)]}'
{"nova/api/openstack/compute/servers.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c596e2ec60aa063bd6629c77fa35556060f1b6ee","unresolved":false,"context_lines":[{"line_number":737,"context_line":"                exception.FlavorMemoryTooSmall,"},{"line_number":738,"context_line":"                exception.InvalidMetadata,"},{"line_number":739,"context_line":"                exception.InvalidVolume,"},{"line_number":740,"context_line":"                exception.MismatchVolumeAZException,"},{"line_number":741,"context_line":"                exception.MultiplePortsNotApplicable,"},{"line_number":742,"context_line":"                exception.InvalidFixedIpAndMaxCountRequest,"},{"line_number":743,"context_line":"                exception.InstanceUserDataMalformed,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_ceb737f5","line":740,"updated":"2019-10-09 16:06:23.000000000","message":"This is tested with the test_cross_az_attach_false_bfv_az_specified_mismatch functional test.","commit_id":"1cc4d6b8cd64cfa8c1952b5260ced8d95134984a"}],"nova/compute/api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7405a00c3220dc2feb57c64a781dbf655c3c0722","unresolved":false,"context_lines":[{"line_number":3858,"context_line":"        self.volume_api.check_detach(context, old_volume)"},{"line_number":3859,"context_line":"        self.volume_api.check_availability_zone(context, new_volume,"},{"line_number":3860,"context_line":"                                                instance\u003dinstance,"},{"line_number":3861,"context_line":"                                                default_az\u003dTrue)"},{"line_number":3862,"context_line":"        self.volume_api.begin_detaching(context, old_volume[\u0027id\u0027])"},{"line_number":3863,"context_line":"        self.volume_api.reserve_volume(context, new_volume[\u0027id\u0027])"},{"line_number":3864,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"df140735_86f9744e","line":3861,"range":{"start_line":3861,"start_character":48,"end_line":3861,"end_character":63},"updated":"2017-05-31 22:50:52.000000000","message":"This is wrong, swap_volume is on an existing instance.","commit_id":"caeb4700461fce0f415598a4ada812c3daf85434"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1c504ac17d29753cfe3fd0b1698bd78773da25e8","unresolved":false,"context_lines":[{"line_number":462,"context_line":"        return kernel_id, ramdisk_id"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"    @staticmethod"},{"line_number":465,"context_line":"    def parse_availability_zone(context, availability_zone):"},{"line_number":466,"context_line":"        # NOTE(vish): We have a legacy hack to allow admins to specify hosts"},{"line_number":467,"context_line":"        #             via az using az:host:node. It might be nice to expose an"},{"line_number":468,"context_line":"        #             api to specify specific hosts to force onto, but for"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_9c6c1634","line":465,"updated":"2018-07-17 17:02:19.000000000","message":"This is where we process the user-requested server AZ (or default to CONF.default_schedule_zone).\n\nThis happens long before we get to _provision_instances.","commit_id":"362d5d89c8bb732354027f8b5d5a462824dfeeb1"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"c9c5f8b36d3b75bc3352019c1f8355558d1daf05","unresolved":false,"context_lines":[{"line_number":910,"context_line":"                    self._bdm_validate_set_size_and_instance(context,"},{"line_number":911,"context_line":"                        instance, instance_type, block_device_mapping,"},{"line_number":912,"context_line":"                        supports_multiattach))"},{"line_number":913,"context_line":"                if volume_az is not None:"},{"line_number":914,"context_line":"                    # The user didn\u0027t request an AZ but they are booting from"},{"line_number":915,"context_line":"                    # volume and the instance needs to be in the same AZ as the"},{"line_number":916,"context_line":"                    # root volume, so update the RequestSpec."},{"line_number":917,"context_line":"                    req_spec.availability_zone \u003d volume_az"},{"line_number":918,"context_line":"                    req_spec.save()"},{"line_number":919,"context_line":"                instance_tags \u003d self._transform_tags(tags, instance.uuid)"},{"line_number":920,"context_line":""},{"line_number":921,"context_line":"                build_request \u003d objects.BuildRequest(context,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_0ed45a46","line":918,"range":{"start_line":913,"start_character":16,"end_line":918,"end_character":35},"updated":"2018-07-17 06:46:28.000000000","message":"don\u0027t see where we checked and judged that user didn\u0027t request an AZ?","commit_id":"362d5d89c8bb732354027f8b5d5a462824dfeeb1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1c504ac17d29753cfe3fd0b1698bd78773da25e8","unresolved":false,"context_lines":[{"line_number":910,"context_line":"                    self._bdm_validate_set_size_and_instance(context,"},{"line_number":911,"context_line":"                        instance, instance_type, block_device_mapping,"},{"line_number":912,"context_line":"                        supports_multiattach))"},{"line_number":913,"context_line":"                if volume_az is not None:"},{"line_number":914,"context_line":"                    # The user didn\u0027t request an AZ but they are booting from"},{"line_number":915,"context_line":"                    # volume and the instance needs to be in the same AZ as the"},{"line_number":916,"context_line":"                    # root volume, so update the RequestSpec."},{"line_number":917,"context_line":"                    req_spec.availability_zone \u003d volume_az"},{"line_number":918,"context_line":"                    req_spec.save()"},{"line_number":919,"context_line":"                instance_tags \u003d self._transform_tags(tags, instance.uuid)"},{"line_number":920,"context_line":""},{"line_number":921,"context_line":"                build_request \u003d objects.BuildRequest(context,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_fc2d0afc","line":918,"range":{"start_line":913,"start_character":16,"end_line":918,"end_character":35},"in_reply_to":"5f7c97a3_0ed45a46","updated":"2018-07-17 17:02:19.000000000","message":"It\u0027s confusing and the flow is very tightly coupled, but when the server create request is being processed, we call the parse_availability_zone() method (in this class) which will parse the AZ that the user specified and if one was not specified by the user, we default to CONF.default_schedule_zone (which defaults to None). So by the time we get to _validate_bdm and it eventually calls nova.volume.cinder.API.check_availability_zone, the instance.availability_zone should be set from the request (or the default_schedule_zone) and we can compare it to the requested volume\u0027s AZ.\n\nI thought about trying to decouple all of this so we don\u0027t need the volume_az coming back from way down in check_availability_zone, but it would be a bit inefficient since we\u0027d have to add a check in here before eventually getting to _validate_bdm, something like:\n\n1. if instance.availability_zone is None\n2. if not CONF.cinder.cross_az_attach\n3. find the root bdm (if there is one for boot from volume)\n4. if the root bdm is a volume, get the volume\n5. compare the instance.availability_zone to the volume\u0027s AZ\n6. then do the same logic as I\u0027ve added to check_availability_zone() in this patch, but instead of returning volume_az back up the stack, just set it on the instance.availability_zone and requestspec here. We\u0027ll still eventually end up getting the volume again and comparing the AZ even though we would have already done that - which is inefficient.\n\nAnyway, what I have here isn\u0027t pretty but it fixes the issue (which has been a problem ever since cross_az_attach was added a long long time ago).","commit_id":"362d5d89c8bb732354027f8b5d5a462824dfeeb1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d07e56c3cba2302e92aba9fd24526c4c09475249","unresolved":false,"context_lines":[{"line_number":910,"context_line":"                    self._bdm_validate_set_size_and_instance(context,"},{"line_number":911,"context_line":"                        instance, instance_type, block_device_mapping,"},{"line_number":912,"context_line":"                        supports_multiattach))"},{"line_number":913,"context_line":"                if volume_az is not None:"},{"line_number":914,"context_line":"                    # The user didn\u0027t request an AZ but they are booting from"},{"line_number":915,"context_line":"                    # volume and the instance needs to be in the same AZ as the"},{"line_number":916,"context_line":"                    # root volume, so update the RequestSpec."},{"line_number":917,"context_line":"                    req_spec.availability_zone \u003d volume_az"},{"line_number":918,"context_line":"                    req_spec.save()"},{"line_number":919,"context_line":"                instance_tags \u003d self._transform_tags(tags, instance.uuid)"},{"line_number":920,"context_line":""},{"line_number":921,"context_line":"                build_request \u003d objects.BuildRequest(context,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_2b66bbef","line":918,"range":{"start_line":913,"start_character":16,"end_line":918,"end_character":35},"in_reply_to":"5f7c97a3_92a39144","updated":"2018-07-18 15:17:02.000000000","message":"\u003e oh, got it we are comparing instance.az with volume az, I thought\n \u003e req_spec.az is the instance.az.\n\nThe RequestSpec.availability_zone is meant to be what the user requested during server create and what will go to the scheduler for the AvailabilityZoneFilter. Once the scheduler picks a host based on the requested AZ, conductor will lookup the AZ for the host and set that on instance.availability_zone:\n\nhttps://github.com/openstack/nova/blob/eb4f65a7951e921b1cd8d05713e144e72f2f254f/nova/conductor/manager.py#L1254","commit_id":"362d5d89c8bb732354027f8b5d5a462824dfeeb1"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"b4c9932c8c2ed45902907222bd406a5ece4a4a53","unresolved":false,"context_lines":[{"line_number":910,"context_line":"                    self._bdm_validate_set_size_and_instance(context,"},{"line_number":911,"context_line":"                        instance, instance_type, block_device_mapping,"},{"line_number":912,"context_line":"                        supports_multiattach))"},{"line_number":913,"context_line":"                if volume_az is not None:"},{"line_number":914,"context_line":"                    # The user didn\u0027t request an AZ but they are booting from"},{"line_number":915,"context_line":"                    # volume and the instance needs to be in the same AZ as the"},{"line_number":916,"context_line":"                    # root volume, so update the RequestSpec."},{"line_number":917,"context_line":"                    req_spec.availability_zone \u003d volume_az"},{"line_number":918,"context_line":"                    req_spec.save()"},{"line_number":919,"context_line":"                instance_tags \u003d self._transform_tags(tags, instance.uuid)"},{"line_number":920,"context_line":""},{"line_number":921,"context_line":"                build_request \u003d objects.BuildRequest(context,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_92a39144","line":918,"range":{"start_line":913,"start_character":16,"end_line":918,"end_character":35},"in_reply_to":"5f7c97a3_fc2d0afc","updated":"2018-07-18 08:09:49.000000000","message":"oh, got it we are comparing instance.az with volume az, I thought req_spec.az is the instance.az.","commit_id":"362d5d89c8bb732354027f8b5d5a462824dfeeb1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"534fd19a1a659bd9f79c7c55766c1e11bed1f45e","unresolved":false,"context_lines":[{"line_number":881,"context_line":"                        instance_uuid, boot_meta, instance_type,"},{"line_number":882,"context_line":"                        base_options[\u0027numa_topology\u0027],"},{"line_number":883,"context_line":"                        base_options[\u0027pci_requests\u0027], filter_properties,"},{"line_number":884,"context_line":"                        instance_group, base_options[\u0027availability_zone\u0027],"},{"line_number":885,"context_line":"                        security_groups\u003dsecurity_groups)"},{"line_number":886,"context_line":""},{"line_number":887,"context_line":"                if block_device_mapping:"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_af865715","line":884,"range":{"start_line":884,"start_character":40,"end_line":884,"end_character":73},"updated":"2018-10-12 21:47:28.000000000","message":"This is where the requested (or configured default) AZ gets stored on the request spec.","commit_id":"f19eef6a1dc2072e60de24560239b1742d01b110"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9beb296c53c571c866792a08cddbdf68a81d06a2","unresolved":false,"context_lines":[{"line_number":881,"context_line":"                        instance_uuid, boot_meta, instance_type,"},{"line_number":882,"context_line":"                        base_options[\u0027numa_topology\u0027],"},{"line_number":883,"context_line":"                        base_options[\u0027pci_requests\u0027], filter_properties,"},{"line_number":884,"context_line":"                        instance_group, base_options[\u0027availability_zone\u0027],"},{"line_number":885,"context_line":"                        security_groups\u003dsecurity_groups)"},{"line_number":886,"context_line":""},{"line_number":887,"context_line":"                if block_device_mapping:"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_419c375a","line":884,"range":{"start_line":884,"start_character":40,"end_line":884,"end_character":73},"in_reply_to":"3f79a3b5_af865715","updated":"2018-10-12 22:50:09.000000000","message":"Thanks.","commit_id":"f19eef6a1dc2072e60de24560239b1742d01b110"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6f646881be4061736ec4b547a60a151915227ce3","unresolved":false,"context_lines":[{"line_number":927,"context_line":"                    # volume and the instance needs to be in the same AZ as the"},{"line_number":928,"context_line":"                    # root volume, so update the RequestSpec."},{"line_number":929,"context_line":"                    req_spec.availability_zone \u003d volume_az"},{"line_number":930,"context_line":"                    req_spec.save()"},{"line_number":931,"context_line":"                instance_tags \u003d self._transform_tags(tags, instance.uuid)"},{"line_number":932,"context_line":""},{"line_number":933,"context_line":"                build_request \u003d objects.BuildRequest(context,"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_c4496e66","line":930,"updated":"2018-10-10 20:56:25.000000000","message":"Is this the only place where we save the AZ to the request spec? I grepped for others and didn\u0027t find any.","commit_id":"f19eef6a1dc2072e60de24560239b1742d01b110"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"534fd19a1a659bd9f79c7c55766c1e11bed1f45e","unresolved":false,"context_lines":[{"line_number":927,"context_line":"                    # volume and the instance needs to be in the same AZ as the"},{"line_number":928,"context_line":"                    # root volume, so update the RequestSpec."},{"line_number":929,"context_line":"                    req_spec.availability_zone \u003d volume_az"},{"line_number":930,"context_line":"                    req_spec.save()"},{"line_number":931,"context_line":"                instance_tags \u003d self._transform_tags(tags, instance.uuid)"},{"line_number":932,"context_line":""},{"line_number":933,"context_line":"                build_request \u003d objects.BuildRequest(context,"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_6f7cbf21","line":930,"in_reply_to":"3f79a3b5_c4496e66","updated":"2018-10-12 21:47:28.000000000","message":"No, see above.","commit_id":"f19eef6a1dc2072e60de24560239b1742d01b110"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6f646881be4061736ec4b547a60a151915227ce3","unresolved":false,"context_lines":[{"line_number":3950,"context_line":""},{"line_number":3951,"context_line":"    def _check_attach_and_reserve_volume(self, context, volume, instance,"},{"line_number":3952,"context_line":"                                         bdm, supports_multiattach\u003dFalse,"},{"line_number":3953,"context_line":"                                         default_az\u003dFalse):"},{"line_number":3954,"context_line":"        volume_id \u003d volume[\u0027id\u0027]"},{"line_number":3955,"context_line":"        volume_az \u003d self.volume_api.check_availability_zone("},{"line_number":3956,"context_line":"            context, volume, instance\u003dinstance, default_az\u003ddefault_az)"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_1e55741e","line":3953,"range":{"start_line":3953,"start_character":41,"end_line":3953,"end_character":57},"updated":"2018-10-10 20:56:25.000000000","message":"Note to self: this is called with this default in attach_volume.","commit_id":"f19eef6a1dc2072e60de24560239b1742d01b110"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ba635e9a991dda55729905d29fa528dfeab1c9c4","unresolved":false,"context_lines":[{"line_number":923,"context_line":"                    self._bdm_validate_set_size_and_instance(context,"},{"line_number":924,"context_line":"                        instance, instance_type, block_device_mapping,"},{"line_number":925,"context_line":"                        supports_multiattach))"},{"line_number":926,"context_line":"                if volume_az is not None:"},{"line_number":927,"context_line":"                    # The user didn\u0027t request an AZ but they are booting from"},{"line_number":928,"context_line":"                    # volume and the instance needs to be in the same AZ as the"},{"line_number":929,"context_line":"                    # root volume, so update the RequestSpec."}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_f3e67f16","line":926,"updated":"2019-02-13 16:36:59.000000000","message":"Per my comment in cinder.py, I think the logic for what to do should be here (or this should be a call to a helper function nearby.\n\n if not instance_az:\n     req_spec.az \u003d volume_az\n elif instance_az !\u003d volume_az and not CONF.cross_az_attach:\n     # explode\n\nNow, perhaps the elif case needs to remain buried in the cinder stuff so that cleanup happens properly? If so, that\u0027s fine, but the very nova-specific bit of choosing to put the volume az in the reqspec should happen out here and not deep in the volume api code, IMHO.\n\nAs far as your comment on PS4 about decoupling the attach bit from the determination fo the volume AZ, I\u0027m not as concerned about that. It\u0027s all crazy tightly coupled as it is. I think not adding more (i.e. my complaint in that file) is good, but this fix is backportable as it is, and further refactoring would make that less good.","commit_id":"2ec7c40a5c5ee6417e1d4500ec9e924c5a5ec6d6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"defd59d9b3b299a422298bf5d3321b745d09b537","unresolved":false,"context_lines":[{"line_number":923,"context_line":"                    self._bdm_validate_set_size_and_instance(context,"},{"line_number":924,"context_line":"                        instance, instance_type, block_device_mapping,"},{"line_number":925,"context_line":"                        supports_multiattach))"},{"line_number":926,"context_line":"                if volume_az is not None:"},{"line_number":927,"context_line":"                    # The user didn\u0027t request an AZ but they are booting from"},{"line_number":928,"context_line":"                    # volume and the instance needs to be in the same AZ as the"},{"line_number":929,"context_line":"                    # root volume, so update the RequestSpec."}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_d7483888","line":926,"in_reply_to":"9fdfeff1_f3e67f16","updated":"2019-10-08 21:17:35.000000000","message":"\u003e Per my comment in cinder.py, I think the logic for what to do\n \u003e should be here (or this should be a call to a helper function\n \u003e nearby.\n \u003e \n \u003e if not instance_az:\n \u003e req_spec.az \u003d volume_az\n \u003e elif instance_az !\u003d volume_az and not CONF.cross_az_attach:\n \u003e # explode\n \u003e \n\nThe tricky thing with that is we don\u0027t want to set req_spec.az \u003d volume_az if the volume_az is \u0027nova\u0027 because of default az in config, because that\u0027s like the user requesting the az to be \u0027nova\u0027 on the server which means it can\u0027t migrate between AZs later, which is the warning we have in the docs:\n\nhttps://docs.openstack.org/nova/latest/admin/availability-zones.html\n\nAnd this:\n\nhttps://docs.openstack.org/nova/latest/admin/availability-zones.html#implications-for-moving-servers\n\nThat\u0027s why the cinder.py code is checking CONF.default_availability_zone which defaults to \u0027nova\u0027.\n\nNote that CONF.default_schedule_zone defaults to None but if it\u0027s set then it will be set in instance_az in the logic above before we even get here.","commit_id":"2ec7c40a5c5ee6417e1d4500ec9e924c5a5ec6d6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dd65206deea0a61f21d61cd0861e457d3442251e","unresolved":false,"context_lines":[{"line_number":923,"context_line":"                    self._bdm_validate_set_size_and_instance(context,"},{"line_number":924,"context_line":"                        instance, instance_type, block_device_mapping,"},{"line_number":925,"context_line":"                        supports_multiattach))"},{"line_number":926,"context_line":"                if volume_az is not None:"},{"line_number":927,"context_line":"                    # The user didn\u0027t request an AZ but they are booting from"},{"line_number":928,"context_line":"                    # volume and the instance needs to be in the same AZ as the"},{"line_number":929,"context_line":"                    # root volume, so update the RequestSpec."}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_8eeeea1f","line":926,"in_reply_to":"9fdfeff1_f3e67f16","updated":"2019-02-13 17:08:51.000000000","message":"\u003e the very nova-specific bit of choosing to put the volume az in the reqspec should happen out here and not deep in the volume api code\n\nIt is happening here. I haven\u0027t read the other comment yet though, but maybe you mean the calculation of whether or not we need the volume AZ here way down there is what you\u0027re talking about, which I\u0027d agree is the gross part.","commit_id":"2ec7c40a5c5ee6417e1d4500ec9e924c5a5ec6d6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ba635e9a991dda55729905d29fa528dfeab1c9c4","unresolved":false,"context_lines":[{"line_number":1468,"context_line":"                        self._check_attach_and_reserve_volume("},{"line_number":1469,"context_line":"                            context, volume, instance, bdm,"},{"line_number":1470,"context_line":"                            supports_multiattach, default_az\u003dTrue))"},{"line_number":1471,"context_line":"                    if bdm.is_root:"},{"line_number":1472,"context_line":"                        # The user hasn\u0027t requested an AZ for the instance"},{"line_number":1473,"context_line":"                        # so we pick the boot volume\u0027s AZ for the instance"},{"line_number":1474,"context_line":"                        # in the case that [cinder]/cross_az_attach\u003dFalse"}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_d8003805","line":1471,"range":{"start_line":1471,"start_character":23,"end_line":1471,"end_character":34},"updated":"2019-02-13 16:36:59.000000000","message":"Why only is_root? If I create an instance with multiple volumes, they better all be in the same az. Also, even if I create an ephemeral instance with a data volume (i.e. not root), it needs to be in the same AZ.","commit_id":"2ec7c40a5c5ee6417e1d4500ec9e924c5a5ec6d6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dd65206deea0a61f21d61cd0861e457d3442251e","unresolved":false,"context_lines":[{"line_number":1468,"context_line":"                        self._check_attach_and_reserve_volume("},{"line_number":1469,"context_line":"                            context, volume, instance, bdm,"},{"line_number":1470,"context_line":"                            supports_multiattach, default_az\u003dTrue))"},{"line_number":1471,"context_line":"                    if bdm.is_root:"},{"line_number":1472,"context_line":"                        # The user hasn\u0027t requested an AZ for the instance"},{"line_number":1473,"context_line":"                        # so we pick the boot volume\u0027s AZ for the instance"},{"line_number":1474,"context_line":"                        # in the case that [cinder]/cross_az_attach\u003dFalse"}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_ce5cf21a","line":1471,"range":{"start_line":1471,"start_character":23,"end_line":1471,"end_character":34},"in_reply_to":"9fdfeff1_d8003805","updated":"2019-02-13 17:08:51.000000000","message":"You\u0027re right, I\u0027m not sure why I was restricting to the root BDM since cross_az_attach applies to all volumes attached to the server.","commit_id":"2ec7c40a5c5ee6417e1d4500ec9e924c5a5ec6d6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ba635e9a991dda55729905d29fa528dfeab1c9c4","unresolved":false,"context_lines":[{"line_number":1474,"context_line":"                        # in the case that [cinder]/cross_az_attach\u003dFalse"},{"line_number":1475,"context_line":"                        # because in that scenario, the instance and root"},{"line_number":1476,"context_line":"                        # volume need to be in the same AZ."},{"line_number":1477,"context_line":"                        volume_az \u003d _volume_az"},{"line_number":1478,"context_line":"                    bdm.volume_size \u003d volume.get(\u0027size\u0027)"},{"line_number":1479,"context_line":""},{"line_number":1480,"context_line":"                    # NOTE(mnaser): If we end up reserving the volume, it will"}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_f312ff57","line":1477,"updated":"2019-02-13 16:36:59.000000000","message":"Presumably we only want to do this once, and on the second and later cases, make sure that they\u0027re equivalent right?","commit_id":"2ec7c40a5c5ee6417e1d4500ec9e924c5a5ec6d6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dd65206deea0a61f21d61cd0861e457d3442251e","unresolved":false,"context_lines":[{"line_number":1474,"context_line":"                        # in the case that [cinder]/cross_az_attach\u003dFalse"},{"line_number":1475,"context_line":"                        # because in that scenario, the instance and root"},{"line_number":1476,"context_line":"                        # volume need to be in the same AZ."},{"line_number":1477,"context_line":"                        volume_az \u003d _volume_az"},{"line_number":1478,"context_line":"                    bdm.volume_size \u003d volume.get(\u0027size\u0027)"},{"line_number":1479,"context_line":""},{"line_number":1480,"context_line":"                    # NOTE(mnaser): If we end up reserving the volume, it will"}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_0ec3da8a","line":1477,"in_reply_to":"9fdfeff1_f312ff57","updated":"2019-02-13 17:08:51.000000000","message":"You mean once volume_az is set from at least one volume, if we encounter another in the list of bdms with a different AZ then we blow up? Yeah I agree.","commit_id":"2ec7c40a5c5ee6417e1d4500ec9e924c5a5ec6d6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"62bb6b34e91a07d0702139feb4304c724f94de7c","unresolved":false,"context_lines":[{"line_number":1045,"context_line":"        :raises: Forbidden - if the user token does not have authority to see"},{"line_number":1046,"context_line":"            a volume"},{"line_number":1047,"context_line":"        \"\"\""},{"line_number":1048,"context_line":"        # It would be nice if the GET /v3/{project_id}/volumes/detail API took"},{"line_number":1049,"context_line":"        # a list of volume IDs as filter parameters so we could make one rather"},{"line_number":1050,"context_line":"        # than N calls."},{"line_number":1051,"context_line":"        volumes \u003d {}"},{"line_number":1052,"context_line":"        for bdm in bdms:"},{"line_number":1053,"context_line":"            if bdm.volume_id:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_f1052fc3","line":1050,"range":{"start_line":1048,"start_character":8,"end_line":1050,"end_character":23},"updated":"2019-10-09 19:07:34.000000000","message":"I should probably remove this. Even if the API did allow a list of IDs it depends on the semantics, i.e. if it\u0027s like an IN filter then it probably wouldn\u0027t 404 if any are missing and that\u0027s not the behavior we want here.","commit_id":"1cc4d6b8cd64cfa8c1952b5260ced8d95134984a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7d768a6d8699dd6c66376d7ce57d070927d3a713","unresolved":false,"context_lines":[{"line_number":1045,"context_line":"        :raises: Forbidden - if the user token does not have authority to see"},{"line_number":1046,"context_line":"            a volume"},{"line_number":1047,"context_line":"        \"\"\""},{"line_number":1048,"context_line":"        # It would be nice if the GET /v3/{project_id}/volumes/detail API took"},{"line_number":1049,"context_line":"        # a list of volume IDs as filter parameters so we could make one rather"},{"line_number":1050,"context_line":"        # than N calls."},{"line_number":1051,"context_line":"        volumes \u003d {}"},{"line_number":1052,"context_line":"        for bdm in bdms:"},{"line_number":1053,"context_line":"            if bdm.volume_id:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_1c9c6dbe","line":1050,"range":{"start_line":1048,"start_character":8,"end_line":1050,"end_character":23},"in_reply_to":"3fa7e38b_f1052fc3","updated":"2019-10-30 16:18:31.000000000","message":"Done","commit_id":"1cc4d6b8cd64cfa8c1952b5260ced8d95134984a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"87fe963554ed49ef21ced13fb0f718c25383d98c","unresolved":false,"context_lines":[{"line_number":1108,"context_line":"        # (which by default matches the default AZ in Cinder, i.e. \u0027nova\u0027)."},{"line_number":1109,"context_line":"        if instance_az is None:"},{"line_number":1110,"context_line":"            # Check if the volume AZ is the same as our default AZ for compute"},{"line_number":1111,"context_line":"            # hosts (nova) and if so, assume we are OK because the user did not"},{"line_number":1112,"context_line":"            # request an AZ and will get the same default. If the volume AZ is"},{"line_number":1113,"context_line":"            # not the same as our default, return the volume AZ so the caller"},{"line_number":1114,"context_line":"            # can put it into the request spec so the instance is scheduled"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_db71fe0e","line":1111,"range":{"start_line":1111,"start_character":37,"end_line":1111,"end_character":54},"updated":"2019-10-30 15:18:08.000000000","message":"Agree that we should be able to ignore, but why not just always return the volume AZ if instance_az is None? The only thing I can think of is something related to pinning the instance in the volume\u0027s AZ or something (can\u0027t remember if it\u0027s sticky now anymore). Just seems like one less branch if we just explicitly take the volume\u0027s AZ.","commit_id":"1cc4d6b8cd64cfa8c1952b5260ced8d95134984a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7d768a6d8699dd6c66376d7ce57d070927d3a713","unresolved":false,"context_lines":[{"line_number":1108,"context_line":"        # (which by default matches the default AZ in Cinder, i.e. \u0027nova\u0027)."},{"line_number":1109,"context_line":"        if instance_az is None:"},{"line_number":1110,"context_line":"            # Check if the volume AZ is the same as our default AZ for compute"},{"line_number":1111,"context_line":"            # hosts (nova) and if so, assume we are OK because the user did not"},{"line_number":1112,"context_line":"            # request an AZ and will get the same default. If the volume AZ is"},{"line_number":1113,"context_line":"            # not the same as our default, return the volume AZ so the caller"},{"line_number":1114,"context_line":"            # can put it into the request spec so the instance is scheduled"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_dca1f5f8","line":1111,"range":{"start_line":1111,"start_character":37,"end_line":1111,"end_character":54},"in_reply_to":"3fa7e38b_969a7bc2","updated":"2019-10-30 16:18:31.000000000","message":"Done","commit_id":"1cc4d6b8cd64cfa8c1952b5260ced8d95134984a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0b02b6a8922dc1f87ede35571bef4fb4e46c6c30","unresolved":false,"context_lines":[{"line_number":1108,"context_line":"        # (which by default matches the default AZ in Cinder, i.e. \u0027nova\u0027)."},{"line_number":1109,"context_line":"        if instance_az is None:"},{"line_number":1110,"context_line":"            # Check if the volume AZ is the same as our default AZ for compute"},{"line_number":1111,"context_line":"            # hosts (nova) and if so, assume we are OK because the user did not"},{"line_number":1112,"context_line":"            # request an AZ and will get the same default. If the volume AZ is"},{"line_number":1113,"context_line":"            # not the same as our default, return the volume AZ so the caller"},{"line_number":1114,"context_line":"            # can put it into the request spec so the instance is scheduled"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_969a7bc2","line":1111,"range":{"start_line":1111,"start_character":37,"end_line":1111,"end_character":54},"in_reply_to":"3fa7e38b_b62c5799","updated":"2019-10-30 15:28:41.000000000","message":"If I respin I\u0027ll add a comment about this as well since it\u0027s confusing.","commit_id":"1cc4d6b8cd64cfa8c1952b5260ced8d95134984a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"58a79dd03594577d02f2a06027ffb0d77df813b3","unresolved":false,"context_lines":[{"line_number":1108,"context_line":"        # (which by default matches the default AZ in Cinder, i.e. \u0027nova\u0027)."},{"line_number":1109,"context_line":"        if instance_az is None:"},{"line_number":1110,"context_line":"            # Check if the volume AZ is the same as our default AZ for compute"},{"line_number":1111,"context_line":"            # hosts (nova) and if so, assume we are OK because the user did not"},{"line_number":1112,"context_line":"            # request an AZ and will get the same default. If the volume AZ is"},{"line_number":1113,"context_line":"            # not the same as our default, return the volume AZ so the caller"},{"line_number":1114,"context_line":"            # can put it into the request spec so the instance is scheduled"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_b62c5799","line":1111,"range":{"start_line":1111,"start_character":37,"end_line":1111,"end_character":54},"in_reply_to":"3fa7e38b_db71fe0e","updated":"2019-10-30 15:26:27.000000000","message":"The case to worry about is the default one, where the default AZ is \u0027nova\u0027 in both nova and cinder:\n\nhttps://docs.openstack.org/nova/latest/configuration/config.html#DEFAULT.default_availability_zone\n\nhttps://github.com/openstack/cinder/blob/6074b9bfcb9278790833e8b3a36803e106d79c67/cinder/common/config.py#L94\n\nAnd that is not good if we pin the instance to the \u0027nova\u0027 AZ per the warning here:\n\nhttps://docs.openstack.org/nova/latest/admin/availability-zones.html\n\nSpecifically:\n\n\"\"\"\nConsequently, it is highly recommended for users to never ever ask for booting an instance by specifying an explicit AZ named nova and for operators to never set the AZ metadata for an aggregate to nova. This can result is some problems due to the fact that the instance AZ information is explicitly attached to nova which could break further move operations when either the host is moved to another aggregate or when the user would like to migrate the instance.\n\"\"\"\n\nSo that\u0027s my reasoning here anyway.","commit_id":"1cc4d6b8cd64cfa8c1952b5260ced8d95134984a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c596e2ec60aa063bd6629c77fa35556060f1b6ee","unresolved":false,"context_lines":[{"line_number":1158,"context_line":"        # Before processing the list of instances get all of the requested"},{"line_number":1159,"context_line":"        # pre-existing volumes so we can do some validation here rather than"},{"line_number":1160,"context_line":"        # down in the bowels of _validate_bdm."},{"line_number":1161,"context_line":"        volumes \u003d self._get_volumes_for_bdms(context, block_device_mapping)"},{"line_number":1162,"context_line":"        volume_az \u003d self._validate_vol_az_for_create("},{"line_number":1163,"context_line":"            base_options[\u0027availability_zone\u0027], volumes.values())"},{"line_number":1164,"context_line":"        if volume_az:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_0ed9cf55","line":1161,"updated":"2019-10-09 16:06:23.000000000","message":"If it makes things easier I could split this out into a separate change before _validate_vol_az_for_create is added.","commit_id":"1cc4d6b8cd64cfa8c1952b5260ced8d95134984a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c596e2ec60aa063bd6629c77fa35556060f1b6ee","unresolved":false,"context_lines":[{"line_number":4233,"context_line":"            should be validated for cross_az_attach, False to not validate AZ"},{"line_number":4234,"context_line":"        \"\"\""},{"line_number":4235,"context_line":"        volume_id \u003d volume[\u0027id\u0027]"},{"line_number":4236,"context_line":"        if validate_az:"},{"line_number":4237,"context_line":"            self.volume_api.check_availability_zone(context, volume,"},{"line_number":4238,"context_line":"                                                    instance\u003dinstance)"},{"line_number":4239,"context_line":"        # If volume.multiattach\u003dTrue and the microversion to"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_4ea6e7ad","line":4236,"updated":"2019-10-09 16:06:23.000000000","message":"This being False is tested by test_validate_bdm_media_service_valid.","commit_id":"1cc4d6b8cd64cfa8c1952b5260ced8d95134984a"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"d2d59bdcc579dca32080e795397aa1f740251e60","unresolved":false,"context_lines":[{"line_number":1053,"context_line":"        return volumes"},{"line_number":1054,"context_line":""},{"line_number":1055,"context_line":"    @staticmethod"},{"line_number":1056,"context_line":"    def _validate_vol_az_for_create(instance_az, volumes):"},{"line_number":1057,"context_line":"        \"\"\"Performs cross_az_attach validation for the instance and volumes."},{"line_number":1058,"context_line":""},{"line_number":1059,"context_line":"        If [cinder]/cross_az_attach\u003dTrue (default) this method is a no-op."}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_974cabc8","line":1056,"updated":"2019-10-31 12:57:06.000000000","message":"and we should be able to use for unshelve also https://github.com/openstack/nova/blob/master/nova/compute/api.py#L3762","commit_id":"4c55304741da6dcf1a7009f74c0eddeebce9514e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eac4ab62075e50068b74f8c393386c33e14cad80","unresolved":false,"context_lines":[{"line_number":1053,"context_line":"        return volumes"},{"line_number":1054,"context_line":""},{"line_number":1055,"context_line":"    @staticmethod"},{"line_number":1056,"context_line":"    def _validate_vol_az_for_create(instance_az, volumes):"},{"line_number":1057,"context_line":"        \"\"\"Performs cross_az_attach validation for the instance and volumes."},{"line_number":1058,"context_line":""},{"line_number":1059,"context_line":"        If [cinder]/cross_az_attach\u003dTrue (default) this method is a no-op."}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_b2b77521","line":1056,"in_reply_to":"3fa7e38b_974cabc8","updated":"2019-10-31 13:32:45.000000000","message":"Maybe sort of but it\u0027s not really the same. I don\u0027t really want to munge that into this patch since it\u0027s already a big change. If there is some optimization where the unshelve flow can reduce some duplication by re-using this method then that\u0027s fine, but it\u0027s not a straight replacement.","commit_id":"4c55304741da6dcf1a7009f74c0eddeebce9514e"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"6969b5eba16eeebff1fbd2bab2c55b17f09c2968","unresolved":false,"context_lines":[{"line_number":1091,"context_line":"                   % \u0027,\u0027.join(vol_zones))"},{"line_number":1092,"context_line":"            raise exception.MismatchVolumeAZException(reason\u003dmsg)"},{"line_number":1093,"context_line":""},{"line_number":1094,"context_line":"        volume_az \u003d vol_zones[0]"},{"line_number":1095,"context_line":"        # In this case the instance.host should not be set so the instance AZ"},{"line_number":1096,"context_line":"        # value should come from instance.availability_zone which will be one"},{"line_number":1097,"context_line":"        # of the following cases:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_770a4f8a","line":1094,"range":{"start_line":1094,"start_character":20,"end_line":1094,"end_character":29},"updated":"2019-10-31 12:48:10.000000000","message":"if there no volumes being attached, this will be failed?","commit_id":"4c55304741da6dcf1a7009f74c0eddeebce9514e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eac4ab62075e50068b74f8c393386c33e14cad80","unresolved":false,"context_lines":[{"line_number":1091,"context_line":"                   % \u0027,\u0027.join(vol_zones))"},{"line_number":1092,"context_line":"            raise exception.MismatchVolumeAZException(reason\u003dmsg)"},{"line_number":1093,"context_line":""},{"line_number":1094,"context_line":"        volume_az \u003d vol_zones[0]"},{"line_number":1095,"context_line":"        # In this case the instance.host should not be set so the instance AZ"},{"line_number":1096,"context_line":"        # value should come from instance.availability_zone which will be one"},{"line_number":1097,"context_line":"        # of the following cases:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_d2c6b19a","line":1094,"range":{"start_line":1094,"start_character":20,"end_line":1094,"end_character":29},"in_reply_to":"3fa7e38b_770a4f8a","updated":"2019-10-31 13:32:45.000000000","message":"Yeah I think you\u0027re right. Unit tests likely missed this since they probably mock this method out and functional tests probably missed it since they use volumes when cross_az_attach\u003dFalse.","commit_id":"4c55304741da6dcf1a7009f74c0eddeebce9514e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c6847212ae920b0426f536e4018fd6f7458d8f9b","unresolved":false,"context_lines":[{"line_number":1091,"context_line":"                   % \u0027,\u0027.join(vol_zones))"},{"line_number":1092,"context_line":"            raise exception.MismatchVolumeAZException(reason\u003dmsg)"},{"line_number":1093,"context_line":""},{"line_number":1094,"context_line":"        volume_az \u003d vol_zones[0]"},{"line_number":1095,"context_line":"        # In this case the instance.host should not be set so the instance AZ"},{"line_number":1096,"context_line":"        # value should come from instance.availability_zone which will be one"},{"line_number":1097,"context_line":"        # of the following cases:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_b2ff7500","line":1094,"range":{"start_line":1094,"start_character":20,"end_line":1094,"end_character":29},"in_reply_to":"3fa7e38b_d2c6b19a","updated":"2019-10-31 14:03:49.000000000","message":"Fixed with a new functional test \"test_cross_az_attach_false_no_volumes\" which is easier and more trustworthy than stubbing everything out through unit tests.","commit_id":"4c55304741da6dcf1a7009f74c0eddeebce9514e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4e8ddb442596a41b534a68fb4ae1a577de396ce2","unresolved":false,"context_lines":[{"line_number":1056,"context_line":"    def _validate_vol_az_for_create(instance_az, volumes):"},{"line_number":1057,"context_line":"        \"\"\"Performs cross_az_attach validation for the instance and volumes."},{"line_number":1058,"context_line":""},{"line_number":1059,"context_line":"        If [cinder]/cross_az_attach\u003dTrue (default) this method is a no-op."},{"line_number":1060,"context_line":""},{"line_number":1061,"context_line":"        If [cinder]/cross_az_attach\u003dFalse, this method will validate that:"},{"line_number":1062,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_2d7eca69","line":1059,"updated":"2019-10-31 14:13:57.000000000","message":"Might\u0027ve been worth calling out what happens if no volumes are used on the instance, since you covered all the other bases here. Sounds like \"this won\u0027t screw with the presence or lack of an instance AZ if no volumes are present to call for a change.\"","commit_id":"07a24dcef7ce6767b4b5bab0c8d3166cbe5b39c0"}],"nova/conf/cinder.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c596e2ec60aa063bd6629c77fa35556060f1b6ee","unresolved":false,"context_lines":[{"line_number":89,"context_line":""},{"line_number":90,"context_line":"If False, volumes attached to an instance must be in the same availability"},{"line_number":91,"context_line":"zone in Cinder as the instance availability zone in Nova."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"This also means care should be taken when booting an instance from a volume"},{"line_number":94,"context_line":"where source is not \"volume\" because Nova will attempt to create a volume using"},{"line_number":95,"context_line":"the same availability zone as what is assigned to the instance."}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_aeb2fb04","line":92,"updated":"2019-10-09 16:06:23.000000000","message":"If desired I could split this and the default_schedule_zone conf option help change out into a separate change since they are worthwhile on their own.","commit_id":"1cc4d6b8cd64cfa8c1952b5260ced8d95134984a"}],"nova/tests/unit/volume/test_cinder.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9beb296c53c571c866792a08cddbdf68a81d06a2","unresolved":false,"context_lines":[{"line_number":281,"context_line":"            availability_zone\u003dCONF.default_schedule_zone)"},{"line_number":282,"context_line":"        self.assertIsNone(self.api.check_availability_zone("},{"line_number":283,"context_line":"            self.ctx, volume, instance, default_az\u003dTrue))"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    @mock.patch(\u0027nova.volume.cinder.cinderclient\u0027)"},{"line_number":286,"context_line":"    def test_reserve_volume(self, mock_cinderclient):"},{"line_number":287,"context_line":"        mock_volumes \u003d mock.MagicMock()"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_418e7732","line":284,"updated":"2018-10-12 22:50:09.000000000","message":"I didn\u0027t notice before, but there\u0027s no test for the cross_az_attach\u003dFalse, volume AZ different from default_availability_zone, and no instance AZ set, where the method should return the volume AZ.","commit_id":"f19eef6a1dc2072e60de24560239b1742d01b110"}],"nova/volume/cinder.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dac43be80384a1fef84f42ad62825f5105048e65","unresolved":false,"context_lines":[{"line_number":291,"context_line":"            # the default AZ (which by default matches the default AZ in"},{"line_number":292,"context_line":"            # Cinder, i.e. \u0027nova\u0027)."},{"line_number":293,"context_line":"            if instance_az is None and default_az:"},{"line_number":294,"context_line":"                instance_az \u003d CONF.default_availability_zone"},{"line_number":295,"context_line":"            if instance_az !\u003d volume[\u0027availability_zone\u0027]:"},{"line_number":296,"context_line":"                msg \u003d _(\"Instance %(instance)s and volume %(vol)s are not in \""},{"line_number":297,"context_line":"                        \"the same availability_zone. Instance is in \""}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_1e0c1193","line":294,"range":{"start_line":294,"start_character":35,"end_line":294,"end_character":60},"updated":"2018-05-16 20:59:46.000000000","message":"Seems like this should be CONF.default_schedule_zone but that defaults to None so it wouldn\u0027t help us here when the volume has a default AZ of \"nova\". But maybe should use CONF.default_schedule_zone or CONF.default_availability_zone in case CONF.default_schedule_zone is set?","commit_id":"6be2626d1ebca43efb48debf9c9b0ff5746ad2cf"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f45ec0fe4725fd303bf250baeab3457c3a71e15b","unresolved":false,"context_lines":[{"line_number":291,"context_line":"            # the default AZ (which by default matches the default AZ in"},{"line_number":292,"context_line":"            # Cinder, i.e. \u0027nova\u0027)."},{"line_number":293,"context_line":"            if instance_az is None and default_az:"},{"line_number":294,"context_line":"                instance_az \u003d CONF.default_availability_zone"},{"line_number":295,"context_line":"            if instance_az !\u003d volume[\u0027availability_zone\u0027]:"},{"line_number":296,"context_line":"                msg \u003d _(\"Instance %(instance)s and volume %(vol)s are not in \""},{"line_number":297,"context_line":"                        \"the same availability_zone. Instance is in \""}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_67d19a56","line":294,"range":{"start_line":294,"start_character":35,"end_line":294,"end_character":60},"in_reply_to":"5f7c97a3_1e0c1193","updated":"2018-07-15 13:05:35.000000000","message":"We should probably use CONF.default_schedule_zone.\n\nAlternatively, if cross_az_attach\u003dFalse and the user didn\u0027t explicitly request an AZ for the instance, we need the instance to be in the same AZ as the volume, but if the volume is in the default AZ (nova), do we just bypass this check - we could set the RequestSpec.availability_zone \u003d volume.availability_zone, but we don\u0027t want to do that if the volume AZ is the default (nova) because we don\u0027t want users requesting \u0027nova\u0027 for the AZ:\n\nhttps://docs.openstack.org/nova/latest/user/aggregates.html#availability-zones-azs\n\nSo maybe we should do something like this:\n\nif instance_az is None and default_az:\n    # Use default_schedule_zone if set (defaults to None).\n    if CONF.default_schedule_zone:\n        instance_az \u003d CONF.default_schedule_zone\n        if instance_az !\u003d volume[\u0027availability_zone\u0027]:\n            msg \u003d _(\"Instance %(instance)s and volume %(vol)s are not in \"\n                    \"the same availability_zone. Instance is in \"\n                    \"%(ins_zone)s. Volume is in %(vol_zone)s\") % {\n                        \"instance\": instance.uuid,\n                         \"vol\": volume[\u0027id\u0027],\n                         \u0027ins_zone\u0027: instance_az,\n                         \u0027vol_zone\u0027: volume[\u0027availability_zone\u0027]}\n            raise exception.InvalidVolume(reason\u003dmsg)\n    else:\n        # Check if the volume AZ is the same as our default\n        # AZ for compute hosts (nova) and if so, assume we are\n        # OK because the user didn\u0027t request an AZ and will\n        # get the same default. If the volume AZ is not the\n        # same as our default, use the volume AZ and put it\n        # into the request spec so the instance is scheduled\n        # to the same zone as the volume.\n        volume_az \u003d volume[\u0027availability_zone\u0027]\n        if volume_az !\u003d CONF.default_availability_zone:\n            return volume_az  # indication to set in request spec\n        # the volume az is the same as the default nova az\n        # so we\u0027ll be OK and just return\n        return","commit_id":"6be2626d1ebca43efb48debf9c9b0ff5746ad2cf"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"494bdf0f841c7a35de4ce71d466391eeeea9cc2d","unresolved":false,"context_lines":[{"line_number":520,"context_line":"            # Cinder, i.e. \u0027nova\u0027)."},{"line_number":521,"context_line":"            if instance_az is None and default_az:"},{"line_number":522,"context_line":"                # Use default_schedule_zone if set (defaults to None)."},{"line_number":523,"context_line":"                if CONF.default_schedule_zone:"},{"line_number":524,"context_line":"                    instance_az \u003d CONF.default_schedule_zone"},{"line_number":525,"context_line":"                else:"},{"line_number":526,"context_line":"                    # Check if the volume AZ is the same as our default"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_e75b4ff6","line":523,"range":{"start_line":523,"start_character":16,"end_line":523,"end_character":46},"updated":"2018-07-16 20:07:59.000000000","message":"This is redundant because instance.availability_zone will be set in parse_availability_zone if the user didn\u0027t request an AZ, and if CONF.default_schedule_zone is not None, then \"if instance_az is None\" on L521 will be False.","commit_id":"d0e24f2f57b5190da8a297d294227774b367fde3"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6f646881be4061736ec4b547a60a151915227ce3","unresolved":false,"context_lines":[{"line_number":498,"context_line":"        :param context: nova.context.RequestContext user auth request context"},{"line_number":499,"context_line":"        :param volume: dict representation of volume being attached to instance"},{"line_number":500,"context_line":"        :param instance: nova.objects.Instance being attached to volume"},{"line_number":501,"context_line":"        :param default_az: Used when [cinder]/cross_az_attach\u003dFalse and a"},{"line_number":502,"context_line":"            specific AZ is not requested by the user for the instance. Should"},{"line_number":503,"context_line":"            only be True before the instance is scheduled to a host."},{"line_number":504,"context_line":"        :returns: None if [cinder]/cross_az_attach\u003dFalse and the instance AZ"},{"line_number":505,"context_line":"            matches the volume AZ. Returns the volume AZ if the"},{"line_number":506,"context_line":"            user did not request a specific AZ for the instance (and"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_be124060","line":503,"range":{"start_line":501,"start_character":8,"end_line":503,"end_character":68},"updated":"2018-10-10 20:56:25.000000000","message":"Just to be clear, \"Should only be True before the instance is scheduled to a host\" because after the instance is scheduled to a host, the request spec or instance will be updated with an AZ? In which case this method should not be called with default_az\u003dTrue?","commit_id":"f19eef6a1dc2072e60de24560239b1742d01b110"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9beb296c53c571c866792a08cddbdf68a81d06a2","unresolved":false,"context_lines":[{"line_number":498,"context_line":"        :param context: nova.context.RequestContext user auth request context"},{"line_number":499,"context_line":"        :param volume: dict representation of volume being attached to instance"},{"line_number":500,"context_line":"        :param instance: nova.objects.Instance being attached to volume"},{"line_number":501,"context_line":"        :param default_az: Used when [cinder]/cross_az_attach\u003dFalse and a"},{"line_number":502,"context_line":"            specific AZ is not requested by the user for the instance. Should"},{"line_number":503,"context_line":"            only be True before the instance is scheduled to a host."},{"line_number":504,"context_line":"        :returns: None if [cinder]/cross_az_attach\u003dFalse and the instance AZ"},{"line_number":505,"context_line":"            matches the volume AZ. Returns the volume AZ if the"},{"line_number":506,"context_line":"            user did not request a specific AZ for the instance (and"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_219fbb51","line":503,"range":{"start_line":501,"start_character":8,"end_line":503,"end_character":68},"in_reply_to":"3f79a3b5_6f911f45","updated":"2018-10-12 22:50:09.000000000","message":"Ack.","commit_id":"f19eef6a1dc2072e60de24560239b1742d01b110"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"534fd19a1a659bd9f79c7c55766c1e11bed1f45e","unresolved":false,"context_lines":[{"line_number":498,"context_line":"        :param context: nova.context.RequestContext user auth request context"},{"line_number":499,"context_line":"        :param volume: dict representation of volume being attached to instance"},{"line_number":500,"context_line":"        :param instance: nova.objects.Instance being attached to volume"},{"line_number":501,"context_line":"        :param default_az: Used when [cinder]/cross_az_attach\u003dFalse and a"},{"line_number":502,"context_line":"            specific AZ is not requested by the user for the instance. Should"},{"line_number":503,"context_line":"            only be True before the instance is scheduled to a host."},{"line_number":504,"context_line":"        :returns: None if [cinder]/cross_az_attach\u003dFalse and the instance AZ"},{"line_number":505,"context_line":"            matches the volume AZ. Returns the volume AZ if the"},{"line_number":506,"context_line":"            user did not request a specific AZ for the instance (and"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_6f911f45","line":503,"range":{"start_line":501,"start_character":8,"end_line":503,"end_character":68},"in_reply_to":"3f79a3b5_be124060","updated":"2018-10-12 21:47:28.000000000","message":"Once the instance is scheduled to a host, conductor updates the instance.availability_zone to the AZ that the selected host is in, yes:\n\nhttps://github.com/openstack/nova/blob/20bc0136d0665bafdcd379f19389a0a5ea7bf310/nova/conductor/manager.py#L1287","commit_id":"f19eef6a1dc2072e60de24560239b1742d01b110"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6f646881be4061736ec4b547a60a151915227ce3","unresolved":false,"context_lines":[{"line_number":501,"context_line":"        :param default_az: Used when [cinder]/cross_az_attach\u003dFalse and a"},{"line_number":502,"context_line":"            specific AZ is not requested by the user for the instance. Should"},{"line_number":503,"context_line":"            only be True before the instance is scheduled to a host."},{"line_number":504,"context_line":"        :returns: None if [cinder]/cross_az_attach\u003dFalse and the instance AZ"},{"line_number":505,"context_line":"            matches the volume AZ. Returns the volume AZ if the"},{"line_number":506,"context_line":"            user did not request a specific AZ for the instance (and"},{"line_number":507,"context_line":"            [DEFAULT]/default_schedule_zone is None) and the"},{"line_number":508,"context_line":"            volume AZ does not match [DEFAULT]/default_availability_zone."},{"line_number":509,"context_line":"        :raises: nova.exception.InvalidVolume if [cinder]/cross_az_attach\u003dFalse"},{"line_number":510,"context_line":"            and the instance AZ does not match the volume AZ."},{"line_number":511,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_23dbb1ea","line":508,"range":{"start_line":504,"start_character":8,"end_line":508,"end_character":73},"updated":"2018-10-10 20:56:25.000000000","message":"So, before this change, this method returns None if cross_az_attach\u003dFalse and the instance AZ matches the volume AZ. So that part stays the same. Before this change, the method raises if the instance AZ does not match the volume AZ, if cross_az_attach\u003dFalse.\n\nWhat\u0027s new is this method will return the volume AZ if the instance has no requested AZ and the volume AZ does not match the default AZ.\n\nThis feels weird and like it\u0027s adding a ternary mode of behavior to this method and adding confusion to how this method works. The code comments help, but still. I don\u0027t have a better idea, though.","commit_id":"f19eef6a1dc2072e60de24560239b1742d01b110"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"534fd19a1a659bd9f79c7c55766c1e11bed1f45e","unresolved":false,"context_lines":[{"line_number":501,"context_line":"        :param default_az: Used when [cinder]/cross_az_attach\u003dFalse and a"},{"line_number":502,"context_line":"            specific AZ is not requested by the user for the instance. Should"},{"line_number":503,"context_line":"            only be True before the instance is scheduled to a host."},{"line_number":504,"context_line":"        :returns: None if [cinder]/cross_az_attach\u003dFalse and the instance AZ"},{"line_number":505,"context_line":"            matches the volume AZ. Returns the volume AZ if the"},{"line_number":506,"context_line":"            user did not request a specific AZ for the instance (and"},{"line_number":507,"context_line":"            [DEFAULT]/default_schedule_zone is None) and the"},{"line_number":508,"context_line":"            volume AZ does not match [DEFAULT]/default_availability_zone."},{"line_number":509,"context_line":"        :raises: nova.exception.InvalidVolume if [cinder]/cross_az_attach\u003dFalse"},{"line_number":510,"context_line":"            and the instance AZ does not match the volume AZ."},{"line_number":511,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_cfb953cb","line":508,"range":{"start_line":504,"start_character":8,"end_line":508,"end_character":73},"in_reply_to":"3f79a3b5_23dbb1ea","updated":"2018-10-12 21:47:28.000000000","message":"Yeah it\u0027s super tightly coupled and nasty. I have an alternative idea to how this can be handled in an earlier patch set comment:\n\nhttps://review.openstack.org/#/c/469675/4/nova/compute/api.py@918\n\nI just haven\u0027t tried writing that alternative yet.","commit_id":"f19eef6a1dc2072e60de24560239b1742d01b110"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9beb296c53c571c866792a08cddbdf68a81d06a2","unresolved":false,"context_lines":[{"line_number":501,"context_line":"        :param default_az: Used when [cinder]/cross_az_attach\u003dFalse and a"},{"line_number":502,"context_line":"            specific AZ is not requested by the user for the instance. Should"},{"line_number":503,"context_line":"            only be True before the instance is scheduled to a host."},{"line_number":504,"context_line":"        :returns: None if [cinder]/cross_az_attach\u003dFalse and the instance AZ"},{"line_number":505,"context_line":"            matches the volume AZ. Returns the volume AZ if the"},{"line_number":506,"context_line":"            user did not request a specific AZ for the instance (and"},{"line_number":507,"context_line":"            [DEFAULT]/default_schedule_zone is None) and the"},{"line_number":508,"context_line":"            volume AZ does not match [DEFAULT]/default_availability_zone."},{"line_number":509,"context_line":"        :raises: nova.exception.InvalidVolume if [cinder]/cross_az_attach\u003dFalse"},{"line_number":510,"context_line":"            and the instance AZ does not match the volume AZ."},{"line_number":511,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_c1904745","line":508,"range":{"start_line":504,"start_character":8,"end_line":508,"end_character":73},"in_reply_to":"3f79a3b5_cfb953cb","updated":"2018-10-12 22:50:09.000000000","message":"Ah, I see. I thought about similar alternatives and the complexity and inefficiency that would have to happen if we went a different route.","commit_id":"f19eef6a1dc2072e60de24560239b1742d01b110"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ba635e9a991dda55729905d29fa528dfeab1c9c4","unresolved":false,"context_lines":[{"line_number":538,"context_line":"                # same as our default, return the volume AZ so the caller can"},{"line_number":539,"context_line":"                # put it into the request spec so the instance is scheduled"},{"line_number":540,"context_line":"                # to the same zone as the volume."},{"line_number":541,"context_line":"                if volume_az !\u003d CONF.default_availability_zone:"},{"line_number":542,"context_line":"                    return volume_az  # indication to set in request spec"},{"line_number":543,"context_line":"                # The volume AZ is the same as the default nova AZ"},{"line_number":544,"context_line":"                # so we\u0027ll be OK."},{"line_number":545,"context_line":"                return"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":"            if instance_az !\u003d volume_az:"},{"line_number":548,"context_line":"                msg \u003d _(\"Instance %(instance)s and volume %(vol)s are not in \""}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_f340bf37","line":545,"range":{"start_line":541,"start_character":0,"end_line":545,"end_character":22},"updated":"2019-02-13 16:36:59.000000000","message":"Okay, this is super confusing to me. I think what default_az\u003dTrue really just means \"return the cinder az, if we should be forcing the instance to the same nova az\" right?\n\nIMHO, that\u0027s burying some very nova-specific logic deep in the volume api code. Really it seems like we should just always return the volume az and then do our \"what should we do with the cinder az\" logic above.\n\nWhen reading all the code that has \"attach\" or \"reserve\" in the name, and has a parameter of default_az\u003dTrue, I had assumed that meant \"create the volume in the default cinder az\". Until I came here to find out what the end result was, I had a very different idea.","commit_id":"2ec7c40a5c5ee6417e1d4500ec9e924c5a5ec6d6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dd65206deea0a61f21d61cd0861e457d3442251e","unresolved":false,"context_lines":[{"line_number":538,"context_line":"                # same as our default, return the volume AZ so the caller can"},{"line_number":539,"context_line":"                # put it into the request spec so the instance is scheduled"},{"line_number":540,"context_line":"                # to the same zone as the volume."},{"line_number":541,"context_line":"                if volume_az !\u003d CONF.default_availability_zone:"},{"line_number":542,"context_line":"                    return volume_az  # indication to set in request spec"},{"line_number":543,"context_line":"                # The volume AZ is the same as the default nova AZ"},{"line_number":544,"context_line":"                # so we\u0027ll be OK."},{"line_number":545,"context_line":"                return"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":"            if instance_az !\u003d volume_az:"},{"line_number":548,"context_line":"                msg \u003d _(\"Instance %(instance)s and volume %(vol)s are not in \""}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_8ebd8ab8","line":545,"range":{"start_line":541,"start_character":0,"end_line":545,"end_character":22},"in_reply_to":"9fdfeff1_f340bf37","updated":"2019-02-13 17:08:51.000000000","message":"\u003e IMHO, that\u0027s burying some very nova-specific logic deep in the volume api code.\n\nWell this is all nova-specific logic since cross_az_attach is a nova-only configuration (even though cinder has an option to workaround it [allow_allow_availability_zone_fallback]) but yeah I get what you\u0027re saying. The default_az parameter was needed to distinguish between when the API calls this before scheduling the instance to host (and AZ) and when nova-compute calls this to check a volume it created. But I think I get your point.\n\n\u003e Really it seems like we should just always return the volume az and then do our \"what should we do with the cinder az\" logic above.\n\nYeah that makes sense, but I\u0027d have to think about refactoring this, since if this method doesn\u0027t raise InvalidVolume like it did, then the compute service usage of it changes.\n\nMaybe the API shouldn\u0027t even be calling this method and the API can have a specific check for cross_az_attach and compute can have a specific check for cross_az_attach and that would help avoid the confusion over trying to make this single method do double duty (I would probably just move this method to nova.virt.block_device in that case).","commit_id":"2ec7c40a5c5ee6417e1d4500ec9e924c5a5ec6d6"}]}