)]}'
{"nova/tests/unit/virt/vmwareapi/test_vmops.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"60781d4a081fdb41ffdd8727c2bb652412764f9e","unresolved":false,"context_lines":[{"line_number":1161,"context_line":"                   extra_specs\u003dNone,"},{"line_number":1162,"context_line":"                   config_drive\u003dFalse):"},{"line_number":1163,"context_line":""},{"line_number":1164,"context_line":"        image_size \u003d (self._instance.root_gb) * units.Gi / 2"},{"line_number":1165,"context_line":"        image \u003d {"},{"line_number":1166,"context_line":"            \u0027id\u0027: self._image_id,"},{"line_number":1167,"context_line":"            \u0027disk_format\u0027: \u0027vmdk\u0027,"}],"source_content_type":"text/x-python","patch_set":31,"id":"9a80dd14_6c2678bc","line":1164,"updated":"2015-03-04 18:12:42.000000000","message":"nit: parens not needed.","commit_id":"264425678fd4a37618c2bceae8e62f6bba778223"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"60781d4a081fdb41ffdd8727c2bb652412764f9e","unresolved":false,"context_lines":[{"line_number":1244,"context_line":"                    mock_attach_root \u003d ("},{"line_number":1245,"context_line":"                        self._vmops._volumeops.attach_root_volume)"},{"line_number":1246,"context_line":"                    mock_attach \u003d self._vmops._volumeops.attach_volume"},{"line_number":1247,"context_line":"                    adapter_type \u003d bdm.get(\u0027disk_bus\u0027) or vi.ii.adapter_type"},{"line_number":1248,"context_line":"                    if bdm.get(\u0027boot_index\u0027) \u003d\u003d 0:"},{"line_number":1249,"context_line":"                        mock_attach_root.assert_any_call("},{"line_number":1250,"context_line":"                            bdm[\u0027connection_info\u0027], self._instance,"}],"source_content_type":"text/x-python","patch_set":31,"id":"9a80dd14_8cee4c21","line":1247,"updated":"2015-03-04 18:12:42.000000000","message":"nit:\n\n adapter_type \u003d bdm.get(\u0027disk_bus\u0027, vi.ii.adapter_type)","commit_id":"264425678fd4a37618c2bceae8e62f6bba778223"}],"nova/tests/virt/vmwareapi/test_driver_api.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"10a4ec532832777f50f9efd0c9b5578a50874944","unresolved":false,"context_lines":[{"line_number":1128,"context_line":"        self.mox.StubOutWithMock(volumeops.VMwareVolumeOps,"},{"line_number":1129,"context_line":"                                 \u0027attach_volume\u0027)"},{"line_number":1130,"context_line":"        volumeops.VMwareVolumeOps.attach_volume(connection_info,"},{"line_number":1131,"context_line":"                self.instance, mox.IgnoreArg())"},{"line_number":1132,"context_line":"        self.mox.ReplayAll()"},{"line_number":1133,"context_line":"        block_device_info \u003d {\u0027mount_device\u0027: \u0027vda\u0027}"},{"line_number":1134,"context_line":"        self.conn.spawn(self.context, self.instance, self.image,"}],"source_content_type":"text/x-python","patch_set":11,"id":"9aa7fdbe_056c4b34","line":1131,"updated":"2014-10-26 08:45:30.000000000","message":"i think here we should try and actually check the correct parameter is possible.","commit_id":"300f9e0899cbc2f1cf0a3eed0439efcfc715a9d2"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"d0236fe064e3184008b32df855752eaf88272d13","unresolved":false,"context_lines":[{"line_number":1128,"context_line":"        self.mox.StubOutWithMock(volumeops.VMwareVolumeOps,"},{"line_number":1129,"context_line":"                                 \u0027attach_volume\u0027)"},{"line_number":1130,"context_line":"        volumeops.VMwareVolumeOps.attach_volume(connection_info,"},{"line_number":1131,"context_line":"                self.instance, mox.IgnoreArg())"},{"line_number":1132,"context_line":"        self.mox.ReplayAll()"},{"line_number":1133,"context_line":"        block_device_info \u003d {\u0027mount_device\u0027: \u0027vda\u0027}"},{"line_number":1134,"context_line":"        self.conn.spawn(self.context, self.instance, self.image,"}],"source_content_type":"text/x-python","patch_set":11,"id":"9aa7fdbe_6acdd2b6","line":1131,"in_reply_to":"9aa7fdbe_056c4b34","updated":"2014-10-27 15:11:21.000000000","message":"Sure.  It just ends up being the default \"lsiLogic\".","commit_id":"300f9e0899cbc2f1cf0a3eed0439efcfc715a9d2"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"10a4ec532832777f50f9efd0c9b5578a50874944","unresolved":false,"context_lines":[{"line_number":1765,"context_line":"        self.mox.StubOutWithMock(volumeops.VMwareVolumeOps,"},{"line_number":1766,"context_line":"                                 \u0027_attach_volume_vmdk\u0027)"},{"line_number":1767,"context_line":"        volumeops.VMwareVolumeOps._attach_volume_vmdk(connection_info,"},{"line_number":1768,"context_line":"                self.instance, mox.IgnoreArg())"},{"line_number":1769,"context_line":"        self.mox.ReplayAll()"},{"line_number":1770,"context_line":"        self.conn.attach_volume(None, connection_info, self.instance,"},{"line_number":1771,"context_line":"                                mount_point)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9aa7fdbe_2575cff9","line":1768,"updated":"2014-10-26 08:45:30.000000000","message":"ditto","commit_id":"300f9e0899cbc2f1cf0a3eed0439efcfc715a9d2"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"10a4ec532832777f50f9efd0c9b5578a50874944","unresolved":false,"context_lines":[{"line_number":1835,"context_line":"        self.mox.StubOutWithMock(volumeops.VMwareVolumeOps,"},{"line_number":1836,"context_line":"                                 \u0027_attach_volume_iscsi\u0027)"},{"line_number":1837,"context_line":"        volumeops.VMwareVolumeOps._attach_volume_iscsi(connection_info,"},{"line_number":1838,"context_line":"                self.instance, mox.IgnoreArg())"},{"line_number":1839,"context_line":"        self.mox.ReplayAll()"},{"line_number":1840,"context_line":"        self.conn.attach_volume(None, connection_info, self.instance,"},{"line_number":1841,"context_line":"                                mount_point)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9aa7fdbe_4572d311","line":1838,"updated":"2014-10-26 08:45:30.000000000","message":"ditto","commit_id":"300f9e0899cbc2f1cf0a3eed0439efcfc715a9d2"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"b823eb15d47a56ff277689565005a01109e90b7c","unresolved":false,"context_lines":[{"line_number":1103,"context_line":"        self.mox.StubOutWithMock(volumeops.VMwareVolumeOps,"},{"line_number":1104,"context_line":"                                 \u0027attach_volume\u0027)"},{"line_number":1105,"context_line":"        volumeops.VMwareVolumeOps.attach_volume(connection_info,"},{"line_number":1106,"context_line":"                self.instance, \u0027lsiLogic\u0027)"},{"line_number":1107,"context_line":"        self.mox.ReplayAll()"},{"line_number":1108,"context_line":"        block_device_info \u003d {\u0027block_device_mapping\u0027: root_disk,"},{"line_number":1109,"context_line":"                             \u0027root_device_name\u0027: \u0027/dev/vda\u0027}"}],"source_content_type":"text/x-python","patch_set":14,"id":"9aa7fdbe_ac2a3a2a","line":1106,"updated":"2014-10-29 11:05:47.000000000","message":"Did you intentionally not use the constant here?","commit_id":"c5ad2a2e2d3bb2a044e749d8cb2123aa53e98cb8"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"d07233fb6545a38df93b3d5fe1340ea4b2c8758c","unresolved":false,"context_lines":[{"line_number":1103,"context_line":"        self.mox.StubOutWithMock(volumeops.VMwareVolumeOps,"},{"line_number":1104,"context_line":"                                 \u0027attach_volume\u0027)"},{"line_number":1105,"context_line":"        volumeops.VMwareVolumeOps.attach_volume(connection_info,"},{"line_number":1106,"context_line":"                self.instance, \u0027lsiLogic\u0027)"},{"line_number":1107,"context_line":"        self.mox.ReplayAll()"},{"line_number":1108,"context_line":"        block_device_info \u003d {\u0027block_device_mapping\u0027: root_disk,"},{"line_number":1109,"context_line":"                             \u0027root_device_name\u0027: \u0027/dev/vda\u0027}"}],"source_content_type":"text/x-python","patch_set":14,"id":"9aa7fdbe_07febc38","line":1106,"in_reply_to":"9aa7fdbe_ac2a3a2a","updated":"2014-10-29 14:15:42.000000000","message":"I will switch it over to use constants.*","commit_id":"c5ad2a2e2d3bb2a044e749d8cb2123aa53e98cb8"}],"nova/tests/virt/vmwareapi/test_vmops.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"cf988e90f9bfec2fc4b6c98e51d2ddaf25eb2fdd","unresolved":false,"context_lines":[{"line_number":583,"context_line":"        # that we checked it was scrubbed"},{"line_number":584,"context_line":"        self.assertTrue(self.password_logged)"},{"line_number":585,"context_line":""},{"line_number":586,"context_line":"        self.assertEqual(1, mock_create_folder.call_count)"},{"line_number":587,"context_line":""},{"line_number":588,"context_line":"    def test_get_ds_browser(self):"},{"line_number":589,"context_line":"        cache \u003d self._vmops._datastore_browser_mapping"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_63a4364b","line":586,"updated":"2014-10-16 13:14:53.000000000","message":"Not relevant to this test. By all means mock it, but it doesn\u0027t actually matter if it was called or not.","commit_id":"7d39e08a6ead3ae02e03ec4e74a90498d1798516"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"557010595f4ad9e45dbf0c9a165d0b0bb3f9e93b","unresolved":false,"context_lines":[{"line_number":583,"context_line":"        # that we checked it was scrubbed"},{"line_number":584,"context_line":"        self.assertTrue(self.password_logged)"},{"line_number":585,"context_line":""},{"line_number":586,"context_line":"        self.assertEqual(1, mock_create_folder.call_count)"},{"line_number":587,"context_line":""},{"line_number":588,"context_line":"    def test_get_ds_browser(self):"},{"line_number":589,"context_line":"        cache \u003d self._vmops._datastore_browser_mapping"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_ce0b4622","line":586,"in_reply_to":"baa201ad_63a4364b","updated":"2014-10-16 15:25:01.000000000","message":"Based on most of the patches I have done, if you mock it, you must verified that it is later called.  Otherwise, in most cases I end up getting -1 for not verifying that the mocked method got called.  It does not hurt to have it verified here.","commit_id":"7d39e08a6ead3ae02e03ec4e74a90498d1798516"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"6d55a1bde30fc1a15a9ab548d92751bc55814273","unresolved":false,"context_lines":[{"line_number":583,"context_line":"        # that we checked it was scrubbed"},{"line_number":584,"context_line":"        self.assertTrue(self.password_logged)"},{"line_number":585,"context_line":""},{"line_number":586,"context_line":"        self.assertEqual(1, mock_create_folder.call_count)"},{"line_number":587,"context_line":""},{"line_number":588,"context_line":"    def test_get_ds_browser(self):"},{"line_number":589,"context_line":"        cache \u003d self._vmops._datastore_browser_mapping"}],"source_content_type":"text/x-python","patch_set":3,"id":"9aa7fdbe_c7e2f402","line":586,"in_reply_to":"baa201ad_ce0b4622","updated":"2014-10-20 08:39:37.000000000","message":"Don\u0027t pander to them! It just makes these tests unnecessarily fragile.","commit_id":"7d39e08a6ead3ae02e03ec4e74a90498d1798516"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"cf988e90f9bfec2fc4b6c98e51d2ddaf25eb2fdd","unresolved":false,"context_lines":[{"line_number":836,"context_line":""},{"line_number":837,"context_line":"            mock_is_neutron.assert_called_once_with()"},{"line_number":838,"context_line":""},{"line_number":839,"context_line":"            self.assertEqual(2, mock_mkdir.call_count)"},{"line_number":840,"context_line":""},{"line_number":841,"context_line":"            mock_get_mo_id_for_instance.assert_called_once_with(self._instance)"},{"line_number":842,"context_line":"            mock_get_res_pool_ref.assert_called_once_with("}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_c3416264","line":839,"updated":"2014-10-16 13:14:53.000000000","message":"Aside: This test is absurdly coupled to the implementation: this is just voodoo. We really need to rewrite this to use and test fake vim.\n\nAnother day, though...","commit_id":"7d39e08a6ead3ae02e03ec4e74a90498d1798516"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"557010595f4ad9e45dbf0c9a165d0b0bb3f9e93b","unresolved":false,"context_lines":[{"line_number":836,"context_line":""},{"line_number":837,"context_line":"            mock_is_neutron.assert_called_once_with()"},{"line_number":838,"context_line":""},{"line_number":839,"context_line":"            self.assertEqual(2, mock_mkdir.call_count)"},{"line_number":840,"context_line":""},{"line_number":841,"context_line":"            mock_get_mo_id_for_instance.assert_called_once_with(self._instance)"},{"line_number":842,"context_line":"            mock_get_res_pool_ref.assert_called_once_with("}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_ee3acacb","line":839,"in_reply_to":"baa201ad_c3416264","updated":"2014-10-16 15:25:01.000000000","message":"Agreed.  The _test_spawn is really nested and needs to be broken out some more.","commit_id":"7d39e08a6ead3ae02e03ec4e74a90498d1798516"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"cf988e90f9bfec2fc4b6c98e51d2ddaf25eb2fdd","unresolved":false,"context_lines":[{"line_number":876,"context_line":"            if block_device_info:"},{"line_number":877,"context_line":"                bdms \u003d block_device_info[\u0027block_device_mapping\u0027]"},{"line_number":878,"context_line":"                for bdm in bdms:"},{"line_number":879,"context_line":"                    root_disk \u003d bdm"},{"line_number":880,"context_line":"                    mock_attach \u003d self._vmops._volumeops.attach_root_volume"},{"line_number":881,"context_line":"                    mock_attach.assert_any_call("},{"line_number":882,"context_line":"                            root_disk[\u0027connection_info\u0027], self._instance,"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_552e058d","line":879,"updated":"2014-10-16 13:14:53.000000000","message":"nit: redundant","commit_id":"7d39e08a6ead3ae02e03ec4e74a90498d1798516"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"557010595f4ad9e45dbf0c9a165d0b0bb3f9e93b","unresolved":false,"context_lines":[{"line_number":876,"context_line":"            if block_device_info:"},{"line_number":877,"context_line":"                bdms \u003d block_device_info[\u0027block_device_mapping\u0027]"},{"line_number":878,"context_line":"                for bdm in bdms:"},{"line_number":879,"context_line":"                    root_disk \u003d bdm"},{"line_number":880,"context_line":"                    mock_attach \u003d self._vmops._volumeops.attach_root_volume"},{"line_number":881,"context_line":"                    mock_attach.assert_any_call("},{"line_number":882,"context_line":"                            root_disk[\u0027connection_info\u0027], self._instance,"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_ce598636","line":879,"in_reply_to":"baa201ad_552e058d","updated":"2014-10-16 15:25:01.000000000","message":"Done","commit_id":"7d39e08a6ead3ae02e03ec4e74a90498d1798516"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"b823eb15d47a56ff277689565005a01109e90b7c","unresolved":false,"context_lines":[{"line_number":935,"context_line":"                    mock_attach \u003d self._vmops._volumeops.attach_root_volume"},{"line_number":936,"context_line":"                    mock_attach.assert_any_call("},{"line_number":937,"context_line":"                            bdm[\u0027connection_info\u0027], self._instance,"},{"line_number":938,"context_line":"                            self._ds.ref, mock.ANY)"},{"line_number":939,"context_line":""},{"line_number":940,"context_line":"            mock_enlist_image.assert_called_once_with("},{"line_number":941,"context_line":"                    self._image_id, self._ds, self._dc_info.ref)"}],"source_content_type":"text/x-python","patch_set":14,"id":"9aa7fdbe_6c4e9298","line":938,"updated":"2014-10-29 11:05:47.000000000","message":"This doesn\u0027t look right. We should only call attach_root_volume for the root volume. Other volumes should be attached with attach_volume(). In fact, in this particular case we will have attached the *image* as the root volume, not the passed in volume at all.","commit_id":"c5ad2a2e2d3bb2a044e749d8cb2123aa53e98cb8"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"d07233fb6545a38df93b3d5fe1340ea4b2c8758c","unresolved":false,"context_lines":[{"line_number":935,"context_line":"                    mock_attach \u003d self._vmops._volumeops.attach_root_volume"},{"line_number":936,"context_line":"                    mock_attach.assert_any_call("},{"line_number":937,"context_line":"                            bdm[\u0027connection_info\u0027], self._instance,"},{"line_number":938,"context_line":"                            self._ds.ref, mock.ANY)"},{"line_number":939,"context_line":""},{"line_number":940,"context_line":"            mock_enlist_image.assert_called_once_with("},{"line_number":941,"context_line":"                    self._image_id, self._ds, self._dc_info.ref)"}],"source_content_type":"text/x-python","patch_set":14,"id":"9aa7fdbe_67622818","line":938,"in_reply_to":"9aa7fdbe_6c4e9298","updated":"2014-10-29 14:15:42.000000000","message":"I will add a if/else logic to detect root and user attach_root_volume, otherwise use attach_volume.","commit_id":"c5ad2a2e2d3bb2a044e749d8cb2123aa53e98cb8"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"b823eb15d47a56ff277689565005a01109e90b7c","unresolved":false,"context_lines":[{"line_number":1032,"context_line":"        block_device_info \u003d {"},{"line_number":1033,"context_line":"            \u0027root_device_name\u0027: \u0027/dev/vda\u0027,"},{"line_number":1034,"context_line":"            \u0027block_device_mapping\u0027: [{\u0027connection_info\u0027: connection_info,"},{"line_number":1035,"context_line":"                                      \u0027mount_device\u0027: \u0027/dev/vda\u0027}]"},{"line_number":1036,"context_line":"        }"},{"line_number":1037,"context_line":"        self._test_spawn(block_device_info\u003dblock_device_info)"},{"line_number":1038,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9aa7fdbe_ec59a25c","line":1035,"updated":"2014-10-29 11:05:47.000000000","message":"Consider multiple bdms and no image.","commit_id":"c5ad2a2e2d3bb2a044e749d8cb2123aa53e98cb8"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"d07233fb6545a38df93b3d5fe1340ea4b2c8758c","unresolved":false,"context_lines":[{"line_number":1032,"context_line":"        block_device_info \u003d {"},{"line_number":1033,"context_line":"            \u0027root_device_name\u0027: \u0027/dev/vda\u0027,"},{"line_number":1034,"context_line":"            \u0027block_device_mapping\u0027: [{\u0027connection_info\u0027: connection_info,"},{"line_number":1035,"context_line":"                                      \u0027mount_device\u0027: \u0027/dev/vda\u0027}]"},{"line_number":1036,"context_line":"        }"},{"line_number":1037,"context_line":"        self._test_spawn(block_device_info\u003dblock_device_info)"},{"line_number":1038,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9aa7fdbe_a743306b","line":1035,"in_reply_to":"9aa7fdbe_ec59a25c","updated":"2014-10-29 14:15:42.000000000","message":"Sure.  This should be a separate test case outside of this.","commit_id":"c5ad2a2e2d3bb2a044e749d8cb2123aa53e98cb8"}],"nova/tests/virt/vmwareapi/test_volumeops.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"10a4ec532832777f50f9efd0c9b5578a50874944","unresolved":false,"context_lines":[{"line_number":101,"context_line":"                           \u0027serial\u0027: \u0027volume-fake-id\u0027,"},{"line_number":102,"context_line":"                           \u0027data\u0027: {\u0027volume\u0027: \u0027vm-10\u0027,"},{"line_number":103,"context_line":"                                    \u0027volume_id\u0027: \u0027volume-fake-id\u0027}}"},{"line_number":104,"context_line":"        instance \u003d mock.MagicMock(name\u003d\u0027fake-name\u0027, vm_state\u003dvm_states.ACTIVE)"},{"line_number":105,"context_line":"        vm_ref \u003d \u0027fake-vm-ref\u0027"},{"line_number":106,"context_line":"        volume_device \u003d mock.MagicMock()"},{"line_number":107,"context_line":"        volume_device.backing.fileName \u003d \u0027fake-path\u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"9aa7fdbe_a558df8a","line":104,"updated":"2014-10-26 08:45:30.000000000","message":"why are we not using a fake instance here?","commit_id":"300f9e0899cbc2f1cf0a3eed0439efcfc715a9d2"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"d0236fe064e3184008b32df855752eaf88272d13","unresolved":false,"context_lines":[{"line_number":101,"context_line":"                           \u0027serial\u0027: \u0027volume-fake-id\u0027,"},{"line_number":102,"context_line":"                           \u0027data\u0027: {\u0027volume\u0027: \u0027vm-10\u0027,"},{"line_number":103,"context_line":"                                    \u0027volume_id\u0027: \u0027volume-fake-id\u0027}}"},{"line_number":104,"context_line":"        instance \u003d mock.MagicMock(name\u003d\u0027fake-name\u0027, vm_state\u003dvm_states.ACTIVE)"},{"line_number":105,"context_line":"        vm_ref \u003d \u0027fake-vm-ref\u0027"},{"line_number":106,"context_line":"        volume_device \u003d mock.MagicMock()"},{"line_number":107,"context_line":"        volume_device.backing.fileName \u003d \u0027fake-path\u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"9aa7fdbe_aa709a72","line":104,"in_reply_to":"9aa7fdbe_a558df8a","updated":"2014-10-27 15:11:21.000000000","message":"Nothing is really needed from the instance, hence I just mocked it, but I can add in an instance.","commit_id":"300f9e0899cbc2f1cf0a3eed0439efcfc715a9d2"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"10a4ec532832777f50f9efd0c9b5578a50874944","unresolved":false,"context_lines":[{"line_number":139,"context_line":"                           \u0027serial\u0027: \u0027volume-fake-id\u0027,"},{"line_number":140,"context_line":"                           \u0027data\u0027: {\u0027volume\u0027: \u0027vm-10\u0027,"},{"line_number":141,"context_line":"                                    \u0027volume_id\u0027: \u0027volume-fake-id\u0027}}"},{"line_number":142,"context_line":"        instance \u003d mock.MagicMock(name\u003d\u0027fake-name\u0027, vm_state\u003dvm_states.ACTIVE)"},{"line_number":143,"context_line":"        vm_ref \u003d \u0027fake-vm-ref\u0027"},{"line_number":144,"context_line":"        default_adapter_type \u003d \u0027lsiLogic\u0027"},{"line_number":145,"context_line":"        path_and_type \u003d (\u0027fake-path\u0027, default_adapter_type, \u0027preallocated\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9aa7fdbe_c55de37a","line":142,"updated":"2014-10-26 08:45:30.000000000","message":"ditto","commit_id":"300f9e0899cbc2f1cf0a3eed0439efcfc715a9d2"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"b823eb15d47a56ff277689565005a01109e90b7c","unresolved":false,"context_lines":[{"line_number":147,"context_line":"                self._instance, adapter_type, \u0027preallocated\u0027,"},{"line_number":148,"context_line":"                vmdk_path\u003d\u0027fake-path\u0027)"},{"line_number":149,"context_line":"            update_volume_details.assert_called_once_with(vm_ref,"},{"line_number":150,"context_line":"                self._instance, connection_info[\u0027data\u0027][\u0027volume_id\u0027])"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    def _test_attach_volume_iscsi(self, adapter_type\u003dNone):"},{"line_number":153,"context_line":"        connection_info \u003d {\u0027driver_volume_type\u0027: \u0027iscsi\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"9aa7fdbe_8cfd7671","line":150,"updated":"2014-10-29 11:05:47.000000000","message":"Aside: I personally don\u0027t see any value in this kind of test. We\u0027re basically asserting that volumeops.attach_volume() calls all the internal functions that it calls. You might as well diff it. I would much rather see a test using the fake driver which attaches a volume to a vm, then tests the state of the fake driver to see if the end result is correct. However, it makes some people happy. In fact, they get grouchy if you don\u0027t do it.\n\nPlease don\u0027t change it in this patch, but if you\u0027re feeling keen a future patch which does this would be nice.","commit_id":"c5ad2a2e2d3bb2a044e749d8cb2123aa53e98cb8"}],"nova/virt/vmwareapi/vmops.py":[{"author":{"_account_id":9172,"name":"Radoslav Gerganov","email":"rgerganov@vmware.com","username":"rgerganov"},"change_message_id":"6f90b04dd4dce5055adf5d18bb351a98a08d4509","unresolved":false,"context_lines":[{"line_number":480,"context_line":""},{"line_number":481,"context_line":"            for disk in block_device_mapping:"},{"line_number":482,"context_line":"                connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":483,"context_line":"                disk_dev \u003d disk[\u0027mount_device\u0027]"},{"line_number":484,"context_line":"                if disk_dev is not None:"},{"line_number":485,"context_line":"                    disk_dev \u003d disk_dev.rpartition(\"/\")[2]"},{"line_number":486,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_2cb7a8f6","line":483,"updated":"2014-10-15 08:41:52.000000000","message":"The vmware driver is not able to mount volumes at specified mountpoint, so this is redundant. I have submitted patch which removes the mountpoint from VMwareVolumeOps: https://review.openstack.org/#/c/128577/","commit_id":"cc03b1bfb444b9a9041f0de21a35774f1ea4309d"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"9de4a6e8238044da2b98ea99af83483cf84841e6","unresolved":false,"context_lines":[{"line_number":480,"context_line":""},{"line_number":481,"context_line":"            for disk in block_device_mapping:"},{"line_number":482,"context_line":"                connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":483,"context_line":"                disk_dev \u003d disk[\u0027mount_device\u0027]"},{"line_number":484,"context_line":"                if disk_dev is not None:"},{"line_number":485,"context_line":"                    disk_dev \u003d disk_dev.rpartition(\"/\")[2]"},{"line_number":486,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_0f4c8c9d","line":483,"in_reply_to":"baa201ad_2cb7a8f6","updated":"2014-10-15 14:35:56.000000000","message":"I can remove the mountpoint here, and leave the attach_root_volume call as before.  Can you make your patch a dependency on this one?  That way other reviewers will know why I left self._default_root_device in the attach_root_volume call.  Otherwise, it would not look correct since self._default_root_device is a constant.","commit_id":"cc03b1bfb444b9a9041f0de21a35774f1ea4309d"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"cf988e90f9bfec2fc4b6c98e51d2ddaf25eb2fdd","unresolved":false,"context_lines":[{"line_number":460,"context_line":"            block_device_mapping \u003d driver.block_device_info_get_mapping("},{"line_number":461,"context_line":"                block_device_info)"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"        if instance.image_ref:"},{"line_number":464,"context_line":"            self._imagecache.enlist_image("},{"line_number":465,"context_line":"                    image_info.image_id, vi.datastore, vi.dc_info.ref)"},{"line_number":466,"context_line":"            self._fetch_image_if_missing(context, vi)"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_03552a4f","line":463,"updated":"2014-10-16 13:14:53.000000000","message":"is not None","commit_id":"7d39e08a6ead3ae02e03ec4e74a90498d1798516"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"557010595f4ad9e45dbf0c9a165d0b0bb3f9e93b","unresolved":false,"context_lines":[{"line_number":460,"context_line":"            block_device_mapping \u003d driver.block_device_info_get_mapping("},{"line_number":461,"context_line":"                block_device_info)"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"        if instance.image_ref:"},{"line_number":464,"context_line":"            self._imagecache.enlist_image("},{"line_number":465,"context_line":"                    image_info.image_id, vi.datastore, vi.dc_info.ref)"},{"line_number":466,"context_line":"            self._fetch_image_if_missing(context, vi)"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_4e459641","line":463,"in_reply_to":"baa201ad_03552a4f","updated":"2014-10-16 15:25:01.000000000","message":"Done","commit_id":"7d39e08a6ead3ae02e03ec4e74a90498d1798516"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"a0a6702e269f693dd8e17835d540727689884069","unresolved":false,"context_lines":[{"line_number":460,"context_line":"            block_device_mapping \u003d driver.block_device_info_get_mapping("},{"line_number":461,"context_line":"                block_device_info)"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"        if instance.image_ref:"},{"line_number":464,"context_line":"            self._imagecache.enlist_image("},{"line_number":465,"context_line":"                    image_info.image_id, vi.datastore, vi.dc_info.ref)"},{"line_number":466,"context_line":"            self._fetch_image_if_missing(context, vi)"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_530b6b7c","line":463,"in_reply_to":"baa201ad_4e459641","updated":"2014-10-17 02:57:17.000000000","message":"Actually, I tried to set \"if instance.image_ref is not None:\" and it croaked.  instance.image_ref is just set to \u0027\u0027 if one is not given, based on my trace.","commit_id":"7d39e08a6ead3ae02e03ec4e74a90498d1798516"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"cf988e90f9bfec2fc4b6c98e51d2ddaf25eb2fdd","unresolved":false,"context_lines":[{"line_number":486,"context_line":"                # than logging, can we simplify this?"},{"line_number":487,"context_line":"                self._volumeops.attach_root_volume(connection_info, instance,"},{"line_number":488,"context_line":"                                                   self._default_root_device,"},{"line_number":489,"context_line":"                                                   vi.datastore.ref)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        if configdrive.required_by(instance):"},{"line_number":492,"context_line":"            self._configure_config_drive("}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_55a4454a","line":489,"updated":"2014-10-16 13:14:53.000000000","message":"A couple of things, here.\n\nFirstly, use of attach_root_volume here didn\u0027t look right as we\u0027d potentially be attaching multiple \u0027root\u0027 volumes, so I looked at what it does. It turns out that the mountpoint passed to attach_volume is never used for anything other than logging. All this is actually doing is relocating a volume to the same resource pool as the instance. We should clean this up and/or remove the confusingly ignored mountpoint argument. I think this cleanup is sufficiently important to come before this change, because without it this change looks wrong.\n\nSecondly, there\u0027s quite a lot more going on in block_device_mapping than we\u0027re using here. Have a look in nova/block_device.py. The code without this change explicitly claims to be broken. If we\u0027re going to claim it\u0027s not I think we ought to do a better job of using the available information. For example, we ought to sort by boot_index. There\u0027s probably more in there, too.","commit_id":"7d39e08a6ead3ae02e03ec4e74a90498d1798516"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"6d55a1bde30fc1a15a9ab548d92751bc55814273","unresolved":false,"context_lines":[{"line_number":486,"context_line":"                # than logging, can we simplify this?"},{"line_number":487,"context_line":"                self._volumeops.attach_root_volume(connection_info, instance,"},{"line_number":488,"context_line":"                                                   self._default_root_device,"},{"line_number":489,"context_line":"                                                   vi.datastore.ref)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        if configdrive.required_by(instance):"},{"line_number":492,"context_line":"            self._configure_config_drive("}],"source_content_type":"text/x-python","patch_set":3,"id":"9aa7fdbe_c787540e","line":489,"in_reply_to":"baa201ad_4ea4b64a","updated":"2014-10-20 08:39:37.000000000","message":"Yes, you\u0027re right. We should add by mount_device, and boot_index is for the boot order, which we don\u0027t currently use (although I\u0027m working on a rescue patch which uses it.).","commit_id":"7d39e08a6ead3ae02e03ec4e74a90498d1798516"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"557010595f4ad9e45dbf0c9a165d0b0bb3f9e93b","unresolved":false,"context_lines":[{"line_number":486,"context_line":"                # than logging, can we simplify this?"},{"line_number":487,"context_line":"                self._volumeops.attach_root_volume(connection_info, instance,"},{"line_number":488,"context_line":"                                                   self._default_root_device,"},{"line_number":489,"context_line":"                                                   vi.datastore.ref)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        if configdrive.required_by(instance):"},{"line_number":492,"context_line":"            self._configure_config_drive("}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_4ea4b64a","line":489,"in_reply_to":"baa201ad_55a4454a","updated":"2014-10-16 15:25:01.000000000","message":"I can talk with rgerganov on having his patch come first before mine.\n\nI will check if the bdm contains the root_device_name, if it does, I will user attach_root_volume.  Otherwise, I will use attach_volume.\n\nAs far as bdm is concerned,  there is not much info to go from there.  The boot_index is not present, so you cannot just sort by it.  However, you could sort by the mount_device, and attach it based on the order.  This will take care of cases where users use --block-device-mapping to specify the block devices.\n\nA vmdk bdm can contain the following:\n\nBlock device information present: {\u0027block_device_mapping\u0027: [{\u0027connection_info\u0027: {u\u0027driver_volume_type\u0027: \nu\u0027vmdk\u0027, \u0027serial\u0027: u\u00270bec34c0-250b-456b-b235-5b2b4279051c\u0027, u\u0027data\u0027: {u\u0027volume\u0027: u\u0027vm-1280\u0027, \nu\u0027access_mode\u0027: u\u0027rw\u0027, u\u0027qos_specs\u0027: None, u\u0027volume_id\u0027: u\u00270bec34c0-250b-456b-b235-5b2b4279051c\u0027}}, \n\u0027mount_device\u0027: u\u0027/dev/sda\u0027, \u0027delete_on_termination\u0027: False}, {\u0027connection_info\u0027: {u\u0027driver_volume_type\u0027:\n u\u0027vmdk\u0027, \u0027serial\u0027: u\u00273daa0344-95f4-4bad-9415-d11fd2c44625\u0027, u\u0027data\u0027: {u\u0027volume\u0027: u\u0027vm-1281\u0027, \nu\u0027access_mode\u0027: u\u0027rw\u0027, u\u0027qos_specs\u0027: None, u\u0027volume_id\u0027: u\u00273daa0344-95f4-4bad-9415-d11fd2c44625\u0027}}, \n\u0027mount_device\u0027: u\u0027/dev/sdb\u0027, \u0027delete_on_termination\u0027: False}], \u0027root_device_name\u0027: u\u0027/dev/sda\u0027, \u0027ephemerals\u0027: [], \n\u0027swap\u0027: None} spawn /opt/stack/nova/nova/virt/vmwareapi/vmops.py:479","commit_id":"7d39e08a6ead3ae02e03ec4e74a90498d1798516"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"0b4a226f369b1a27ba5d77c4129712353c615f0e","unresolved":false,"context_lines":[{"line_number":459,"context_line":"        if block_device_info is not None:"},{"line_number":460,"context_line":"            block_device_mapping \u003d sorted("},{"line_number":461,"context_line":"                driver.block_device_info_get_mapping(block_device_info),"},{"line_number":462,"context_line":"                key\u003doperator.itemgetter(\u0027mount_device\u0027))"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"        if instance.image_ref:"},{"line_number":465,"context_line":"            self._imagecache.enlist_image("}],"source_content_type":"text/x-python","patch_set":8,"id":"9aa7fdbe_cd0ca545","line":462,"updated":"2014-10-21 15:59:34.000000000","message":"Sorry if I\u0027ve given you a bum steer on this, but we\u0027ve been discussing this today wrt ephemeral disks, and we shouldn\u0027t be using mount_device here. The block_device_mapping list is already in order wrt order on a bus (i.e. this code was correct before).","commit_id":"ce11d29f2eb79b1a26a2fa0101361adbe4c3773b"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"fc90ed82fd9d07ed73c6a265281b15c0f7aa6e93","unresolved":false,"context_lines":[{"line_number":459,"context_line":"        if block_device_info is not None:"},{"line_number":460,"context_line":"            block_device_mapping \u003d sorted("},{"line_number":461,"context_line":"                driver.block_device_info_get_mapping(block_device_info),"},{"line_number":462,"context_line":"                key\u003doperator.itemgetter(\u0027mount_device\u0027))"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"        if instance.image_ref:"},{"line_number":465,"context_line":"            self._imagecache.enlist_image("}],"source_content_type":"text/x-python","patch_set":8,"id":"9aa7fdbe_a9de6c3d","line":462,"in_reply_to":"9aa7fdbe_cd0ca545","updated":"2014-10-21 19:42:15.000000000","message":"Ok, no problem.  I can remove this part.","commit_id":"ce11d29f2eb79b1a26a2fa0101361adbe4c3773b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"0b4a226f369b1a27ba5d77c4129712353c615f0e","unresolved":false,"context_lines":[{"line_number":480,"context_line":"            LOG.debug(strutils.mask_password(msg), instance\u003dinstance)"},{"line_number":481,"context_line":""},{"line_number":482,"context_line":"            root_device \u003d block_device_info.get(\u0027root_device_name\u0027)"},{"line_number":483,"context_line":"            for disk in block_device_mapping:"},{"line_number":484,"context_line":"                connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"                # TODO(hartsocks): instance is unnecessary, remove it"}],"source_content_type":"text/x-python","patch_set":8,"id":"9aa7fdbe_7b860f67","line":483,"updated":"2014-10-21 15:59:34.000000000","message":"We should honour disk_bus if it\u0027s set, and explicitly set the adapter type of each disk as we add it. We should default it if it\u0027s not set. Doing this would require using attach_disk_to_vm() below, and refactoring attach_root_volume (if it\u0027s still required).","commit_id":"ce11d29f2eb79b1a26a2fa0101361adbe4c3773b"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"2aeb1b2d9e69371a6be2cb857aa62a2699d66690","unresolved":false,"context_lines":[{"line_number":480,"context_line":"            LOG.debug(strutils.mask_password(msg), instance\u003dinstance)"},{"line_number":481,"context_line":""},{"line_number":482,"context_line":"            root_device \u003d block_device_info.get(\u0027root_device_name\u0027)"},{"line_number":483,"context_line":"            for disk in block_device_mapping:"},{"line_number":484,"context_line":"                connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"                # TODO(hartsocks): instance is unnecessary, remove it"}],"source_content_type":"text/x-python","patch_set":8,"id":"9aa7fdbe_e913eb63","line":483,"in_reply_to":"9aa7fdbe_296d1c99","updated":"2014-10-22 21:23:55.000000000","message":"So there is no disk_bus in block_device_info.  The only way (that I know right now) to determine that is by querying the db thru\n\nbdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid(context, instance.uuid)\n\nThis is done for you in the compute/manager.py, so it is a duplicate lookup if we do it here.  It does not seem like the right solution to do the look up here.","commit_id":"ce11d29f2eb79b1a26a2fa0101361adbe4c3773b"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"fc90ed82fd9d07ed73c6a265281b15c0f7aa6e93","unresolved":false,"context_lines":[{"line_number":480,"context_line":"            LOG.debug(strutils.mask_password(msg), instance\u003dinstance)"},{"line_number":481,"context_line":""},{"line_number":482,"context_line":"            root_device \u003d block_device_info.get(\u0027root_device_name\u0027)"},{"line_number":483,"context_line":"            for disk in block_device_mapping:"},{"line_number":484,"context_line":"                connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"                # TODO(hartsocks): instance is unnecessary, remove it"}],"source_content_type":"text/x-python","patch_set":8,"id":"9aa7fdbe_296d1c99","line":483,"in_reply_to":"9aa7fdbe_7b860f67","updated":"2014-10-21 19:42:15.000000000","message":"Ok, I can add logic to handle disk_bus.","commit_id":"ce11d29f2eb79b1a26a2fa0101361adbe4c3773b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"0b4a226f369b1a27ba5d77c4129712353c615f0e","unresolved":false,"context_lines":[{"line_number":487,"context_line":"                # we still use instance in many locations for no other purpose"},{"line_number":488,"context_line":"                # than logging, can we simplify this?"},{"line_number":489,"context_line":"                if disk.get(\u0027mount_device\u0027) \u003d\u003d root_device:"},{"line_number":490,"context_line":"                    self._volumeops.attach_root_volume(connection_info,"},{"line_number":491,"context_line":"                        instance, vi.datastore.ref)"},{"line_number":492,"context_line":"                else:"},{"line_number":493,"context_line":"                    self._volumeops.attach_volume(connection_info,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9aa7fdbe_9b0ad31f","line":490,"updated":"2014-10-21 15:59:34.000000000","message":"See https://bugs.launchpad.net/nova/+bug/1223709 for why attach_root_volume was added. I don\u0027t understand why it\u0027s necessary for the boot disk to be on the same datastore, or if it is still necessary. I also don\u0027t understand if all disks have the same restriction. We could do with having this cleared up.","commit_id":"ce11d29f2eb79b1a26a2fa0101361adbe4c3773b"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"fc90ed82fd9d07ed73c6a265281b15c0f7aa6e93","unresolved":false,"context_lines":[{"line_number":487,"context_line":"                # we still use instance in many locations for no other purpose"},{"line_number":488,"context_line":"                # than logging, can we simplify this?"},{"line_number":489,"context_line":"                if disk.get(\u0027mount_device\u0027) \u003d\u003d root_device:"},{"line_number":490,"context_line":"                    self._volumeops.attach_root_volume(connection_info,"},{"line_number":491,"context_line":"                        instance, vi.datastore.ref)"},{"line_number":492,"context_line":"                else:"},{"line_number":493,"context_line":"                    self._volumeops.attach_volume(connection_info,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9aa7fdbe_e920744b","line":490,"in_reply_to":"9aa7fdbe_9b0ad31f","updated":"2014-10-21 19:42:15.000000000","message":"I have to ask Gary about this, maybe during the IRC meeting tomorrow.","commit_id":"ce11d29f2eb79b1a26a2fa0101361adbe4c3773b"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"2aeb1b2d9e69371a6be2cb857aa62a2699d66690","unresolved":false,"context_lines":[{"line_number":487,"context_line":"                # we still use instance in many locations for no other purpose"},{"line_number":488,"context_line":"                # than logging, can we simplify this?"},{"line_number":489,"context_line":"                if disk.get(\u0027mount_device\u0027) \u003d\u003d root_device:"},{"line_number":490,"context_line":"                    self._volumeops.attach_root_volume(connection_info,"},{"line_number":491,"context_line":"                        instance, vi.datastore.ref)"},{"line_number":492,"context_line":"                else:"},{"line_number":493,"context_line":"                    self._volumeops.attach_volume(connection_info,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9aa7fdbe_7ea08f9f","line":490,"in_reply_to":"9aa7fdbe_e920744b","updated":"2014-10-22 21:23:55.000000000","message":"I spoke with arnaud on IRC, and arnaud says: a vmdk volume from another datastore cannot be attached to a VM in a different datastore, if it is supposed to be the root disk.","commit_id":"ce11d29f2eb79b1a26a2fa0101361adbe4c3773b"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"10a4ec532832777f50f9efd0c9b5578a50874944","unresolved":false,"context_lines":[{"line_number":494,"context_line":"                # TODO(hartsocks): instance is unnecessary, remove it"},{"line_number":495,"context_line":"                # we still use instance in many locations for no other purpose"},{"line_number":496,"context_line":"                # than logging, can we simplify this?"},{"line_number":497,"context_line":"                if disk.get(\u0027mount_device\u0027) \u003d\u003d root_device:"},{"line_number":498,"context_line":"                    self._volumeops.attach_root_volume(connection_info,"},{"line_number":499,"context_line":"                        instance, vi.datastore.ref, adapter_type)"},{"line_number":500,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":11,"id":"9aa7fdbe_65c99725","line":497,"updated":"2014-10-26 08:45:30.000000000","message":"could there be a case when the root_device is None and the mount_device does not appear?","commit_id":"300f9e0899cbc2f1cf0a3eed0439efcfc715a9d2"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"d0236fe064e3184008b32df855752eaf88272d13","unresolved":false,"context_lines":[{"line_number":494,"context_line":"                # TODO(hartsocks): instance is unnecessary, remove it"},{"line_number":495,"context_line":"                # we still use instance in many locations for no other purpose"},{"line_number":496,"context_line":"                # than logging, can we simplify this?"},{"line_number":497,"context_line":"                if disk.get(\u0027mount_device\u0027) \u003d\u003d root_device:"},{"line_number":498,"context_line":"                    self._volumeops.attach_root_volume(connection_info,"},{"line_number":499,"context_line":"                        instance, vi.datastore.ref, adapter_type)"},{"line_number":500,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":11,"id":"9aa7fdbe_2942f7ad","line":497,"in_reply_to":"9aa7fdbe_65c99725","updated":"2014-10-27 15:11:21.000000000","message":"I do not believe so, if the following trace is right.\n\nIf there are BDMs, the block_device_info is determined by nova/compute/manager.py:_prep_block_device,  which extracts it from objects.BlockDeviceMappingList.get_by_instance_uuid.  \n\nmount_device is actually just the device_name of the BDM (https://github.com/openstack/nova/blob/master/nova/virt/block_device.py#L210) and device_name and root_device_name  is always guarantee by _default_block_device_names (https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1428)","commit_id":"300f9e0899cbc2f1cf0a3eed0439efcfc715a9d2"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"10a4ec532832777f50f9efd0c9b5578a50874944","unresolved":false,"context_lines":[{"line_number":509,"context_line":"        if power_on:"},{"line_number":510,"context_line":"            vm_util.power_on_instance(self._session, instance, vm_ref\u003dvm_ref)"},{"line_number":511,"context_line":""},{"line_number":512,"context_line":"    def _get_bdm_disk_bus(self, context, instance):"},{"line_number":513,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":514,"context_line":"            context, instance.uuid)"},{"line_number":515,"context_line":"        bdm_disk_bus \u003d {}"}],"source_content_type":"text/x-python","patch_set":11,"id":"9aa7fdbe_254cefc7","line":512,"updated":"2014-10-26 08:45:30.000000000","message":"can we just pass the instance uuid here?","commit_id":"300f9e0899cbc2f1cf0a3eed0439efcfc715a9d2"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"d0236fe064e3184008b32df855752eaf88272d13","unresolved":false,"context_lines":[{"line_number":509,"context_line":"        if power_on:"},{"line_number":510,"context_line":"            vm_util.power_on_instance(self._session, instance, vm_ref\u003dvm_ref)"},{"line_number":511,"context_line":""},{"line_number":512,"context_line":"    def _get_bdm_disk_bus(self, context, instance):"},{"line_number":513,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":514,"context_line":"            context, instance.uuid)"},{"line_number":515,"context_line":"        bdm_disk_bus \u003d {}"}],"source_content_type":"text/x-python","patch_set":11,"id":"9aa7fdbe_8aa676ac","line":512,"in_reply_to":"9aa7fdbe_05da8be7","updated":"2014-10-27 15:11:21.000000000","message":"I do not know of another driver that might need this, but I can put it in nova/virt/block_device.py.","commit_id":"300f9e0899cbc2f1cf0a3eed0439efcfc715a9d2"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"d0236fe064e3184008b32df855752eaf88272d13","unresolved":false,"context_lines":[{"line_number":509,"context_line":"        if power_on:"},{"line_number":510,"context_line":"            vm_util.power_on_instance(self._session, instance, vm_ref\u003dvm_ref)"},{"line_number":511,"context_line":""},{"line_number":512,"context_line":"    def _get_bdm_disk_bus(self, context, instance):"},{"line_number":513,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":514,"context_line":"            context, instance.uuid)"},{"line_number":515,"context_line":"        bdm_disk_bus \u003d {}"}],"source_content_type":"text/x-python","patch_set":11,"id":"9aa7fdbe_8a983679","line":512,"in_reply_to":"9aa7fdbe_254cefc7","updated":"2014-10-27 15:11:21.000000000","message":"Sure","commit_id":"300f9e0899cbc2f1cf0a3eed0439efcfc715a9d2"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"009a47a634b60d0ad47b94d540ab9b4268fbd779","unresolved":false,"context_lines":[{"line_number":509,"context_line":"        if power_on:"},{"line_number":510,"context_line":"            vm_util.power_on_instance(self._session, instance, vm_ref\u003dvm_ref)"},{"line_number":511,"context_line":""},{"line_number":512,"context_line":"    def _get_bdm_disk_bus(self, context, instance):"},{"line_number":513,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":514,"context_line":"            context, instance.uuid)"},{"line_number":515,"context_line":"        bdm_disk_bus \u003d {}"}],"source_content_type":"text/x-python","patch_set":11,"id":"9aa7fdbe_05da8be7","line":512,"in_reply_to":"9aa7fdbe_254cefc7","updated":"2014-10-26 08:46:08.000000000","message":"this also seems to be a common utility method that can be used by all drivers","commit_id":"300f9e0899cbc2f1cf0a3eed0439efcfc715a9d2"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"6c12003f037aadb17a6869686732c247b34f79cf","unresolved":false,"context_lines":[{"line_number":481,"context_line":"            # block_device_mapping does not provide the disk_bus, so get it"},{"line_number":482,"context_line":"            # by querying the objects"},{"line_number":483,"context_line":"            bdm_disk_bus \u003d block_device.get_bdm_disk_bus(context,"},{"line_number":484,"context_line":"                                                         instance.uuid)"},{"line_number":485,"context_line":"            root_device \u003d block_device_info.get(\u0027root_device_name\u0027)"},{"line_number":486,"context_line":"            for disk in block_device_mapping:"},{"line_number":487,"context_line":"                connection_info \u003d disk[\u0027connection_info\u0027]"}],"source_content_type":"text/x-python","patch_set":12,"id":"9aa7fdbe_95591b90","line":484,"updated":"2014-10-28 11:57:16.000000000","message":"I couldn\u0027t work out why this was required, because _prep_block_device() looks like it puts the BDM objects directly into this array. Also this will result in another hit on the DB which is obviously undesirable. Then I noticed that it converts them *back* into legacy dicts if the driver doesn\u0027t declare it doesn\u0027t want legacy dicts :/\n\nI think this is the source of some of my confusion. We need to define:\n\n    @property\n    def need_legacy_block_device_info(self):\n        return False\n\nin driver.py. See libvirt as an example.\n\nWith this in place this should all make more sense.","commit_id":"be2a2d3063fe05ded61701c173bdbe9703a40922"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"a5c506ab857d074440dfa0276a305a499ec336dc","unresolved":false,"context_lines":[{"line_number":481,"context_line":"            # block_device_mapping does not provide the disk_bus, so get it"},{"line_number":482,"context_line":"            # by querying the objects"},{"line_number":483,"context_line":"            bdm_disk_bus \u003d block_device.get_bdm_disk_bus(context,"},{"line_number":484,"context_line":"                                                         instance.uuid)"},{"line_number":485,"context_line":"            root_device \u003d block_device_info.get(\u0027root_device_name\u0027)"},{"line_number":486,"context_line":"            for disk in block_device_mapping:"},{"line_number":487,"context_line":"                connection_info \u003d disk[\u0027connection_info\u0027]"}],"source_content_type":"text/x-python","patch_set":12,"id":"9aa7fdbe_411a95c6","line":484,"in_reply_to":"9aa7fdbe_95591b90","updated":"2014-10-28 14:10:15.000000000","message":"This was my concern I posted a couple of patches ago.  But I will try out the approach you mention above.  Thanks.","commit_id":"be2a2d3063fe05ded61701c173bdbe9703a40922"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"b823eb15d47a56ff277689565005a01109e90b7c","unresolved":false,"context_lines":[{"line_number":476,"context_line":"            # so we have to scrub the message before logging it."},{"line_number":477,"context_line":"            LOG.debug(strutils.mask_password(msg), instance\u003dinstance)"},{"line_number":478,"context_line":""},{"line_number":479,"context_line":"            root_device \u003d block_device_info.get(\u0027root_device_name\u0027)"},{"line_number":480,"context_line":"            for disk in block_device_mapping:"},{"line_number":481,"context_line":"                connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":482,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9aa7fdbe_0e025e0b","line":479,"updated":"2014-10-29 11:05:47.000000000","message":"Now we\u0027ve switch to the new BDM objects, root_device_name and mount_device are deprecated. The root device is the one with boot_index of 0. The API asserts (in _validate_bdm) that this exists.","commit_id":"c5ad2a2e2d3bb2a044e749d8cb2123aa53e98cb8"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"d07233fb6545a38df93b3d5fe1340ea4b2c8758c","unresolved":false,"context_lines":[{"line_number":476,"context_line":"            # so we have to scrub the message before logging it."},{"line_number":477,"context_line":"            LOG.debug(strutils.mask_password(msg), instance\u003dinstance)"},{"line_number":478,"context_line":""},{"line_number":479,"context_line":"            root_device \u003d block_device_info.get(\u0027root_device_name\u0027)"},{"line_number":480,"context_line":"            for disk in block_device_mapping:"},{"line_number":481,"context_line":"                connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":482,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9aa7fdbe_c73f54d5","line":479,"in_reply_to":"9aa7fdbe_0e025e0b","updated":"2014-10-29 14:15:42.000000000","message":"Done","commit_id":"c5ad2a2e2d3bb2a044e749d8cb2123aa53e98cb8"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"b823eb15d47a56ff277689565005a01109e90b7c","unresolved":false,"context_lines":[{"line_number":481,"context_line":"                connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":482,"context_line":""},{"line_number":483,"context_line":"                # The only options for disk_bus are ide, scsi, usb, or virtio,"},{"line_number":484,"context_line":"                # but only ide and scsi are relevant to VMware"},{"line_number":485,"context_line":"                if disk.get(\u0027disk_bus\u0027) \u003d\u003d \u0027ide\u0027:"},{"line_number":486,"context_line":"                    adapter_type \u003d constants.ADAPTER_TYPE_IDE"},{"line_number":487,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":14,"id":"9aa7fdbe_0c8fc676","line":484,"updated":"2014-10-29 11:05:47.000000000","message":"I\u0027ve been looking, and I don\u0027t think that\u0027s true. I think that\u0027s just the constraint that the libvirt driver puts on it. The field seems to be a simple string.\n\nI think we should check that the adapter type is one of the known types defined in constants.py, and raise UnsupportedHardware if it isn\u0027t. If it\u0027s undefined, we should default to DEFAULT_ADAPTER_TYPE as below.\n\nI tested this, btw, and was able to boot an instance with a root volume on pvscsi by setting bus\u003dparaVirtual on the command line.","commit_id":"c5ad2a2e2d3bb2a044e749d8cb2123aa53e98cb8"},{"author":{"_account_id":9172,"name":"Radoslav Gerganov","email":"rgerganov@vmware.com","username":"rgerganov"},"change_message_id":"f1ca7a6e076f5f09bde5d3c8b4fa1f09cecdc455","unresolved":false,"context_lines":[{"line_number":481,"context_line":"                connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":482,"context_line":""},{"line_number":483,"context_line":"                # The only options for disk_bus are ide, scsi, usb, or virtio,"},{"line_number":484,"context_line":"                # but only ide and scsi are relevant to VMware"},{"line_number":485,"context_line":"                if disk.get(\u0027disk_bus\u0027) \u003d\u003d \u0027ide\u0027:"},{"line_number":486,"context_line":"                    adapter_type \u003d constants.ADAPTER_TYPE_IDE"},{"line_number":487,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":14,"id":"9aa7fdbe_5d9fb6b4","line":484,"in_reply_to":"9aa7fdbe_0c8fc676","updated":"2014-10-29 12:17:34.000000000","message":"I agree with Matt, disk_bus should be equivalent to what we call adapter_type in the vmware driver, so we should try to match with the constants for adapter types that we have.","commit_id":"c5ad2a2e2d3bb2a044e749d8cb2123aa53e98cb8"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"d07233fb6545a38df93b3d5fe1340ea4b2c8758c","unresolved":false,"context_lines":[{"line_number":481,"context_line":"                connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":482,"context_line":""},{"line_number":483,"context_line":"                # The only options for disk_bus are ide, scsi, usb, or virtio,"},{"line_number":484,"context_line":"                # but only ide and scsi are relevant to VMware"},{"line_number":485,"context_line":"                if disk.get(\u0027disk_bus\u0027) \u003d\u003d \u0027ide\u0027:"},{"line_number":486,"context_line":"                    adapter_type \u003d constants.ADAPTER_TYPE_IDE"},{"line_number":487,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":14,"id":"9aa7fdbe_aa085529","line":484,"in_reply_to":"9aa7fdbe_5d9fb6b4","updated":"2014-10-29 14:15:42.000000000","message":"Ok, I will have disk_bus accept all values for adapter_type and default to vi.ii.adapter_type if one is not set.","commit_id":"c5ad2a2e2d3bb2a044e749d8cb2123aa53e98cb8"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"6efe282e14c7230e5db09a6c27f0cf7b1b55de8a","unresolved":false,"context_lines":[{"line_number":485,"context_line":"                if disk.get(\u0027disk_bus\u0027) \u003d\u003d \u0027ide\u0027:"},{"line_number":486,"context_line":"                    adapter_type \u003d constants.ADAPTER_TYPE_IDE"},{"line_number":487,"context_line":"                else:"},{"line_number":488,"context_line":"                    adapter_type \u003d constants.DEFAULT_ADAPTER_TYPE"},{"line_number":489,"context_line":""},{"line_number":490,"context_line":"                # TODO(hartsocks): instance is unnecessary, remove it"},{"line_number":491,"context_line":"                # we still use instance in many locations for no other purpose"}],"source_content_type":"text/x-python","patch_set":14,"id":"9aa7fdbe_5d2fb6f6","line":488,"updated":"2014-10-29 12:33:21.000000000","message":"On second thoughts, I think the default should be vi.ii.adapter_type, which is what garyk uses in the ephemerals patch. The advantage of this is that it will default to whatever is set on the image, or DEFAULT_ADAPTER_TYPE if there is nothing set.","commit_id":"c5ad2a2e2d3bb2a044e749d8cb2123aa53e98cb8"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"b823eb15d47a56ff277689565005a01109e90b7c","unresolved":false,"context_lines":[{"line_number":490,"context_line":"                # TODO(hartsocks): instance is unnecessary, remove it"},{"line_number":491,"context_line":"                # we still use instance in many locations for no other purpose"},{"line_number":492,"context_line":"                # than logging, can we simplify this?"},{"line_number":493,"context_line":"                if disk.get(\u0027mount_device\u0027) \u003d\u003d root_device:"},{"line_number":494,"context_line":"                    self._volumeops.attach_root_volume(connection_info,"},{"line_number":495,"context_line":"                        instance, vi.datastore.ref, adapter_type)"},{"line_number":496,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":14,"id":"9aa7fdbe_b111b355","line":493,"updated":"2014-10-29 11:05:47.000000000","message":"Deprecated (See above)","commit_id":"c5ad2a2e2d3bb2a044e749d8cb2123aa53e98cb8"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"d07233fb6545a38df93b3d5fe1340ea4b2c8758c","unresolved":false,"context_lines":[{"line_number":490,"context_line":"                # TODO(hartsocks): instance is unnecessary, remove it"},{"line_number":491,"context_line":"                # we still use instance in many locations for no other purpose"},{"line_number":492,"context_line":"                # than logging, can we simplify this?"},{"line_number":493,"context_line":"                if disk.get(\u0027mount_device\u0027) \u003d\u003d root_device:"},{"line_number":494,"context_line":"                    self._volumeops.attach_root_volume(connection_info,"},{"line_number":495,"context_line":"                        instance, vi.datastore.ref, adapter_type)"},{"line_number":496,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":14,"id":"9aa7fdbe_ca0d9939","line":493,"in_reply_to":"9aa7fdbe_b111b355","updated":"2014-10-29 14:15:42.000000000","message":"Ok, I will use boot_index.","commit_id":"c5ad2a2e2d3bb2a044e749d8cb2123aa53e98cb8"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e408b5c8071e82f998178afdb82e18af85f306c0","unresolved":false,"context_lines":[{"line_number":479,"context_line":"            for disk in block_device_mapping:"},{"line_number":480,"context_line":"                connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":481,"context_line":""},{"line_number":482,"context_line":"                disk_bus \u003d disk.get(\u0027disk_bus\u0027)"},{"line_number":483,"context_line":"                if (disk_bus is not None and disk_bus not in"},{"line_number":484,"context_line":"                    (constants.DEFAULT_ADAPTER_TYPE,"},{"line_number":485,"context_line":"                     constants.ADAPTER_TYPE_BUSLOGIC,"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a8c0949_89e72ae4","line":482,"updated":"2014-10-30 11:10:16.000000000","message":"nit: You could do:\n\nadapter_type \u003d disk.get(\u0027disk_bus\u0027, vi.ii.adapter_type)\n\nYou also wouldn\u0027t have to check for None below. This is fine, though.","commit_id":"79de1ad55b8c1b4bb8ad99ae31ad87deacc0d3f4"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"b48894244d10c4f24e305cf5f6fd2179f0752dab","unresolved":false,"context_lines":[{"line_number":582,"context_line":"            for disk in block_device_mapping:"},{"line_number":583,"context_line":"                connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":584,"context_line":""},{"line_number":585,"context_line":"                adapter_type \u003d disk.get(\u0027disk_bus\u0027) or vi.ii.adapter_type"},{"line_number":586,"context_line":"                if (adapter_type not in"},{"line_number":587,"context_line":"                    (constants.DEFAULT_ADAPTER_TYPE,"},{"line_number":588,"context_line":"                     constants.ADAPTER_TYPE_BUSLOGIC,"}],"source_content_type":"text/x-python","patch_set":25,"id":"3a961159_c48c4d38","line":585,"updated":"2014-12-10 17:25:35.000000000","message":"why not:\nadapter_type \u003d disk.get(\u0027disk_bus\u0027, vi.ii.adapter_type)","commit_id":"e7a351d82793d2613abf8815277b9326b777a446"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"9dd07e32b8b44e4f966e1d78f00d7829c9791bfd","unresolved":false,"context_lines":[{"line_number":582,"context_line":"            for disk in block_device_mapping:"},{"line_number":583,"context_line":"                connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":584,"context_line":""},{"line_number":585,"context_line":"                adapter_type \u003d disk.get(\u0027disk_bus\u0027) or vi.ii.adapter_type"},{"line_number":586,"context_line":"                if (adapter_type not in"},{"line_number":587,"context_line":"                    (constants.DEFAULT_ADAPTER_TYPE,"},{"line_number":588,"context_line":"                     constants.ADAPTER_TYPE_BUSLOGIC,"}],"source_content_type":"text/x-python","patch_set":25,"id":"3a961159_84024509","line":585,"in_reply_to":"3a961159_c48c4d38","updated":"2014-12-10 17:35:57.000000000","message":"I did try that, but disk.get returns the value for key if key is in the dictionary, else default.\n\nI found in testing that \u0027disk_bus\u0027 is set to None.  Since \u0027disk_bus\u0027 is defined in the dict but set to None, it will not get set to the default vi.ii.adapter_type I want if I use it as the default in the get().","commit_id":"e7a351d82793d2613abf8815277b9326b777a446"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"b48894244d10c4f24e305cf5f6fd2179f0752dab","unresolved":false,"context_lines":[{"line_number":583,"context_line":"                connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":584,"context_line":""},{"line_number":585,"context_line":"                adapter_type \u003d disk.get(\u0027disk_bus\u0027) or vi.ii.adapter_type"},{"line_number":586,"context_line":"                if (adapter_type not in"},{"line_number":587,"context_line":"                    (constants.DEFAULT_ADAPTER_TYPE,"},{"line_number":588,"context_line":"                     constants.ADAPTER_TYPE_BUSLOGIC,"},{"line_number":589,"context_line":"                     constants.ADAPTER_TYPE_IDE,"}],"source_content_type":"text/x-python","patch_set":25,"id":"3a961159_a4b741d7","line":586,"updated":"2014-12-10 17:25:35.000000000","message":"one concern here - and sorry for getting to this too late - say we have 3 devices, and the first 2 have valid device type and the 3rd does not. that will cause volume attachments.\nwould it be possible that we validate the adapter types in a different loop?","commit_id":"e7a351d82793d2613abf8815277b9326b777a446"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"6b54b8d81d74d52c6e514bb77efcd4ad686e769b","unresolved":false,"context_lines":[{"line_number":583,"context_line":"                connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":584,"context_line":""},{"line_number":585,"context_line":"                adapter_type \u003d disk.get(\u0027disk_bus\u0027) or vi.ii.adapter_type"},{"line_number":586,"context_line":"                if (adapter_type not in"},{"line_number":587,"context_line":"                    (constants.DEFAULT_ADAPTER_TYPE,"},{"line_number":588,"context_line":"                     constants.ADAPTER_TYPE_BUSLOGIC,"},{"line_number":589,"context_line":"                     constants.ADAPTER_TYPE_IDE,"}],"source_content_type":"text/x-python","patch_set":25,"id":"3a961159_bc2e6641","line":586,"in_reply_to":"3a961159_57bacd23","updated":"2014-12-11 04:24:40.000000000","message":"I will add a _is_bdm_valid() method to check the bdm.  Will have another patch up shortly.","commit_id":"e7a351d82793d2613abf8815277b9326b777a446"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"9dd07e32b8b44e4f966e1d78f00d7829c9791bfd","unresolved":false,"context_lines":[{"line_number":583,"context_line":"                connection_info \u003d disk[\u0027connection_info\u0027]"},{"line_number":584,"context_line":""},{"line_number":585,"context_line":"                adapter_type \u003d disk.get(\u0027disk_bus\u0027) or vi.ii.adapter_type"},{"line_number":586,"context_line":"                if (adapter_type not in"},{"line_number":587,"context_line":"                    (constants.DEFAULT_ADAPTER_TYPE,"},{"line_number":588,"context_line":"                     constants.ADAPTER_TYPE_BUSLOGIC,"},{"line_number":589,"context_line":"                     constants.ADAPTER_TYPE_IDE,"}],"source_content_type":"text/x-python","patch_set":25,"id":"3a961159_57bacd23","line":586,"in_reply_to":"3a961159_a4b741d7","updated":"2014-12-10 17:35:57.000000000","message":"We could, but it would be an extra loop and repetitive code.  I am conflicted about this.  I could add a function, e.g. _check_bdm_disk_bus, to do the check?","commit_id":"e7a351d82793d2613abf8815277b9326b777a446"},{"author":{"_account_id":9172,"name":"Radoslav Gerganov","email":"rgerganov@vmware.com","username":"rgerganov"},"change_message_id":"ad07e6472fa3f76b0b7164eb085eb7dcd260b7bf","unresolved":false,"context_lines":[{"line_number":523,"context_line":"              admin_password, network_info, block_device_info\u003dNone,"},{"line_number":524,"context_line":"              instance_name\u003dNone, power_on\u003dTrue,"},{"line_number":525,"context_line":"              flavor\u003dNone):"},{"line_number":526,"context_line":""},{"line_number":527,"context_line":"        client_factory \u003d self._session.vim.client.factory"},{"line_number":528,"context_line":"        image_info \u003d images.VMwareImage.from_image(instance.image_ref,"},{"line_number":529,"context_line":"                                                   image_meta)"}],"source_content_type":"text/x-python","patch_set":28,"id":"1a930d6b_b1832b03","side":"PARENT","line":526,"updated":"2015-01-22 17:24:15.000000000","message":"nit: revert this change if you respin","commit_id":"9a05457a354f056bbd1cd87918efd46e42eb45fd"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"7f71871d17c59d81b814fb9a2ce83ab8a91850b3","unresolved":false,"context_lines":[{"line_number":523,"context_line":"              admin_password, network_info, block_device_info\u003dNone,"},{"line_number":524,"context_line":"              instance_name\u003dNone, power_on\u003dTrue,"},{"line_number":525,"context_line":"              flavor\u003dNone):"},{"line_number":526,"context_line":""},{"line_number":527,"context_line":"        client_factory \u003d self._session.vim.client.factory"},{"line_number":528,"context_line":"        image_info \u003d images.VMwareImage.from_image(instance.image_ref,"},{"line_number":529,"context_line":"                                                   image_meta)"}],"source_content_type":"text/x-python","patch_set":28,"id":"1a930d6b_e6b3d2e6","side":"PARENT","line":526,"in_reply_to":"1a930d6b_b1832b03","updated":"2015-01-26 17:51:28.000000000","message":"Done","commit_id":"9a05457a354f056bbd1cd87918efd46e42eb45fd"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"60781d4a081fdb41ffdd8727c2bb652412764f9e","unresolved":false,"context_lines":[{"line_number":665,"context_line":""},{"line_number":666,"context_line":"        for disk in block_device_mapping:"},{"line_number":667,"context_line":"            adapter_type \u003d disk.get(\u0027disk_bus\u0027)"},{"line_number":668,"context_line":"            if (adapter_type is not None and adapter_type not in valid_bus):"},{"line_number":669,"context_line":"                raise exception.UnsupportedHardware(model\u003dadapter_type,"},{"line_number":670,"context_line":"                                                    virt\u003d\"vmware\")"},{"line_number":671,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"9a80dd14_31e5b9a8","line":668,"updated":"2015-03-04 18:12:42.000000000","message":"nit: parens not necessary :)","commit_id":"264425678fd4a37618c2bceae8e62f6bba778223"}]}