)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4e15e115c7fcfa312f5dd22ca509f2c89b874f19","unresolved":false,"context_lines":[{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: Ia91e2f9c316e25394a0f41dc341d903dfcff6921"},{"line_number":15,"context_line":"Co-authored-by: Mehdi Abaakouk \u003csileht@sileht.net\u003e"},{"line_number":16,"context_line":"Closes-bug: #1729584"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"df7087c5_659db664","line":16,"updated":"2018-03-15 00:39:27.000000000","message":"Based on sahid\u0027s inline comment and the content of the bug, we should have Closes-bug: #1753394 on here too.","commit_id":"3b079f55641e443b7c51bc1ee7aef677c6179e0a"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d0a3caea23b27e0354878ea0ae68c8cc8b88b3f1","unresolved":false,"context_lines":[{"line_number":3522,"context_line":"              fake_block_device.FakeDbBlockDeviceDict("},{"line_number":3523,"context_line":"                    {\u0027id\u0027: 1,"},{"line_number":3524,"context_line":"                     \u0027source_type\u0027: \u0027volume\u0027, \u0027destination_type\u0027: \u0027volume\u0027,"},{"line_number":3525,"context_line":"                     \u0027boot_index\u0027: 0}),"},{"line_number":3526,"context_line":"              fake_block_device.FakeDbBlockDeviceDict("},{"line_number":3527,"context_line":"                    {\u0027id\u0027: 2,"},{"line_number":3528,"context_line":"                     \u0027source_type\u0027: \u0027volume\u0027, \u0027destination_type\u0027: \u0027volume\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa0c359_4424e9c6","line":3525,"updated":"2018-02-05 20:08:55.000000000","message":"This is what I based my example on.","commit_id":"3b079f55641e443b7c51bc1ee7aef677c6179e0a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d0a3caea23b27e0354878ea0ae68c8cc8b88b3f1","unresolved":false,"context_lines":[{"line_number":3740,"context_line":"                fake_block_device.FakeDbBlockDeviceDict("},{"line_number":3741,"context_line":"                    {\u0027id\u0027: 1,"},{"line_number":3742,"context_line":"                     \u0027source_type\u0027: \u0027volume\u0027, \u0027destination_type\u0027: \u0027volume\u0027,"},{"line_number":3743,"context_line":"                     \u0027device_name\u0027: \u0027/dev/sdc\u0027, \u0027disk_bus\u0027: \u0027scsi\u0027}),"},{"line_number":3744,"context_line":"                ]"},{"line_number":3745,"context_line":"        )"},{"line_number":3746,"context_line":"        bd_info \u003d {"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa0c359_24567d2b","line":3743,"updated":"2018-02-05 20:08:55.000000000","message":"So this one is the configdrive. To make this also boot-from-volume, I *think* you need to add another block device mapping like:\n\n {\u0027id\u0027: 2,\n  \u0027source_type\u0027: \u0027volume\u0027, \u0027destination_type\u0027: \u0027volume\u0027, \u0027boot_index\u0027: 0}","commit_id":"3b079f55641e443b7c51bc1ee7aef677c6179e0a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4e15e115c7fcfa312f5dd22ca509f2c89b874f19","unresolved":false,"context_lines":[{"line_number":3740,"context_line":"                fake_block_device.FakeDbBlockDeviceDict("},{"line_number":3741,"context_line":"                    {\u0027id\u0027: 1,"},{"line_number":3742,"context_line":"                     \u0027source_type\u0027: \u0027volume\u0027, \u0027destination_type\u0027: \u0027volume\u0027,"},{"line_number":3743,"context_line":"                     \u0027device_name\u0027: \u0027/dev/sdc\u0027, \u0027disk_bus\u0027: \u0027scsi\u0027}),"},{"line_number":3744,"context_line":"                ]"},{"line_number":3745,"context_line":"        )"},{"line_number":3746,"context_line":"        bd_info \u003d {"}],"source_content_type":"text/x-python","patch_set":2,"id":"df7087c5_25a49eb9","line":3743,"in_reply_to":"3fa0c359_24567d2b","updated":"2018-03-15 00:39:27.000000000","message":"Correction: I now realize this is not the configdrive. The configdrive is cfg.devices[2] on L3760, /dev/hda with an \u0027ide\u0027 bus.\n\nThis one is just one attached scsi volume.","commit_id":"3b079f55641e443b7c51bc1ee7aef677c6179e0a"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"4eafbe56972c1db2ea23ecb073de51d7405ae602","unresolved":false,"context_lines":[{"line_number":3675,"context_line":"            self.assertEqual(cfg.devices[4].model, \u0027virtio-scsi\u0027)"},{"line_number":3676,"context_line":"            mock_save.assert_called_with()"},{"line_number":3677,"context_line":""},{"line_number":3678,"context_line":"    def test_get_guest_config_one_scsi_volume_with_configdrive(self):"},{"line_number":3679,"context_line":"        \"\"\"Tests that the unit attribute is only incremented for block devices"},{"line_number":3680,"context_line":"        that have a scsi bus."},{"line_number":3681,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"df7087c5_5aef2c1d","line":3678,"range":{"start_line":3678,"start_character":8,"end_line":3678,"end_character":62},"updated":"2018-03-15 04:50:07.000000000","message":"Is this testcase doing anything that isn\u0027t also covered in the other testcase?","commit_id":"e4a9b11d6d4488b4c7e270e8e965f49e79806eef"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6d06a17cb0d30dbe603661d20100f11b86702320","unresolved":false,"context_lines":[{"line_number":3675,"context_line":"            self.assertEqual(cfg.devices[4].model, \u0027virtio-scsi\u0027)"},{"line_number":3676,"context_line":"            mock_save.assert_called_with()"},{"line_number":3677,"context_line":""},{"line_number":3678,"context_line":"    def test_get_guest_config_one_scsi_volume_with_configdrive(self):"},{"line_number":3679,"context_line":"        \"\"\"Tests that the unit attribute is only incremented for block devices"},{"line_number":3680,"context_line":"        that have a scsi bus."},{"line_number":3681,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"df7087c5_81445e75","line":3678,"range":{"start_line":3678,"start_character":8,"end_line":3678,"end_character":62},"in_reply_to":"df7087c5_5aef2c1d","updated":"2018-03-15 22:10:57.000000000","message":"For the most part, no, but this test case is different in that if we aren\u0027t booting from volume, then we\u0027ll use unit number 0 for a normal scsi device instead of reserving it for the boot volume for a boot-from-volume instance. I\u0027ll update this test to add more asserts and add to the docstring to make it clear.","commit_id":"e4a9b11d6d4488b4c7e270e8e965f49e79806eef"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"4eafbe56972c1db2ea23ecb073de51d7405ae602","unresolved":false,"context_lines":[{"line_number":3789,"context_line":"            # The fourth device will be the other attached volume."},{"line_number":3790,"context_line":"            self.assertIsInstance(cfg.devices[3],"},{"line_number":3791,"context_line":"                                  vconfig.LibvirtConfigGuestDisk)"},{"line_number":3792,"context_line":"            self.assertEqual(\u0027sdc\u0027, cfg.devices[3].target_dev)"},{"line_number":3793,"context_line":"            self.assertEqual(\u0027scsi\u0027, cfg.devices[3].target_bus)"},{"line_number":3794,"context_line":"            self.assertEqual(2, cfg.devices[3].device_addr.unit)"},{"line_number":3795,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"df7087c5_1af534ed","line":3792,"range":{"start_line":3792,"start_character":30,"end_line":3792,"end_character":33},"updated":"2018-03-15 04:50:07.000000000","message":"just curious, how did we get to \"sdc\" without any \"sdb\"?","commit_id":"e4a9b11d6d4488b4c7e270e8e965f49e79806eef"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6d06a17cb0d30dbe603661d20100f11b86702320","unresolved":false,"context_lines":[{"line_number":3789,"context_line":"            # The fourth device will be the other attached volume."},{"line_number":3790,"context_line":"            self.assertIsInstance(cfg.devices[3],"},{"line_number":3791,"context_line":"                                  vconfig.LibvirtConfigGuestDisk)"},{"line_number":3792,"context_line":"            self.assertEqual(\u0027sdc\u0027, cfg.devices[3].target_dev)"},{"line_number":3793,"context_line":"            self.assertEqual(\u0027scsi\u0027, cfg.devices[3].target_bus)"},{"line_number":3794,"context_line":"            self.assertEqual(2, cfg.devices[3].device_addr.unit)"},{"line_number":3795,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"df7087c5_012daea6","line":3792,"range":{"start_line":3792,"start_character":30,"end_line":3792,"end_character":33},"in_reply_to":"df7087c5_1af534ed","updated":"2018-03-15 22:10:57.000000000","message":"I was wondering if anyone would complain about that. :) The ephemeral disk on L3771 is actually \u0027sdb\u0027 but I forgot to add an assert for it. I\u0027ll add it.","commit_id":"e4a9b11d6d4488b4c7e270e8e965f49e79806eef"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"04b1ff64a4a1537b63d63700efa86320359de0e6","unresolved":false,"context_lines":[{"line_number":3734,"context_line":""},{"line_number":3735,"context_line":"            # This is the config drive. It should not have unit number set."},{"line_number":3736,"context_line":"            self.assertIsInstance(cfg.devices[2],"},{"line_number":3737,"context_line":"                                  vconfig.LibvirtConfigGuestDisk)"},{"line_number":3738,"context_line":"            self.assertEqual(\u0027hda\u0027, cfg.devices[2].target_dev)"},{"line_number":3739,"context_line":"            self.assertEqual(\u0027ide\u0027, cfg.devices[2].target_bus)"},{"line_number":3740,"context_line":"            self.assertIsNone(cfg.devices[2].device_addr)"}],"source_content_type":"text/x-python","patch_set":4,"id":"df7087c5_2e5be4f5","line":3737,"updated":"2018-03-16 17:27:55.000000000","message":"Bah, I forgot to add here:\n\n self.assertIn(\u0027disk.config\u0027, cfg.devices[2].source_path)","commit_id":"1687817915eb78c94bf59feaa6fd53f7384d6cd9"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e8539cf69a7850bd60fc3720859f26dd5b1a9d1a","unresolved":false,"context_lines":[{"line_number":3723,"context_line":"                LOG.debug(\u0027Config drive not found in RBD, falling back to the \u0027"},{"line_number":3724,"context_line":"                          \u0027instance directory\u0027, instance\u003dinstance)"},{"line_number":3725,"context_line":"        disk_info \u003d disk_mapping[name]"},{"line_number":3726,"context_line":"        if \u0027unit\u0027 in disk_mapping and disk_info[\u0027bus\u0027] \u003d\u003d \u0027scsi\u0027:"},{"line_number":3727,"context_line":"            disk_unit \u003d disk_mapping[\u0027unit\u0027]"},{"line_number":3728,"context_line":"            disk_mapping[\u0027unit\u0027] +\u003d 1  # Increments for the next disk added"},{"line_number":3729,"context_line":"        conf \u003d disk.libvirt_info(disk_info[\u0027bus\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f93b717_b7be1363","line":3726,"updated":"2018-01-26 19:54:22.000000000","message":"This seems like a right thing to do. All of the code that ever sets disk_mapping[\u0027unit\u0027] is guarded by checks for the scsi_controller, so I think this had assumed that if \u0027unit\u0027 is in disk_mapping, it\u0027s scsi. But that\u0027s not necessarily true because you could be using the scsi_controller and still have disks that are not scsi (like the configdrive). So it seems like it makes sense to guard this with a scsi check too.\n\nBased on comment 2 in the LP bug, we need to do a bit more to handle the case where we have boot-from-volume plus an additional attached storage volume. We need to make sure that the boot volume (boot_index\u003d0) gets disk_mapping[\u0027unit\u0027] \u003d 0.","commit_id":"cbe80c1d1bd11263839958f104109ff7ab9c0757"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e8539cf69a7850bd60fc3720859f26dd5b1a9d1a","unresolved":false,"context_lines":[{"line_number":3762,"context_line":"            # use disk_mapping as container to keep reference of the"},{"line_number":3763,"context_line":"            # unit added and be able to increment it for each disk"},{"line_number":3764,"context_line":"            # added."},{"line_number":3765,"context_line":"            disk_mapping[\u0027unit\u0027] \u003d 0"},{"line_number":3766,"context_line":""},{"line_number":3767,"context_line":"        def _get_ephemeral_devices():"},{"line_number":3768,"context_line":"            eph_devices \u003d []"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f93b717_770e4b8e","line":3765,"updated":"2018-01-26 19:54:22.000000000","message":"Maybe here we could check if boot-from-volume and start disk_mapping[\u0027unit\u0027] \u003d 1 in that case, else start from 0.\n\n disk_mapping[\u0027unit\u0027] \u003d 1 if self._is_booted_from_volume(block_device_info) else 0\n\nAnd then we\u0027ll make sure to use 0 for the bootable volume.","commit_id":"cbe80c1d1bd11263839958f104109ff7ab9c0757"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e8539cf69a7850bd60fc3720859f26dd5b1a9d1a","unresolved":false,"context_lines":[{"line_number":3853,"context_line":"            self._connect_volume(context, connection_info, instance)"},{"line_number":3854,"context_line":"            if scsi_controller and scsi_controller.model \u003d\u003d \u0027virtio-scsi\u0027:"},{"line_number":3855,"context_line":"                info[\u0027unit\u0027] \u003d disk_mapping[\u0027unit\u0027]"},{"line_number":3856,"context_line":"                disk_mapping[\u0027unit\u0027] +\u003d 1"},{"line_number":3857,"context_line":"            cfg \u003d self._get_volume_config(connection_info, info)"},{"line_number":3858,"context_line":"            devices.append(cfg)"},{"line_number":3859,"context_line":"            vol[\u0027connection_info\u0027] \u003d connection_info"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f93b717_9749978c","line":3856,"updated":"2018-01-26 19:54:22.000000000","message":"Here it looks like we need to check the boot_index and if it\u0027s 0, set disk_mapping[\u0027unit\u0027] \u003d 0, else do the incrementing thing.\n\n if info[\u0027boot_index\u0027] \u003d\u003d 0:\n     info[\u0027unit\u0027] \u003d 0\n else:\n     info[\u0027unit\u0027] \u003d disk_mapping[\u0027unit\u0027]\n     disk_mapping[\u0027unit\u0027] +\u003d 1","commit_id":"cbe80c1d1bd11263839958f104109ff7ab9c0757"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"21203f7add4412c4e43ce87d7459893b77f0e6b1","unresolved":false,"context_lines":[{"line_number":3853,"context_line":"            self._connect_volume(context, connection_info, instance)"},{"line_number":3854,"context_line":"            if scsi_controller and scsi_controller.model \u003d\u003d \u0027virtio-scsi\u0027:"},{"line_number":3855,"context_line":"                info[\u0027unit\u0027] \u003d disk_mapping[\u0027unit\u0027]"},{"line_number":3856,"context_line":"                disk_mapping[\u0027unit\u0027] +\u003d 1"},{"line_number":3857,"context_line":"            cfg \u003d self._get_volume_config(connection_info, info)"},{"line_number":3858,"context_line":"            devices.append(cfg)"},{"line_number":3859,"context_line":"            vol[\u0027connection_info\u0027] \u003d connection_info"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa0c359_609891bf","line":3856,"in_reply_to":"3fa0c359_663e8ddb","updated":"2018-02-08 18:21:42.000000000","message":"I had a browse through the libvirt source today and I think I found what Vlad described. This is a function that takes a disk name and returns an \"index\":\n\nhttps://github.com/libvirt/libvirt/blob/5315d20/src/util/virutil.c#L553-L624\n\nAnd it looks like this is how it\u0027s used to come up with the unit number:\n\nhttps://github.com/libvirt/libvirt/blob/5315d20/src/conf/domain_conf.c#L7883\n\nSo, we could discuss and consider forklifting that code into nova to provide back the nice ordering that was there pre-Ocata. But I don\u0027t think it should be part of this bug fix patch.","commit_id":"cbe80c1d1bd11263839958f104109ff7ab9c0757"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6d093d02a8d9fbfbcce87e8e47bf43490570ac17","unresolved":false,"context_lines":[{"line_number":3853,"context_line":"            self._connect_volume(context, connection_info, instance)"},{"line_number":3854,"context_line":"            if scsi_controller and scsi_controller.model \u003d\u003d \u0027virtio-scsi\u0027:"},{"line_number":3855,"context_line":"                info[\u0027unit\u0027] \u003d disk_mapping[\u0027unit\u0027]"},{"line_number":3856,"context_line":"                disk_mapping[\u0027unit\u0027] +\u003d 1"},{"line_number":3857,"context_line":"            cfg \u003d self._get_volume_config(connection_info, info)"},{"line_number":3858,"context_line":"            devices.append(cfg)"},{"line_number":3859,"context_line":"            vol[\u0027connection_info\u0027] \u003d connection_info"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa0c359_663e8ddb","line":3856,"in_reply_to":"3fa0c359_86ddd915","updated":"2018-02-07 18:38:05.000000000","message":"Thanks for your prompt reply.\n\nOkay, so libvirt\u0027s automatic assignment of unit number coincided with the alphabetical order of the device names. I\u0027ll take a look at libvirt to see what it does there and whether we can provide similar behavior here.\n\nIn your example with the block_device_mapping table, with this patch (see revision 2), /dev/sda will receive unit 0 because it is boot_index 0. And /dev/sdb will receive unit 1.\n\nBut the remaining concern is that the unit numbers may not coincide with the alphabetical ordering of the device names as they used to when libvirt was automatically assigning unit numbers in Newton.","commit_id":"cbe80c1d1bd11263839958f104109ff7ab9c0757"},{"author":{"_account_id":9373,"name":"Vlad Gusev","email":"vlad.esten@gmail.com","username":"s10"},"change_message_id":"1b03498226553c5ce58e67e3b8c46e4301160a75","unresolved":false,"context_lines":[{"line_number":3853,"context_line":"            self._connect_volume(context, connection_info, instance)"},{"line_number":3854,"context_line":"            if scsi_controller and scsi_controller.model \u003d\u003d \u0027virtio-scsi\u0027:"},{"line_number":3855,"context_line":"                info[\u0027unit\u0027] \u003d disk_mapping[\u0027unit\u0027]"},{"line_number":3856,"context_line":"                disk_mapping[\u0027unit\u0027] +\u003d 1"},{"line_number":3857,"context_line":"            cfg \u003d self._get_volume_config(connection_info, info)"},{"line_number":3858,"context_line":"            devices.append(cfg)"},{"line_number":3859,"context_line":"            vol[\u0027connection_info\u0027] \u003d connection_info"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa0c359_86ddd915","line":3856,"in_reply_to":"3fa0c359_bb1cb1c1","updated":"2018-02-07 18:26:00.000000000","message":"For example, in Newton, if we create volume-backed VM with 2 volumes (/dev/sda, /dev/sdb) and with the config drive, all those sd* volumes will appear inside the VM by alphabetical order. It looks like libvirt just gave them scsi bus unit number automatically accordingly to their device letter.\n\nIn Ocata, disk_mapping will be:\n {u\u0027/dev/sdb\u0027: {\u0027bus\u0027: u\u0027scsi\u0027, \u0027boot_index\u0027: \u00272\u0027, \u0027type\u0027: u\u0027disk\u0027, \u0027dev\u0027: u\u0027sdb\u0027}, \u0027disk.config\u0027: {\u0027bus\u0027: \u0027ide\u0027, \u0027type\u0027: \u0027cdrom\u0027, \u0027dev\u0027: \u0027hda\u0027}, \u0027root\u0027: {\u0027bus\u0027: u\u0027scsi\u0027, \u0027boot_index\u0027: \u00271\u0027, \u0027type\u0027: u\u0027disk\u0027, \u0027dev\u0027: u\u0027sda\u0027}, u\u0027/dev/sda\u0027: {\u0027bus\u0027: u\u0027scsi\u0027, \u0027boot_index\u0027: \u00271\u0027, \u0027type\u0027: u\u0027disk\u0027, \u0027dev\u0027: u\u0027sda\u0027}}\n\nIn the table nova.block_device_mapping we see:\n select volume_id,device_name,boot_index from nova.block_device_mapping where instance_uuid \u003d \u00275a02dc9e-a9b0-4bfc-a932-a4f57f60aaaf\u0027;\n +--------------------------------------+-------------+------------+\n | volume_id                            | device_name | boot_index |\n +--------------------------------------+-------------+------------+\n | dd9eb2ff-c82f-4be9-8115-900c227eb130 | /dev/sda    |          0 |\n | 518c1f46-a23c-48a4-9258-65f34ab11e6e | /dev/sdb    |          1 |\n +--------------------------------------+-------------+------------+\n\n\nSo even with patch from this review volume /dev/sdb will receive unit 0 (and become /dev/sda inside the vm), and /dev/sda volume will be attached with the unit 1 (and appear as /dev/sdb inside vm; without a patch unit will be 2).","commit_id":"cbe80c1d1bd11263839958f104109ff7ab9c0757"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"06c8c574353bbc196566b800338c935b6847c7fa","unresolved":false,"context_lines":[{"line_number":3853,"context_line":"            self._connect_volume(context, connection_info, instance)"},{"line_number":3854,"context_line":"            if scsi_controller and scsi_controller.model \u003d\u003d \u0027virtio-scsi\u0027:"},{"line_number":3855,"context_line":"                info[\u0027unit\u0027] \u003d disk_mapping[\u0027unit\u0027]"},{"line_number":3856,"context_line":"                disk_mapping[\u0027unit\u0027] +\u003d 1"},{"line_number":3857,"context_line":"            cfg \u003d self._get_volume_config(connection_info, info)"},{"line_number":3858,"context_line":"            devices.append(cfg)"},{"line_number":3859,"context_line":"            vol[\u0027connection_info\u0027] \u003d connection_info"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa0c359_bb1cb1c1","line":3856,"in_reply_to":"5f93b717_39f15a96","updated":"2018-02-07 16:28:04.000000000","message":"What do you mean by \"random order\"? What was the ordering before Ocata and how has it changed?","commit_id":"cbe80c1d1bd11263839958f104109ff7ab9c0757"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ad74cfb0a4de60c7bcc62acb5ff29ee59edb1813","unresolved":false,"context_lines":[{"line_number":3853,"context_line":"            self._connect_volume(context, connection_info, instance)"},{"line_number":3854,"context_line":"            if scsi_controller and scsi_controller.model \u003d\u003d \u0027virtio-scsi\u0027:"},{"line_number":3855,"context_line":"                info[\u0027unit\u0027] \u003d disk_mapping[\u0027unit\u0027]"},{"line_number":3856,"context_line":"                disk_mapping[\u0027unit\u0027] +\u003d 1"},{"line_number":3857,"context_line":"            cfg \u003d self._get_volume_config(connection_info, info)"},{"line_number":3858,"context_line":"            devices.append(cfg)"},{"line_number":3859,"context_line":"            vol[\u0027connection_info\u0027] \u003d connection_info"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f93b717_b27381b1","line":3856,"in_reply_to":"5f93b717_9749978c","updated":"2018-01-26 20:07:20.000000000","message":"Sorry, I think actually you probably have to check the BDM for the boot_index instead:\n\n if vol.get(\u0027boot_index\u0027) \u003d\u003d 0:\n\ninstead of info[\u0027boot_index\u0027]. I\u0027m not sure when/how info[\u0027boot_index\u0027] gets set.","commit_id":"cbe80c1d1bd11263839958f104109ff7ab9c0757"},{"author":{"_account_id":9373,"name":"Vlad Gusev","email":"vlad.esten@gmail.com","username":"s10"},"change_message_id":"85691bac118307fd893b82e648f6a3a128322b7a","unresolved":false,"context_lines":[{"line_number":3853,"context_line":"            self._connect_volume(context, connection_info, instance)"},{"line_number":3854,"context_line":"            if scsi_controller and scsi_controller.model \u003d\u003d \u0027virtio-scsi\u0027:"},{"line_number":3855,"context_line":"                info[\u0027unit\u0027] \u003d disk_mapping[\u0027unit\u0027]"},{"line_number":3856,"context_line":"                disk_mapping[\u0027unit\u0027] +\u003d 1"},{"line_number":3857,"context_line":"            cfg \u003d self._get_volume_config(connection_info, info)"},{"line_number":3858,"context_line":"            devices.append(cfg)"},{"line_number":3859,"context_line":"            vol[\u0027connection_info\u0027] \u003d connection_info"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f93b717_39f15a96","line":3856,"in_reply_to":"5f93b717_b27381b1","updated":"2018-01-30 09:32:25.000000000","message":"I think we need to calculate info[\u0027unit\u0027] value based on the vol_dev variable. Something like based on ord(block_device.get_device_letter(vol_dev)). Because disk_mapping is an unsorted mess. If we connect more than one volume to the vm, all of them might attach to scsi controller in random order, not in desired, like it was before Ocata (/dev/sdb, /dev/sdc, /dev/sdd and so on).","commit_id":"cbe80c1d1bd11263839958f104109ff7ab9c0757"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"8800a85757e455451d9580b6d4799a0da11eb2be","unresolved":false,"context_lines":[{"line_number":3850,"context_line":"            # we need to start the disk mapping unit at 1 since we set the"},{"line_number":3851,"context_line":"            # bootable volume\u0027s unit to 0 for the bootable volume."},{"line_number":3852,"context_line":"            disk_mapping[\u0027unit\u0027] \u003d 0"},{"line_number":3853,"context_line":"            if self._is_booted_from_volume(block_device_info):"},{"line_number":3854,"context_line":"                disk_mapping[\u0027unit\u0027] \u003d 1"},{"line_number":3855,"context_line":""},{"line_number":3856,"context_line":"        def _get_ephemeral_devices():"}],"source_content_type":"text/x-python","patch_set":2,"id":"df7087c5_6e61d339","line":3853,"updated":"2018-03-07 07:53:02.000000000","message":"This is probably fixing: bug 1753394","commit_id":"3b079f55641e443b7c51bc1ee7aef677c6179e0a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d0a3caea23b27e0354878ea0ae68c8cc8b88b3f1","unresolved":false,"context_lines":[{"line_number":3948,"context_line":"                    info[\u0027unit\u0027] \u003d 0"},{"line_number":3949,"context_line":"                else:"},{"line_number":3950,"context_line":"                    info[\u0027unit\u0027] \u003d disk_mapping[\u0027unit\u0027]"},{"line_number":3951,"context_line":"                    disk_mapping[\u0027unit\u0027] +\u003d 1"},{"line_number":3952,"context_line":"            cfg \u003d self._get_volume_config(connection_info, info)"},{"line_number":3953,"context_line":"            devices.append(cfg)"},{"line_number":3954,"context_line":"            vol[\u0027connection_info\u0027] \u003d connection_info"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa0c359_ff49fe5f","line":3951,"updated":"2018-02-05 20:08:55.000000000","message":"I dunno if you saw the comment from Vlad about basing the unit number on the vol_dev name. I honestly don\u0027t understand how the unit numbering used to work in Ocata (when we weren\u0027t setting unit number), so I\u0027m not understanding how we should be doing it now.","commit_id":"3b079f55641e443b7c51bc1ee7aef677c6179e0a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"825705e831732c7d93aa9e1ff4ecbd243f235bc6","unresolved":false,"context_lines":[{"line_number":3941,"context_line":"            info \u003d disk_mapping[vol_dev]"},{"line_number":3942,"context_line":"            self._connect_volume(context, connection_info, instance)"},{"line_number":3943,"context_line":"            if scsi_controller and scsi_controller.model \u003d\u003d \u0027virtio-scsi\u0027:"},{"line_number":3944,"context_line":"                # Check if this is the bootable volume when in a"},{"line_number":3945,"context_line":"                # boot-from-volume instance, and if so, ensure the unit"},{"line_number":3946,"context_line":"                # attribute is 0."},{"line_number":3947,"context_line":"                if vol.get(\u0027boot_index\u0027) \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":5,"id":"bf659307_4d2a3d2c","line":3944,"updated":"2018-03-29 23:19:36.000000000","message":"I wonder how any of this could get screwed up during a migration where the destination node doesn\u0027t have this code. I think we only hit this during cold migration, which is rebuilding the guest on another host, so should be OK. And live migration doesn\u0027t go through this flow as far as I know. See thoughts below.","commit_id":"2616b384e642b6eb58eef7da87b6e893f25a949e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"825705e831732c7d93aa9e1ff4ecbd243f235bc6","unresolved":false,"context_lines":[{"line_number":6799,"context_line":"            params \u003d None"},{"line_number":6800,"context_line":"            new_xml_str \u003d None"},{"line_number":6801,"context_line":"            if CONF.libvirt.virt_type !\u003d \"parallels\":"},{"line_number":6802,"context_line":"                new_xml_str \u003d libvirt_migrate.get_updated_guest_xml("},{"line_number":6803,"context_line":"                    # TODO(sahid): It\u0027s not a really good idea to pass"},{"line_number":6804,"context_line":"                    # the method _get_volume_config and we should to find"},{"line_number":6805,"context_line":"                    # a way to avoid this in future."}],"source_content_type":"text/x-python","patch_set":5,"id":"bf659307_4dad5d99","line":6802,"updated":"2018-03-29 23:19:36.000000000","message":"Live migration goes through this using the existing guest xml and then tweaks some things for the destination host like the volume config and that relies on the migrate_data.bdms which come back from the dest host during pre_live_migration, and that doesn\u0027t have anything about disk indexes, it looks for volume configs based on the serial (volume id) and might replace entries. Can any of that get messed up though?\n\nLike, will migrating from a source host that has the correct units to an older compute break the guest? Will migrating from an older compute to a newer dest compute with this fix resolve the issue (rebuild the config?) or just be the same as before? Maybe the worst case scenario is nothing changes and you\u0027re stuck with the bug regardless of live migration until you backport this fix to all of your affected computes.\n\nAnyway, mostly just thinking out loud here, maybe Sahid can answer some of this.","commit_id":"2616b384e642b6eb58eef7da87b6e893f25a949e"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"fcf3ee29721a61e3bb0851d9f4f17fcf02f162ce","unresolved":false,"context_lines":[{"line_number":6799,"context_line":"            params \u003d None"},{"line_number":6800,"context_line":"            new_xml_str \u003d None"},{"line_number":6801,"context_line":"            if CONF.libvirt.virt_type !\u003d \"parallels\":"},{"line_number":6802,"context_line":"                new_xml_str \u003d libvirt_migrate.get_updated_guest_xml("},{"line_number":6803,"context_line":"                    # TODO(sahid): It\u0027s not a really good idea to pass"},{"line_number":6804,"context_line":"                    # the method _get_volume_config and we should to find"},{"line_number":6805,"context_line":"                    # a way to avoid this in future."}],"source_content_type":"text/x-python","patch_set":5,"id":"bf659307_ea707a74","line":6802,"in_reply_to":"bf659307_4dad5d99","updated":"2018-04-04 14:33:21.000000000","message":"New to old LM should work as the volume config generated by using   LibvirtLiveMigrateBDMInfo from the old destination compute shouldn\u0027t include any unit details[1][2][3]. So the unit referenced in the new / existing instance XML should remain unchanged.\n\nOld to New and subsequent new to new LM attempts without a hard reboot are not going to change the unit ordering for the same reason and I think that\u0027s fine.\n\n[1] https://github.com/openstack/nova/blob/ed52fae692d79289028abacfc694f802f3ac6b5e/nova/objects/migrate_data.py#L76-L86\n[2] https://github.com/openstack/nova/blob/ed52fae692d79289028abacfc694f802f3ac6b5e/nova/virt/libvirt/migration.py#L150-L151\n[3] https://github.com/openstack/nova/blob/ed52fae692d79289028abacfc694f802f3ac6b5e/nova/virt/libvirt/volume/volume.py#L93-L102","commit_id":"2616b384e642b6eb58eef7da87b6e893f25a949e"}]}