)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"7a4b4d339ee81521b22fb523a161e86127d90f60","unresolved":false,"context_lines":[{"line_number":11,"context_line":"has config drive in format iso9660 and is placed on local storage."},{"line_number":12,"context_line":"Since libvirt 1.2.17 it is possible to migrate instance with such"},{"line_number":13,"context_line":"config drive format. However, config drive needs to be copied to"},{"line_number":14,"context_line":"destination node prior to live migration."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Co-Authored-By: Bartosz Fic \u003cbartosz.fic@intel.com\u003e"},{"line_number":17,"context_line":"Co-Authored-By: Michael Still \u003cmikal@stillhq.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":31,"id":"3aaa91ec_5ee27d02","line":14,"updated":"2016-06-28 13:14:42.000000000","message":"Have you genuinely tested that this works ? AFAIK, this should still be impossible. The CDROM drive media is read-only, so the QEMU process will be blocked from writing to the ISO image by both file permissions and by SELinux or ApArmour. So it should still be impossible to block-migrate the ISO image, even if you pre-create it on the target.","commit_id":"2abeda1090ef7643c25587a83cde5941a4000ac4"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"2c45780d1b5a32a0f61c0b08206a0480cce795a9","unresolved":false,"context_lines":[{"line_number":11,"context_line":"has config drive in format iso9660 and is placed on local storage."},{"line_number":12,"context_line":"Since libvirt 1.2.17 it is possible to migrate instance with such"},{"line_number":13,"context_line":"config drive format. However, config drive needs to be copied to"},{"line_number":14,"context_line":"destination node prior to live migration."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Co-Authored-By: Bartosz Fic \u003cbartosz.fic@intel.com\u003e"},{"line_number":17,"context_line":"Co-Authored-By: Michael Still \u003cmikal@stillhq.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":31,"id":"3aaa91ec_49745b9b","line":14,"in_reply_to":"3aaa91ec_5ee27d02","updated":"2016-06-29 08:56:44.000000000","message":"Yes, I\u0027ve tested all this stuff and it works. There is no need to exclude config drive from devices that will be migrated to destination, because libvirt does not copy files that are already there on destination host. AFAIK iWeb is running similar code in production. Mathieu Gagne posted sample of code somewhere in comments there with what they have in their production environment.\n\nHowever, let me double check what happens with privileges and will let you know. I haven\u0027t observed any problem so far.","commit_id":"2abeda1090ef7643c25587a83cde5941a4000ac4"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"ee1144923b06921fc1a6f82117e00e2958121815","unresolved":false,"context_lines":[{"line_number":7905,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.os.path.exists\u0027)"},{"line_number":7906,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.utils.get_instance_path_at_destination\u0027)"},{"line_number":7907,"context_line":"    @mock.patch(\u0027nova.virt.driver.block_device_info_get_mapping\u0027)"},{"line_number":7908,"context_line":"    @mock.patch.object(host.Host, \u0027has_min_version\u0027, return_value\u003dTrue)"},{"line_number":7909,"context_line":"    @mock.patch(\u0027nova.virt.configdrive.required_by\u0027, return_value\u003dTrue)"},{"line_number":7910,"context_line":"    def test_pre_live_migration_block_with_config_drive_success("},{"line_number":7911,"context_line":"            self, mock_required_by, mock_min_version,"}],"source_content_type":"text/x-python","patch_set":20,"id":"ba63e191_dadd8671","line":7908,"updated":"2016-01-19 11:30:55.000000000","message":"Simply making has_min_version() always return True seems like a big hammer - shouldn\u0027t we ensure it\u0027s only called once with 1.2.17?","commit_id":"417d72a10f5507b6659ae79178ef037a0dc0a4d8"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"8dd7f817adf1d1348d5a7914a60173d27807075b","unresolved":false,"context_lines":[{"line_number":7905,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.os.path.exists\u0027)"},{"line_number":7906,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.utils.get_instance_path_at_destination\u0027)"},{"line_number":7907,"context_line":"    @mock.patch(\u0027nova.virt.driver.block_device_info_get_mapping\u0027)"},{"line_number":7908,"context_line":"    @mock.patch.object(host.Host, \u0027has_min_version\u0027, return_value\u003dTrue)"},{"line_number":7909,"context_line":"    @mock.patch(\u0027nova.virt.configdrive.required_by\u0027, return_value\u003dTrue)"},{"line_number":7910,"context_line":"    def test_pre_live_migration_block_with_config_drive_success("},{"line_number":7911,"context_line":"            self, mock_required_by, mock_min_version,"}],"source_content_type":"text/x-python","patch_set":20,"id":"ba63e191_10445d72","line":7908,"in_reply_to":"ba63e191_dadd8671","updated":"2016-01-19 12:16:07.000000000","message":"I\u0027d rather leave it as it is because it is used only once. Even if I change it and someone adds another check in pre_live_migration  then this test will need to be fixed.","commit_id":"417d72a10f5507b6659ae79178ef037a0dc0a4d8"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"ee1144923b06921fc1a6f82117e00e2958121815","unresolved":false,"context_lines":[{"line_number":7927,"context_line":"        migrate_data.block_migration \u003d True"},{"line_number":7928,"context_line":""},{"line_number":7929,"context_line":"        result \u003d drvr.pre_live_migration("},{"line_number":7930,"context_line":"            self.context, instance, vol, [], None, migrate_data)"},{"line_number":7931,"context_line":""},{"line_number":7932,"context_line":"        block_device_info_get_mapping.assert_called_once_with("},{"line_number":7933,"context_line":"            {\u0027block_device_mapping\u0027: ["}],"source_content_type":"text/x-python","patch_set":20,"id":"ba63e191_1a41de0b","line":7930,"updated":"2016-01-19 11:30:55.000000000","message":"I\u0027d be inclined to just pass block_device_info\u003dNone in here - it doesn\u0027t really help test the configdrive libvirt version check code path","commit_id":"417d72a10f5507b6659ae79178ef037a0dc0a4d8"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"8dd7f817adf1d1348d5a7914a60173d27807075b","unresolved":false,"context_lines":[{"line_number":7927,"context_line":"        migrate_data.block_migration \u003d True"},{"line_number":7928,"context_line":""},{"line_number":7929,"context_line":"        result \u003d drvr.pre_live_migration("},{"line_number":7930,"context_line":"            self.context, instance, vol, [], None, migrate_data)"},{"line_number":7931,"context_line":""},{"line_number":7932,"context_line":"        block_device_info_get_mapping.assert_called_once_with("},{"line_number":7933,"context_line":"            {\u0027block_device_mapping\u0027: ["}],"source_content_type":"text/x-python","patch_set":20,"id":"ba63e191_b65eaae7","line":7930,"in_reply_to":"ba63e191_1a41de0b","updated":"2016-01-19 12:16:07.000000000","message":"I can remove this if I need to reroll. Doesn\u0027t seem like an issue.","commit_id":"417d72a10f5507b6659ae79178ef037a0dc0a4d8"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"ee1144923b06921fc1a6f82117e00e2958121815","unresolved":false,"context_lines":[{"line_number":7938,"context_line":""},{"line_number":7939,"context_line":"        migrate_data.graphics_listen_addrs_vnc \u003d \u0027127.0.0.1\u0027"},{"line_number":7940,"context_line":"        migrate_data.graphics_listen_addrs_spice \u003d \u0027127.0.0.1\u0027"},{"line_number":7941,"context_line":"        migrate_data.serial_listen_addr \u003d \u0027127.0.0.1\u0027"},{"line_number":7942,"context_line":""},{"line_number":7943,"context_line":"        self.assertEqual(migrate_data, result)"},{"line_number":7944,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"ba63e191_ff22bcb8","line":7941,"updated":"2016-01-19 11:30:55.000000000","message":"Iefec6cdfabba105bb183795add0a94885e13143e will change this - these values won\u0027t be set by pre_live_migration()","commit_id":"417d72a10f5507b6659ae79178ef037a0dc0a4d8"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"8dd7f817adf1d1348d5a7914a60173d27807075b","unresolved":false,"context_lines":[{"line_number":7938,"context_line":""},{"line_number":7939,"context_line":"        migrate_data.graphics_listen_addrs_vnc \u003d \u0027127.0.0.1\u0027"},{"line_number":7940,"context_line":"        migrate_data.graphics_listen_addrs_spice \u003d \u0027127.0.0.1\u0027"},{"line_number":7941,"context_line":"        migrate_data.serial_listen_addr \u003d \u0027127.0.0.1\u0027"},{"line_number":7942,"context_line":""},{"line_number":7943,"context_line":"        self.assertEqual(migrate_data, result)"},{"line_number":7944,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"ba63e191_10b45df0","line":7941,"in_reply_to":"ba63e191_ff22bcb8","updated":"2016-01-19 12:16:07.000000000","message":"Thanks for the information. Right now it is still correct.","commit_id":"417d72a10f5507b6659ae79178ef037a0dc0a4d8"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"ee1144923b06921fc1a6f82117e00e2958121815","unresolved":false,"context_lines":[{"line_number":7940,"context_line":"        migrate_data.graphics_listen_addrs_spice \u003d \u0027127.0.0.1\u0027"},{"line_number":7941,"context_line":"        migrate_data.serial_listen_addr \u003d \u0027127.0.0.1\u0027"},{"line_number":7942,"context_line":""},{"line_number":7943,"context_line":"        self.assertEqual(migrate_data, result)"},{"line_number":7944,"context_line":""},{"line_number":7945,"context_line":"    @mock.patch.object(host.Host, \u0027has_min_version\u0027, return_value\u003dFalse)"},{"line_number":7946,"context_line":"    @mock.patch(\u0027nova.virt.configdrive.required_by\u0027, return_value\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":20,"id":"ba63e191_dadb46ec","line":7943,"updated":"2016-01-19 11:30:55.000000000","message":"This seems useless - they\u0027re the same objects, no?\n\nWhat are we really trying to verify here? That NoLiveMigrationForConfigDriveInLibVirt isn\u0027t raised, correct?","commit_id":"417d72a10f5507b6659ae79178ef037a0dc0a4d8"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"8dd7f817adf1d1348d5a7914a60173d27807075b","unresolved":false,"context_lines":[{"line_number":7940,"context_line":"        migrate_data.graphics_listen_addrs_spice \u003d \u0027127.0.0.1\u0027"},{"line_number":7941,"context_line":"        migrate_data.serial_listen_addr \u003d \u0027127.0.0.1\u0027"},{"line_number":7942,"context_line":""},{"line_number":7943,"context_line":"        self.assertEqual(migrate_data, result)"},{"line_number":7944,"context_line":""},{"line_number":7945,"context_line":"    @mock.patch.object(host.Host, \u0027has_min_version\u0027, return_value\u003dFalse)"},{"line_number":7946,"context_line":"    @mock.patch(\u0027nova.virt.configdrive.required_by\u0027, return_value\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":20,"id":"ba63e191_70303957","line":7943,"in_reply_to":"ba63e191_dadb46ec","updated":"2016-01-19 12:16:07.000000000","message":"We are verifying that this particular code path returns correct migrate data in case of block live migration with libvirt that supports selective block migration.","commit_id":"417d72a10f5507b6659ae79178ef037a0dc0a4d8"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"ee1144923b06921fc1a6f82117e00e2958121815","unresolved":false,"context_lines":[{"line_number":7948,"context_line":"            self, mock_required_by, mock_min_version):"},{"line_number":7949,"context_line":"        # Creating testdata"},{"line_number":7950,"context_line":"        self.flags(config_drive_format\u003d\u0027iso9960\u0027)"},{"line_number":7951,"context_line":"        vol \u003d {\u0027block_device_mapping\u0027: ["},{"line_number":7952,"context_line":"                  {\u0027connection_info\u0027: \u0027dummy\u0027, \u0027mount_device\u0027: \u0027/dev/sda\u0027},"},{"line_number":7953,"context_line":"                  {\u0027connection_info\u0027: \u0027dummy\u0027, \u0027mount_device\u0027: \u0027/dev/sdb\u0027}]}"},{"line_number":7954,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"}],"source_content_type":"text/x-python","patch_set":20,"id":"ba63e191_faf5ea82","line":7951,"updated":"2016-01-19 11:30:55.000000000","message":"Could pass None for block_device_info here too","commit_id":"417d72a10f5507b6659ae79178ef037a0dc0a4d8"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"8dd7f817adf1d1348d5a7914a60173d27807075b","unresolved":false,"context_lines":[{"line_number":7948,"context_line":"            self, mock_required_by, mock_min_version):"},{"line_number":7949,"context_line":"        # Creating testdata"},{"line_number":7950,"context_line":"        self.flags(config_drive_format\u003d\u0027iso9960\u0027)"},{"line_number":7951,"context_line":"        vol \u003d {\u0027block_device_mapping\u0027: ["},{"line_number":7952,"context_line":"                  {\u0027connection_info\u0027: \u0027dummy\u0027, \u0027mount_device\u0027: \u0027/dev/sda\u0027},"},{"line_number":7953,"context_line":"                  {\u0027connection_info\u0027: \u0027dummy\u0027, \u0027mount_device\u0027: \u0027/dev/sdb\u0027}]}"},{"line_number":7954,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"}],"source_content_type":"text/x-python","patch_set":20,"id":"ba63e191_b0c5515c","line":7951,"in_reply_to":"ba63e191_faf5ea82","updated":"2016-01-19 12:16:07.000000000","message":"I don\u0027t want to change logic of existing tests, it\u0027s out of scope","commit_id":"417d72a10f5507b6659ae79178ef037a0dc0a4d8"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"fd3fdfcb85b8e9cb6cad1c5a8849fd20cd6b3af2","unresolved":false,"context_lines":[{"line_number":8022,"context_line":"        vol \u003d {\u0027block_device_mapping\u0027: ["},{"line_number":8023,"context_line":"                  {\u0027connection_info\u0027: \u0027dummy\u0027, \u0027mount_device\u0027: \u0027/dev/sda\u0027},"},{"line_number":8024,"context_line":"                  {\u0027connection_info\u0027: \u0027dummy\u0027, \u0027mount_device\u0027: \u0027/dev/sdb\u0027}]}"},{"line_number":8025,"context_line":"        mock_get_instance_path.return_value \u003d \u0027fake_instance_path\u0027"},{"line_number":8026,"context_line":"        mock_path.return_value \u003d False"},{"line_number":8027,"context_line":""},{"line_number":8028,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"}],"source_content_type":"text/x-python","patch_set":21,"id":"9a68dd71_be9cdb0f","line":8025,"updated":"2016-01-22 18:23:19.000000000","message":"Testing locally, I\u0027m seeing this cause a \u0027fake_instance_path\u0027 directory to be created, not cleaned up, and then cause subsequent test runs to fail.\n\nThat needs to be fixed, at least.","commit_id":"5cdf9ae6147c01e079fc53fbe2192f26cd844248"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"9d799fada8438dd7e6c0da84e12d0020f93affa1","unresolved":false,"context_lines":[{"line_number":8022,"context_line":"        vol \u003d {\u0027block_device_mapping\u0027: ["},{"line_number":8023,"context_line":"                  {\u0027connection_info\u0027: \u0027dummy\u0027, \u0027mount_device\u0027: \u0027/dev/sda\u0027},"},{"line_number":8024,"context_line":"                  {\u0027connection_info\u0027: \u0027dummy\u0027, \u0027mount_device\u0027: \u0027/dev/sdb\u0027}]}"},{"line_number":8025,"context_line":"        mock_get_instance_path.return_value \u003d \u0027fake_instance_path\u0027"},{"line_number":8026,"context_line":"        mock_path.return_value \u003d False"},{"line_number":8027,"context_line":""},{"line_number":8028,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"}],"source_content_type":"text/x-python","patch_set":21,"id":"9a68dd71_8df7f38b","line":8025,"in_reply_to":"9a68dd71_be9cdb0f","updated":"2016-01-26 13:30:00.000000000","message":"One way to solve that would be create a fixture for instance backend. This would build and clean it up after. \n\nHaving the 5 mocks is kind of an indication that too much mocking is going on here. This would remove 2 of them.","commit_id":"5cdf9ae6147c01e079fc53fbe2192f26cd844248"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"24ca6db324193e86cd92aa41331398810bc7b699","unresolved":false,"context_lines":[{"line_number":9427,"context_line":""},{"line_number":9428,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.remotefs.\u0027"},{"line_number":9429,"context_line":"                \u0027RemoteFilesystem.copy_file\u0027)"},{"line_number":9430,"context_line":"    def test_fetch_instance_config_drive(self, mock_create_file):"},{"line_number":9431,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":9432,"context_line":"        instance \u003d objects.Instance(**self.test_instance)"},{"line_number":9433,"context_line":"        instance_dir \u003d \u0027test\u0027"}],"source_content_type":"text/x-python","patch_set":36,"id":"fad55976_70e528a7","line":9430,"updated":"2016-07-15 15:09:52.000000000","message":"Not a fan of tests like this: they\u0027re basically just a diff. They don\u0027t really add anything, and they make it really hard to see interesting changes when you\u0027re reorganising code. I mentioned in another comment that this method is too simple to exist anyway. I\u0027d just fold this into the test below and reduce the noise.","commit_id":"f193503fe1e93cec52e5cbe4d60a3a9014bc6f9b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"24ca6db324193e86cd92aa41331398810bc7b699","unresolved":false,"context_lines":[{"line_number":9436,"context_line":"        mock_create_file.assert_called_once_with(src, instance_dir)"},{"line_number":9437,"context_line":""},{"line_number":9438,"context_line":"    @mock.patch.object(libvirt_driver.LibvirtDriver,"},{"line_number":9439,"context_line":"                       \u0027_fetch_instance_config_drive\u0027)"},{"line_number":9440,"context_line":"    @mock.patch(\u0027nova.virt.driver.block_device_info_get_mapping\u0027)"},{"line_number":9441,"context_line":"    @mock.patch(\u0027nova.virt.configdrive.required_by\u0027, return_value\u003dTrue)"},{"line_number":9442,"context_line":"    def test_pre_live_migration_block_with_config_drive_success("}],"source_content_type":"text/x-python","patch_set":36,"id":"fad55976_50a7444f","line":9439,"updated":"2016-07-15 15:09:52.000000000","message":"You could mock RemoteFilesystem.copy_file here instead.","commit_id":"f193503fe1e93cec52e5cbe4d60a3a9014bc6f9b"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"e764627b12ca684871c068967b01d4221f75ed44","unresolved":false,"context_lines":[{"line_number":375,"context_line":"# Relative block commit \u0026 rebase (feature is detected,"},{"line_number":376,"context_line":"# this version is only used for messaging)"},{"line_number":377,"context_line":"MIN_LIBVIRT_BLOCKJOB_RELATIVE_VERSION \u003d (1, 2, 7)"},{"line_number":378,"context_line":"# Libvirt version 1.2.17 is required for successfull block live migration"},{"line_number":379,"context_line":"# of vm booted from image with attached devices"},{"line_number":380,"context_line":"MIN_LIBVIRT_BLOCK_LM_WITH_VOLUMES_VERSION \u003d (1, 2, 17)"},{"line_number":381,"context_line":"# libvirt discard feature"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba8a016a_e80c3b3d","line":378,"updated":"2015-11-17 09:54:57.000000000","message":"@pkoniszewski\nhow about add some notes here ?","commit_id":"7972637f6c9933071afde16da4cee675720d1b3f"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"8e1f7f67dbe96948d9072e0f1656f939f5465f0f","unresolved":false,"context_lines":[{"line_number":6254,"context_line":"            # format does not work because of a bug in libvirt with read only"},{"line_number":6255,"context_line":"            # devices. However one can use vfat as config_drive_format which"},{"line_number":6256,"context_line":"            # works fine. Please see bug/1246201 for details on the libvirt"},{"line_number":6257,"context_line":"            # bug."},{"line_number":6258,"context_line":"            if (is_shared_block_storage or"},{"line_number":6259,"context_line":"                is_shared_instance_path or"},{"line_number":6260,"context_line":"                CONF.config_drive_format \u003d\u003d \u0027vfat\u0027):"}],"source_content_type":"text/x-python","patch_set":11,"id":"da85f559_d96fb0fa","line":6257,"updated":"2015-11-16 00:47:31.000000000","message":"These block comments are a bit redundant now, but its a nit.","commit_id":"7972637f6c9933071afde16da4cee675720d1b3f"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"349e74b8acb505ff2b21360bf54e97d9e94f2ea1","unresolved":false,"context_lines":[{"line_number":6081,"context_line":"                block_devices.append(device_name)"},{"line_number":6082,"context_line":""},{"line_number":6083,"context_line":"        for dev in guest.get_all_disks():"},{"line_number":6084,"context_line":"            if dev.readonly or dev.shareable:"},{"line_number":6085,"context_line":"                continue"},{"line_number":6086,"context_line":"            if dev.source_type not in [\"file\", \"block\"]:"},{"line_number":6087,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_d0e33f04","line":6084,"updated":"2016-02-09 10:02:11.000000000","message":"read only disks should be migrated in this change. As Daniel\u0027s comment in https://review.openstack.org/#/c/227278/31/nova/virt/libvirt/driver.py","commit_id":"ea51d32780e2cd601f2b74d1fe1755760b405672"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"0e394bff94898f592a8d79951eb7c5271a42b4e1","unresolved":false,"context_lines":[{"line_number":6081,"context_line":"                block_devices.append(device_name)"},{"line_number":6082,"context_line":""},{"line_number":6083,"context_line":"        for dev in guest.get_all_disks():"},{"line_number":6084,"context_line":"            if dev.readonly or dev.shareable:"},{"line_number":6085,"context_line":"                continue"},{"line_number":6086,"context_line":"            if dev.source_type not in [\"file\", \"block\"]:"},{"line_number":6087,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_062afd2c","line":6084,"in_reply_to":"5a5ae5dd_a6dc4975","updated":"2016-02-09 10:35:06.000000000","message":"Thanks for agreement, will fix it there.","commit_id":"ea51d32780e2cd601f2b74d1fe1755760b405672"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"fae3e7646e90275c6e9de36df6adeca591a385d3","unresolved":false,"context_lines":[{"line_number":6081,"context_line":"                block_devices.append(device_name)"},{"line_number":6082,"context_line":""},{"line_number":6083,"context_line":"        for dev in guest.get_all_disks():"},{"line_number":6084,"context_line":"            if dev.readonly or dev.shareable:"},{"line_number":6085,"context_line":"                continue"},{"line_number":6086,"context_line":"            if dev.source_type not in [\"file\", \"block\"]:"},{"line_number":6087,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_a6dc4975","line":6084,"in_reply_to":"5a5ae5dd_d0e33f04","updated":"2016-02-09 10:33:33.000000000","message":"Yep, make this configurable based on libvirt version too","commit_id":"ea51d32780e2cd601f2b74d1fe1755760b405672"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c8b33c446d2da762998e46e3f5ccbbbf16fe77cd","unresolved":false,"context_lines":[{"line_number":6483,"context_line":"            # kvm is a block device, live migration will work"},{"line_number":6484,"context_line":"            # NOTE(sileht): the configdrive is stored into a shared path"},{"line_number":6485,"context_line":"            # kvm don\u0027t need to migrate it, live migration will work"},{"line_number":6486,"context_line":"            # NOTE(dims): Prior to libvirt 1.2.17 using config drive with iso"},{"line_number":6487,"context_line":"            # format does not work because of a bug in libvirt with read only"},{"line_number":6488,"context_line":"            # devices. However one can use vfat as config_drive_format which"},{"line_number":6489,"context_line":"            # works fine. Please see bug/1246201 for details on the libvirt"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_af49ab32","line":6486,"range":{"start_line":6486,"start_character":19,"end_line":6486,"end_character":23},"updated":"2016-02-08 17:48:10.000000000","message":"nit: Might be best to change the name of this note, seeing as you are now the \"expert\" with the most up-to-date information on this specific issue?","commit_id":"ea51d32780e2cd601f2b74d1fe1755760b405672"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"be27d30c74d6a0df58bf7731699a2a3997cd6f87","unresolved":false,"context_lines":[{"line_number":6487,"context_line":"            # format does not work because of a bug in libvirt with read only"},{"line_number":6488,"context_line":"            # devices. However one can use vfat as config_drive_format which"},{"line_number":6489,"context_line":"            # works fine. Please see bug/1246201 for details on the libvirt"},{"line_number":6490,"context_line":"            # bug."},{"line_number":6491,"context_line":"            if (is_shared_block_storage or"},{"line_number":6492,"context_line":"                is_shared_instance_path or"},{"line_number":6493,"context_line":"                CONF.config_drive_format \u003d\u003d \u0027vfat\u0027):"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_5c053312","line":6490,"updated":"2016-02-09 05:38:46.000000000","message":"I feel like this entire note block could do with a re-write, but its not critical.","commit_id":"ea51d32780e2cd601f2b74d1fe1755760b405672"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"fc754aef42c57fc2467c1f00a4cb8f597ad923a5","unresolved":false,"context_lines":[{"line_number":6362,"context_line":"            is_block_migration \u003d migrate_data.block_migration"},{"line_number":6363,"context_line":""},{"line_number":6364,"context_line":"        # NOTE(pkoniszewski): Since libvirt 1.2.17 it is possible to block live"},{"line_number":6365,"context_line":"        # migrate a VM which has a read-only config drive. If older libvirt"},{"line_number":6366,"context_line":"        # is in use it is possible to block live migrate only with vfat config"},{"line_number":6367,"context_line":"        # drive or when config drive is stored into the block storage or when"},{"line_number":6368,"context_line":"        # instance path is shared"}],"source_content_type":"text/x-python","patch_set":31,"id":"3aaa91ec_73e7b212","line":6365,"range":{"start_line":6365,"start_character":35,"end_line":6365,"end_character":44},"updated":"2016-06-28 13:02:05.000000000","message":"Nit: good to say read-only \u003d iso9660 as later you mention vfat config drive.","commit_id":"2abeda1090ef7643c25587a83cde5941a4000ac4"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"24ca6db324193e86cd92aa41331398810bc7b699","unresolved":false,"context_lines":[{"line_number":6485,"context_line":"                if (configdrive.required_by(instance) and"},{"line_number":6486,"context_line":"                        CONF.config_drive_format \u003d\u003d \u0027iso9660\u0027):"},{"line_number":6487,"context_line":"                    self._fetch_instance_config_drive(instance.host,"},{"line_number":6488,"context_line":"                                                      instance_dir)"},{"line_number":6489,"context_line":""},{"line_number":6490,"context_line":"            if not is_block_migration:"},{"line_number":6491,"context_line":"                # NOTE(angdraug): when block storage is shared between source"}],"source_content_type":"text/x-python","patch_set":36,"id":"fad55976_b07840fa","line":6488,"updated":"2016-07-15 15:09:52.000000000","message":"This is seriously quirky. We need to do 2 things here:\n\n1. Determine if we need to copy the config drive because of its location.\n\n2. Copy the config disk if required.\n\nThe config disk can currently be on either the Rbd or Flat backend. Flat uses the instance directory, so we need to copy it if we don\u0027t have a shared instance directory, but not if we\u0027re on Rbd. If we\u0027re on Rbd then is_shared_block_storage will be True. We\u0027re then assuming the path of the config disk if we\u0027re not on shared block storage, and that it\u0027s a file.\n\nIOW, putting this here happens to work right now, but based on a ton of context spread between this module and imagebackend. This is going to be a bug some day.\n\nAt the very least this requires a full length comment explaining all the assumptions this code makes. Ideally it requires a better design for negotiating between source and destination what needs to be copied, though.","commit_id":"f193503fe1e93cec52e5cbe4d60a3a9014bc6f9b"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"2923b9b7b97021391b3fa41b684bc0edfd26460b","unresolved":false,"context_lines":[{"line_number":6485,"context_line":"                if (configdrive.required_by(instance) and"},{"line_number":6486,"context_line":"                        CONF.config_drive_format \u003d\u003d \u0027iso9660\u0027):"},{"line_number":6487,"context_line":"                    self._fetch_instance_config_drive(instance.host,"},{"line_number":6488,"context_line":"                                                      instance_dir)"},{"line_number":6489,"context_line":""},{"line_number":6490,"context_line":"            if not is_block_migration:"},{"line_number":6491,"context_line":"                # NOTE(angdraug): when block storage is shared between source"}],"source_content_type":"text/x-python","patch_set":36,"id":"dada55a8_84095734","line":6488,"in_reply_to":"dada55a8_80c4ba92","updated":"2016-07-18 16:35:07.000000000","message":"Thanks Matthew, got it. However, I still don\u0027t know how negotiation would help us. We would need to move even more logic from libvirt to nova (handling config drives during LM) and again increase code complexity of already very complex code. I think that long term solution should mean changes to libvirt/QEMU in this case, not nova.","commit_id":"f193503fe1e93cec52e5cbe4d60a3a9014bc6f9b"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"19f056d6f2d4647f4e9963fb9a7eadf4fad68f9d","unresolved":false,"context_lines":[{"line_number":6485,"context_line":"                if (configdrive.required_by(instance) and"},{"line_number":6486,"context_line":"                        CONF.config_drive_format \u003d\u003d \u0027iso9660\u0027):"},{"line_number":6487,"context_line":"                    self._fetch_instance_config_drive(instance.host,"},{"line_number":6488,"context_line":"                                                      instance_dir)"},{"line_number":6489,"context_line":""},{"line_number":6490,"context_line":"            if not is_block_migration:"},{"line_number":6491,"context_line":"                # NOTE(angdraug): when block storage is shared between source"}],"source_content_type":"text/x-python","patch_set":36,"id":"fad55976_f9af437b","line":6488,"in_reply_to":"fad55976_b07840fa","updated":"2016-07-15 15:38:10.000000000","message":"Why would we need any negotiation here? This config drive is a special case, all other files are always copied to destination in case of block migrations. Basically this one is read-only and we need to take care of it. Otherwise QEMU will fail migration, because it will try to write to read-only device.\n\nYour comment about RBD sounds like you just answered your concerns. Can you explain in a little bit more in details?\n\nGoing a bit deeper - if instance path is not shared and block storage is not shared we assume that we need to copy all filed to destination during live migration. VFAT config drive is handled exactly the same way, so if your concerns are true (and if i understand your concerns correctly), we already have a bug.","commit_id":"f193503fe1e93cec52e5cbe4d60a3a9014bc6f9b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"9fba81bde45067ba0d8b46608b1c5c31b802fb64","unresolved":false,"context_lines":[{"line_number":6485,"context_line":"                if (configdrive.required_by(instance) and"},{"line_number":6486,"context_line":"                        CONF.config_drive_format \u003d\u003d \u0027iso9660\u0027):"},{"line_number":6487,"context_line":"                    self._fetch_instance_config_drive(instance.host,"},{"line_number":6488,"context_line":"                                                      instance_dir)"},{"line_number":6489,"context_line":""},{"line_number":6490,"context_line":"            if not is_block_migration:"},{"line_number":6491,"context_line":"                # NOTE(angdraug): when block storage is shared between source"}],"source_content_type":"text/x-python","patch_set":36,"id":"dada55a8_80c4ba92","line":6488,"in_reply_to":"fad55976_f9af437b","updated":"2016-07-18 09:34:44.000000000","message":"I often suffer from being too wordy :) This code is not incorrect today, but I\u0027m still strongly -1 on this unless 1 of 2 things happen:\n\n1. We change the way we negotiate shared disks such that config drives no longer have to be a special case here.\n\nor\n\n2. We add a long-form comment here explaining all the assumptions which allow this to currently be correct.\n\nAs (1) is likely a multi-cycle fix, I suspect you\u0027ll go for (2) :) These innocuous 4 lines of code require a large amount of context to understand. We understand it today, but contributor X doesn\u0027t, and I\u0027ll have forgotten it by next month.","commit_id":"f193503fe1e93cec52e5cbe4d60a3a9014bc6f9b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"24ca6db324193e86cd92aa41331398810bc7b699","unresolved":false,"context_lines":[{"line_number":6566,"context_line":""},{"line_number":6567,"context_line":"        return migrate_data"},{"line_number":6568,"context_line":""},{"line_number":6569,"context_line":"    def _fetch_instance_config_drive(self, host, instance_dir):"},{"line_number":6570,"context_line":"        src \u003d \"%s:%s/disk.config\" % (host, instance_dir)"},{"line_number":6571,"context_line":"        self._remotefs.copy_file(src, instance_dir)"},{"line_number":6572,"context_line":""}],"source_content_type":"text/x-python","patch_set":36,"id":"fad55976_0d793530","line":6569,"updated":"2016-07-15 15:09:52.000000000","message":"I don\u0027t think this function should exist. It\u0027s a 2 line method with a single caller, which also assumes the path of the config disk outside the context of imagebackend. At the very least it should be inline above.","commit_id":"f193503fe1e93cec52e5cbe4d60a3a9014bc6f9b"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"19f056d6f2d4647f4e9963fb9a7eadf4fad68f9d","unresolved":false,"context_lines":[{"line_number":6566,"context_line":""},{"line_number":6567,"context_line":"        return migrate_data"},{"line_number":6568,"context_line":""},{"line_number":6569,"context_line":"    def _fetch_instance_config_drive(self, host, instance_dir):"},{"line_number":6570,"context_line":"        src \u003d \"%s:%s/disk.config\" % (host, instance_dir)"},{"line_number":6571,"context_line":"        self._remotefs.copy_file(src, instance_dir)"},{"line_number":6572,"context_line":""}],"source_content_type":"text/x-python","patch_set":36,"id":"fad55976_d9eb5faf","line":6569,"in_reply_to":"fad55976_0d793530","updated":"2016-07-15 15:38:10.000000000","message":"Okay, i will merge it.","commit_id":"f193503fe1e93cec52e5cbe4d60a3a9014bc6f9b"}]}