From 76c6359ed44702279ba4bb9d2542d3cc9772c4f2 Mon Sep 17 00:00:00 2001 From: Adelina Tuvenie Date: Tue, 17 Nov 2015 02:14:41 -0800 Subject: [PATCH] Added support for new block device format in vmops Modified vmops methods to make use of the new block device format. This new format allows for more flexibility when using block device, such as the ability to select the bus for the attachment and the boot index. Co-Authored-By: Claudiu Belu Partially implements: blueprint hyper-v-block-device-mapping-support Change-Id: Ib6ff12b2710143798c1c29376edd9c1364d451b9 --- .../virt/hyperv/test_block_device_manager.py | 57 ++-- .../unit/virt/hyperv/test_livemigrationops.py | 10 +- .../unit/virt/hyperv/test_migrationops.py | 202 +++++++----- nova/tests/unit/virt/hyperv/test_vmops.py | 292 ++++++++++-------- nova/tests/unit/virt/hyperv/test_volumeops.py | 43 +-- nova/virt/hyperv/block_device_manager.py | 10 +- nova/virt/hyperv/constants.py | 2 + nova/virt/hyperv/driver.py | 4 + nova/virt/hyperv/livemigrationops.py | 4 +- nova/virt/hyperv/migrationops.py | 112 +++++-- nova/virt/hyperv/pathutils.py | 9 +- nova/virt/hyperv/vmops.py | 107 ++++--- nova/virt/hyperv/volumeops.py | 36 +-- 13 files changed, 528 insertions(+), 360 deletions(-) diff --git a/nova/tests/unit/virt/hyperv/test_block_device_manager.py b/nova/tests/unit/virt/hyperv/test_block_device_manager.py index e35e2ad14904..810a0b589167 100644 --- a/nova/tests/unit/virt/hyperv/test_block_device_manager.py +++ b/nova/tests/unit/virt/hyperv/test_block_device_manager.py @@ -50,14 +50,7 @@ class BlockDeviceManagerTestCase(test_base.HyperVBaseTestCase): self.assertEqual(slot_map[constants.CTRL_TYPE_IDE][1], os_win_const.IDE_CONTROLLER_SLOTS_NUMBER - 1) - @mock.patch('nova.virt.configdrive.required_by') - def test_init_controller_slot_counter_gen2(self, mock_cfg_drive_req): - slot_map = self._bdman._initialize_controller_slot_counter( - mock.sentinel.FAKE_INSTANCE, constants.VM_GEN_2) - - self.assertEqual(slot_map[constants.CTRL_TYPE_SCSI][0], - os_win_const.SCSI_CONTROLLER_SLOTS_NUMBER - 1) - + @mock.patch.object(block_device_manager.configdrive, 'required_by') @mock.patch.object(block_device_manager.BlockDeviceInfoManager, '_initialize_controller_slot_counter') @mock.patch.object(block_device_manager.BlockDeviceInfoManager, @@ -66,28 +59,44 @@ class BlockDeviceManagerTestCase(test_base.HyperVBaseTestCase): '_check_and_update_ephemerals') @mock.patch.object(block_device_manager.BlockDeviceInfoManager, '_check_and_update_volumes') - def test_validate_and_update_bdi(self, mock_check_and_update_vol, - mock_check_and_update_eph, - mock_check_and_update_root, - mock_init_ctrl_cntr): - mock_init_ctrl_cntr.return_value = mock.sentinel.FAKE_SLOT_MAP + def _check_validate_and_update_bdi(self, mock_check_and_update_vol, + mock_check_and_update_eph, + mock_check_and_update_root, + mock_init_ctrl_cntr, + mock_required_by, available_slots=1): + mock_required_by.return_value = True + slot_map = {constants.CTRL_TYPE_SCSI: [available_slots]} + mock_init_ctrl_cntr.return_value = slot_map - self._bdman.validate_and_update_bdi(mock.sentinel.FAKE_INSTANCE, - mock.sentinel.IMAGE_META, - mock.sentinel.VM_GEN, - mock.sentinel.BLOCK_DEV_INFO) + if available_slots: + self._bdman.validate_and_update_bdi(mock.sentinel.FAKE_INSTANCE, + mock.sentinel.IMAGE_META, + constants.VM_GEN_2, + mock.sentinel.BLOCK_DEV_INFO) + else: + self.assertRaises(exception.InvalidBDMFormat, + self._bdman.validate_and_update_bdi, + mock.sentinel.FAKE_INSTANCE, + mock.sentinel.IMAGE_META, + constants.VM_GEN_2, + mock.sentinel.BLOCK_DEV_INFO) mock_init_ctrl_cntr.assert_called_once_with( - mock.sentinel.FAKE_INSTANCE, mock.sentinel.VM_GEN) + mock.sentinel.FAKE_INSTANCE, constants.VM_GEN_2) mock_check_and_update_root.assert_called_once_with( - mock.sentinel.VM_GEN, mock.sentinel.IMAGE_META, - mock.sentinel.BLOCK_DEV_INFO, mock.sentinel.FAKE_SLOT_MAP) + constants.VM_GEN_2, mock.sentinel.IMAGE_META, + mock.sentinel.BLOCK_DEV_INFO, slot_map) mock_check_and_update_eph.assert_called_once_with( - mock.sentinel.VM_GEN, mock.sentinel.BLOCK_DEV_INFO, - mock.sentinel.FAKE_SLOT_MAP) + constants.VM_GEN_2, mock.sentinel.BLOCK_DEV_INFO, slot_map) mock_check_and_update_vol.assert_called_once_with( - mock.sentinel.VM_GEN, mock.sentinel.BLOCK_DEV_INFO, - mock.sentinel.FAKE_SLOT_MAP) + constants.VM_GEN_2, mock.sentinel.BLOCK_DEV_INFO, slot_map) + mock_required_by.assert_called_once_with(mock.sentinel.FAKE_INSTANCE) + + def test_validate_and_update_bdi(self): + self._check_validate_and_update_bdi() + + def test_validate_and_update_bdi_insufficient_slots(self): + self._check_validate_and_update_bdi(available_slots=0) @mock.patch.object(block_device_manager.BlockDeviceInfoManager, '_get_available_controller_slot') diff --git a/nova/tests/unit/virt/hyperv/test_livemigrationops.py b/nova/tests/unit/virt/hyperv/test_livemigrationops.py index 388b382cc686..ce2dc0584702 100644 --- a/nova/tests/unit/virt/hyperv/test_livemigrationops.py +++ b/nova/tests/unit/virt/hyperv/test_livemigrationops.py @@ -34,6 +34,7 @@ class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase): self._livemigrops = livemigrationops.LiveMigrationOps() self._livemigrops._livemigrutils = mock.MagicMock() self._livemigrops._pathutils = mock.MagicMock() + self._livemigrops._block_dev_man = mock.MagicMock() @mock.patch.object(serialconsoleops.SerialConsoleOps, 'stop_console_handler') @@ -78,22 +79,21 @@ class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase): self._test_live_migration(side_effect=os_win_exc.HyperVException) @mock.patch('nova.virt.hyperv.volumeops.VolumeOps.get_disk_path_mapping') - @mock.patch('nova.virt.hyperv.volumeops.VolumeOps' - '.ebs_root_in_block_devices') @mock.patch('nova.virt.hyperv.imagecache.ImageCache.get_cached_image') @mock.patch('nova.virt.hyperv.volumeops.VolumeOps' '.initialize_volumes_connection') def _test_pre_live_migration(self, mock_initialize_connection, mock_get_cached_image, - mock_ebs_root_in_block_devices, mock_get_disk_path_mapping, phys_disks_attached=True): mock_instance = fake_instance.fake_instance_obj(self.context) mock_instance.image_ref = "fake_image_ref" - mock_ebs_root_in_block_devices.return_value = None mock_get_disk_path_mapping.return_value = ( mock.sentinel.disk_path_mapping if phys_disks_attached else None) + bdman = self._livemigrops._block_dev_man + mock_is_boot_from_vol = bdman.is_boot_from_volume + mock_is_boot_from_vol.return_value = None CONF.set_override('use_cow_images', True) self._livemigrops.pre_live_migration( self.context, mock_instance, @@ -103,7 +103,7 @@ class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase): check_config = ( self._livemigrops._livemigrutils.check_live_migration_config) check_config.assert_called_once_with() - mock_ebs_root_in_block_devices.assert_called_once_with( + mock_is_boot_from_vol.assert_called_once_with( mock.sentinel.BLOCK_INFO) mock_get_cached_image.assert_called_once_with(self.context, mock_instance) diff --git a/nova/tests/unit/virt/hyperv/test_migrationops.py b/nova/tests/unit/virt/hyperv/test_migrationops.py index e8a4edcce728..5ae97dc569dc 100644 --- a/nova/tests/unit/virt/hyperv/test_migrationops.py +++ b/nova/tests/unit/virt/hyperv/test_migrationops.py @@ -23,6 +23,7 @@ from nova import objects from nova import test from nova.tests.unit import fake_instance from nova.tests.unit.virt.hyperv import test_base +from nova.virt.hyperv import constants from nova.virt.hyperv import migrationops @@ -46,6 +47,7 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase): self._migrationops._pathutils = mock.MagicMock() self._migrationops._volumeops = mock.MagicMock() self._migrationops._imagecache = mock.MagicMock() + self._migrationops._block_dev_man = mock.MagicMock() def _check_migrate_disk_files(self, host): instance_path = 'fake/instance/path' @@ -223,56 +225,61 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase): @mock.patch.object(migrationops.MigrationOps, '_check_and_attach_config_drive') @mock.patch.object(migrationops.MigrationOps, '_revert_migration_files') + @mock.patch.object(migrationops.MigrationOps, '_check_ephemeral_disks') @mock.patch.object(objects.ImageMeta, "from_instance") def _check_finish_revert_migration(self, mock_image, + mock_check_eph_disks, mock_revert_migration_files, mock_check_attach_config_drive, - boot_from_volume=False): + disk_type=constants.DISK): mock_image.return_value = objects.ImageMeta.from_dict({}) mock_instance = fake_instance.fake_instance_obj(self.context) - mock_ebs_root_in_block_devices = ( - self._migrationops._volumeops.ebs_root_in_block_devices) - mock_ebs_root_in_block_devices.return_value = boot_from_volume - lookup_ephemeral = ( - self._migrationops._pathutils.lookup_ephemeral_vhd_path) + root_device = {'type': disk_type} + block_device_info = {'root_disk': root_device, 'ephemerals': []} self._migrationops.finish_revert_migration( context=self.context, instance=mock_instance, network_info=mock.sentinel.network_info, - block_device_info=mock.sentinel.block_device, + block_device_info=block_device_info, power_on=True) mock_revert_migration_files.assert_called_once_with( mock_instance.name) - mock_ebs_root_in_block_devices.assert_called_once_with( - mock.sentinel.block_device) - if not boot_from_volume: + if root_device['type'] == constants.DISK: lookup_root_vhd = ( self._migrationops._pathutils.lookup_root_vhd_path) lookup_root_vhd.assert_called_once_with(mock_instance.name) - fake_root_path = lookup_root_vhd.return_value - else: - fake_root_path = None + self.assertEqual(lookup_root_vhd.return_value, + root_device['path']) - lookup_ephemeral.assert_called_with(mock_instance.name) get_image_vm_gen = self._migrationops._vmops.get_image_vm_generation get_image_vm_gen.assert_called_once_with( - mock_instance.uuid, fake_root_path, - test.MatchType(objects.ImageMeta)) + mock_instance.uuid, test.MatchType(objects.ImageMeta)) self._migrationops._vmops.create_instance.assert_called_once_with( - mock_instance, mock.sentinel.network_info, - mock.sentinel.block_device, fake_root_path, - lookup_ephemeral.return_value, get_image_vm_gen.return_value) + mock_instance, mock.sentinel.network_info, root_device, + block_device_info, get_image_vm_gen.return_value) mock_check_attach_config_drive.assert_called_once_with( mock_instance, get_image_vm_gen.return_value) self._migrationops._vmops.power_on.assert_called_once_with( mock_instance) def test_finish_revert_migration_boot_from_volume(self): - self._check_finish_revert_migration(boot_from_volume=True) + self._check_finish_revert_migration(disk_type=constants.VOLUME) - def test_finish_revert_migration_not_in_block_device(self): - self._check_finish_revert_migration() + def test_finish_revert_migration_boot_from_disk(self): + self._check_finish_revert_migration(disk_type=constants.DISK) + + @mock.patch.object(objects.ImageMeta, "from_instance") + def test_finish_revert_migration_no_root_vhd(self, mock_image): + mock_instance = fake_instance.fake_instance_obj(self.context) + self._migrationops._pathutils.lookup_root_vhd_path.return_value = None + bdi = {'root_disk': {'type': constants.DISK}, + 'ephemerals': []} + + self.assertRaises( + exception.DiskNotFound, + self._migrationops.finish_revert_migration, self.context, + mock_instance, mock.sentinel.network_info, bdi, True) def test_merge_base_vhd(self): fake_diff_vhd_path = 'fake/diff/path' @@ -362,24 +369,21 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase): '_check_and_attach_config_drive') @mock.patch.object(migrationops.MigrationOps, '_check_base_disk') @mock.patch.object(migrationops.MigrationOps, '_check_resize_vhd') - def _check_finish_migration(self, mock_check_resize_vhd, + @mock.patch.object(migrationops.MigrationOps, '_check_ephemeral_disks') + def _check_finish_migration(self, mock_check_eph_disks, + mock_check_resize_vhd, mock_check_base_disk, mock_check_attach_config_drive, - ephemeral_path=None, - boot_from_volume=False): + disk_type=constants.DISK): mock_instance = fake_instance.fake_instance_obj(self.context) mock_instance.ephemeral_gb = 1 - mock_vhd_info = mock.MagicMock() - mock_eph_info = mock.MagicMock() + root_device = {'type': disk_type} + block_device_info = {'root_disk': root_device, 'ephemerals': []} + lookup_root_vhd = self._migrationops._pathutils.lookup_root_vhd_path - side_effect = [mock_eph_info] if boot_from_volume else [mock_vhd_info, - mock_eph_info] - mock_ebs_root_in_block_devices = ( - self._migrationops._volumeops.ebs_root_in_block_devices) - mock_ebs_root_in_block_devices.return_value = boot_from_volume - self._migrationops._vhdutils.get_vhd_info.side_effect = side_effect - look_up_ephem = self._migrationops._pathutils.lookup_ephemeral_vhd_path - look_up_ephem.return_value = ephemeral_path + get_vhd_info = self._migrationops._vhdutils.get_vhd_info + mock_vhd_info = get_vhd_info.return_value + expected_check_resize = [] expected_get_info = [] @@ -387,71 +391,131 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase): context=self.context, migration=mock.sentinel.migration, instance=mock_instance, disk_info=mock.sentinel.disk_info, network_info=mock.sentinel.network_info, - image_meta=mock.sentinel.image_meta, resize_instance=True) + image_meta=mock.sentinel.image_meta, resize_instance=True, + block_device_info=block_device_info) - mock_ebs_root_in_block_devices.assert_called_once_with(None) - if not boot_from_volume: - root_vhd_path = lookup_root_vhd.return_value + if root_device['type'] == constants.DISK: + root_device_path = lookup_root_vhd.return_value lookup_root_vhd.assert_called_with(mock_instance.name) - expected_get_info = [mock.call(root_vhd_path)] + expected_get_info = [mock.call(root_device_path)] mock_vhd_info.get.assert_called_once_with("ParentPath") mock_check_base_disk.assert_called_once_with( - self.context, mock_instance, root_vhd_path, + self.context, mock_instance, root_device_path, mock_vhd_info.get.return_value) expected_check_resize.append( - mock.call(root_vhd_path, mock_vhd_info, + mock.call(root_device_path, mock_vhd_info, mock_instance.root_gb * units.Gi)) - else: - root_vhd_path = None - look_up_ephem.assert_called_once_with(mock_instance.name) - if ephemeral_path is None: - create_eph_vhd = self._migrationops._vmops.create_ephemeral_vhd - create_eph_vhd.assert_called_once_with(mock_instance) - ephemeral_path = create_eph_vhd.return_value - else: - expected_get_info.append(mock.call(ephemeral_path)) - expected_check_resize.append( - mock.call(ephemeral_path, mock_eph_info, - mock_instance.ephemeral_gb * units.Gi)) + ephemerals = block_device_info['ephemerals'] + mock_check_eph_disks.assert_called_once_with( + mock_instance, ephemerals, True) mock_check_resize_vhd.assert_has_calls(expected_check_resize) self._migrationops._vhdutils.get_vhd_info.assert_has_calls( expected_get_info) get_image_vm_gen = self._migrationops._vmops.get_image_vm_generation get_image_vm_gen.assert_called_once_with(mock_instance.uuid, - root_vhd_path, mock.sentinel.image_meta) self._migrationops._vmops.create_instance.assert_called_once_with( - mock_instance, mock.sentinel.network_info, None, root_vhd_path, - ephemeral_path, get_image_vm_gen.return_value) + mock_instance, mock.sentinel.network_info, root_device, + block_device_info, get_image_vm_gen.return_value) mock_check_attach_config_drive.assert_called_once_with( mock_instance, get_image_vm_gen.return_value) self._migrationops._vmops.power_on.assert_called_once_with( mock_instance) def test_finish_migration(self): - self._check_finish_migration( - ephemeral_path=mock.sentinel.ephemeral_path) + self._check_finish_migration(disk_type=constants.DISK) def test_finish_migration_boot_from_volume(self): - self._check_finish_migration( - ephemeral_path=mock.sentinel.ephemeral_path, - boot_from_volume=True) - - def test_finish_migration_no_ephemeral(self): - self._check_finish_migration() + self._check_finish_migration(disk_type=constants.VOLUME) def test_finish_migration_no_root(self): mock_instance = fake_instance.fake_instance_obj(self.context) - mock_ebs_root_in_block_devices = ( - self._migrationops._volumeops.ebs_root_in_block_devices) - mock_ebs_root_in_block_devices.return_value = False self._migrationops._pathutils.lookup_root_vhd_path.return_value = None + bdi = {'root_disk': {'type': constants.DISK}, + 'ephemerals': []} self.assertRaises(exception.DiskNotFound, self._migrationops.finish_migration, self.context, mock.sentinel.migration, mock_instance, mock.sentinel.disk_info, mock.sentinel.network_info, - mock.sentinel.image_meta, True, None, True) + mock.sentinel.image_meta, True, bdi, True) + + @mock.patch.object(migrationops.MigrationOps, '_check_resize_vhd') + @mock.patch.object(migrationops.LOG, 'warn') + def test_check_ephemeral_disks_multiple_eph_warn(self, mock_warn, + mock_check_resize_vhd): + mock_instance = fake_instance.fake_instance_obj(self.context) + mock_instance.ephemeral_gb = 3 + mock_ephemerals = [{'size': 1}, {'size': 1}] + + self._migrationops._check_ephemeral_disks(mock_instance, + mock_ephemerals, + True) + + mock_warn.assert_called_once_with( + "Cannot resize multiple ephemeral disks for instance.", + instance=mock_instance) + + def test_check_ephemeral_disks_exception(self): + mock_instance = fake_instance.fake_instance_obj(self.context) + mock_ephemerals = [dict()] + + lookup_eph_path = ( + self._migrationops._pathutils.lookup_ephemeral_vhd_path) + lookup_eph_path.return_value = None + + self.assertRaises(exception.DiskNotFound, + self._migrationops._check_ephemeral_disks, + mock_instance, mock_ephemerals) + + @mock.patch.object(migrationops.MigrationOps, '_check_resize_vhd') + def _test_check_ephemeral_disks(self, mock_check_resize_vhd, + existing_eph_path=None, new_eph_size=42): + mock_instance = fake_instance.fake_instance_obj(self.context) + mock_instance.ephemeral_gb = new_eph_size + eph = {} + mock_ephemerals = [eph] + + mock_pathutils = self._migrationops._pathutils + lookup_eph_path = mock_pathutils.lookup_ephemeral_vhd_path + lookup_eph_path.return_value = existing_eph_path + mock_get_eph_vhd_path = mock_pathutils.get_ephemeral_vhd_path + mock_get_eph_vhd_path.return_value = mock.sentinel.get_path + + mock_vhdutils = self._migrationops._vhdutils + mock_get_vhd_format = mock_vhdutils.get_best_supported_vhd_format + mock_get_vhd_format.return_value = mock.sentinel.vhd_format + + self._migrationops._check_ephemeral_disks(mock_instance, + mock_ephemerals, + True) + + self.assertEqual(mock_instance.ephemeral_gb, eph['size']) + if not existing_eph_path: + mock_vmops = self._migrationops._vmops + mock_vmops.create_ephemeral_disk.assert_called_once_with( + mock_instance.name, eph) + self.assertEqual(mock.sentinel.vhd_format, eph['format']) + self.assertEqual(mock.sentinel.get_path, eph['path']) + elif new_eph_size: + mock_check_resize_vhd.assert_called_once_with( + existing_eph_path, + self._migrationops._vhdutils.get_vhd_info.return_value, + mock_instance.ephemeral_gb * units.Gi) + self.assertEqual(existing_eph_path, eph['path']) + else: + self._migrationops._pathutils.remove.assert_called_once_with( + existing_eph_path) + + def test_check_ephemeral_disks_create(self): + self._test_check_ephemeral_disks() + + def test_check_ephemeral_disks_resize(self): + self._test_check_ephemeral_disks(existing_eph_path=mock.sentinel.path) + + def test_check_ephemeral_disks_remove(self): + self._test_check_ephemeral_disks(existing_eph_path=mock.sentinel.path, + new_eph_size=0) diff --git a/nova/tests/unit/virt/hyperv/test_vmops.py b/nova/tests/unit/virt/hyperv/test_vmops.py index 0284be32c4d8..4d016487f59b 100644 --- a/nova/tests/unit/virt/hyperv/test_vmops.py +++ b/nova/tests/unit/virt/hyperv/test_vmops.py @@ -67,6 +67,7 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): self._vmops._pathutils = mock.MagicMock() self._vmops._hostutils = mock.MagicMock() self._vmops._serial_console_ops = mock.MagicMock() + self._vmops._block_dev_man = mock.MagicMock() @mock.patch('nova.network.is_neutron') @mock.patch('nova.virt.hyperv.vmops.importutils.import_object') @@ -151,7 +152,24 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): def test_get_info_exception(self): self._test_get_info(vm_exists=False) - def _prepare_create_root_vhd_mocks(self, use_cow_images, vhd_format, + @mock.patch.object(vmops.VMOps, 'check_vm_image_type') + @mock.patch.object(vmops.VMOps, '_create_root_vhd') + def test_create_root_device_type_disk(self, mock_create_root_device, + mock_check_vm_image_type): + mock_instance = fake_instance.fake_instance_obj(self.context) + mock_root_disk_info = {'type': constants.DISK} + + self._vmops._create_root_device(self.context, mock_instance, + mock_root_disk_info, + mock.sentinel.VM_GEN_1) + + mock_create_root_device.assert_called_once_with( + self.context, mock_instance) + mock_check_vm_image_type.assert_called_once_with( + mock_instance.uuid, mock.sentinel.VM_GEN_1, + mock_create_root_device.return_value) + + def _prepare_create_root_device_mocks(self, use_cow_images, vhd_format, vhd_size): mock_instance = fake_instance.fake_instance_obj(self.context) mock_instance.root_gb = self.FAKE_SIZE @@ -169,7 +187,7 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): @mock.patch('nova.virt.hyperv.imagecache.ImageCache.get_cached_image') def _test_create_root_vhd_exception(self, mock_get_cached_image, vhd_format): - mock_instance = self._prepare_create_root_vhd_mocks( + mock_instance = self._prepare_create_root_device_mocks( use_cow_images=False, vhd_format=vhd_format, vhd_size=(self.FAKE_SIZE + 1)) fake_vhd_path = self.FAKE_ROOT_PATH % vhd_format @@ -188,7 +206,7 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): @mock.patch('nova.virt.hyperv.imagecache.ImageCache.get_cached_image') def _test_create_root_vhd_qcow(self, mock_get_cached_image, vhd_format): - mock_instance = self._prepare_create_root_vhd_mocks( + mock_instance = self._prepare_create_root_device_mocks( use_cow_images=True, vhd_format=vhd_format, vhd_size=(self.FAKE_SIZE - 1)) fake_vhd_path = self.FAKE_ROOT_PATH % vhd_format @@ -221,7 +239,7 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): @mock.patch('nova.virt.hyperv.imagecache.ImageCache.get_cached_image') def _test_create_root_vhd(self, mock_get_cached_image, vhd_format, is_rescue_vhd=False): - mock_instance = self._prepare_create_root_vhd_mocks( + mock_instance = self._prepare_create_root_device_mocks( use_cow_images=False, vhd_format=vhd_format, vhd_size=(self.FAKE_SIZE - 1)) fake_vhd_path = self.FAKE_ROOT_PATH % vhd_format @@ -292,21 +310,36 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): self.assertFalse(self._vmops._is_resize_needed( mock.sentinel.FAKE_PATH, self.FAKE_SIZE, self.FAKE_SIZE, inst)) - def test_create_ephemeral_vhd(self): + @mock.patch.object(vmops.VMOps, 'create_ephemeral_disk') + def test_create_ephemerals(self, mock_create_ephemeral_disk): mock_instance = fake_instance.fake_instance_obj(self.context) - mock_instance.ephemeral_gb = self.FAKE_SIZE - best_supported = self._vmops._vhdutils.get_best_supported_vhd_format - best_supported.return_value = mock.sentinel.FAKE_FORMAT - self._vmops._pathutils.get_ephemeral_vhd_path.return_value = ( - mock.sentinel.FAKE_PATH) - response = self._vmops.create_ephemeral_vhd(instance=mock_instance) + fake_ephemerals = [dict(), dict()] + self._vmops._vhdutils.get_best_supported_vhd_format.return_value = ( + mock.sentinel.format) + self._vmops._pathutils.get_ephemeral_vhd_path.side_effect = [ + mock.sentinel.FAKE_PATH0, mock.sentinel.FAKE_PATH1] - self._vmops._pathutils.get_ephemeral_vhd_path.assert_called_with( - mock_instance.name, mock.sentinel.FAKE_FORMAT) - self._vmops._vhdutils.create_dynamic_vhd.assert_called_with( - mock.sentinel.FAKE_PATH, mock_instance.ephemeral_gb * units.Gi) - self.assertEqual(mock.sentinel.FAKE_PATH, response) + self._vmops._create_ephemerals(mock_instance, fake_ephemerals) + + self._vmops._pathutils.get_ephemeral_vhd_path.assert_has_calls( + [mock.call(mock_instance.name, mock.sentinel.format, 'eph0'), + mock.call(mock_instance.name, mock.sentinel.format, 'eph1')]) + mock_create_ephemeral_disk.assert_has_calls( + [mock.call(mock_instance.name, fake_ephemerals[0]), + mock.call(mock_instance.name, fake_ephemerals[1])]) + + def test_create_ephemeral_disk(self): + mock_instance = fake_instance.fake_instance_obj(self.context) + mock_ephemeral_info = {'path': 'fake_eph_path', + 'size': 10} + + self._vmops.create_ephemeral_disk(mock_instance.name, + mock_ephemeral_info) + + mock_create_dynamic_vhd = self._vmops._vhdutils.create_dynamic_vhd + mock_create_dynamic_vhd.assert_called_once_with('fake_eph_path', + 10 * units.Gi) @mock.patch('nova.virt.hyperv.vmops.VMOps.destroy') @mock.patch('nova.virt.hyperv.vmops.VMOps.power_on') @@ -315,62 +348,59 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): @mock.patch('nova.virt.configdrive.required_by') @mock.patch('nova.virt.hyperv.vmops.VMOps.create_instance') @mock.patch('nova.virt.hyperv.vmops.VMOps.get_image_vm_generation') - @mock.patch('nova.virt.hyperv.vmops.VMOps.create_ephemeral_vhd') - @mock.patch('nova.virt.hyperv.vmops.VMOps._create_root_vhd') - @mock.patch('nova.virt.hyperv.volumeops.VolumeOps.' - 'ebs_root_in_block_devices') + @mock.patch('nova.virt.hyperv.vmops.VMOps._create_ephemerals') + @mock.patch('nova.virt.hyperv.vmops.VMOps._create_root_device') @mock.patch('nova.virt.hyperv.vmops.VMOps._delete_disk_files') - def _test_spawn(self, mock_delete_disk_files, - mock_ebs_root_in_block_devices, mock_create_root_vhd, - mock_create_ephemeral_vhd, mock_get_image_vm_gen, + def _test_spawn(self, mock_delete_disk_files, mock_create_root_device, + mock_create_ephemerals, mock_get_image_vm_gen, mock_create_instance, mock_configdrive_required, mock_create_config_drive, mock_attach_config_drive, - mock_power_on, mock_destroy, exists, boot_from_volume, + mock_power_on, mock_destroy, exists, configdrive_required, fail): mock_instance = fake_instance.fake_instance_obj(self.context) mock_image_meta = mock.MagicMock() - - fake_root_path = mock_create_root_vhd.return_value - fake_root_path = None if boot_from_volume else fake_root_path - fake_ephemeral_path = mock_create_ephemeral_vhd.return_value + root_device_info = mock.sentinel.ROOT_DEV_INFO fake_vm_gen = mock_get_image_vm_gen.return_value fake_config_drive_path = mock_create_config_drive.return_value + block_device_info = {'ephemerals': [], 'root_disk': root_device_info} self._vmops._vmutils.vm_exists.return_value = exists - mock_ebs_root_in_block_devices.return_value = boot_from_volume - mock_create_root_vhd.return_value = fake_root_path mock_configdrive_required.return_value = configdrive_required mock_create_instance.side_effect = fail if exists: self.assertRaises(exception.InstanceExists, self._vmops.spawn, self.context, mock_instance, mock_image_meta, [mock.sentinel.FILE], mock.sentinel.PASSWORD, - mock.sentinel.INFO, mock.sentinel.DEV_INFO) + mock.sentinel.INFO, block_device_info) elif fail is os_win_exc.HyperVException: self.assertRaises(os_win_exc.HyperVException, self._vmops.spawn, self.context, mock_instance, mock_image_meta, [mock.sentinel.FILE], mock.sentinel.PASSWORD, - mock.sentinel.INFO, mock.sentinel.DEV_INFO) + mock.sentinel.INFO, block_device_info) mock_destroy.assert_called_once_with(mock_instance) else: self._vmops.spawn(self.context, mock_instance, mock_image_meta, [mock.sentinel.FILE], mock.sentinel.PASSWORD, - mock.sentinel.INFO, mock.sentinel.DEV_INFO) + mock.sentinel.INFO, block_device_info) self._vmops._vmutils.vm_exists.assert_called_once_with( mock_instance.name) mock_delete_disk_files.assert_called_once_with( mock_instance.name) - mock_ebs_root_in_block_devices.assert_called_once_with( - mock.sentinel.DEV_INFO) - if not boot_from_volume: - mock_create_root_vhd.assert_called_once_with(self.context, - mock_instance) - mock_create_ephemeral_vhd.assert_called_once_with(mock_instance) - mock_get_image_vm_gen.assert_called_once_with( - mock_instance.uuid, fake_root_path, mock_image_meta) + mock_validate_and_update_bdi = ( + self._vmops._block_dev_man.validate_and_update_bdi) + mock_validate_and_update_bdi.assert_called_once_with( + mock_instance, mock_image_meta, fake_vm_gen, block_device_info) + mock_create_root_device.assert_called_once_with(self.context, + mock_instance, + root_device_info, + fake_vm_gen) + mock_create_ephemerals.assert_called_once_with( + mock_instance, block_device_info['ephemerals']) + mock_get_image_vm_gen.assert_called_once_with(mock_instance.uuid, + mock_image_meta) mock_create_instance.assert_called_once_with( - mock_instance, mock.sentinel.INFO, mock.sentinel.DEV_INFO, - fake_root_path, fake_ephemeral_path, fake_vm_gen) + mock_instance, mock.sentinel.INFO, root_device_info, + block_device_info, fake_vm_gen) mock_configdrive_required.assert_called_once_with(mock_instance) if configdrive_required: mock_create_config_drive.assert_called_once_with( @@ -382,25 +412,17 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): mock_power_on.assert_called_once_with(mock_instance) def test_spawn(self): - self._test_spawn(exists=False, boot_from_volume=False, - configdrive_required=True, fail=None) + self._test_spawn(exists=False, configdrive_required=True, fail=None) def test_spawn_instance_exists(self): - self._test_spawn(exists=True, boot_from_volume=False, - configdrive_required=True, fail=None) + self._test_spawn(exists=True, configdrive_required=True, fail=None) def test_spawn_create_instance_exception(self): - self._test_spawn(exists=False, boot_from_volume=False, - configdrive_required=True, + self._test_spawn(exists=False, configdrive_required=True, fail=os_win_exc.HyperVException) def test_spawn_not_required(self): - self._test_spawn(exists=False, boot_from_volume=False, - configdrive_required=False, fail=None) - - def test_spawn_root_in_block(self): - self._test_spawn(exists=False, boot_from_volume=True, - configdrive_required=False, fail=None) + self._test_spawn(exists=False, configdrive_required=False, fail=None) def test_spawn_no_admin_permissions(self): self._vmops._vmutils.check_admin_permissions.side_effect = ( @@ -413,19 +435,23 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): @mock.patch('nova.virt.hyperv.volumeops.VolumeOps' '.attach_volumes') - @mock.patch.object(vmops.VMOps, '_attach_drive') @mock.patch.object(vmops.VMOps, '_create_vm_com_port_pipes') + @mock.patch.object(vmops.VMOps, '_attach_ephemerals') + @mock.patch.object(vmops.VMOps, '_attach_root_device') @mock.patch.object(vmops.VMOps, '_configure_remotefx') def _test_create_instance(self, mock_configure_remotefx, - mock_create_pipes, mock_attach_drive, - mock_attach_volumes, fake_root_path, - fake_ephemeral_path, + mock_attach_root_device, + mock_attach_ephemerals, + mock_create_pipes, + mock_attach_volumes, enable_instance_metrics, vm_gen=constants.VM_GEN_1): mock_vif_driver = mock.MagicMock() self._vmops._vif_driver = mock_vif_driver self.flags(enable_instance_metrics_collection=enable_instance_metrics, group='hyperv') + root_device_info = mock.sentinel.ROOT_DEV_INFO + block_device_info = {'ephemerals': [], 'block_device_mapping': []} fake_network_info = {'id': mock.sentinel.ID, 'address': mock.sentinel.ADDRESS} mock_instance = fake_instance.fake_instance_obj(self.context) @@ -436,9 +462,8 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): self._vmops.create_instance(instance=mock_instance, network_info=[fake_network_info], - block_device_info=mock.sentinel.DEV_INFO, - root_vhd_path=fake_root_path, - eph_vhd_path=fake_ephemeral_path, + root_device=root_device_info, + block_device_info=block_device_info, vm_gen=vm_gen) self._vmops._vmutils.create_vm.assert_called_once_with( mock_instance.name, mock_instance.memory_mb, @@ -447,32 +472,15 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): [mock_instance.uuid]) mock_configure_remotefx.assert_called_once_with(mock_instance, vm_gen) - expected = [] - ctrl_type = vmops.VM_GENERATIONS_CONTROLLER_TYPES[vm_gen] - ctrl_disk_addr = 0 - if fake_root_path: - expected.append(mock.call(mock_instance.name, fake_root_path, - 0, ctrl_disk_addr, ctrl_type, - constants.DISK)) - ctrl_disk_addr = 1 - if fake_ephemeral_path: - expected.append(mock.call(mock_instance.name, - fake_ephemeral_path, 0, ctrl_disk_addr, - ctrl_type, constants.DISK)) - mock_attach_drive.has_calls(expected) - self._vmops._vmutils.create_scsi_controller.assert_called_once_with( - mock_instance.name) + mock_create_scsi_ctrl = self._vmops._vmutils.create_scsi_controller + mock_create_scsi_ctrl.assert_called_once_with(mock_instance.name) - ebs_root = vm_gen is not constants.VM_GEN_2 and fake_root_path is None - mock_attach_volumes.assert_called_once_with(mock.sentinel.DEV_INFO, - mock_instance.name, - ebs_root) - - expected_port_settings = { - constants.DEFAULT_SERIAL_CONSOLE_PORT: - constants.SERIAL_PORT_TYPE_RW} - mock_create_pipes.assert_called_once_with( - mock_instance, expected_port_settings) + mock_attach_root_device.assert_called_once_with(mock_instance.name, + root_device_info) + mock_attach_ephemerals.assert_called_once_with(mock_instance.name, + block_device_info['ephemerals']) + mock_attach_volumes.assert_called_once_with( + block_device_info['block_device_mapping'], mock_instance.name) self._vmops._vmutils.create_nic.assert_called_once_with( mock_instance.name, mock.sentinel.ID, mock.sentinel.ADDRESS) @@ -483,39 +491,72 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): mock_enable.assert_called_once_with(mock_instance.name) def test_create_instance(self): - fake_ephemeral_path = mock.sentinel.FAKE_EPHEMERAL_PATH - self._test_create_instance(fake_root_path=mock.sentinel.FAKE_ROOT_PATH, - fake_ephemeral_path=fake_ephemeral_path, - enable_instance_metrics=True) - - def test_create_instance_no_root_path(self): - fake_ephemeral_path = mock.sentinel.FAKE_EPHEMERAL_PATH - self._test_create_instance(fake_root_path=None, - fake_ephemeral_path=fake_ephemeral_path, - enable_instance_metrics=True) - - def test_create_instance_no_ephemeral_path(self): - self._test_create_instance(fake_root_path=mock.sentinel.FAKE_ROOT_PATH, - fake_ephemeral_path=None, - enable_instance_metrics=True) - - def test_create_instance_no_path(self): - self._test_create_instance(fake_root_path=None, - fake_ephemeral_path=None, - enable_instance_metrics=False) + self._test_create_instance(enable_instance_metrics=True) def test_create_instance_enable_instance_metrics_false(self): - fake_ephemeral_path = mock.sentinel.FAKE_EPHEMERAL_PATH - self._test_create_instance(fake_root_path=mock.sentinel.FAKE_ROOT_PATH, - fake_ephemeral_path=fake_ephemeral_path, - enable_instance_metrics=False) + self._test_create_instance(enable_instance_metrics=False) def test_create_instance_gen2(self): - self._test_create_instance(fake_root_path=None, - fake_ephemeral_path=None, - enable_instance_metrics=False, + self._test_create_instance(enable_instance_metrics=False, vm_gen=constants.VM_GEN_2) + @mock.patch.object(vmops.volumeops.VolumeOps, 'attach_volume') + def test_attach_root_device_volume(self, mock_attach_volume): + mock_instance = fake_instance.fake_instance_obj(self.context) + root_device_info = {'type': constants.VOLUME, + 'connection_info': mock.sentinel.CONN_INFO, + 'disk_bus': constants.CTRL_TYPE_IDE} + + self._vmops._attach_root_device(mock_instance.name, root_device_info) + + mock_attach_volume.assert_called_once_with( + root_device_info['connection_info'], mock_instance.name, + disk_bus=root_device_info['disk_bus']) + + @mock.patch.object(vmops.VMOps, '_attach_drive') + def test_attach_root_device_disk(self, mock_attach_drive): + mock_instance = fake_instance.fake_instance_obj(self.context) + root_device_info = {'type': constants.DISK, + 'boot_index': 0, + 'disk_bus': constants.CTRL_TYPE_IDE, + 'path': 'fake_path', + 'drive_addr': 0, + 'ctrl_disk_addr': 1} + + self._vmops._attach_root_device(mock_instance.name, root_device_info) + + mock_attach_drive.assert_called_once_with( + mock_instance.name, root_device_info['path'], + root_device_info['drive_addr'], root_device_info['ctrl_disk_addr'], + root_device_info['disk_bus'], root_device_info['type']) + + @mock.patch.object(vmops.VMOps, '_attach_drive') + def test_attach_ephemerals(self, mock_attach_drive): + mock_instance = fake_instance.fake_instance_obj(self.context) + + ephemerals = [{'path': mock.sentinel.PATH1, + 'boot_index': 1, + 'disk_bus': constants.CTRL_TYPE_IDE, + 'device_type': 'disk', + 'drive_addr': 0, + 'ctrl_disk_addr': 1}, + {'path': mock.sentinel.PATH2, + 'boot_index': 2, + 'disk_bus': constants.CTRL_TYPE_SCSI, + 'device_type': 'disk', + 'drive_addr': 0, + 'ctrl_disk_addr': 0}, + {'path': None}] + + self._vmops._attach_ephemerals(mock_instance.name, ephemerals) + + mock_attach_drive.assert_has_calls( + [mock.call(mock_instance.name, mock.sentinel.PATH1, 0, + 1, constants.CTRL_TYPE_IDE, constants.DISK), + mock.call(mock_instance.name, mock.sentinel.PATH2, 0, + 0, constants.CTRL_TYPE_SCSI, constants.DISK) + ]) + def test_attach_drive_vm_to_scsi(self): self._vmops._attach_drive( mock.sentinel.FAKE_VM_NAME, mock.sentinel.FAKE_PATH, @@ -545,7 +586,7 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): constants.IMAGE_PROP_VM_GEN_1, constants.IMAGE_PROP_VM_GEN_2] response = self._vmops.get_image_vm_generation( - mock.sentinel.instance_id, mock.sentinel.FAKE_PATH, image_meta) + mock.sentinel.instance_id, image_meta) self.assertEqual(constants.VM_GEN_1, response) @@ -555,28 +596,20 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): {"hw_machine_type": constants.IMAGE_PROP_VM_GEN_2}}) self._vmops._hostutils.get_supported_vm_types.return_value = [ constants.IMAGE_PROP_VM_GEN_1, constants.IMAGE_PROP_VM_GEN_2] - self._vmops._vhdutils.get_vhd_format.return_value = ( - constants.DISK_FORMAT_VHDX) response = self._vmops.get_image_vm_generation( - mock.sentinel.instance_id, mock.sentinel.FAKE_PATH, image_meta) + mock.sentinel.instance_id, image_meta) self.assertEqual(constants.VM_GEN_2, response) - def test_get_image_vm_generation_not_vhdx(self): - image_meta = objects.ImageMeta.from_dict( - {"properties": - {'hw_machine_type': constants.IMAGE_PROP_VM_GEN_2}}) - self._vmops._hostutils.get_supported_vm_types.return_value = [ - constants.IMAGE_PROP_VM_GEN_1, constants.IMAGE_PROP_VM_GEN_2] + def test_check_vm_image_type_exception(self): self._vmops._vhdutils.get_vhd_format.return_value = ( constants.DISK_FORMAT_VHD) self.assertRaises(exception.InstanceUnacceptable, - self._vmops.get_image_vm_generation, - mock.sentinel.instance_id, - mock.sentinel.FAKE_PATH, - image_meta) + self._vmops.check_vm_image_type, + mock.sentinel.instance_id, constants.VM_GEN_2, + mock.sentinel.FAKE_PATH) @mock.patch('nova.api.metadata.base.InstanceMetadata') @mock.patch('nova.virt.configdrive.ConfigDriveBuilder') @@ -1244,8 +1277,7 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase): mock.sentinel.rescue_password) mock_get_image_vm_gen.assert_called_once_with( - mock_instance.uuid, mock.sentinel.rescue_vhd_path, - mock_image_meta) + mock_instance.uuid, mock_image_meta) self._vmops._vmutils.detach_vm_disk.assert_called_once_with( mock_instance.name, mock.sentinel.root_vhd_path, is_physical=False) diff --git a/nova/tests/unit/virt/hyperv/test_volumeops.py b/nova/tests/unit/virt/hyperv/test_volumeops.py index f4a8401f1c18..4bcc7d6f7bb1 100644 --- a/nova/tests/unit/virt/hyperv/test_volumeops.py +++ b/nova/tests/unit/virt/hyperv/test_volumeops.py @@ -24,6 +24,7 @@ from nova import exception from nova import test from nova.tests.unit import fake_block_device from nova.tests.unit.virt.hyperv import test_base +from nova.virt.hyperv import constants from nova.virt.hyperv import volumeops CONF = cfg.CONF @@ -76,13 +77,13 @@ class VolumeOpsTestCase(test_base.HyperVBaseTestCase): def test_attach_volumes(self, mock_attach_volume): block_device_info = get_fake_block_dev_info() - self._volumeops.attach_volumes(block_device_info, - mock.sentinel.instance_name, - ebs_root=True) + self._volumeops.attach_volumes( + block_device_info['block_device_mapping'], + mock.sentinel.instance_name) mock_attach_volume.assert_called_once_with( block_device_info['block_device_mapping'][0]['connection_info'], - mock.sentinel.instance_name, True) + mock.sentinel.instance_name) def test_fix_instance_volume_disk_paths_empty_bdm(self): self._volumeops.fix_instance_volume_disk_paths( @@ -144,16 +145,6 @@ class VolumeOpsTestCase(test_base.HyperVBaseTestCase): fake_volume_driver.disconnect_volumes.assert_called_once_with( block_device_mapping) - @mock.patch('nova.block_device.volume_in_mapping') - def test_ebs_root_in_block_devices(self, mock_vol_in_mapping): - block_device_info = get_fake_block_dev_info() - - response = self._volumeops.ebs_root_in_block_devices(block_device_info) - - mock_vol_in_mapping.assert_called_once_with( - self._volumeops._default_root_device, block_device_info) - self.assertEqual(mock_vol_in_mapping.return_value, response) - def test_get_volume_connector(self): mock_instance = mock.DEFAULT initiator = self._volumeops._volutils.get_iscsi_initiator.return_value @@ -324,7 +315,7 @@ class ISCSIVolumeDriverTestCase(test_base.HyperVBaseTestCase): '_get_mounted_disk_from_lun') @mock.patch.object(volumeops.ISCSIVolumeDriver, 'login_storage_target') def _check_attach_volume(self, mock_login_storage_target, - mock_get_mounted_disk_from_lun, ebs_root): + mock_get_mounted_disk_from_lun, disk_bus): connection_info = get_fake_connection_info() get_ide_path = self._volume_driver._vmutils.get_vm_ide_controller @@ -340,14 +331,14 @@ class ISCSIVolumeDriverTestCase(test_base.HyperVBaseTestCase): self._volume_driver.attach_volume( connection_info=connection_info, instance_name=mock.sentinel.instance_name, - ebs_root=ebs_root) + disk_bus=disk_bus) mock_login_storage_target.assert_called_once_with(connection_info) mock_get_mounted_disk_from_lun.assert_called_once_with( mock.sentinel.fake_iqn, mock.sentinel.fake_lun, wait_for_device=True) - if ebs_root: + if disk_bus == constants.CTRL_TYPE_IDE: get_ide_path.assert_called_once_with( mock.sentinel.instance_name, 0) attach_vol.assert_called_once_with(mock.sentinel.instance_name, @@ -362,11 +353,11 @@ class ISCSIVolumeDriverTestCase(test_base.HyperVBaseTestCase): fake_mounted_disk_path, serial=mock.sentinel.serial) - def test_attach_volume_ebs(self): - self._check_attach_volume(ebs_root=True) + def test_attach_volume_ide(self): + self._check_attach_volume(disk_bus=constants.CTRL_TYPE_IDE) - def test_attach_volume(self): - self._check_attach_volume(ebs_root=False) + def test_attach_volume_scsi(self): + self._check_attach_volume(disk_bus=constants.CTRL_TYPE_SCSI) @mock.patch.object(volumeops.ISCSIVolumeDriver, '_get_mounted_disk_from_lun') @@ -491,15 +482,15 @@ class SMBFSVolumeDriverTestCase(test_base.HyperVBaseTestCase): @mock.patch.object(volumeops.SMBFSVolumeDriver, 'ensure_share_mounted') @mock.patch.object(volumeops.SMBFSVolumeDriver, '_get_disk_path') def _check_attach_volume(self, mock_get_disk_path, - mock_ensure_share_mounted, ebs_root=False): + mock_ensure_share_mounted, disk_bus): mock_get_disk_path.return_value = mock.sentinel.disk_path self._volume_driver.attach_volume( self._FAKE_CONNECTION_INFO, mock.sentinel.instance_name, - ebs_root) + disk_bus) - if ebs_root: + if disk_bus == constants.CTRL_TYPE_IDE: get_vm_ide_controller = ( self._volume_driver._vmutils.get_vm_ide_controller) get_vm_ide_controller.assert_called_once_with( @@ -527,10 +518,10 @@ class SMBFSVolumeDriverTestCase(test_base.HyperVBaseTestCase): ctrller_path, slot) def test_attach_volume_ide(self): - self._check_attach_volume(ebs_root=True) + self._check_attach_volume(disk_bus=constants.CTRL_TYPE_IDE) def test_attach_volume_scsi(self): - self._check_attach_volume() + self._check_attach_volume(disk_bus=constants.CTRL_TYPE_SCSI) @mock.patch.object(volumeops.SMBFSVolumeDriver, 'ensure_share_mounted') @mock.patch.object(volumeops.SMBFSVolumeDriver, '_get_disk_path') diff --git a/nova/virt/hyperv/block_device_manager.py b/nova/virt/hyperv/block_device_manager.py index 1e6cdc679599..c5f88eb4d4ba 100644 --- a/nova/virt/hyperv/block_device_manager.py +++ b/nova/virt/hyperv/block_device_manager.py @@ -58,8 +58,6 @@ class BlockDeviceInfoManager(object): # reserve one slot for the config drive on the second # controller in case of generation 1 virtual machines free_slots_by_device_type[constants.CTRL_TYPE_IDE][1] -= 1 - else: - free_slots_by_device_type[constants.CTRL_TYPE_SCSI][0] -= 1 return free_slots_by_device_type def validate_and_update_bdi(self, instance, image_meta, vm_gen, @@ -70,6 +68,14 @@ class BlockDeviceInfoManager(object): self._check_and_update_ephemerals(vm_gen, block_device_info, slot_map) self._check_and_update_volumes(vm_gen, block_device_info, slot_map) + if vm_gen == constants.VM_GEN_2 and configdrive.required_by(instance): + # for Generation 2 VMs, the configdrive is attached to the SCSI + # controller. Check that there is still a slot available for it. + if slot_map[constants.CTRL_TYPE_SCSI][0] == 0: + msg = _("There are no more free slots on controller %s for " + "configdrive.") % constants.CTRL_TYPE_SCSI + raise exception.InvalidBDMFormat(details=msg) + def _check_and_update_root_device(self, vm_gen, image_meta, block_device_info, slot_map): # either booting from volume, or booting from image/iso diff --git a/nova/virt/hyperv/constants.py b/nova/virt/hyperv/constants.py index f5c40196357b..a5c569724db3 100644 --- a/nova/virt/hyperv/constants.py +++ b/nova/virt/hyperv/constants.py @@ -56,6 +56,8 @@ DISK_FORMAT_MAP = { DVD_FORMAT.lower(): DVD } +BDI_DEVICE_TYPE_TO_DRIVE_TYPE = {'disk': DISK} + DISK_FORMAT_VHD = "VHD" DISK_FORMAT_VHDX = "VHDX" diff --git a/nova/virt/hyperv/driver.py b/nova/virt/hyperv/driver.py index 2de94357c805..3c7bbb341a89 100644 --- a/nova/virt/hyperv/driver.py +++ b/nova/virt/hyperv/driver.py @@ -128,6 +128,10 @@ class HyperVDriver(driver.ComputeDriver): 'Windows has been removed in Mitaka.')) raise exception.HypervisorTooOld(version='6.2') + @property + def need_legacy_block_device_info(self): + return False + def init_host(self, host): self._serialconsoleops.start_console_handlers() event_handler = eventhandler.InstanceEventHandler( diff --git a/nova/virt/hyperv/livemigrationops.py b/nova/virt/hyperv/livemigrationops.py index 865afae6c056..5d652e0a080a 100644 --- a/nova/virt/hyperv/livemigrationops.py +++ b/nova/virt/hyperv/livemigrationops.py @@ -23,6 +23,7 @@ from oslo_utils import excutils import nova.conf from nova.objects import migrate_data as migrate_data_obj +from nova.virt.hyperv import block_device_manager from nova.virt.hyperv import imagecache from nova.virt.hyperv import pathutils from nova.virt.hyperv import serialconsoleops @@ -42,6 +43,7 @@ class LiveMigrationOps(object): self._serial_console_ops = serialconsoleops.SerialConsoleOps() self._imagecache = imagecache.ImageCache() self._vmutils = utilsfactory.get_vmutils() + self._block_dev_man = block_device_manager.BlockDeviceInfoManager() def live_migration(self, context, instance_ref, dest, post_method, recover_method, block_migration=False, @@ -75,7 +77,7 @@ class LiveMigrationOps(object): self._livemigrutils.check_live_migration_config() if CONF.use_cow_images: - boot_from_volume = self._volumeops.ebs_root_in_block_devices( + boot_from_volume = self._block_dev_man.is_boot_from_volume( block_device_info) if not boot_from_volume and instance.image_ref: self._imagecache.get_cached_image(context, instance) diff --git a/nova/virt/hyperv/migrationops.py b/nova/virt/hyperv/migrationops.py index c1853301fbcb..d3a7b94a0cf0 100644 --- a/nova/virt/hyperv/migrationops.py +++ b/nova/virt/hyperv/migrationops.py @@ -24,9 +24,11 @@ from oslo_utils import excutils from oslo_utils import units from nova import exception -from nova.i18n import _, _LE +from nova.i18n import _, _LW, _LE from nova import objects from nova.virt import configdrive +from nova.virt.hyperv import block_device_manager +from nova.virt.hyperv import constants from nova.virt.hyperv import imagecache from nova.virt.hyperv import pathutils from nova.virt.hyperv import vmops @@ -44,6 +46,7 @@ class MigrationOps(object): self._volumeops = volumeops.VolumeOps() self._vmops = vmops.VMOps() self._imagecache = imagecache.ImageCache() + self._block_dev_man = block_device_manager.BlockDeviceInfoManager() def _migrate_disk_files(self, instance_name, disk_files, dest): # TODO(mikal): it would be nice if this method took a full instance, @@ -167,18 +170,25 @@ class MigrationOps(object): instance_name = instance.name self._revert_migration_files(instance_name) - if self._volumeops.ebs_root_in_block_devices(block_device_info): - root_vhd_path = None - else: - root_vhd_path = self._pathutils.lookup_root_vhd_path(instance_name) - - eph_vhd_path = self._pathutils.lookup_ephemeral_vhd_path(instance_name) - image_meta = objects.ImageMeta.from_instance(instance) - vm_gen = self._vmops.get_image_vm_generation( - instance.uuid, root_vhd_path, image_meta) - self._vmops.create_instance(instance, network_info, block_device_info, - root_vhd_path, eph_vhd_path, vm_gen) + vm_gen = self._vmops.get_image_vm_generation(instance.uuid, image_meta) + + self._block_dev_man.validate_and_update_bdi(instance, image_meta, + vm_gen, block_device_info) + root_device = block_device_info['root_disk'] + + if root_device['type'] == constants.DISK: + root_vhd_path = self._pathutils.lookup_root_vhd_path(instance_name) + root_device['path'] = root_vhd_path + if not root_vhd_path: + base_vhd_path = self._pathutils.get_instance_dir(instance_name) + raise exception.DiskNotFound(location=base_vhd_path) + + ephemerals = block_device_info['ephemerals'] + self._check_ephemeral_disks(instance, ephemerals) + + self._vmops.create_instance(instance, network_info, root_device, + block_device_info, vm_gen) self._check_and_attach_config_drive(instance, vm_gen) @@ -221,8 +231,8 @@ class MigrationOps(object): reason=_("Cannot resize the root disk to a smaller size. " "Current size: %(curr_root_gb)s GB. Requested " "size: %(new_root_gb)s GB.") % { - 'curr_root_gb': curr_size, - 'new_root_gb': new_size}) + 'curr_root_gb': curr_size / units.Gi, + 'new_root_gb': new_size / units.Gi}) elif new_size > curr_size: self._resize_vhd(vhd_path, new_size) @@ -260,13 +270,18 @@ class MigrationOps(object): LOG.debug("finish_migration called", instance=instance) instance_name = instance.name + vm_gen = self._vmops.get_image_vm_generation(instance.uuid, image_meta) - if self._volumeops.ebs_root_in_block_devices(block_device_info): - root_vhd_path = None - else: + self._block_dev_man.validate_and_update_bdi(instance, image_meta, + vm_gen, block_device_info) + root_device = block_device_info['root_disk'] + + if root_device['type'] == constants.DISK: root_vhd_path = self._pathutils.lookup_root_vhd_path(instance_name) + root_device['path'] = root_vhd_path if not root_vhd_path: - raise exception.DiskNotFound(location=root_vhd_path) + base_vhd_path = self._pathutils.get_instance_dir(instance_name) + raise exception.DiskNotFound(location=base_vhd_path) root_vhd_info = self._vhdutils.get_vhd_info(root_vhd_path) src_base_disk_path = root_vhd_info.get("ParentPath") @@ -278,22 +293,57 @@ class MigrationOps(object): new_size = instance.root_gb * units.Gi self._check_resize_vhd(root_vhd_path, root_vhd_info, new_size) - eph_vhd_path = self._pathutils.lookup_ephemeral_vhd_path(instance_name) - if resize_instance: - new_size = instance.get('ephemeral_gb', 0) * units.Gi - if not eph_vhd_path: - if new_size: - eph_vhd_path = self._vmops.create_ephemeral_vhd(instance) - else: - eph_vhd_info = self._vhdutils.get_vhd_info(eph_vhd_path) - self._check_resize_vhd(eph_vhd_path, eph_vhd_info, new_size) + ephemerals = block_device_info['ephemerals'] + self._check_ephemeral_disks(instance, ephemerals, resize_instance) - vm_gen = self._vmops.get_image_vm_generation( - instance.uuid, root_vhd_path, image_meta) - self._vmops.create_instance(instance, network_info, block_device_info, - root_vhd_path, eph_vhd_path, vm_gen) + self._vmops.create_instance(instance, network_info, root_device, + block_device_info, vm_gen) self._check_and_attach_config_drive(instance, vm_gen) if power_on: self._vmops.power_on(instance) + + def _check_ephemeral_disks(self, instance, ephemerals, + resize_instance=False): + instance_name = instance.name + new_eph_gb = instance.get('ephemeral_gb', 0) + + if len(ephemerals) == 1: + # NOTE(claudiub): Resize only if there is one ephemeral. If there + # are more than 1, resizing them can be problematic. This behaviour + # also exists in the libvirt driver and it has to be addressed in + # the future. + ephemerals[0]['size'] = new_eph_gb + elif sum(eph['size'] for eph in ephemerals) != new_eph_gb: + # New ephemeral size is different from the original ephemeral size + # and there are multiple ephemerals. + LOG.warn(_LW("Cannot resize multiple ephemeral disks for " + "instance."), instance=instance) + + for index, eph in enumerate(ephemerals): + eph_name = "eph%s" % index + existing_eph_path = self._pathutils.lookup_ephemeral_vhd_path( + instance_name, eph_name) + + if not existing_eph_path: + eph['format'] = self._vhdutils.get_best_supported_vhd_format() + eph['path'] = self._pathutils.get_ephemeral_vhd_path( + instance_name, eph['format'], eph_name) + if not resize_instance: + # ephemerals should have existed. + raise exception.DiskNotFound(location=eph['path']) + + if eph['size']: + # create ephemerals + self._vmops.create_ephemeral_disk(instance.name, eph) + elif eph['size'] > 0: + # ephemerals exist. resize them. + eph['path'] = existing_eph_path + eph_vhd_info = self._vhdutils.get_vhd_info(eph['path']) + self._check_resize_vhd( + eph['path'], eph_vhd_info, eph['size'] * units.Gi) + else: + # ephemeral new size is 0, remove it. + self._pathutils.remove(existing_eph_path) + eph['path'] = None diff --git a/nova/virt/hyperv/pathutils.py b/nova/virt/hyperv/pathutils.py index dfde741a55d5..63dabb695d05 100644 --- a/nova/virt/hyperv/pathutils.py +++ b/nova/virt/hyperv/pathutils.py @@ -106,9 +106,10 @@ class PathUtils(pathutils.PathUtils): break return configdrive_path - def lookup_ephemeral_vhd_path(self, instance_name): + def lookup_ephemeral_vhd_path(self, instance_name, eph_name): return self._lookup_vhd_path(instance_name, - self.get_ephemeral_vhd_path) + self.get_ephemeral_vhd_path, + eph_name) def get_root_vhd_path(self, instance_name, format_ext, rescue=False): instance_path = self.get_instance_dir(instance_name) @@ -127,9 +128,9 @@ class PathUtils(pathutils.PathUtils): return os.path.join(instance_path, configdrive_image_name + '.' + format_ext.lower()) - def get_ephemeral_vhd_path(self, instance_name, format_ext): + def get_ephemeral_vhd_path(self, instance_name, format_ext, eph_name): instance_path = self.get_instance_dir(instance_name) - return os.path.join(instance_path, 'ephemeral.' + format_ext.lower()) + return os.path.join(instance_path, eph_name + '.' + format_ext.lower()) def get_base_vhd_dir(self): return self._get_instances_sub_dir('_base') diff --git a/nova/virt/hyperv/vmops.py b/nova/virt/hyperv/vmops.py index 42144df669e3..f68d758d26ec 100644 --- a/nova/virt/hyperv/vmops.py +++ b/nova/virt/hyperv/vmops.py @@ -42,6 +42,7 @@ from nova.i18n import _, _LI, _LE, _LW from nova import utils from nova.virt import configdrive from nova.virt import hardware +from nova.virt.hyperv import block_device_manager from nova.virt.hyperv import constants from nova.virt.hyperv import imagecache from nova.virt.hyperv import pathutils @@ -107,6 +108,8 @@ class VMOps(object): self._volumeops = volumeops.VolumeOps() self._imagecache = imagecache.ImageCache() self._serial_console_ops = serialconsoleops.SerialConsoleOps() + self._block_dev_man = ( + block_device_manager.BlockDeviceInfoManager()) self._vif_driver = None self._load_vif_driver_class() @@ -157,6 +160,13 @@ class VMOps(object): num_cpu=info['NumberOfProcessors'], cpu_time_ns=info['UpTime']) + def _create_root_device(self, context, instance, root_disk_info, vm_gen): + path = None + if root_disk_info['type'] == constants.DISK: + path = self._create_root_vhd(context, instance) + self.check_vm_image_type(instance.uuid, vm_gen, path) + root_disk_info['path'] = path + def _create_root_vhd(self, context, instance, rescue_image_id=None): is_rescue_vhd = rescue_image_id is not None @@ -223,15 +233,17 @@ class VMOps(object): return True return False - def create_ephemeral_vhd(self, instance): - eph_vhd_size = instance.get('ephemeral_gb', 0) * units.Gi - if eph_vhd_size: - vhd_format = self._vhdutils.get_best_supported_vhd_format() + def _create_ephemerals(self, instance, ephemerals): + for index, eph in enumerate(ephemerals): + eph['format'] = self._vhdutils.get_best_supported_vhd_format() + eph_name = "eph%s" % index + eph['path'] = self._pathutils.get_ephemeral_vhd_path( + instance.name, eph['format'], eph_name) + self.create_ephemeral_disk(instance.name, eph) - eph_vhd_path = self._pathutils.get_ephemeral_vhd_path( - instance.name, vhd_format) - self._vhdutils.create_dynamic_vhd(eph_vhd_path, eph_vhd_size) - return eph_vhd_path + def create_ephemeral_disk(self, instance_name, eph_info): + self._vhdutils.create_dynamic_vhd(eph_info['path'], + eph_info['size'] * units.Gi) @check_admin_permissions def spawn(self, context, instance, image_meta, injected_files, @@ -246,19 +258,17 @@ class VMOps(object): # Make sure we're starting with a clean slate. self._delete_disk_files(instance_name) - if self._volumeops.ebs_root_in_block_devices(block_device_info): - root_vhd_path = None - else: - root_vhd_path = self._create_root_vhd(context, instance) + vm_gen = self.get_image_vm_generation(instance.uuid, image_meta) - eph_vhd_path = self.create_ephemeral_vhd(instance) - vm_gen = self.get_image_vm_generation( - instance.uuid, root_vhd_path, image_meta) + self._block_dev_man.validate_and_update_bdi( + instance, image_meta, vm_gen, block_device_info) + root_device = block_device_info['root_disk'] + self._create_root_device(context, instance, root_device, vm_gen) + self._create_ephemerals(instance, block_device_info['ephemerals']) try: - self.create_instance(instance, network_info, block_device_info, - root_vhd_path, eph_vhd_path, - vm_gen) + self.create_instance(instance, network_info, root_device, + block_device_info, vm_gen) if configdrive.required_by(instance): configdrive_path = self._create_config_drive(instance, @@ -273,8 +283,8 @@ class VMOps(object): with excutils.save_and_reraise_exception(): self.destroy(instance) - def create_instance(self, instance, network_info, block_device_info, - root_vhd_path, eph_vhd_path, vm_gen): + def create_instance(self, instance, network_info, root_device, + block_device_info, vm_gen): instance_name = instance.name instance_path = os.path.join(CONF.instances_path, instance_name) @@ -290,24 +300,10 @@ class VMOps(object): self._configure_remotefx(instance, vm_gen) self._vmutils.create_scsi_controller(instance_name) - controller_type = VM_GENERATIONS_CONTROLLER_TYPES[vm_gen] - - ctrl_disk_addr = 0 - if root_vhd_path: - self._attach_drive(instance_name, root_vhd_path, 0, ctrl_disk_addr, - controller_type) - - ctrl_disk_addr = 1 - if eph_vhd_path: - self._attach_drive(instance_name, eph_vhd_path, 0, ctrl_disk_addr, - controller_type) - - # If ebs_root is False, the first volume will be attached to SCSI - # controller. Generation 2 VMs only has a SCSI controller. - ebs_root = vm_gen is not constants.VM_GEN_2 and root_vhd_path is None - self._volumeops.attach_volumes(block_device_info, - instance_name, - ebs_root) + self._attach_root_device(instance_name, root_device) + self._attach_ephemerals(instance_name, block_device_info['ephemerals']) + self._volumeops.attach_volumes( + block_device_info['block_device_mapping'], instance_name) # For the moment, we use COM port 1 when getting the serial console # log as well as interactive sessions. In the future, the way in which @@ -368,6 +364,29 @@ class VMOps(object): remotefx_max_resolution, vram_bytes) + def _attach_root_device(self, instance_name, root_dev_info): + if root_dev_info['type'] == constants.VOLUME: + self._volumeops.attach_volume(root_dev_info['connection_info'], + instance_name, + disk_bus=root_dev_info['disk_bus']) + else: + self._attach_drive(instance_name, root_dev_info['path'], + root_dev_info['drive_addr'], + root_dev_info['ctrl_disk_addr'], + root_dev_info['disk_bus'], + root_dev_info['type']) + + def _attach_ephemerals(self, instance_name, ephemerals): + for eph in ephemerals: + # if an ephemeral doesn't have a path, it might have been removed + # during resize. + if eph.get('path'): + self._attach_drive( + instance_name, eph['path'], eph['drive_addr'], + eph['ctrl_disk_addr'], eph['disk_bus'], + constants.BDI_DEVICE_TYPE_TO_DRIVE_TYPE[ + eph['device_type']]) + def _attach_drive(self, instance_name, path, drive_addr, ctrl_disk_addr, controller_type, drive_type=constants.DISK): if controller_type == constants.CTRL_TYPE_SCSI: @@ -376,18 +395,19 @@ class VMOps(object): self._vmutils.attach_ide_drive(instance_name, path, drive_addr, ctrl_disk_addr, drive_type) - def get_image_vm_generation(self, instance_id, root_vhd_path, image_meta): + def get_image_vm_generation(self, instance_id, image_meta): default_vm_gen = self._hostutils.get_default_vm_generation() - image_prop_vm = image_meta.properties.get( - 'hw_machine_type', default_vm_gen) + image_prop_vm = image_meta.properties.get('hw_machine_type', + default_vm_gen) if image_prop_vm not in self._hostutils.get_supported_vm_types(): reason = _LE('Requested VM Generation %s is not supported on ' 'this OS.') % image_prop_vm raise exception.InstanceUnacceptable(instance_id=instance_id, reason=reason) - vm_gen = VM_GENERATIONS[image_prop_vm] + return VM_GENERATIONS[image_prop_vm] + def check_vm_image_type(self, instance_id, vm_gen, root_vhd_path): if (vm_gen != constants.VM_GEN_1 and root_vhd_path and self._vhdutils.get_vhd_format( root_vhd_path) == constants.DISK_FORMAT_VHD): @@ -396,8 +416,6 @@ class VMOps(object): raise exception.InstanceUnacceptable(instance_id=instance_id, reason=reason) - return vm_gen - def _create_config_drive(self, instance, injected_files, admin_password, network_info, rescue=False): if CONF.config_drive_format != 'iso9660': @@ -748,7 +766,6 @@ class VMOps(object): context, instance, rescue_image_id=rescue_image_id) rescue_vm_gen = self.get_image_vm_generation(instance.uuid, - rescue_vhd_path, image_meta) vm_gen = self._vmutils.get_vm_generation(instance.name) if rescue_vm_gen != vm_gen: diff --git a/nova/virt/hyperv/volumeops.py b/nova/virt/hyperv/volumeops.py index 793fae4d2142..152baad63dc0 100644 --- a/nova/virt/hyperv/volumeops.py +++ b/nova/virt/hyperv/volumeops.py @@ -28,12 +28,12 @@ from oslo_log import log as logging from oslo_utils import excutils from six.moves import range -from nova import block_device import nova.conf from nova import exception from nova.i18n import _, _LE, _LW from nova import utils from nova.virt import driver +from nova.virt.hyperv import constants LOG = logging.getLogger(__name__) @@ -59,14 +59,8 @@ class VolumeOps(object): raise exception.VolumeDriverNotFound(driver_type=driver_type) return self.volume_drivers[driver_type] - def attach_volumes(self, block_device_info, instance_name, ebs_root): - mapping = driver.block_device_info_get_mapping(block_device_info) - - if ebs_root: - self.attach_volume(mapping[0]['connection_info'], - instance_name, True) - mapping = mapping[1:] - for vol in mapping: + def attach_volumes(self, volumes, instance_name): + for vol in volumes: self.attach_volume(vol['connection_info'], instance_name) def disconnect_volumes(self, block_device_info): @@ -77,24 +71,18 @@ class VolumeOps(object): volume_driver = self._get_volume_driver(driver_type) volume_driver.disconnect_volumes(block_device_mapping) - def attach_volume(self, connection_info, instance_name, ebs_root=False): + def attach_volume(self, connection_info, instance_name, + disk_bus=constants.CTRL_TYPE_SCSI): volume_driver = self._get_volume_driver( connection_info=connection_info) - volume_driver.attach_volume(connection_info, instance_name, ebs_root) + volume_driver.attach_volume(connection_info, instance_name, + disk_bus=disk_bus) def detach_volume(self, connection_info, instance_name): volume_driver = self._get_volume_driver( connection_info=connection_info) volume_driver.detach_volume(connection_info, instance_name) - def ebs_root_in_block_devices(self, block_device_info): - if block_device_info: - root_device = block_device_info.get('root_device_name') - if not root_device: - root_device = self._default_root_device - return block_device.volume_in_mapping(root_device, - block_device_info) - def fix_instance_volume_disk_paths(self, instance_name, block_device_info): # Mapping containing the current disk paths for each volume. actual_disk_mapping = self.get_disk_path_mapping(block_device_info) @@ -230,7 +218,8 @@ class ISCSIVolumeDriver(object): return self._get_mounted_disk_from_lun(target_iqn, target_lun, wait_for_device=True) - def attach_volume(self, connection_info, instance_name, ebs_root=False): + def attach_volume(self, connection_info, instance_name, + disk_bus=constants.CTRL_TYPE_SCSI): """Attach a volume to the SCSI controller or to the IDE controller if ebs_root is True """ @@ -246,7 +235,7 @@ class ISCSIVolumeDriver(object): mounted_disk_path = self.get_mounted_disk_path_from_volume( connection_info) - if ebs_root: + if disk_bus == constants.CTRL_TYPE_IDE: # Find the IDE controller for the vm. ctrller_path = self._vmutils.get_vm_ide_controller( instance_name, 0) @@ -359,13 +348,14 @@ class SMBFSVolumeDriver(object): return self._get_disk_path(connection_info) @export_path_synchronized - def attach_volume(self, connection_info, instance_name, ebs_root=False): + def attach_volume(self, connection_info, instance_name, + disk_bus=constants.CTRL_TYPE_SCSI): self.ensure_share_mounted(connection_info) disk_path = self._get_disk_path(connection_info) try: - if ebs_root: + if disk_bus == constants.CTRL_TYPE_IDE: ctrller_path = self._vmutils.get_vm_ide_controller( instance_name, 0) slot = 0