)]}'
{"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5e3b23bebdf2b200b0b3799b857866cd696e9226","unresolved":false,"context_lines":[{"line_number":6257,"context_line":"              mock_build_metadata, mock_save):"},{"line_number":6258,"context_line":"            for state in (power_state.RUNNING, power_state.PAUSED):"},{"line_number":6259,"context_line":"                mock_dom.info.return_value \u003d [state, 512, 512, 2, 1234, 5678]"},{"line_number":6260,"context_line":"                mock_build_metadata.return_value \u003d \\"},{"line_number":6261,"context_line":"                    objects.InstanceDeviceMetadata()"},{"line_number":6262,"context_line":""},{"line_number":6263,"context_line":"                drvr.attach_volume(self.context, connection_info, instance,"},{"line_number":6264,"context_line":"                                   \"/dev/vdb\", disk_bus\u003dbdm[\u0027disk_bus\u0027],"}],"source_content_type":"text/x-python","patch_set":20,"id":"ffe62b97_1ae45fe0","line":6261,"range":{"start_line":6260,"start_character":16,"end_line":6261,"end_character":52},"updated":"2017-03-28 08:41:16.000000000","message":"nit: Could go in the context manager, a la line 6249-6250","commit_id":"09ac95260ed6a1a4a56501cb96c44bbb9a9c1a43"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7d7133c0d5b3314d12c81e3fdd7179b126f10710","unresolved":false,"context_lines":[{"line_number":6358,"context_line":"                mock_dom.attachDeviceFlags.assert_called_with("},{"line_number":6359,"context_line":"                    mock_conf.to_xml(), flags\u003dflags)"},{"line_number":6360,"context_line":"                mock_check_discard.assert_called_with(mock_conf, instance)"},{"line_number":6361,"context_line":"                mock_build_metadata.assert_called_with(self.context, instance)"},{"line_number":6362,"context_line":"                mock_save.assert_called_with()"},{"line_number":6363,"context_line":""},{"line_number":6364,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_domain\u0027)"}],"source_content_type":"text/x-python","patch_set":37,"id":"ff0f0b1f_4eb1858f","line":6361,"range":{"start_line":6361,"start_character":36,"end_line":6361,"end_character":54},"updated":"2017-05-17 19:51:14.000000000","message":"nit: would prefer using assert_called_once_with but won\u0027t block on it.","commit_id":"daed08796b71e456c9282f221b9b12c333e08e8f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7d7133c0d5b3314d12c81e3fdd7179b126f10710","unresolved":false,"context_lines":[{"line_number":6359,"context_line":"                    mock_conf.to_xml(), flags\u003dflags)"},{"line_number":6360,"context_line":"                mock_check_discard.assert_called_with(mock_conf, instance)"},{"line_number":6361,"context_line":"                mock_build_metadata.assert_called_with(self.context, instance)"},{"line_number":6362,"context_line":"                mock_save.assert_called_with()"},{"line_number":6363,"context_line":""},{"line_number":6364,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_domain\u0027)"},{"line_number":6365,"context_line":"    def test_detach_volume_with_vir_domain_affect_live_flag(self,"}],"source_content_type":"text/x-python","patch_set":37,"id":"ff0f0b1f_0eab8d1d","line":6362,"range":{"start_line":6362,"start_character":26,"end_line":6362,"end_character":44},"updated":"2017-05-17 19:51:14.000000000","message":"same","commit_id":"daed08796b71e456c9282f221b9b12c333e08e8f"}],"nova/tests/unit/virt/test_block_device.py":[{"author":{"_account_id":18339,"name":"Sivasathurappan Radhakrishnan","email":"siva.radhakrishnan@intel.com","username":"sivakrishnan"},"change_message_id":"48b8699e22eb14f2b4f3e4a9476ae4fb24942f64","unresolved":false,"context_lines":[{"line_number":438,"context_line":"                        bdm_dict[\u0027device_name\u0027],"},{"line_number":439,"context_line":"                        disk_bus\u003dbdm_dict[\u0027disk_bus\u0027],"},{"line_number":440,"context_line":"                        device_type\u003dbdm_dict[\u0027device_type\u0027],"},{"line_number":441,"context_line":"                        encryption\u003denc_data,"},{"line_number":442,"context_line":"                        tag\u003dNone).AndRaise(test.TestingException)"},{"line_number":443,"context_line":"                self.volume_api.terminate_connection("},{"line_number":444,"context_line":"                        elevated_context, fake_volume[\u0027id\u0027],"},{"line_number":445,"context_line":"                        connector).AndReturn(None)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9a30ddce_5dd68428","line":442,"range":{"start_line":441,"start_character":0,"end_line":442,"end_character":65},"updated":"2017-03-11 23:53:39.000000000","message":"We can move this to decorator but I think this should be done as part of mox to mock changes","commit_id":"b52275ef9065d461e4262f29c40d5e377680e262"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a65a19297f3bce1ab5fbc92d95ae2c16455bd7e0","unresolved":false,"context_lines":[{"line_number":438,"context_line":"                        bdm_dict[\u0027device_name\u0027],"},{"line_number":439,"context_line":"                        disk_bus\u003dbdm_dict[\u0027disk_bus\u0027],"},{"line_number":440,"context_line":"                        device_type\u003dbdm_dict[\u0027device_type\u0027],"},{"line_number":441,"context_line":"                        encryption\u003denc_data,"},{"line_number":442,"context_line":"                        tag\u003dNone).AndRaise(test.TestingException)"},{"line_number":443,"context_line":"                self.volume_api.terminate_connection("},{"line_number":444,"context_line":"                        elevated_context, fake_volume[\u0027id\u0027],"},{"line_number":445,"context_line":"                        connector).AndReturn(None)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9a30ddce_ee14cea0","line":442,"range":{"start_line":441,"start_character":0,"end_line":442,"end_character":65},"in_reply_to":"9a30ddce_5dd68428","updated":"2017-03-13 12:08:30.000000000","message":"Yeah, not overly fond of doing anything in Mox now, but I\u0027m not going to block on it","commit_id":"b52275ef9065d461e4262f29c40d5e377680e262"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"68b8603822cb039e258189a0f2525d3a3f7cb814","unresolved":false,"context_lines":[{"line_number":438,"context_line":"                        bdm_dict[\u0027device_name\u0027],"},{"line_number":439,"context_line":"                        disk_bus\u003dbdm_dict[\u0027disk_bus\u0027],"},{"line_number":440,"context_line":"                        device_type\u003dbdm_dict[\u0027device_type\u0027],"},{"line_number":441,"context_line":"                        encryption\u003denc_data,"},{"line_number":442,"context_line":"                        tag\u003dNone).AndRaise(test.TestingException)"},{"line_number":443,"context_line":"                self.volume_api.terminate_connection("},{"line_number":444,"context_line":"                        elevated_context, fake_volume[\u0027id\u0027],"},{"line_number":445,"context_line":"                        connector).AndReturn(None)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9a30ddce_b99774ea","line":442,"range":{"start_line":441,"start_character":0,"end_line":442,"end_character":65},"in_reply_to":"9a30ddce_ee14cea0","updated":"2017-03-13 16:07:46.000000000","message":"Removed in new patchset.","commit_id":"b52275ef9065d461e4262f29c40d5e377680e262"}],"nova/virt/block_device.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"6247b7f149782f8fe1e6a12ac833f6e07c26c221","unresolved":false,"context_lines":[{"line_number":260,"context_line":"            connection_info[\u0027serial\u0027] \u003d self.volume_id"},{"line_number":261,"context_line":"        self._preserve_multipath_id(connection_info)"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        tag \u003d kwargs[\u0027tag\u0027] if \u0027tag\u0027 in kwargs else None"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        # If do_driver_attach is False, we will attach a volume to an instance"},{"line_number":266,"context_line":"        # at boot time. So actual attach is done by instance creation code."}],"source_content_type":"text/x-python","patch_set":4,"id":"9a57fde8_30a45456","line":263,"updated":"2017-01-12 17:02:46.000000000","message":"nit: kwargs.get(\u0027tag\u0027)","commit_id":"7ac756d0d34d453e49b9d70bc9bf3267b4ab6382"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a65a19297f3bce1ab5fbc92d95ae2c16455bd7e0","unresolved":false,"context_lines":[{"line_number":260,"context_line":"            connection_info[\u0027serial\u0027] \u003d self.volume_id"},{"line_number":261,"context_line":"        self._preserve_multipath_id(connection_info)"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        tag \u003d kwargs[\u0027tag\u0027] if \u0027tag\u0027 in kwargs else None"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        # If do_driver_attach is False, we will attach a volume to an instance"},{"line_number":266,"context_line":"        # at boot time. So actual attach is done by instance creation code."}],"source_content_type":"text/x-python","patch_set":18,"id":"9a30ddce_2ed0a6c8","line":263,"range":{"start_line":263,"start_character":8,"end_line":263,"end_character":56},"updated":"2017-03-13 12:08:30.000000000","message":"Why is it necessary to do this, rather than explicitly adding a new parameter?","commit_id":"b52275ef9065d461e4262f29c40d5e377680e262"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"68b8603822cb039e258189a0f2525d3a3f7cb814","unresolved":false,"context_lines":[{"line_number":260,"context_line":"            connection_info[\u0027serial\u0027] \u003d self.volume_id"},{"line_number":261,"context_line":"        self._preserve_multipath_id(connection_info)"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        tag \u003d kwargs[\u0027tag\u0027] if \u0027tag\u0027 in kwargs else None"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        # If do_driver_attach is False, we will attach a volume to an instance"},{"line_number":266,"context_line":"        # at boot time. So actual attach is done by instance creation code."}],"source_content_type":"text/x-python","patch_set":18,"id":"9a30ddce_f981ecc3","line":263,"range":{"start_line":263,"start_character":8,"end_line":263,"end_character":56},"in_reply_to":"9a30ddce_2ed0a6c8","updated":"2017-03-13 16:07:46.000000000","message":"Removed in next pathset.","commit_id":"b52275ef9065d461e4262f29c40d5e377680e262"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"8dd936f6c97f59f4c5e8f1ec2479860d9010f962","unresolved":false,"context_lines":[{"line_number":7680,"context_line":"        # methods that call this function."},{"line_number":7681,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":7682,"context_line":"            context, instance.uuid)"},{"line_number":7683,"context_line":"        tagged_bdms \u003d {bdm.volume_id: bdm for bdm in bdms if bdm.tag}"},{"line_number":7684,"context_line":""},{"line_number":7685,"context_line":"        devices \u003d []"},{"line_number":7686,"context_line":"        guest \u003d self._host.get_guest(instance)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_a7bfc3fa","line":7683,"range":{"start_line":7683,"start_character":23,"end_line":7683,"end_character":36},"updated":"2017-01-11 13:48:52.000000000","message":"I still don\u0027t understand this change..\n \nConfig classes will show a device name (target_dev) that the guest will see for a specific device - why is it being changed here to a volume_id? Also, on line 7709, how will it get a serial from a dict of volume_ids? \nWhat am I missing here?","commit_id":"ff81772f716f626b4cd03d6f36f6bd75ff9b5335"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"bff393052a7fb673ea45b516092f1e9c2387c126","unresolved":false,"context_lines":[{"line_number":7706,"context_line":""},{"line_number":7707,"context_line":"            # Build disks related metadata"},{"line_number":7708,"context_line":"            if isinstance(dev, vconfig.LibvirtConfigGuestDisk):"},{"line_number":7709,"context_line":"                bdm \u003d tagged_bdms.get(dev.serial)"},{"line_number":7710,"context_line":"                if not bdm:"},{"line_number":7711,"context_line":"                    continue"},{"line_number":7712,"context_line":"                bus \u003d self._prepare_device_bus(dev)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba5201f7_b880afc0","line":7709,"range":{"start_line":7709,"start_character":38,"end_line":7709,"end_character":48},"updated":"2017-01-11 19:37:04.000000000","message":"dev.serial gets its value from cinder. I think relying on the fact that all cinder backends will provide this value as a volume_id is not desirable, IMO. This is also optional in libvirt..\nOn the other hand, keep using the device_name is better, because you can rely that it will be there, as it is being generated as set by nova.","commit_id":"7ac756d0d34d453e49b9d70bc9bf3267b4ab6382"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"95df31f89da8efd998739536f0876ad208983503","unresolved":false,"context_lines":[{"line_number":7706,"context_line":""},{"line_number":7707,"context_line":"            # Build disks related metadata"},{"line_number":7708,"context_line":"            if isinstance(dev, vconfig.LibvirtConfigGuestDisk):"},{"line_number":7709,"context_line":"                bdm \u003d tagged_bdms.get(dev.serial)"},{"line_number":7710,"context_line":"                if not bdm:"},{"line_number":7711,"context_line":"                    continue"},{"line_number":7712,"context_line":"                bus \u003d self._prepare_device_bus(dev)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba5201f7_bba269fa","line":7709,"range":{"start_line":7709,"start_character":38,"end_line":7709,"end_character":48},"in_reply_to":"ba5201f7_b880afc0","updated":"2017-01-11 19:49:46.000000000","message":"The thing with device name is that it doesn\u0027t always match between the instance xml and the value in the bdm object. We got it working for the initial device tagging, so it\u0027s clearly good enough for that, but in my tests for volume attachment they were different.\n\nAlso, I was talking to Matt Riedemann on IRC and he was saying the device name can\u0027t be trusted to match between xml and bdm, especially if we do stuff like rebooting the instance.\n\nLet\u0027s try and get Matt Booth\u0027s eyes on this tomorrow.","commit_id":"7ac756d0d34d453e49b9d70bc9bf3267b4ab6382"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"6247b7f149782f8fe1e6a12ac833f6e07c26c221","unresolved":false,"context_lines":[{"line_number":7706,"context_line":""},{"line_number":7707,"context_line":"            # Build disks related metadata"},{"line_number":7708,"context_line":"            if isinstance(dev, vconfig.LibvirtConfigGuestDisk):"},{"line_number":7709,"context_line":"                bdm \u003d tagged_bdms.get(dev.serial)"},{"line_number":7710,"context_line":"                if not bdm:"},{"line_number":7711,"context_line":"                    continue"},{"line_number":7712,"context_line":"                bus \u003d self._prepare_device_bus(dev)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a57fde8_9655f8c7","line":7709,"range":{"start_line":7709,"start_character":38,"end_line":7709,"end_character":48},"in_reply_to":"ba5201f7_bba269fa","updated":"2017-01-12 17:02:46.000000000","message":"Yep, as it stands device name can change over the lifetime of an instance. However, I\u0027m pretty sure it will be borked in practical cases during reboot and migration, for eg.\n\nHowever, I also take Vladik\u0027s point about serial specifically. From LibvirtBaseVolumeDriver.get_config:\n\n  conf.serial \u003d connection_info.get(\u0027serial\u0027)\n\nSo what we\u0027re doing here is adding a local device, and then matching against the local device that we just created based on a key which was hopefully created correctly in a driver-dependent opaque data structure of an external service.i.e. If future cinder  ever starts sending connection_info with serial !\u003d volume id we\u0027re going to break here in a really obtuse way.  That doesn\u0027t sound like a good idea, either.\n\nI think _build_device_metadata needs a rethink. If the goal is to map tags to guest hardware addressing, we need to do this every time guest hardware addressing can change, which is every time we rewrite the domain xml. I think we need to run it on reboot, cold migration, and possibly live migration too. And potentially resume from sleep. Go audit calls to _create_domain_and_network.\n\nGiven that we should be populating device metadata in the same operation as creating the domain xml in all these cases, we should be able to find a robust and unambiguous way to map them. For example, we should be able to simply use the order they appear in the domain xml.\n\nFor attach_volume and attach_interface we\u0027re currently recreating all device metadata, even though we\u0027re only touching a single device. This seems inefficient and potentially error prone. We should look into how to touch only the device we just added. Again, I\u0027m pretty sure that as we just added we can probably safely use the order it appears in the domain xml. I expect it\u0027s last, but you should check.\n\nIncidentally, I don\u0027t immediately see where we\u0027re cleaning up metadata from devices that we\u0027ve removed.","commit_id":"7ac756d0d34d453e49b9d70bc9bf3267b4ab6382"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e853548ecbb856f3715ddfdcd44f2d6c433d2961","unresolved":false,"context_lines":[{"line_number":1229,"context_line":"            # instance.save()."},{"line_number":1230,"context_line":"            instance.device_metadata \u003d self._build_device_metadata("},{"line_number":1231,"context_line":"                context, instance)"},{"line_number":1232,"context_line":"            instance.save()"},{"line_number":1233,"context_line":"        except Exception as ex:"},{"line_number":1234,"context_line":"            LOG.exception(_LE(\u0027Failed to attach volume at mountpoint: %s\u0027),"},{"line_number":1235,"context_line":"                          mountpoint, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":20,"id":"ffe62b97_ecd3fbff","line":1232,"updated":"2017-03-29 15:53:45.000000000","message":"Don\u0027t you need a corresponding call in detach, otherwise we\u0027ll be left with stale device metadata?","commit_id":"09ac95260ed6a1a4a56501cb96c44bbb9a9c1a43"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"acd4845a127480d8443c73afe9b70d5d0e6769ae","unresolved":false,"context_lines":[{"line_number":1229,"context_line":"            # instance.save()."},{"line_number":1230,"context_line":"            instance.device_metadata \u003d self._build_device_metadata("},{"line_number":1231,"context_line":"                context, instance)"},{"line_number":1232,"context_line":"            instance.save()"},{"line_number":1233,"context_line":"        except Exception as ex:"},{"line_number":1234,"context_line":"            LOG.exception(_LE(\u0027Failed to attach volume at mountpoint: %s\u0027),"},{"line_number":1235,"context_line":"                          mountpoint, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":20,"id":"ffe62b97_4fb1b946","line":1232,"in_reply_to":"ffe62b97_ecd3fbff","updated":"2017-03-29 16:04:54.000000000","message":"We can do that in compute manager without hitting the drivers [1]\n\n[1] https://review.openstack.org/#/c/435082/11/nova/compute/manager.py","commit_id":"09ac95260ed6a1a4a56501cb96c44bbb9a9c1a43"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7d7133c0d5b3314d12c81e3fdd7179b126f10710","unresolved":false,"context_lines":[{"line_number":1249,"context_line":"        except Exception as ex:"},{"line_number":1250,"context_line":"            LOG.exception(_LE(\u0027Failed to attach volume at mountpoint: %s\u0027),"},{"line_number":1251,"context_line":"                          mountpoint, instance\u003dinstance)"},{"line_number":1252,"context_line":"            if isinstance(ex, libvirt.libvirtError):"},{"line_number":1253,"context_line":"                errcode \u003d ex.get_error_code()"},{"line_number":1254,"context_line":"                if errcode \u003d\u003d libvirt.VIR_ERR_OPERATION_FAILED:"},{"line_number":1255,"context_line":"                    self._disconnect_volume(connection_info, disk_dev,"},{"line_number":1256,"context_line":"                                            instance)"},{"line_number":1257,"context_line":"                    raise exception.DeviceIsBusy(device\u003ddisk_dev)"},{"line_number":1258,"context_line":""},{"line_number":1259,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1260,"context_line":"                self._disconnect_volume(connection_info, disk_dev, instance)"}],"source_content_type":"text/x-python","patch_set":37,"id":"ff0f0b1f_0e4f0dbe","line":1257,"range":{"start_line":1252,"start_character":12,"end_line":1257,"end_character":65},"updated":"2017-05-17 19:51:14.000000000","message":"Totally unrelated, but it\u0027s weird that we handle the libvirtError to raise DeviceIsBusy when nothing is handling that specific exception in the upper layers. Probably vestigial code at this point.","commit_id":"daed08796b71e456c9282f221b9b12c333e08e8f"}]}