)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dd9ea30311fdc1e05d6a11d2c9d8d62f8e5bb77e","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Jiří Suchomel \u003cjsuchome@suse.cz\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-09-18 21:08:17 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Get the instance availability zone before creating volume"},{"line_number":8,"context_line":"so we can pass actual AZ value to volume API."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"Change-Id: I45085ba4c5386ea5be3ad4095c8043c986110a5a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba15a1d1_bbb333e9","line":7,"updated":"2015-09-18 20:30:59.000000000","message":"pep8 is going to fail b/c your commit message title is wrapping past 80 characters.","commit_id":"92b59753639457cd0efec1dcaee88a0973a42d81"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dd9ea30311fdc1e05d6a11d2c9d8d62f8e5bb77e","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Get the instance availability zone before creating volume"},{"line_number":8,"context_line":"so we can pass actual AZ value to volume API."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"Change-Id: I45085ba4c5386ea5be3ad4095c8043c986110a5a"},{"line_number":11,"context_line":"Closes-Bug: #1497253"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba15a1d1_fbe0cbce","line":9,"updated":"2015-09-18 20:30:59.000000000","message":"Please provide more detailed background about what the bug is and why your change fixes it.  It\u0027s not clear to me what the issue is here.  If Cinder has some restrictions/assumptions and Nova is failing to meet those, that should be described here.","commit_id":"92b59753639457cd0efec1dcaee88a0973a42d81"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e2ffa902f142ecef25d748d002e6a449a77020a8","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Get the instance availability zone before creating volume"},{"line_number":8,"context_line":"so we can pass actual AZ value to volume API."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"Change-Id: I45085ba4c5386ea5be3ad4095c8043c986110a5a"},{"line_number":11,"context_line":"Closes-Bug: #1497253"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9a1a9d01_cf597890","line":9,"in_reply_to":"9a1a9d01_4fdb281c","updated":"2015-09-24 18:03:15.000000000","message":"Sorry, typo, I meant to say you\u0027re going to need to set allow_availability_zone_fallback\u003dTrue in cinder.conf to workaround this.","commit_id":"92b59753639457cd0efec1dcaee88a0973a42d81"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"f43d29c83313795e1eed8034be1cab8653f0bad8","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Get the instance availability zone before creating volume"},{"line_number":8,"context_line":"so we can pass actual AZ value to volume API."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"Change-Id: I45085ba4c5386ea5be3ad4095c8043c986110a5a"},{"line_number":11,"context_line":"Closes-Bug: #1497253"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9a1a9d01_eb20d04b","line":9,"in_reply_to":"9a1a9d01_cf597890","updated":"2015-09-25 13:22:12.000000000","message":"I do not think that allow_availability_zone_fallback could help here. \n\nThe failure in my case is because with volume and instance have different AZ, not that cinder\u0027s AZ does not exist or is not avialable.\n\nWith https://review.openstack.org/#/c/157041/ , when cross_az_attach is False, nova should pass it\u0027s AZ info to cinder so it is created with same AZ. I think my patch is mostly extension of this situation for the state when nova\u0027s AZ was not provided. So I think it is correct to (again, as in \nhttps://review.openstack.org/#/c/157041/) pass the same AZ to volume as the one which will be used for instance. The problem was, that it\u0027s not known in time, hence I used get_instance_availability_zone to find out earlier.\n\n(Sorry if I\u0027m (ab)using this commit message discussion for discussion about the change itself.)","commit_id":"92b59753639457cd0efec1dcaee88a0973a42d81"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"24409c1de5a5e61aeaf4e1b0a6fe949308a915d8","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Get the instance availability zone before creating volume"},{"line_number":8,"context_line":"so we can pass actual AZ value to volume API."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"Change-Id: I45085ba4c5386ea5be3ad4095c8043c986110a5a"},{"line_number":11,"context_line":"Closes-Bug: #1497253"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9a1a9d01_4fdb281c","line":9,"in_reply_to":"ba15a1d1_3ea9517c","updated":"2015-09-24 18:02:44.000000000","message":"A lot of the background on this same problem is in this thread now:\n\nhttp://lists.openstack.org/pipermail/openstack-dev/2015-September/075264.html\n\nThe problem is also in the bug report.  If cinder.cross_az_attach is False in nova and you boot an instance from a volume with source\u003dimage, nova creates the volume but doesn\u0027t specify an AZ (since instance.az is None).  Since this is boot from volume, once the volume is created it\u0027s attached and since do_check_attach\u003dTrue in this case, the volume_api.check_attach is called and the instance.az and volume.az are different, so it fails (because cross_az_attach is False).\n\nWhat\u0027s not clear to me is what\u0027s the volume.az when this fails?  Is Cinder providing a default AZ on the volume create?\n\nUltimately I think you\u0027re going to just need to set allow_availability_zone_fallback\u003dFalse in cinder.conf per this change in liberty:\n\nhttps://review.openstack.org/#/c/217857/\n\nAnd we really really need to get this code out of the compute node, or at least validate it higher up the stack before we get to the compute node, and have cinder tell us if the AZ we\u0027re using is going to be OK for creating volumes.","commit_id":"92b59753639457cd0efec1dcaee88a0973a42d81"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"66259b1cc8d942013765b4dd97c3464f4754b1f5","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Get the instance availability zone before creating volume"},{"line_number":8,"context_line":"so we can pass actual AZ value to volume API."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"Change-Id: I45085ba4c5386ea5be3ad4095c8043c986110a5a"},{"line_number":11,"context_line":"Closes-Bug: #1497253"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba15a1d1_3ea9517c","line":9,"in_reply_to":"ba15a1d1_fbe0cbce","updated":"2015-09-18 21:03:40.000000000","message":"Yeah, same concerns here. I wonder why Cinder needs to know the Nova AZ information since we\u0027re speaking about 2 very different failure domains (AZs are related to hosts in Nova, while it relates to volumes in Cinder I guess).\n\nTo be clear, Nova AZs just guarantee that 2 instances having distinct AZs won\u0027t be scheduled on the same host, it makes no assumption about different controlplanes or even dataplanes. If the controller dies or the network node dies, then all hosts behind all the AZs will say bye-bye.","commit_id":"92b59753639457cd0efec1dcaee88a0973a42d81"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9aae654addc51f3199765c39d8976ac82794560e","unresolved":false,"context_lines":[{"line_number":12,"context_line":"But user may not want to explicitly provide a name of AZ at the"},{"line_number":13,"context_line":"time of creating the instance, so that scheduler could distribute"},{"line_number":14,"context_line":"the load evenly and according to available resources."},{"line_number":15,"context_line":"In such case, nova uses None value for availability zone at the time"},{"line_number":16,"context_line":"volume_api.create is called, and cinder creates the volume in whatever"},{"line_number":17,"context_line":"AZ it has available, while none of these components tries to check if"},{"line_number":18,"context_line":"availability zones of volume and instance are the same."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"With this patch, if cross_az_attach is set to False, we try to ensure that"},{"line_number":21,"context_line":"cinder and nova AZ\u0027s are matching, even when AZ wasn\u0027t specified by user. So"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"ff0f0b1f_803e81b5","line":18,"range":{"start_line":15,"start_character":0,"end_line":18,"end_character":55},"updated":"2017-05-24 15:58:16.000000000","message":"Does anything actually fail? Does the cinder volume creation fail because the AZ defined that the instance is in does not exist in Cinder? Or does the volume get created successfully, but the volume is in AZ \u0027nova\u0027 (default) and the instance is in some non-default (non-nova) AZ, so maybe they can\u0027t actually communicate or something.","commit_id":"571ea4eed79689e729e119a0c2fdf7a10bea26b6"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"9303bda6d063eb7c55520b898d63c1d18a77c7ee","unresolved":false,"context_lines":[{"line_number":12,"context_line":"But user may not want to explicitly provide a name of AZ at the"},{"line_number":13,"context_line":"time of creating the instance, so that scheduler could distribute"},{"line_number":14,"context_line":"the load evenly and according to available resources."},{"line_number":15,"context_line":"In such case, nova uses None value for availability zone at the time"},{"line_number":16,"context_line":"volume_api.create is called, and cinder creates the volume in whatever"},{"line_number":17,"context_line":"AZ it has available, while none of these components tries to check if"},{"line_number":18,"context_line":"availability zones of volume and instance are the same."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"With this patch, if cross_az_attach is set to False, we try to ensure that"},{"line_number":21,"context_line":"cinder and nova AZ\u0027s are matching, even when AZ wasn\u0027t specified by user. So"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"ff0f0b1f_006951f7","line":18,"range":{"start_line":15,"start_character":0,"end_line":18,"end_character":55},"in_reply_to":"ff0f0b1f_803e81b5","updated":"2017-05-24 16:26:37.000000000","message":"Matt, I\u0027m not sure at this time, I would have to look again at it. I think this patch got actually replaced by https://review.openstack.org/#/c/366724/","commit_id":"571ea4eed79689e729e119a0c2fdf7a10bea26b6"}],"nova/virt/block_device.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dd9ea30311fdc1e05d6a11d2c9d8d62f8e5bb77e","unresolved":false,"context_lines":[{"line_number":358,"context_line":"               virt_driver, wait_func\u003dNone, do_check_attach\u003dTrue):"},{"line_number":359,"context_line":"        if not self.volume_id:"},{"line_number":360,"context_line":"            av_zone \u003d instance.availability_zone"},{"line_number":361,"context_line":"            if not av_zone and not CONF.cinder.cross_az_attach:"},{"line_number":362,"context_line":"                # If AV zone for instance was not specified, find out its name"},{"line_number":363,"context_line":"                # so we can use the same one for volume"},{"line_number":364,"context_line":"                av_zone \u003d az.get_instance_availability_zone(context, instance)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_5bd0d7f7","line":361,"updated":"2015-09-18 20:30:59.000000000","message":"You need a unit test for this condition.","commit_id":"92b59753639457cd0efec1dcaee88a0973a42d81"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b193fe589e4e6c55f086c00b2d455f81944b17a6","unresolved":false,"context_lines":[{"line_number":358,"context_line":"               virt_driver, wait_func\u003dNone, do_check_attach\u003dTrue):"},{"line_number":359,"context_line":"        if not self.volume_id:"},{"line_number":360,"context_line":"            av_zone \u003d instance.availability_zone"},{"line_number":361,"context_line":"            if not av_zone and not CONF.cinder.cross_az_attach:"},{"line_number":362,"context_line":"                # If AV zone for instance was not specified, find out its name"},{"line_number":363,"context_line":"                # so we can use the same one for volume"},{"line_number":364,"context_line":"                av_zone \u003d az.get_instance_availability_zone(context, instance)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_5baf772b","line":361,"in_reply_to":"ba15a1d1_5bd0d7f7","updated":"2015-09-18 20:46:54.000000000","message":"You should also import the right CONF opt, not implicitely calling it because it\u0027s a global object","commit_id":"92b59753639457cd0efec1dcaee88a0973a42d81"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b193fe589e4e6c55f086c00b2d455f81944b17a6","unresolved":false,"context_lines":[{"line_number":361,"context_line":"            if not av_zone and not CONF.cinder.cross_az_attach:"},{"line_number":362,"context_line":"                # If AV zone for instance was not specified, find out its name"},{"line_number":363,"context_line":"                # so we can use the same one for volume"},{"line_number":364,"context_line":"                av_zone \u003d az.get_instance_availability_zone(context, instance)"},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"            vol \u003d volume_api.create(context, self.volume_size,"},{"line_number":367,"context_line":"                                    \u0027\u0027, \u0027\u0027, image_id\u003dself.image_id,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_3e47114d","line":364,"updated":"2015-09-18 20:46:54.000000000","message":"so, I really dislike having the AZ helper being called by the nova-compute service, I wonder how we could get the real AZ other way...","commit_id":"92b59753639457cd0efec1dcaee88a0973a42d81"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3919cca7de2594f51e89a38affc98e9ddd2c563","unresolved":false,"context_lines":[{"line_number":361,"context_line":"            if not av_zone and not CONF.cinder.cross_az_attach:"},{"line_number":362,"context_line":"                # If AV zone for instance was not specified, find out its name"},{"line_number":363,"context_line":"                # so we can use the same one for volume"},{"line_number":364,"context_line":"                av_zone \u003d az.get_instance_availability_zone(context, instance)"},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"            vol \u003d volume_api.create(context, self.volume_size,"},{"line_number":367,"context_line":"                                    \u0027\u0027, \u0027\u0027, image_id\u003dself.image_id,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_a17a2476","line":364,"in_reply_to":"ba15a1d1_3e02b10c","updated":"2015-09-18 21:30:00.000000000","message":"Hmm, in the boot from volume case, is there a reason we wouldn\u0027t check this in the compute API layer here?\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/api.py#L1318\n\nLike what volume_api.check_attach is doing?","commit_id":"92b59753639457cd0efec1dcaee88a0973a42d81"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"a2782601cb9e52c60f683bc828da41331055b5b7","unresolved":false,"context_lines":[{"line_number":361,"context_line":"            if not av_zone and not CONF.cinder.cross_az_attach:"},{"line_number":362,"context_line":"                # If AV zone for instance was not specified, find out its name"},{"line_number":363,"context_line":"                # so we can use the same one for volume"},{"line_number":364,"context_line":"                av_zone \u003d az.get_instance_availability_zone(context, instance)"},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"            vol \u003d volume_api.create(context, self.volume_size,"},{"line_number":367,"context_line":"                                    \u0027\u0027, \u0027\u0027, image_id\u003dself.image_id,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_2ab816e5","line":364,"in_reply_to":"ba15a1d1_3e02b10c","updated":"2015-09-21 06:58:31.000000000","message":"The call of az.get_instance_availabiltiy_zone I\u0027ve tried to add here was indeed inspired by its presens in volume_api.check_attach.\n\nBut check_attach itself was called too  late, that\u0027s why I added separate call to get_instance_availabiltiy_zone to this place in virt/block_device.py","commit_id":"92b59753639457cd0efec1dcaee88a0973a42d81"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"66259b1cc8d942013765b4dd97c3464f4754b1f5","unresolved":false,"context_lines":[{"line_number":361,"context_line":"            if not av_zone and not CONF.cinder.cross_az_attach:"},{"line_number":362,"context_line":"                # If AV zone for instance was not specified, find out its name"},{"line_number":363,"context_line":"                # so we can use the same one for volume"},{"line_number":364,"context_line":"                av_zone \u003d az.get_instance_availability_zone(context, instance)"},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"            vol \u003d volume_api.create(context, self.volume_size,"},{"line_number":367,"context_line":"                                    \u0027\u0027, \u0027\u0027, image_id\u003dself.image_id,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_de861516","line":364,"in_reply_to":"ba15a1d1_3e47114d","updated":"2015-09-18 21:03:40.000000000","message":"Sorry, I was very short in explanation in my previous comment, let me clarify.\n\nThe AZ helper was originally planned for being used by the Nova API service and not by the other services, and that rule is still valid now. If you grep the calls to that method, you\u0027ll see they\u0027re all done within the n-api process.\n\nSince the AZ information provided in the Instance object corresponds to what the user asked, not what he got (ie. if no AZ provided, it will default to instance.az \u003d None while the default host AZ is \"nova\"). \n\nI still don\u0027t know why Cinder needs to know the Nova AZ for that instance - because Nova AZs and Cinder AZs are very different failure domains, but given you would still need to provide that, I\u0027d recommend to provide a new method for the Instance object (not needed to be a remotable one) which would be called .get_host_az() or something equivalent and would return the real AZ information attached to the host the instance belongs to.\nThat way, you wouldn\u0027t need to call the AZ helper, rather just called the new method you\u0027d add.","commit_id":"92b59753639457cd0efec1dcaee88a0973a42d81"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0301559b543421b0d8a7c0759b42a7d7c1161a45","unresolved":false,"context_lines":[{"line_number":361,"context_line":"            if not av_zone and not CONF.cinder.cross_az_attach:"},{"line_number":362,"context_line":"                # If AV zone for instance was not specified, find out its name"},{"line_number":363,"context_line":"                # so we can use the same one for volume"},{"line_number":364,"context_line":"                av_zone \u003d az.get_instance_availability_zone(context, instance)"},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"            vol \u003d volume_api.create(context, self.volume_size,"},{"line_number":367,"context_line":"                                    \u0027\u0027, \u0027\u0027, image_id\u003dself.image_id,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_3e02b10c","line":364,"in_reply_to":"ba15a1d1_de861516","updated":"2015-09-18 21:24:45.000000000","message":"az.get_instance_availabiltiy_zone is called from volume_api.check_attach:\n\nhttps://github.com/openstack/nova/blob/master/nova/volume/cinder.py#L296\n\nWhich is something that can already be called from this module (L236).  Having said that, I don\u0027t think the compute manager actually calls block_device.attach with do_check_attach\u003dTrue.  The nova-api code actually does the volume_api.check_attach call:\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/api.py#L3065\n\nThis is the call to attach from the compute manager:\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/manager.py#L4553","commit_id":"92b59753639457cd0efec1dcaee88a0973a42d81"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e9d1d481ee29e5d7ad25c2d8a79ec5d9bb532d17","unresolved":false,"context_lines":[{"line_number":86,"context_line":"    # NoValidHost error."},{"line_number":87,"context_line":"    av_zone \u003d instance.availability_zone"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    if not av_zone:"},{"line_number":90,"context_line":"        # If availability zone for instance was not explicitely specified,"},{"line_number":91,"context_line":"        # find out host\u0027s AZ, so it can be used also for volume, thus"},{"line_number":92,"context_line":"        # fulfilling what cinder.cross_az_attach\u003dFalse expects."}],"source_content_type":"text/x-python","patch_set":8,"id":"7aa08908_9a824351","line":89,"updated":"2016-06-06 14:02:26.000000000","message":"There are no unit tests added for any of this.","commit_id":"af530aec87a62a6be4efed067f13b1480d75dcbd"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e9d1d481ee29e5d7ad25c2d8a79ec5d9bb532d17","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        aggregates \u003d objects.AggregateList.get_by_host(context, host,"},{"line_number":95,"context_line":"                                                   key\u003d\u0027availability_zone\u0027)"},{"line_number":96,"context_line":"        if aggregates:"},{"line_number":97,"context_line":"            av_zone \u003d aggregates[0].metadata[\u0027availability_zone\u0027]"},{"line_number":98,"context_line":"        else:"},{"line_number":99,"context_line":"            av_zone \u003d CONF.default_availability_zone"},{"line_number":100,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7aa08908_cf0febdf","line":97,"range":{"start_line":97,"start_character":22,"end_line":97,"end_character":35},"updated":"2016-06-06 14:02:26.000000000","message":"What if the compute is in more than one aggregate? Then we\u0027re randomly picking the AZ, which might be OK, but still might not exist in Cinder. You could loop over the aggregates and check with Cinder until you find an AZ that exists there, and use that match - but it\u0027s more work and something that sucks to do in the compute. This kind of validation should really happen in the API, but that\u0027s probably a much bigger change.","commit_id":"af530aec87a62a6be4efed067f13b1480d75dcbd"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"3e0fb9ebb368ed1b98ec4321415fc5674dedfe9d","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        aggregates \u003d objects.AggregateList.get_by_host(context, host,"},{"line_number":95,"context_line":"                                                   key\u003d\u0027availability_zone\u0027)"},{"line_number":96,"context_line":"        if aggregates:"},{"line_number":97,"context_line":"            av_zone \u003d aggregates[0].metadata[\u0027availability_zone\u0027]"},{"line_number":98,"context_line":"        else:"},{"line_number":99,"context_line":"            av_zone \u003d CONF.default_availability_zone"},{"line_number":100,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"5a9d85d2_700fc5e1","line":97,"range":{"start_line":97,"start_character":22,"end_line":97,"end_character":35},"in_reply_to":"7aa08908_cf0febdf","updated":"2016-06-20 16:10:49.000000000","message":"For this one, I think picking the first one is (almost) as good as anything. Yes, it\u0027s random, but the proposed solution is already a backup for likely corner case, so IMHO it is sufficient to be \u0027good enough\u0027...","commit_id":"af530aec87a62a6be4efed067f13b1480d75dcbd"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e9d1d481ee29e5d7ad25c2d8a79ec5d9bb532d17","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        if aggregates:"},{"line_number":97,"context_line":"            av_zone \u003d aggregates[0].metadata[\u0027availability_zone\u0027]"},{"line_number":98,"context_line":"        else:"},{"line_number":99,"context_line":"            av_zone \u003d CONF.default_availability_zone"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    return av_zone"},{"line_number":102,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7aa08908_0fb1f38f","line":99,"updated":"2016-06-06 14:02:26.000000000","message":"I\u0027m not really sure about this. Alternatively we could fail but that\u0027s really something that should happen in the API (again), since by the time you get to the compute it\u0027s a nasty failure experience.\n\nThe default AZ in nova.conf might not exist in Cinder, even though by default it does. And on the cinder side they might have configured it to not fail when the AZ doesn\u0027t exist - which is doubly confusing. Really this is all stuff that should be hidden from the user since the behavior is all built into config options, which is pretty bad.\n\nI don\u0027t really have a good answer or alternative though besides moving this stuff to the nova-api where we can fail faster before getting to a compute. And in the API we could check if cross_az_attach was False and an AZ wasn\u0027t specified, then we could just fail (or use the default).\n\nThis problem might be worth bringing up in the openstack-operators mailing list to see how people using the cross_az_attach\u003dFalse option would expect this to behave.\n\nI think this would also at least require a release note in this change given it\u0027s a behavior change.","commit_id":"af530aec87a62a6be4efed067f13b1480d75dcbd"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"784106f2074e9199a4c0da35e7fcfe186d48310d","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        if aggregates:"},{"line_number":97,"context_line":"            av_zone \u003d aggregates[0].metadata[\u0027availability_zone\u0027]"},{"line_number":98,"context_line":"        else:"},{"line_number":99,"context_line":"            av_zone \u003d CONF.default_availability_zone"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    return av_zone"},{"line_number":102,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_6f318e0b","line":99,"updated":"2016-07-04 17:44:23.000000000","message":"We should really use this util function:\nhttps://github.com/openstack/nova/blob/bc5035343d366a18cae587f92ecb4e871aba974a/nova/availability_zones.py#L89\n\nKeeps things DRY, and that should help make that test simpler too.","commit_id":"571ea4eed79689e729e119a0c2fdf7a10bea26b6"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"ab34ab09390119f4d32043accc737e4fecf663ff","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        if aggregates:"},{"line_number":97,"context_line":"            av_zone \u003d aggregates[0].metadata[\u0027availability_zone\u0027]"},{"line_number":98,"context_line":"        else:"},{"line_number":99,"context_line":"            av_zone \u003d CONF.default_availability_zone"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    return av_zone"},{"line_number":102,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"1a95cdbc_e862ded7","line":99,"in_reply_to":"1a95cdbc_99b60f89","updated":"2016-10-05 09:12:49.000000000","message":"Indeed. That\u0027s what I had in Patch set 2 :-)","commit_id":"571ea4eed79689e729e119a0c2fdf7a10bea26b6"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"89e9842c484aee3e890541a1f9af7e824ab38269","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        if aggregates:"},{"line_number":97,"context_line":"            av_zone \u003d aggregates[0].metadata[\u0027availability_zone\u0027]"},{"line_number":98,"context_line":"        else:"},{"line_number":99,"context_line":"            av_zone \u003d CONF.default_availability_zone"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    return av_zone"},{"line_number":102,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"dada55a8_d494a645","line":99,"in_reply_to":"1aa78d24_6f318e0b","updated":"2016-07-22 17:56:54.000000000","message":"Makes sense to me... but I have used availability_zones.py already in Patch set 2, and it was refused at that time....","commit_id":"571ea4eed79689e729e119a0c2fdf7a10bea26b6"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"f227892042992983ec22c5bb0d5c67bfe39c9969","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        if aggregates:"},{"line_number":97,"context_line":"            av_zone \u003d aggregates[0].metadata[\u0027availability_zone\u0027]"},{"line_number":98,"context_line":"        else:"},{"line_number":99,"context_line":"            av_zone \u003d CONF.default_availability_zone"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    return av_zone"},{"line_number":102,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"1a95cdbc_99b60f89","line":99,"in_reply_to":"dada55a8_d494a645","updated":"2016-10-05 08:45:35.000000000","message":"Hmm, isn\u0027t https://review.openstack.org/#/c/366724/ a simpler way to achieve the very same thing?","commit_id":"571ea4eed79689e729e119a0c2fdf7a10bea26b6"}]}
