)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"25928e9c0d1485adc2c19c99027a5448d5e2c8bd","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Check for multiattach before removing connections"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"With the addition of multiattach we need to ensure that we"},{"line_number":10,"context_line":"don\u0027t make brick calls to remove connections on detach volume"},{"line_number":11,"context_line":"if that volume is attached to another Instance on the same"},{"line_number":12,"context_line":"node."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This patch adds some logic in the virt driver _disconnect_volume"},{"line_number":15,"context_line":"method to check the BDM\u0027s and verify that we don\u0027t have any"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1f9dbf25_b64ffe90","line":12,"range":{"start_line":9,"start_character":0,"end_line":12,"end_character":5},"updated":"2018-03-02 14:43:12.000000000","message":"I was going to ask about the shared_targets lock but see Ildiko already did in the bug report.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a09959376961216ebf4994db9befbed0d65a379d","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Check for multiattach before removing connections"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"With the addition of multiattach we need to ensure that we"},{"line_number":10,"context_line":"don\u0027t make brick calls to remove connections on detach volume"},{"line_number":11,"context_line":"if that volume is attached to another Instance on the same"},{"line_number":12,"context_line":"node."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This patch adds some logic in the virt driver _disconnect_volume"},{"line_number":15,"context_line":"method to check the BDM\u0027s and verify that we don\u0027t have any"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1f9dbf25_b0cff9ae","line":12,"range":{"start_line":9,"start_character":0,"end_line":12,"end_character":5},"in_reply_to":"1f9dbf25_9c9541e7","updated":"2018-03-03 10:26:41.000000000","message":"Up to you. It\u0027s kind of tribal knowledge, but might help to remind us that when we get into this code patch in the driver, we have a lock so we shouldn\u0027t need to worry about races to disconnect on the same compute host. So if we figure out the attachment count via BDMs or Cinder API, it should be race-free within the lock either way.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"da3618cb99e15704aa5b2444ecdd882a2347d5f8","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Check for multiattach before removing connections"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"With the addition of multiattach we need to ensure that we"},{"line_number":10,"context_line":"don\u0027t make brick calls to remove connections on detach volume"},{"line_number":11,"context_line":"if that volume is attached to another Instance on the same"},{"line_number":12,"context_line":"node."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This patch adds some logic in the virt driver _disconnect_volume"},{"line_number":15,"context_line":"method to check the BDM\u0027s and verify that we don\u0027t have any"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1f9dbf25_9c9541e7","line":12,"range":{"start_line":9,"start_character":0,"end_line":12,"end_character":5},"in_reply_to":"1f9dbf25_b64ffe90","updated":"2018-03-02 16:14:09.000000000","message":"Yeah, do we want to reference that in the commit message as well?","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"}],"nova/objects/block_device.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"25928e9c0d1485adc2c19c99027a5448d5e2c8bd","unresolved":false,"context_lines":[{"line_number":323,"context_line":"                context, volume_id, _expected_cols(expected_attrs))"},{"line_number":324,"context_line":"        if not db_bdms:"},{"line_number":325,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"},{"line_number":326,"context_line":"        if len(db_bdms) \u003e 1:"},{"line_number":327,"context_line":"            raise exception.VolumeBDMIsMultiAttach(volume_id\u003dvolume_id)"},{"line_number":328,"context_line":"        return cls._from_db_object(context, cls(), db_bdms[0],"},{"line_number":329,"context_line":"                                   expected_attrs\u003dexpected_attrs)"},{"line_number":330,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_99ff9300","line":327,"range":{"start_line":326,"start_character":8,"end_line":327,"end_character":71},"updated":"2018-03-02 14:43:12.000000000","message":"Could we call this method, and if we get this exception, we know (albeit a bit grossly since it\u0027s tightly coupled to the logic in this interface) that the volume is attached to more than one instance, and therefore we need to avoid the disconnect for attachments on the same host (how do we know the instances that the volume are attached to are on the same host via the bdm? Maybe I haven\u0027t gotten that far yet).","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0b6639e99c935b014ccd61383055165b19696981","unresolved":false,"context_lines":[{"line_number":323,"context_line":"                context, volume_id, _expected_cols(expected_attrs))"},{"line_number":324,"context_line":"        if not db_bdms:"},{"line_number":325,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"},{"line_number":326,"context_line":"        if len(db_bdms) \u003e 1:"},{"line_number":327,"context_line":"            raise exception.VolumeBDMIsMultiAttach(volume_id\u003dvolume_id)"},{"line_number":328,"context_line":"        return cls._from_db_object(context, cls(), db_bdms[0],"},{"line_number":329,"context_line":"                                   expected_attrs\u003dexpected_attrs)"},{"line_number":330,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_f9fd77b1","line":327,"range":{"start_line":326,"start_character":8,"end_line":327,"end_character":71},"in_reply_to":"1f9dbf25_99ff9300","updated":"2018-03-02 14:47:37.000000000","message":"Alternatively, if we have to, we could do two patches:\n\n1. A backportable patch that calls the volume API from the virt driver to get the list of attachments for the volume, and for any instances in the list of attachments that aren\u0027t the instance we care about, we then use that list to get those instances and see which host they are on - ugly, but it\u0027s something we could backport w/o RPC API changes.\n\n2. Then we could remove the volume API changes and use something like what this patch is doing with a new BlockDeviceMappingList method and we don\u0027t backport it. Having said that, the BDM query is a touch racy since it depends on the nova db rather than the volume API, but let\u0027s boil that ocean later.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"da3618cb99e15704aa5b2444ecdd882a2347d5f8","unresolved":false,"context_lines":[{"line_number":323,"context_line":"                context, volume_id, _expected_cols(expected_attrs))"},{"line_number":324,"context_line":"        if not db_bdms:"},{"line_number":325,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"},{"line_number":326,"context_line":"        if len(db_bdms) \u003e 1:"},{"line_number":327,"context_line":"            raise exception.VolumeBDMIsMultiAttach(volume_id\u003dvolume_id)"},{"line_number":328,"context_line":"        return cls._from_db_object(context, cls(), db_bdms[0],"},{"line_number":329,"context_line":"                                   expected_attrs\u003dexpected_attrs)"},{"line_number":330,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_c702160a","line":327,"range":{"start_line":326,"start_character":8,"end_line":327,"end_character":71},"in_reply_to":"1f9dbf25_99ff9300","updated":"2018-03-02 16:14:09.000000000","message":"I tried that, the problem I ran into was that it doesn\u0027t indicate if theyr\u0027e on the same host or not and as such I can\u0027t actually retrieve the BDM\u0027s of interest.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a09959376961216ebf4994db9befbed0d65a379d","unresolved":false,"context_lines":[{"line_number":323,"context_line":"                context, volume_id, _expected_cols(expected_attrs))"},{"line_number":324,"context_line":"        if not db_bdms:"},{"line_number":325,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"},{"line_number":326,"context_line":"        if len(db_bdms) \u003e 1:"},{"line_number":327,"context_line":"            raise exception.VolumeBDMIsMultiAttach(volume_id\u003dvolume_id)"},{"line_number":328,"context_line":"        return cls._from_db_object(context, cls(), db_bdms[0],"},{"line_number":329,"context_line":"                                   expected_attrs\u003dexpected_attrs)"},{"line_number":330,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_70c17180","line":327,"range":{"start_line":326,"start_character":8,"end_line":327,"end_character":71},"in_reply_to":"1f9dbf25_c702160a","updated":"2018-03-03 10:26:41.000000000","message":"\u003e I tried that, the problem I ran into was that it doesn\u0027t indicate\n \u003e if theyr\u0027e on the same host or not and as such I can\u0027t actually\n \u003e retrieve the BDM\u0027s of interest.\n\nWhich thing did you try, calling this method and handling VolumeBDMIsMultiAttach? I guess that makes sense, because VolumeBDMIsMultiAttach tells us the volume is attached to more than one instance, but not that the volume is attached to more than one instance *on the same host*. So I guess for a backportable fix we need to get the attachments from cinder, pull the instances from the list of attachments, and then figure out if those are on the same host or not.\n\nOn master we could follow up and use your new BDMList method below, then we don\u0027t have to call out to Cinder from the virt driver, which might be a bit cleaner.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"da3618cb99e15704aa5b2444ecdd882a2347d5f8","unresolved":false,"context_lines":[{"line_number":323,"context_line":"                context, volume_id, _expected_cols(expected_attrs))"},{"line_number":324,"context_line":"        if not db_bdms:"},{"line_number":325,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"},{"line_number":326,"context_line":"        if len(db_bdms) \u003e 1:"},{"line_number":327,"context_line":"            raise exception.VolumeBDMIsMultiAttach(volume_id\u003dvolume_id)"},{"line_number":328,"context_line":"        return cls._from_db_object(context, cls(), db_bdms[0],"},{"line_number":329,"context_line":"                                   expected_attrs\u003dexpected_attrs)"},{"line_number":330,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_47460624","line":327,"range":{"start_line":326,"start_character":8,"end_line":327,"end_character":71},"in_reply_to":"1f9dbf25_f9fd77b1","updated":"2018-03-02 16:14:09.000000000","message":"I think there might be a version of what you suggest that isn\u0027t bad.  All of the info is available in the attachment objects from Cinder so I could just pull those over if \u003e1 and use that data without using Nova BDM\u0027s at all.\n\nI had proposed something like that in the past but it was nixed due to fears of race conditions.\n\nThe other solution at one point was returning a \"safe to disconnect\" value on cinder.attachment_delete.  That was frowned upon by some (I don\u0027t remember why exactly) but there\u0027s been some changes in the flow since that was initially done and the problem is that the connection is terminated before calling cinder.attachment_delete now so that won\u0027t work without an explicit call out to Cinder for the info.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"25928e9c0d1485adc2c19c99027a5448d5e2c8bd","unresolved":false,"context_lines":[{"line_number":383,"context_line":"    # Version 1.15: BlockDeviceMapping \u003c\u003d version 1.14"},{"line_number":384,"context_line":"    # Version 1.16: BlockDeviceMapping \u003c\u003d version 1.15"},{"line_number":385,"context_line":"    # Version 1.17: Add get_by_instance_uuids()"},{"line_number":386,"context_line":"    # Version 1.18: Add get_by_volume_id()"},{"line_number":387,"context_line":"    VERSION \u003d \u00271.18\u0027"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_365bee54","line":386,"updated":"2018-03-02 14:43:12.000000000","message":"We can\u0027t backport this, it\u0027s an RPC API change.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"da3618cb99e15704aa5b2444ecdd882a2347d5f8","unresolved":false,"context_lines":[{"line_number":383,"context_line":"    # Version 1.15: BlockDeviceMapping \u003c\u003d version 1.14"},{"line_number":384,"context_line":"    # Version 1.16: BlockDeviceMapping \u003c\u003d version 1.15"},{"line_number":385,"context_line":"    # Version 1.17: Add get_by_instance_uuids()"},{"line_number":386,"context_line":"    # Version 1.18: Add get_by_volume_id()"},{"line_number":387,"context_line":"    VERSION \u003d \u00271.18\u0027"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_bc983d1b","line":386,"in_reply_to":"1f9dbf25_365bee54","updated":"2018-03-02 16:14:09.000000000","message":"Snap, yes I forgot about no RPC backports.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8091b11d01ad8711c908e3bbea882dd839d993b","unresolved":false,"context_lines":[{"line_number":6745,"context_line":"        mock_encryptor.detach_volume.called_once_with(self.context,"},{"line_number":6746,"context_line":"                                                      **encryption)"},{"line_number":6747,"context_line":""},{"line_number":6748,"context_line":"    def _fake_get_attachment(obj, context, attachment_id):"},{"line_number":6749,"context_line":"        if attachment_id \u003d\u003d \"9402c249-99df-4f72-89e7-fd611493ee5d\":"},{"line_number":6750,"context_line":"            return {\u0027connection_info\u0027: {"},{"line_number":6751,"context_line":"                \u0027status\u0027: \u0027attached\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff6b8bd7_fc04a5c8","line":6748,"updated":"2018-03-06 16:47:17.000000000","message":"This can be removed.","commit_id":"7e9f01cca255ff9b6f619a2db70078430408e44c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8091b11d01ad8711c908e3bbea882dd839d993b","unresolved":false,"context_lines":[{"line_number":6765,"context_line":"    @mock.patch(\u0027nova.objects.InstanceList.get_uuids_by_host\u0027)"},{"line_number":6766,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver._get_volume_driver\u0027)"},{"line_number":6767,"context_line":"    @mock.patch(\u0027nova.volume.cinder.API.get\u0027)"},{"line_number":6768,"context_line":"    @mock.patch(\u0027nova.volume.cinder.API.attachment_get\u0027)"},{"line_number":6769,"context_line":"    def test_disconnect_multiattach_single_connection("},{"line_number":6770,"context_line":"            self, mock_attachment_get,"},{"line_number":6771,"context_line":"            mock_volume_get, mock_get_volume_driver,"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff6b8bd7_fc5fc5e2","line":6768,"range":{"start_line":6768,"start_character":4,"end_line":6768,"end_character":56},"updated":"2018-03-06 16:47:17.000000000","message":"Can remove this now since it\u0027s no longer used.","commit_id":"7e9f01cca255ff9b6f619a2db70078430408e44c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8091b11d01ad8711c908e3bbea882dd839d993b","unresolved":false,"context_lines":[{"line_number":6776,"context_line":"            spec\u003dvolume_drivers.LibvirtBaseVolumeDriver)"},{"line_number":6777,"context_line":"        mock_get_volume_driver.return_value \u003d mock_volume_driver"},{"line_number":6778,"context_line":""},{"line_number":6779,"context_line":"        attachments \u003d ("},{"line_number":6780,"context_line":"            [(\u002770ab645f-6ffc-406a-b3d2-5007a0c01b82\u0027,"},{"line_number":6781,"context_line":"              {\u0027mountpoint\u0027: u\u0027/dev/vdb\u0027,"},{"line_number":6782,"context_line":"               \u0027attachment_id\u0027: u\u00279402c249-99df-4f72-89e7-fd611493ee5d\u0027}),"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff6b8bd7_9cf569ce","line":6779,"updated":"2018-03-06 16:47:17.000000000","message":"OK so the volume is attached to two servers, but only 00803490-f768-4049-aa7d-151f54e6311e is on this host, per the mock_get_instances mock below. Makes sense.","commit_id":"7e9f01cca255ff9b6f619a2db70078430408e44c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8091b11d01ad8711c908e3bbea882dd839d993b","unresolved":false,"context_lines":[{"line_number":6807,"context_line":"    @mock.patch(\u0027nova.objects.InstanceList.get_uuids_by_host\u0027)"},{"line_number":6808,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver._get_volume_driver\u0027)"},{"line_number":6809,"context_line":"    @mock.patch(\u0027nova.volume.cinder.API.get\u0027)"},{"line_number":6810,"context_line":"    @mock.patch(\u0027nova.volume.cinder.API.attachment_get\u0027)"},{"line_number":6811,"context_line":"    def test_disconnect_multiattach_multi_connection("},{"line_number":6812,"context_line":"            self, mock_attachment_get,"},{"line_number":6813,"context_line":"            mock_volume_get, mock_get_volume_driver,"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff6b8bd7_3c021db6","line":6810,"updated":"2018-03-06 16:47:17.000000000","message":"same","commit_id":"7e9f01cca255ff9b6f619a2db70078430408e44c"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"25928e9c0d1485adc2c19c99027a5448d5e2c8bd","unresolved":false,"context_lines":[{"line_number":1252,"context_line":"            # instances sharing this connection"},{"line_number":1253,"context_line":"            # iterate through to see if any match instance.host"},{"line_number":1254,"context_line":"            bdm_list \u003d objects.BlockDeviceMappingList.get_by_volume_id("},{"line_number":1255,"context_line":"                nova_context.get_admin_context(), connection_info[\u0027volume_id\u0027])"},{"line_number":1256,"context_line":"            if len(bdm_list) \u003e 1:"},{"line_number":1257,"context_line":"                for bdm in bdm_list:"},{"line_number":1258,"context_line":"                    if bdm.deleted:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_390e7f32","line":1255,"range":{"start_line":1255,"start_character":16,"end_line":1255,"end_character":48},"updated":"2018-03-02 14:43:12.000000000","message":"Should be able to just use context here...unless the volume is attached to instances from different projects (somehow?). Could also just use an elevated version of the context, but that\u0027s what this is doing essentially I guess.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"25928e9c0d1485adc2c19c99027a5448d5e2c8bd","unresolved":false,"context_lines":[{"line_number":1252,"context_line":"            # instances sharing this connection"},{"line_number":1253,"context_line":"            # iterate through to see if any match instance.host"},{"line_number":1254,"context_line":"            bdm_list \u003d objects.BlockDeviceMappingList.get_by_volume_id("},{"line_number":1255,"context_line":"                nova_context.get_admin_context(), connection_info[\u0027volume_id\u0027])"},{"line_number":1256,"context_line":"            if len(bdm_list) \u003e 1:"},{"line_number":1257,"context_line":"                for bdm in bdm_list:"},{"line_number":1258,"context_line":"                    if bdm.deleted:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_59226ba6","line":1255,"range":{"start_line":1255,"start_character":50,"end_line":1255,"end_character":78},"updated":"2018-03-02 14:43:12.000000000","message":"connection_info[\u0027volume_id\u0027] is not set for all types of volumes.\n\nNeed to use:\n\ndriver_block_device.get_volume_id(connection_info)","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"02a09e04e1b8b61c77ae7814e07ee72455ba6ecc","unresolved":false,"context_lines":[{"line_number":1252,"context_line":"            # instances sharing this connection"},{"line_number":1253,"context_line":"            # iterate through to see if any match instance.host"},{"line_number":1254,"context_line":"            bdm_list \u003d objects.BlockDeviceMappingList.get_by_volume_id("},{"line_number":1255,"context_line":"                nova_context.get_admin_context(), connection_info[\u0027volume_id\u0027])"},{"line_number":1256,"context_line":"            if len(bdm_list) \u003e 1:"},{"line_number":1257,"context_line":"                for bdm in bdm_list:"},{"line_number":1258,"context_line":"                    if bdm.deleted:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_d08ac519","line":1255,"range":{"start_line":1255,"start_character":16,"end_line":1255,"end_character":48},"in_reply_to":"1f9dbf25_30ec49f2","updated":"2018-03-03 11:17:46.000000000","message":"I\u0027ve posted patch https://review.openstack.org/549411 to pass the user context down to this method so we can get the volume information from the cinder API in here, and from the volume we have the list of attachments, and from those we have the list of server IDs, and from those we can get the list of instance UUIDs for instances running on this host (InstanceList.get_uuids_by_host(context, instance.host), and then we can cross-check that list of instance uuids from nova\u0027s db against the list of attachment server IDs and bingo.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"da3618cb99e15704aa5b2444ecdd882a2347d5f8","unresolved":false,"context_lines":[{"line_number":1252,"context_line":"            # instances sharing this connection"},{"line_number":1253,"context_line":"            # iterate through to see if any match instance.host"},{"line_number":1254,"context_line":"            bdm_list \u003d objects.BlockDeviceMappingList.get_by_volume_id("},{"line_number":1255,"context_line":"                nova_context.get_admin_context(), connection_info[\u0027volume_id\u0027])"},{"line_number":1256,"context_line":"            if len(bdm_list) \u003e 1:"},{"line_number":1257,"context_line":"                for bdm in bdm_list:"},{"line_number":1258,"context_line":"                    if bdm.deleted:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_9cbea163","line":1255,"range":{"start_line":1255,"start_character":16,"end_line":1255,"end_character":48},"in_reply_to":"1f9dbf25_390e7f32","updated":"2018-03-02 16:14:09.000000000","message":"context is None here IIRC, I\u0027ve never gone back and figured out why that is, perhaps another bug.  You caught me being lazy again here.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"da3618cb99e15704aa5b2444ecdd882a2347d5f8","unresolved":false,"context_lines":[{"line_number":1252,"context_line":"            # instances sharing this connection"},{"line_number":1253,"context_line":"            # iterate through to see if any match instance.host"},{"line_number":1254,"context_line":"            bdm_list \u003d objects.BlockDeviceMappingList.get_by_volume_id("},{"line_number":1255,"context_line":"                nova_context.get_admin_context(), connection_info[\u0027volume_id\u0027])"},{"line_number":1256,"context_line":"            if len(bdm_list) \u003e 1:"},{"line_number":1257,"context_line":"                for bdm in bdm_list:"},{"line_number":1258,"context_line":"                    if bdm.deleted:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_7cbb9571","line":1255,"range":{"start_line":1255,"start_character":50,"end_line":1255,"end_character":78},"in_reply_to":"1f9dbf25_59226ba6","updated":"2018-03-02 16:14:09.000000000","message":"Yep, good point, I\u0027ll certainly convert these to get calls for safety.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"7114f1e5389c68fa7c6992b7c835dfc62e9328ef","unresolved":false,"context_lines":[{"line_number":1252,"context_line":"            # instances sharing this connection"},{"line_number":1253,"context_line":"            # iterate through to see if any match instance.host"},{"line_number":1254,"context_line":"            bdm_list \u003d objects.BlockDeviceMappingList.get_by_volume_id("},{"line_number":1255,"context_line":"                nova_context.get_admin_context(), connection_info[\u0027volume_id\u0027])"},{"line_number":1256,"context_line":"            if len(bdm_list) \u003e 1:"},{"line_number":1257,"context_line":"                for bdm in bdm_list:"},{"line_number":1258,"context_line":"                    if bdm.deleted:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_bc62783d","line":1255,"range":{"start_line":1255,"start_character":16,"end_line":1255,"end_character":48},"in_reply_to":"1f9dbf25_9cbea163","updated":"2018-03-02 22:58:23.000000000","message":"Yeah, so context is explicitly None (see lyarwoods comment in the code about avoiding signature changes in the drivers), and the standard nova_context.get_admin_context is fine for internal Nova calls here; however it isn\u0027t permitted via policy to make calls out to Cinder API to get updates from Cinder on attachments.\n\nRather annoying; so I supppose it\u0027s back to using the try/catch for now at least, and cleaning up the other items.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a09959376961216ebf4994db9befbed0d65a379d","unresolved":false,"context_lines":[{"line_number":1252,"context_line":"            # instances sharing this connection"},{"line_number":1253,"context_line":"            # iterate through to see if any match instance.host"},{"line_number":1254,"context_line":"            bdm_list \u003d objects.BlockDeviceMappingList.get_by_volume_id("},{"line_number":1255,"context_line":"                nova_context.get_admin_context(), connection_info[\u0027volume_id\u0027])"},{"line_number":1256,"context_line":"            if len(bdm_list) \u003e 1:"},{"line_number":1257,"context_line":"                for bdm in bdm_list:"},{"line_number":1258,"context_line":"                    if bdm.deleted:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_d03725b0","line":1255,"range":{"start_line":1255,"start_character":16,"end_line":1255,"end_character":48},"in_reply_to":"1f9dbf25_bc62783d","updated":"2018-03-03 10:26:41.000000000","message":"Hmm, so starting in queens we do now have a [cinder] auth section in nova.conf so operators can add service/admin credentials so nova periodic tasks and such can use the cinder API. We could rely on that, however, it\u0027s optional config and something people upgrading *to* Queens wouldn\u0027t have set. If we did use the cinder API here, we\u0027d need to add a release note about the need to configure [cinder] in nova.conf for auth if you\u0027re going to enable multiattach in your cloud.\n\nAlternatively, we figure out how to pass down the real user context to this method so it\u0027s not None (I\u0027ll have to investigate Lee\u0027s note about this).","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d71447f082278acd71f05f0e23c85f411ae7a8d4","unresolved":false,"context_lines":[{"line_number":1252,"context_line":"            # instances sharing this connection"},{"line_number":1253,"context_line":"            # iterate through to see if any match instance.host"},{"line_number":1254,"context_line":"            bdm_list \u003d objects.BlockDeviceMappingList.get_by_volume_id("},{"line_number":1255,"context_line":"                nova_context.get_admin_context(), connection_info[\u0027volume_id\u0027])"},{"line_number":1256,"context_line":"            if len(bdm_list) \u003e 1:"},{"line_number":1257,"context_line":"                for bdm in bdm_list:"},{"line_number":1258,"context_line":"                    if bdm.deleted:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_30ec49f2","line":1255,"range":{"start_line":1255,"start_character":16,"end_line":1255,"end_character":48},"in_reply_to":"1f9dbf25_d03725b0","updated":"2018-03-03 10:54:43.000000000","message":"Looking at the Cinder API, we can list attachments but there is no way to filter attachments based on volume_id - like a filter query param to the GET /attachments API. So that\u0027s unfortunate.\n\nThe volume details will have the list of attachments, and conveniently for us, the list of attachments has a \u0027host_name\u0027 field which has the host the attachment is connected to - however, see bug 1740950; we discussed that bug at the PTG and said Cinder would add a policy rule to mask that field in the response based on a policy rule, defaulting to admin-only, which would break nova here unless we were getting attachments using admin credentials configured in the [cinder] section of nova.conf (which might not be configured).\n\nSo I think we\u0027re back to getting the list of attachments from the volume via the cinder API, and then from that list get the server IDs, and then get those servers by ID and compare their hosts to this host.\n\nIf at some point in the future we require the [cinder] section in nova.conf, we could rely on using cinder admin creds to get the attachment host details directly from the cinder API and not have to do the instance lookup in the nova DB.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"25928e9c0d1485adc2c19c99027a5448d5e2c8bd","unresolved":false,"context_lines":[{"line_number":1255,"context_line":"                nova_context.get_admin_context(), connection_info[\u0027volume_id\u0027])"},{"line_number":1256,"context_line":"            if len(bdm_list) \u003e 1:"},{"line_number":1257,"context_line":"                for bdm in bdm_list:"},{"line_number":1258,"context_line":"                    if bdm.deleted:"},{"line_number":1259,"context_line":"                        LOG.debug(\"skipping checks for deleted bdm entry\")"},{"line_number":1260,"context_line":"                        continue"},{"line_number":1261,"context_line":"                    con_info \u003d jsonutils.loads(bdm.connection_info)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_b9504f0c","line":1258,"range":{"start_line":1258,"start_character":20,"end_line":1258,"end_character":35},"updated":"2018-03-02 14:43:12.000000000","message":"This shouldn\u0027t ever be true, the bdm query will filter out deleted instances (or it should) since read_deleted is not set to \u0027yes\u0027 on the context.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"da3618cb99e15704aa5b2444ecdd882a2347d5f8","unresolved":false,"context_lines":[{"line_number":1255,"context_line":"                nova_context.get_admin_context(), connection_info[\u0027volume_id\u0027])"},{"line_number":1256,"context_line":"            if len(bdm_list) \u003e 1:"},{"line_number":1257,"context_line":"                for bdm in bdm_list:"},{"line_number":1258,"context_line":"                    if bdm.deleted:"},{"line_number":1259,"context_line":"                        LOG.debug(\"skipping checks for deleted bdm entry\")"},{"line_number":1260,"context_line":"                        continue"},{"line_number":1261,"context_line":"                    con_info \u003d jsonutils.loads(bdm.connection_info)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_1c77f1d6","line":1258,"range":{"start_line":1258,"start_character":20,"end_line":1258,"end_character":35},"in_reply_to":"1f9dbf25_b9504f0c","updated":"2018-03-02 16:14:09.000000000","message":"Yeah, I just chose to be explicit but I\u0027ll remove it if it\u0027s bothersome and verify that deleted is in fact filtered out; it appears that it is so should be good.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"25928e9c0d1485adc2c19c99027a5448d5e2c8bd","unresolved":false,"context_lines":[{"line_number":1260,"context_line":"                        continue"},{"line_number":1261,"context_line":"                    con_info \u003d jsonutils.loads(bdm.connection_info)"},{"line_number":1262,"context_line":"                    if (bdm.instance.host \u003d\u003d instance.host and"},{"line_number":1263,"context_line":"                            con_info[\u0027data\u0027][\u0027target_iqn\u0027] \u003d\u003d"},{"line_number":1264,"context_line":"                            connection_info[\u0027data\u0027][\u0027target_iqn\u0027]):"},{"line_number":1265,"context_line":"                        connection_count +\u003d 1"},{"line_number":1266,"context_line":"        if connection_count \u003e 1:"},{"line_number":1267,"context_line":"            LOG.debug(\u0027skipping initiator destroy for volume \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_b9792f94","line":1264,"range":{"start_line":1263,"start_character":28,"end_line":1264,"end_character":65},"updated":"2018-03-02 14:43:12.000000000","message":"Yikes, doesn\u0027t this get super specific to the type of volume we have attached? connection_info[\u0027data\u0027][\u0027target_iqn\u0027] might not even be set.\n\nDo we really need this part? Can\u0027t we just say that if there is more than one instance attached to the same volume on the same host, we don\u0027t disconnect until we\u0027re down to the last attachment for that volume on this host?","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"da3618cb99e15704aa5b2444ecdd882a2347d5f8","unresolved":false,"context_lines":[{"line_number":1260,"context_line":"                        continue"},{"line_number":1261,"context_line":"                    con_info \u003d jsonutils.loads(bdm.connection_info)"},{"line_number":1262,"context_line":"                    if (bdm.instance.host \u003d\u003d instance.host and"},{"line_number":1263,"context_line":"                            con_info[\u0027data\u0027][\u0027target_iqn\u0027] \u003d\u003d"},{"line_number":1264,"context_line":"                            connection_info[\u0027data\u0027][\u0027target_iqn\u0027]):"},{"line_number":1265,"context_line":"                        connection_count +\u003d 1"},{"line_number":1266,"context_line":"        if connection_count \u003e 1:"},{"line_number":1267,"context_line":"            LOG.debug(\u0027skipping initiator destroy for volume \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_7c3e35db","line":1264,"range":{"start_line":1263,"start_character":28,"end_line":1264,"end_character":65},"in_reply_to":"1f9dbf25_998f9322","updated":"2018-03-02 16:14:09.000000000","message":"Understood, nor do a number of others; if we stuck with this proposal I\u0027d either remove that altogether or make it a safe get that defaults to always True if the field isn\u0027t present.  FWIW I think there\u0027s a better way to solve this and be more generic so I\u0027ll work on proposing that alternative shortly.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":12988,"name":"Peter Penchev","email":"openstack-dev@storpool.com","username":"ppenchev"},"change_message_id":"ab2b19c5d191c6b508bbfc8ccccc9f451b1e68c0","unresolved":false,"context_lines":[{"line_number":1260,"context_line":"                        continue"},{"line_number":1261,"context_line":"                    con_info \u003d jsonutils.loads(bdm.connection_info)"},{"line_number":1262,"context_line":"                    if (bdm.instance.host \u003d\u003d instance.host and"},{"line_number":1263,"context_line":"                            con_info[\u0027data\u0027][\u0027target_iqn\u0027] \u003d\u003d"},{"line_number":1264,"context_line":"                            connection_info[\u0027data\u0027][\u0027target_iqn\u0027]):"},{"line_number":1265,"context_line":"                        connection_count +\u003d 1"},{"line_number":1266,"context_line":"        if connection_count \u003e 1:"},{"line_number":1267,"context_line":"            LOG.debug(\u0027skipping initiator destroy for volume \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_998f9322","line":1264,"range":{"start_line":1263,"start_character":28,"end_line":1264,"end_character":65},"in_reply_to":"1f9dbf25_b9792f94","updated":"2018-03-02 14:52:52.000000000","message":"FWIW, I\u0027ll look into multiattach for the StorPool volumes soon, but yes, the StorPool connection_info certainly does not contain \u0027target_iqn\u0027 at all - the addressing protocol is completely different.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"da3618cb99e15704aa5b2444ecdd882a2347d5f8","unresolved":false,"context_lines":[{"line_number":1260,"context_line":"                        continue"},{"line_number":1261,"context_line":"                    con_info \u003d jsonutils.loads(bdm.connection_info)"},{"line_number":1262,"context_line":"                    if (bdm.instance.host \u003d\u003d instance.host and"},{"line_number":1263,"context_line":"                            con_info[\u0027data\u0027][\u0027target_iqn\u0027] \u003d\u003d"},{"line_number":1264,"context_line":"                            connection_info[\u0027data\u0027][\u0027target_iqn\u0027]):"},{"line_number":1265,"context_line":"                        connection_count +\u003d 1"},{"line_number":1266,"context_line":"        if connection_count \u003e 1:"},{"line_number":1267,"context_line":"            LOG.debug(\u0027skipping initiator destroy for volume \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_3c5bcd54","line":1264,"range":{"start_line":1263,"start_character":28,"end_line":1264,"end_character":65},"in_reply_to":"1f9dbf25_b9792f94","updated":"2018-03-02 16:14:09.000000000","message":"That\u0027s a really good point regarding the specifity, I added this out of paranoia so it\u0027s really not necessary.  Given some of the other things you brought up I\u0027m considering a different approach to this using a Cinder call to determine this instead.\n\nI started that way initially but I thought folks might find it offensive to add a call out to Cinder here in this module but given the ability to backport and race potential it might be the better option.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"25928e9c0d1485adc2c19c99027a5448d5e2c8bd","unresolved":false,"context_lines":[{"line_number":1267,"context_line":"            LOG.debug(\u0027skipping initiator destroy for volume \u0027"},{"line_number":1268,"context_line":"                      \u0027%(vol)s because another attachment \u0027"},{"line_number":1269,"context_line":"                      \u0027exists on the same host\u0027,"},{"line_number":1270,"context_line":"                      {\u0027vol\u0027: connection_info[\u0027volume_id\u0027]})"},{"line_number":1271,"context_line":"        else:"},{"line_number":1272,"context_line":"            vol_driver.disconnect_volume(connection_info, instance)"},{"line_number":1273,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_39651fe2","line":1270,"range":{"start_line":1270,"start_character":30,"end_line":1270,"end_character":58},"updated":"2018-03-02 14:43:12.000000000","message":"same as above, this key might not be set based on volume type","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"da3618cb99e15704aa5b2444ecdd882a2347d5f8","unresolved":false,"context_lines":[{"line_number":1267,"context_line":"            LOG.debug(\u0027skipping initiator destroy for volume \u0027"},{"line_number":1268,"context_line":"                      \u0027%(vol)s because another attachment \u0027"},{"line_number":1269,"context_line":"                      \u0027exists on the same host\u0027,"},{"line_number":1270,"context_line":"                      {\u0027vol\u0027: connection_info[\u0027volume_id\u0027]})"},{"line_number":1271,"context_line":"        else:"},{"line_number":1272,"context_line":"            vol_driver.disconnect_volume(connection_info, instance)"},{"line_number":1273,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_dc2e69a4","line":1270,"range":{"start_line":1270,"start_character":30,"end_line":1270,"end_character":58},"in_reply_to":"1f9dbf25_39651fe2","updated":"2018-03-02 16:14:09.000000000","message":"Yep, I\u0027m with ya","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bd90b30df0c90092a482e7808cc0455b2344af4c","unresolved":false,"context_lines":[{"line_number":1608,"context_line":"            else:"},{"line_number":1609,"context_line":"                raise"},{"line_number":1610,"context_line":""},{"line_number":1611,"context_line":"        # NOTE(lyarwood): We can provide None as the request context here as we"},{"line_number":1612,"context_line":"        # already have the encryption metadata dict from the compute layer."},{"line_number":1613,"context_line":"        # This avoids the need to add the request context to the signature of"},{"line_number":1614,"context_line":"        # detach_volume requiring changes across all drivers."},{"line_number":1615,"context_line":"        self._disconnect_volume(None, connection_info, instance,"},{"line_number":1616,"context_line":"                                encryption\u003dencryption)"},{"line_number":1617,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_b05db9ea","line":1614,"range":{"start_line":1611,"start_character":8,"end_line":1614,"end_character":61},"updated":"2018-03-03 10:32:49.000000000","message":"OK I see....well, I think we\u0027re going to need to change that so we can pass in the user request context. Changing the virt driver detach_volume method signature is OK IMO, nova only has a few in-tree drivers.","commit_id":"55b80ee190e02c87aa7ffbdc2a83a6a1cd2f043e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ade64dadccb72aa6879b2ef0596d2427e6f868f","unresolved":false,"context_lines":[{"line_number":1248,"context_line":"        self._attach_encryptor(context, connection_info, encryption,"},{"line_number":1249,"context_line":"                               allow_native_luks)"},{"line_number":1250,"context_line":""},{"line_number":1251,"context_line":"    def _should_disconnect_target(self, context, connection_info, instance):"},{"line_number":1252,"context_line":"        connection_count \u003d 0"},{"line_number":1253,"context_line":"        if connection_info.get(\u0027multiattach\u0027, False):"},{"line_number":1254,"context_line":"            volume \u003d self._volume_api.get(context,"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff6b8bd7_0b327ff2","line":1251,"updated":"2018-03-06 00:13:33.000000000","message":"Thanks for splitting this into it\u0027s own method, I was going to ask for that but now I don\u0027t have to.","commit_id":"2cf2f9e1fe0259368d6dacbfe49767a017959d3c"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"a1924d893f81468af282b15eab31a671de547526","unresolved":false,"context_lines":[{"line_number":1248,"context_line":"        self._attach_encryptor(context, connection_info, encryption,"},{"line_number":1249,"context_line":"                               allow_native_luks)"},{"line_number":1250,"context_line":""},{"line_number":1251,"context_line":"    def _should_disconnect_target(self, context, connection_info, instance):"},{"line_number":1252,"context_line":"        connection_count \u003d 0"},{"line_number":1253,"context_line":"        if connection_info.get(\u0027multiattach\u0027, False):"},{"line_number":1254,"context_line":"            volume \u003d self._volume_api.get(context,"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff6b8bd7_0bc6ff68","line":1251,"in_reply_to":"ff6b8bd7_0b327ff2","updated":"2018-03-06 01:00:42.000000000","message":"Done","commit_id":"2cf2f9e1fe0259368d6dacbfe49767a017959d3c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ade64dadccb72aa6879b2ef0596d2427e6f868f","unresolved":false,"context_lines":[{"line_number":1250,"context_line":""},{"line_number":1251,"context_line":"    def _should_disconnect_target(self, context, connection_info, instance):"},{"line_number":1252,"context_line":"        connection_count \u003d 0"},{"line_number":1253,"context_line":"        if connection_info.get(\u0027multiattach\u0027, False):"},{"line_number":1254,"context_line":"            volume \u003d self._volume_api.get(context,"},{"line_number":1255,"context_line":"                                          connection_info[\u0027volume_id\u0027])"},{"line_number":1256,"context_line":"            attachments \u003d volume.get(\u0027attachments\u0027, [])"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff6b8bd7_8b5bef2c","line":1253,"updated":"2018-03-06 00:13:33.000000000","message":"nit: could use a comment here about why we only care about multiattach volumes (as a reminder)","commit_id":"2cf2f9e1fe0259368d6dacbfe49767a017959d3c"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"a1924d893f81468af282b15eab31a671de547526","unresolved":false,"context_lines":[{"line_number":1250,"context_line":""},{"line_number":1251,"context_line":"    def _should_disconnect_target(self, context, connection_info, instance):"},{"line_number":1252,"context_line":"        connection_count \u003d 0"},{"line_number":1253,"context_line":"        if connection_info.get(\u0027multiattach\u0027, False):"},{"line_number":1254,"context_line":"            volume \u003d self._volume_api.get(context,"},{"line_number":1255,"context_line":"                                          connection_info[\u0027volume_id\u0027])"},{"line_number":1256,"context_line":"            attachments \u003d volume.get(\u0027attachments\u0027, [])"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff6b8bd7_aba033a7","line":1253,"in_reply_to":"ff6b8bd7_8b5bef2c","updated":"2018-03-06 01:00:42.000000000","message":"Done","commit_id":"2cf2f9e1fe0259368d6dacbfe49767a017959d3c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ade64dadccb72aa6879b2ef0596d2427e6f868f","unresolved":false,"context_lines":[{"line_number":1252,"context_line":"        connection_count \u003d 0"},{"line_number":1253,"context_line":"        if connection_info.get(\u0027multiattach\u0027, False):"},{"line_number":1254,"context_line":"            volume \u003d self._volume_api.get(context,"},{"line_number":1255,"context_line":"                                          connection_info[\u0027volume_id\u0027])"},{"line_number":1256,"context_line":"            attachments \u003d volume.get(\u0027attachments\u0027, [])"},{"line_number":1257,"context_line":"            if len(attachments) \u003e 1:"},{"line_number":1258,"context_line":"                servers_this_host \u003d objects.InstanceList.get_uuids_by_host("}],"source_content_type":"text/x-python","patch_set":7,"id":"ff6b8bd7_eb494b60","line":1255,"range":{"start_line":1255,"start_character":42,"end_line":1255,"end_character":70},"updated":"2018-03-06 00:13:33.000000000","message":"We need to use the helper utility for this in nova.virt.block_device.get_volume_id(connection_info) - it\u0027s used elsewhere in here; not all volume types store the volume_id in the top level connection_info dict.","commit_id":"2cf2f9e1fe0259368d6dacbfe49767a017959d3c"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"a1924d893f81468af282b15eab31a671de547526","unresolved":false,"context_lines":[{"line_number":1252,"context_line":"        connection_count \u003d 0"},{"line_number":1253,"context_line":"        if connection_info.get(\u0027multiattach\u0027, False):"},{"line_number":1254,"context_line":"            volume \u003d self._volume_api.get(context,"},{"line_number":1255,"context_line":"                                          connection_info[\u0027volume_id\u0027])"},{"line_number":1256,"context_line":"            attachments \u003d volume.get(\u0027attachments\u0027, [])"},{"line_number":1257,"context_line":"            if len(attachments) \u003e 1:"},{"line_number":1258,"context_line":"                servers_this_host \u003d objects.InstanceList.get_uuids_by_host("}],"source_content_type":"text/x-python","patch_set":7,"id":"ff6b8bd7_cbd7a723","line":1255,"range":{"start_line":1255,"start_character":42,"end_line":1255,"end_character":70},"in_reply_to":"ff6b8bd7_eb494b60","updated":"2018-03-06 01:00:42.000000000","message":"Gahhh!  Yup, you pointed that out to me somewhere before and I forgot, I\u0027ll fix it up.","commit_id":"2cf2f9e1fe0259368d6dacbfe49767a017959d3c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ade64dadccb72aa6879b2ef0596d2427e6f868f","unresolved":false,"context_lines":[{"line_number":1255,"context_line":"                                          connection_info[\u0027volume_id\u0027])"},{"line_number":1256,"context_line":"            attachments \u003d volume.get(\u0027attachments\u0027, [])"},{"line_number":1257,"context_line":"            if len(attachments) \u003e 1:"},{"line_number":1258,"context_line":"                servers_this_host \u003d objects.InstanceList.get_uuids_by_host("},{"line_number":1259,"context_line":"                    context, instance.host)"},{"line_number":1260,"context_line":"                for id, data in attachments.items():"},{"line_number":1261,"context_line":"                    attref \u003d self._volume_api.attachment_get("}],"source_content_type":"text/x-python","patch_set":7,"id":"ff6b8bd7_ab75939a","line":1258,"updated":"2018-03-06 00:13:33.000000000","message":"nit: could use some comments in here before the more complicated logic starts","commit_id":"2cf2f9e1fe0259368d6dacbfe49767a017959d3c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6cf186c496a1347981333e2f103d424caa3a3aa8","unresolved":false,"context_lines":[{"line_number":1257,"context_line":"            if len(attachments) \u003e 1:"},{"line_number":1258,"context_line":"                servers_this_host \u003d objects.InstanceList.get_uuids_by_host("},{"line_number":1259,"context_line":"                    context, instance.host)"},{"line_number":1260,"context_line":"                for id, data in attachments.items():"},{"line_number":1261,"context_line":"                    attref \u003d self._volume_api.attachment_get("},{"line_number":1262,"context_line":"                        context,"},{"line_number":1263,"context_line":"                        data[\u0027attachment_id\u0027])"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff6b8bd7_199acb13","line":1260,"range":{"start_line":1260,"start_character":20,"end_line":1260,"end_character":22},"updated":"2018-03-06 16:23:10.000000000","message":"So this id field is a server_id correct? If I\u0027m looking at how the attachments dict is constructed, it\u0027s a dict keyed by server_id mapped to a dict value with two keys, attachment_id and mountpoint. And I think we just want the keys (server ids) to figure out the servers attached to this volume.","commit_id":"2cf2f9e1fe0259368d6dacbfe49767a017959d3c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ade64dadccb72aa6879b2ef0596d2427e6f868f","unresolved":false,"context_lines":[{"line_number":1257,"context_line":"            if len(attachments) \u003e 1:"},{"line_number":1258,"context_line":"                servers_this_host \u003d objects.InstanceList.get_uuids_by_host("},{"line_number":1259,"context_line":"                    context, instance.host)"},{"line_number":1260,"context_line":"                for id, data in attachments.items():"},{"line_number":1261,"context_line":"                    attref \u003d self._volume_api.attachment_get("},{"line_number":1262,"context_line":"                        context,"},{"line_number":1263,"context_line":"                        data[\u0027attachment_id\u0027])"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff6b8bd7_6b6f7bc5","line":1260,"range":{"start_line":1260,"start_character":32,"end_line":1260,"end_character":51},"updated":"2018-03-06 00:13:33.000000000","message":"attachments is a list of dicts, so calling items() on the list will explode. You need to iterate the list of attachments and pull the server id out of each attachment dict in the list.","commit_id":"2cf2f9e1fe0259368d6dacbfe49767a017959d3c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6cf186c496a1347981333e2f103d424caa3a3aa8","unresolved":false,"context_lines":[{"line_number":1257,"context_line":"            if len(attachments) \u003e 1:"},{"line_number":1258,"context_line":"                servers_this_host \u003d objects.InstanceList.get_uuids_by_host("},{"line_number":1259,"context_line":"                    context, instance.host)"},{"line_number":1260,"context_line":"                for id, data in attachments.items():"},{"line_number":1261,"context_line":"                    attref \u003d self._volume_api.attachment_get("},{"line_number":1262,"context_line":"                        context,"},{"line_number":1263,"context_line":"                        data[\u0027attachment_id\u0027])"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff6b8bd7_d9d533bb","line":1260,"range":{"start_line":1260,"start_character":32,"end_line":1260,"end_character":51},"in_reply_to":"ff6b8bd7_2bf9837f","updated":"2018-03-06 16:23:10.000000000","message":"Wow yeah I see what you mean - that\u0027s really weird.","commit_id":"2cf2f9e1fe0259368d6dacbfe49767a017959d3c"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"a1924d893f81468af282b15eab31a671de547526","unresolved":false,"context_lines":[{"line_number":1257,"context_line":"            if len(attachments) \u003e 1:"},{"line_number":1258,"context_line":"                servers_this_host \u003d objects.InstanceList.get_uuids_by_host("},{"line_number":1259,"context_line":"                    context, instance.host)"},{"line_number":1260,"context_line":"                for id, data in attachments.items():"},{"line_number":1261,"context_line":"                    attref \u003d self._volume_api.attachment_get("},{"line_number":1262,"context_line":"                        context,"},{"line_number":1263,"context_line":"                        data[\u0027attachment_id\u0027])"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff6b8bd7_2bf9837f","line":1260,"range":{"start_line":1260,"start_character":32,"end_line":1260,"end_character":51},"in_reply_to":"ff6b8bd7_6b6f7bc5","updated":"2018-03-06 01:00:42.000000000","message":"nova.volume.cinder is converting it to a set though:\nhttps://github.com/openstack/nova/blob/master/nova/volume/cinder.py#L207\n\nSo that\u0027s why I called items, this seems to be correct (I tested it before posting)","commit_id":"2cf2f9e1fe0259368d6dacbfe49767a017959d3c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ade64dadccb72aa6879b2ef0596d2427e6f868f","unresolved":false,"context_lines":[{"line_number":1258,"context_line":"                servers_this_host \u003d objects.InstanceList.get_uuids_by_host("},{"line_number":1259,"context_line":"                    context, instance.host)"},{"line_number":1260,"context_line":"                for id, data in attachments.items():"},{"line_number":1261,"context_line":"                    attref \u003d self._volume_api.attachment_get("},{"line_number":1262,"context_line":"                        context,"},{"line_number":1263,"context_line":"                        data[\u0027attachment_id\u0027])"},{"line_number":1264,"context_line":"                    server_id \u003d attref[\u0027connection_info\u0027].get(\u0027instance\u0027, None)"},{"line_number":1265,"context_line":"                    if server_id in servers_this_host:"},{"line_number":1266,"context_line":"                        connection_count +\u003d 1"},{"line_number":1267,"context_line":"        return (False if connection_count \u003e 1 else True)"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff6b8bd7_8b744f9b","line":1264,"range":{"start_line":1261,"start_character":20,"end_line":1264,"end_character":79},"updated":"2018-03-06 00:13:33.000000000","message":"We shouldn\u0027t need this. The volume.attachments list of dicts should have the attached server uuid in each attachment dict:\n\nhttps://developer.openstack.org/api-ref/block-storage/v3/#list-accessible-volumes-with-details\n\nSee the sample attachment dict in ^:\n\n            \"attachments\": [\n                {\n                    \"server_id\": \"f4fda93b-06e0-4743-8117-bc8bcecd651b\",\n                    \"attachment_id\": \"3b4db356-253d-4fab-bfa0-e3626c0b8405\",\n                    \"host_name\": null,\n                    \"volume_id\": \"6edbc2f4-1507-44f8-ac0d-eed1d2608d38\",\n                    \"device\": \"/dev/vdb\",\n                    \"id\": \"6edbc2f4-1507-44f8-ac0d-eed1d2608d38\"\n                }\n            ]","commit_id":"2cf2f9e1fe0259368d6dacbfe49767a017959d3c"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"a1924d893f81468af282b15eab31a671de547526","unresolved":false,"context_lines":[{"line_number":1258,"context_line":"                servers_this_host \u003d objects.InstanceList.get_uuids_by_host("},{"line_number":1259,"context_line":"                    context, instance.host)"},{"line_number":1260,"context_line":"                for id, data in attachments.items():"},{"line_number":1261,"context_line":"                    attref \u003d self._volume_api.attachment_get("},{"line_number":1262,"context_line":"                        context,"},{"line_number":1263,"context_line":"                        data[\u0027attachment_id\u0027])"},{"line_number":1264,"context_line":"                    server_id \u003d attref[\u0027connection_info\u0027].get(\u0027instance\u0027, None)"},{"line_number":1265,"context_line":"                    if server_id in servers_this_host:"},{"line_number":1266,"context_line":"                        connection_count +\u003d 1"},{"line_number":1267,"context_line":"        return (False if connection_count \u003e 1 else True)"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff6b8bd7_6b0d3b8f","line":1264,"range":{"start_line":1261,"start_character":20,"end_line":1264,"end_character":79},"in_reply_to":"ff6b8bd7_8b744f9b","updated":"2018-03-06 01:00:42.000000000","message":"yeah, I took this route instead of messing with the *set* issue that I pointed out in teh above comment.  I\u0027m not sure why it\u0027s translated that way to be honest, but didn\u0027t want to try and modify that and risk breaking other things or introducing compatibility issues etc.\n\nI see your point either way though, there\u0027s no real value in wasting another call out to Cinder here so I\u0027ll futz with the set that we already have and put that together.","commit_id":"2cf2f9e1fe0259368d6dacbfe49767a017959d3c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8ade64dadccb72aa6879b2ef0596d2427e6f868f","unresolved":false,"context_lines":[{"line_number":1275,"context_line":"        else:"},{"line_number":1276,"context_line":"            LOG.info(\"Detected multiple connections on this host for volume: \""},{"line_number":1277,"context_line":"                     \"%s, skipping target disconnect.\","},{"line_number":1278,"context_line":"                     connection_info.get(\u0027volume_id\u0027, \"\"), instance\u003dinstance)"},{"line_number":1279,"context_line":""},{"line_number":1280,"context_line":"    def _extend_volume(self, connection_info, instance):"},{"line_number":1281,"context_line":"        vol_driver \u003d self._get_volume_driver(connection_info)"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff6b8bd7_eb62abd8","line":1278,"range":{"start_line":1278,"start_character":21,"end_line":1278,"end_character":57},"updated":"2018-03-06 00:13:33.000000000","message":"Use the same utility method as mentioned above.","commit_id":"2cf2f9e1fe0259368d6dacbfe49767a017959d3c"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"a1924d893f81468af282b15eab31a671de547526","unresolved":false,"context_lines":[{"line_number":1275,"context_line":"        else:"},{"line_number":1276,"context_line":"            LOG.info(\"Detected multiple connections on this host for volume: \""},{"line_number":1277,"context_line":"                     \"%s, skipping target disconnect.\","},{"line_number":1278,"context_line":"                     connection_info.get(\u0027volume_id\u0027, \"\"), instance\u003dinstance)"},{"line_number":1279,"context_line":""},{"line_number":1280,"context_line":"    def _extend_volume(self, connection_info, instance):"},{"line_number":1281,"context_line":"        vol_driver \u003d self._get_volume_driver(connection_info)"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff6b8bd7_8b120fb1","line":1278,"range":{"start_line":1278,"start_character":21,"end_line":1278,"end_character":57},"in_reply_to":"ff6b8bd7_eb62abd8","updated":"2018-03-06 01:00:42.000000000","message":"Done","commit_id":"2cf2f9e1fe0259368d6dacbfe49767a017959d3c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8091b11d01ad8711c908e3bbea882dd839d993b","unresolved":false,"context_lines":[{"line_number":1264,"context_line":"            volume \u003d self._volume_api.get("},{"line_number":1265,"context_line":"                context,"},{"line_number":1266,"context_line":"                driver_block_device.get_volume_id(connection_info))"},{"line_number":1267,"context_line":"            attachments \u003d volume.get(\u0027attachments\u0027, [])"},{"line_number":1268,"context_line":"            if len(attachments) \u003e 1:"},{"line_number":1269,"context_line":"                # First we get a list of all Server UUID\u0027s associated with"},{"line_number":1270,"context_line":"                # this Host (Compute Node).  We\u0027re going to use this to"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff6b8bd7_dc820150","line":1267,"range":{"start_line":1267,"start_character":52,"end_line":1267,"end_character":54},"updated":"2018-03-06 16:47:17.000000000","message":"nit: this should default to an empty dict since that\u0027s what the attachments are after the cinder code (in nova) translates the response body:\n\nhttps://github.com/openstack/nova/blob/5d2add74534719c5670b29152964a60e8f23b42b/nova/volume/cinder.py#L205","commit_id":"7e9f01cca255ff9b6f619a2db70078430408e44c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8091b11d01ad8711c908e3bbea882dd839d993b","unresolved":false,"context_lines":[{"line_number":1276,"context_line":"                    context, instance.host)"},{"line_number":1277,"context_line":""},{"line_number":1278,"context_line":"                # NOTE(jdg): nova.volume.cinder translates the"},{"line_number":1279,"context_line":"                # volume[\u0027attachments\u0027] response into a set which includes"},{"line_number":1280,"context_line":"                # the Server UUID as the first member, so we\u0027re using that"},{"line_number":1281,"context_line":"                # here to check against our server_this_host list"},{"line_number":1282,"context_line":"                for server_id, data in attachments.items():"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff6b8bd7_9cc24985","line":1279,"range":{"start_line":1279,"start_character":56,"end_line":1279,"end_character":59},"updated":"2018-03-06 16:47:17.000000000","message":"dict: https://github.com/openstack/nova/blob/5d2add74534719c5670b29152964a60e8f23b42b/nova/volume/cinder.py#L205\n\nhttps://docs.python.org/2/library/collections.html#collections.OrderedDict\n\nUnless I\u0027m misreading the code.","commit_id":"7e9f01cca255ff9b6f619a2db70078430408e44c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8091b11d01ad8711c908e3bbea882dd839d993b","unresolved":false,"context_lines":[{"line_number":1291,"context_line":"            vol_driver \u003d self._get_volume_driver(connection_info)"},{"line_number":1292,"context_line":"            vol_driver.disconnect_volume(connection_info, instance)"},{"line_number":1293,"context_line":"        else:"},{"line_number":1294,"context_line":"            LOG.info(\"Detected multiple connections on this host for volume: \""},{"line_number":1295,"context_line":"                     \"%s, skipping target disconnect.\","},{"line_number":1296,"context_line":"                     driver_block_device.get_volume_id(connection_info),"},{"line_number":1297,"context_line":"                     instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff6b8bd7_9c66c9ed","line":1294,"updated":"2018-03-06 16:47:17.000000000","message":"Hmm, so I\u0027m seeing this in the logs for Steve\u0027s patch, which is good, but I\u0027m also seeing it for non-multiattach jobs, which is not expected:\n\nhttp://logs.openstack.org/56/548356/3/check/nova-multiattach/31e72e9/logs/screen-n-cpu.txt.gz#_Mar_06_15_12_33_371333\n\nMar 06 15:12:33.371333 ubuntu-xenial-rax-dfw-0002808845 nova-compute[25586]: INFO nova.virt.libvirt.driver [None req-658fefab-355b-400f-9022-1ec170825f11 tempest-ImagesTestJSON-516208247 tempest-ImagesTestJSON-516208247] [instance: f8ab4ddd-097d-445a-80df-d7282818e762] Detected multiple connections on this host for volume: 05e6c750-3a41-4754-b9b4-73ca7f67ea20, skipping target disconnect.\n\nBut I do see earlier in the logs for that same request, it\u0027s working with a multiattach volume:\n\nhttp://logs.openstack.org/56/548356/3/check/nova-multiattach/31e72e9/logs/screen-n-cpu.txt.gz#_Mar_06_15_12_33_343415\n\nSo maybe it\u0027s just something weird in tempest or the logs where it\u0027s overlapping log entries - I\u0027ve seen weird stuff happen like that before.","commit_id":"7e9f01cca255ff9b6f619a2db70078430408e44c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6460012e28fa694bc22bce10fb34a86826fe99d6","unresolved":false,"context_lines":[{"line_number":1260,"context_line":"        # will indiscriminantly delete the connection for all Server and that\u0027s"},{"line_number":1261,"context_line":"        # no good.  So check if it\u0027s attached multiple times on this node"},{"line_number":1262,"context_line":"        # if it is we skip the call to brick to delete the connection."},{"line_number":1263,"context_line":"        if connection_info.get(\u0027multiattach\u0027, False):"},{"line_number":1264,"context_line":"            volume \u003d self._volume_api.get("},{"line_number":1265,"context_line":"                context,"},{"line_number":1266,"context_line":"                driver_block_device.get_volume_id(connection_info))"}],"source_content_type":"text/x-python","patch_set":9,"id":"ff6b8bd7_ffd25734","line":1263,"updated":"2018-03-06 17:21:37.000000000","message":"Fortunately, we only verify the attachments if we have multi attached volume.","commit_id":"139426d51416313e8b14a335695adc7165bc4848"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6460012e28fa694bc22bce10fb34a86826fe99d6","unresolved":false,"context_lines":[{"line_number":1263,"context_line":"        if connection_info.get(\u0027multiattach\u0027, False):"},{"line_number":1264,"context_line":"            volume \u003d self._volume_api.get("},{"line_number":1265,"context_line":"                context,"},{"line_number":1266,"context_line":"                driver_block_device.get_volume_id(connection_info))"},{"line_number":1267,"context_line":"            attachments \u003d volume.get(\u0027attachments\u0027, {})"},{"line_number":1268,"context_line":"            if len(attachments) \u003e 1:"},{"line_number":1269,"context_line":"                # First we get a list of all Server UUID\u0027s associated with"}],"source_content_type":"text/x-python","patch_set":9,"id":"ff6b8bd7_5f9b4371","line":1266,"updated":"2018-03-06 17:21:37.000000000","message":"I guess we could somehow ask Cinder to give us information about host information, so we shouldn\u0027t need to verify the instances after that, but then it wouldn\u0027t be possible to backport the change.","commit_id":"139426d51416313e8b14a335695adc7165bc4848"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9dbda9b0bcdfa4323dc22c07a98e2c2c57e59cf5","unresolved":false,"context_lines":[{"line_number":1263,"context_line":"        if connection_info.get(\u0027multiattach\u0027, False):"},{"line_number":1264,"context_line":"            volume \u003d self._volume_api.get("},{"line_number":1265,"context_line":"                context,"},{"line_number":1266,"context_line":"                driver_block_device.get_volume_id(connection_info))"},{"line_number":1267,"context_line":"            attachments \u003d volume.get(\u0027attachments\u0027, {})"},{"line_number":1268,"context_line":"            if len(attachments) \u003e 1:"},{"line_number":1269,"context_line":"                # First we get a list of all Server UUID\u0027s associated with"}],"source_content_type":"text/x-python","patch_set":9,"id":"ff6b8bd7_df12f3b1","line":1266,"in_reply_to":"ff6b8bd7_5f9b4371","updated":"2018-03-06 17:27:10.000000000","message":"Yeah so actually today the cinder API returns the attached host information for each attachment. That would be all we\u0027d need here and we wouldn\u0027t need the call to objects.InstanceList.get_uuids_by_host - however, as discussed at the Rocky PTG, that\u0027s a bug in cinder API and they are going to add a policy rule to only return the attached host for admin-users only (by default), so we can\u0027t rely on it here.\n\nNow that nova has (since queens) the ability to configure admin creds for cinder, we could rely on that to get the volume information with the attached host data using an admin token, like nova does for things with neutron like port bindings, but neutron config for admin tokens in nova has been required a long time - we don\u0027t really have that expectation/requirement for operators yet wrt nova/cinder config.","commit_id":"139426d51416313e8b14a335695adc7165bc4848"}]}