VMware: Support volume hotplug

When a volume is attached to an instance, the disk_type and
adapter_type are found by making a get_dynamic_property vim
call to the instance. If the adapter_type returned is
"ide", the volume attach fails with "operation cannot be
performed in the current state". This is because VMware IDE
adapter does not support disk hotplug.

Instead of making a get_dynamic_property vim call to the
instance, it should be made to the volume (shadow VM). This
properly indicates what adapter_type is needed by the volume.
If the instance does not have a controller to support the
volume adapter_type (e.g. lsiLogic), one will be added.
If the volume adapter_type is "ide", a exception.Invalid will
be thrown.

Change-Id: Id826a33298885e9dbd4b52c9fe518ed2e6d50469
Closes-Bug: #1226543
This commit is contained in:
Thang Pham 2014-09-17 17:14:15 -04:00
parent 4c3b5fd1ec
commit e3fd9b377c
3 changed files with 150 additions and 55 deletions

View File

@ -1802,48 +1802,75 @@ class VMwareAPIVMTestCase(test.NoDBTestCase):
def test_attach_vmdk_disk_to_vm(self):
self._create_vm()
connection_info = self._test_vmdk_connection_info('vmdk')
mount_point = '/dev/vdc'
# create fake backing info
volume_device = vmwareapi_fake.DataObject()
volume_device.backing = vmwareapi_fake.DataObject()
volume_device.backing.fileName = 'fake_path'
adapter_type = constants.DEFAULT_ADAPTER_TYPE
disk_type = constants.DEFAULT_DISK_TYPE
path_and_type = ('fake-path', adapter_type, disk_type)
with contextlib.nested(
mock.patch.object(vm_util, 'get_vm_ref',
return_value=mock.sentinel.vm_ref),
mock.patch.object(volumeops.VMwareVolumeOps, '_get_volume_ref'),
mock.patch.object(vm_util, 'get_vmdk_path_and_adapter_type',
return_value=path_and_type),
mock.patch.object(volumeops.VMwareVolumeOps, 'attach_disk_to_vm'),
mock.patch.object(volumeops.VMwareVolumeOps,
'_update_volume_details')
) as (get_vm_ref, get_volume_ref, get_vmdk_path_and_adapter_type,
attach_disk_to_vm, update_volume_details):
self.conn.attach_volume(None, connection_info, self.instance,
'/dev/vdc')
self.mox.StubOutWithMock(volumeops.VMwareVolumeOps,
'_get_vmdk_base_volume_device')
volumeops.VMwareVolumeOps._get_vmdk_base_volume_device(
mox.IgnoreArg()).AndReturn(volume_device)
self.mox.StubOutWithMock(volumeops.VMwareVolumeOps,
'attach_disk_to_vm')
volumeops.VMwareVolumeOps.attach_disk_to_vm(mox.IgnoreArg(),
self.instance, mox.IgnoreArg(), mox.IgnoreArg(),
vmdk_path='fake_path')
self.mox.ReplayAll()
self.conn.attach_volume(None, connection_info, self.instance,
mount_point)
get_vm_ref.assert_called_once_with(self.conn._session,
self.instance)
get_volume_ref.assert_called_once_with(
connection_info['data']['volume'])
self.assertTrue(get_vmdk_path_and_adapter_type.called)
attach_disk_to_vm.assert_called_once_with(mock.sentinel.vm_ref,
self.instance, adapter_type, disk_type, vmdk_path='fake-path')
update_volume_details.assert_called_once_with(
mock.sentinel.vm_ref, self.instance,
connection_info['data']['volume_id'])
def test_detach_vmdk_disk_from_vm(self):
self._create_vm()
connection_info = self._test_vmdk_connection_info('vmdk')
mount_point = '/dev/vdc'
self.mox.StubOutWithMock(volumeops.VMwareVolumeOps,
'_get_volume_uuid')
volumeops.VMwareVolumeOps._get_volume_uuid(mox.IgnoreArg(),
'volume-fake-id').AndReturn('fake_disk_uuid')
self.mox.StubOutWithMock(vm_util, 'get_vmdk_backed_disk_device')
vm_util.get_vmdk_backed_disk_device(mox.IgnoreArg(),
'fake_disk_uuid').AndReturn('fake_device')
self.mox.StubOutWithMock(volumeops.VMwareVolumeOps,
'_consolidate_vmdk_volume')
volumeops.VMwareVolumeOps._consolidate_vmdk_volume(self.instance,
mox.IgnoreArg(), 'fake_device', mox.IgnoreArg())
self.mox.StubOutWithMock(volumeops.VMwareVolumeOps,
'detach_disk_from_vm')
volumeops.VMwareVolumeOps.detach_disk_from_vm(mox.IgnoreArg(),
self.instance, mox.IgnoreArg())
self.mox.ReplayAll()
self.conn.detach_volume(connection_info, self.instance, mount_point,
encryption=None)
adapter_type = constants.DEFAULT_ADAPTER_TYPE
disk_type = constants.DEFAULT_DISK_TYPE
path_and_type = ('fake-path', adapter_type, disk_type)
with contextlib.nested(
mock.patch.object(vm_util, 'get_vm_ref',
return_value=mock.sentinel.vm_ref),
mock.patch.object(volumeops.VMwareVolumeOps, '_get_volume_ref',
return_value=mock.sentinel.volume_ref),
mock.patch.object(volumeops.VMwareVolumeOps,
'_get_vmdk_backed_disk_device',
return_value=mock.sentinel.disk_device),
mock.patch.object(vm_util, 'get_vmdk_path_and_adapter_type',
return_value=path_and_type),
mock.patch.object(volumeops.VMwareVolumeOps,
'_consolidate_vmdk_volume'),
mock.patch.object(volumeops.VMwareVolumeOps,
'detach_disk_from_vm')
) as (get_vm_ref, get_volume_ref, get_vmdk_backed_disk_device,
get_vmdk_path_and_adapter_type, consolidate_vmdk_volume,
detach_disk_from_vm):
self.conn.detach_volume(connection_info, self.instance,
'/dev/vdc', encryption=None)
get_vm_ref.assert_called_once_with(self.conn._session,
self.instance)
get_volume_ref.assert_called_once_with(
connection_info['data']['volume'])
get_vmdk_backed_disk_device.assert_called_once_with(
mock.sentinel.vm_ref, connection_info['data'])
self.assertTrue(get_vmdk_path_and_adapter_type.called)
consolidate_vmdk_volume.assert_called_once_with(self.instance,
mock.sentinel.vm_ref, mock.sentinel.disk_device,
mock.sentinel.volume_ref, adapter_type=adapter_type,
disk_type=disk_type)
detach_disk_from_vm.assert_called_once_with(mock.sentinel.vm_ref,
self.instance, mock.sentinel.disk_device)
def test_volume_attach_iscsi(self):
self._create_vm()

View File

@ -16,10 +16,13 @@ import contextlib
import mock
from nova.compute import vm_states
from nova import exception
from nova import test
from nova.tests.unit.virt.vmwareapi import fake as vmwareapi_fake
from nova.tests.unit.virt.vmwareapi import stubs
from nova.virt.vmwareapi import driver
from nova.virt.vmwareapi import vm_util
from nova.virt.vmwareapi import volumeops
@ -93,3 +96,55 @@ class VMwareVolumeOpsTestCase(test.NoDBTestCase):
with mock.patch.object(self._session, "_call_method", fake_call):
val = self._volumeops._get_volume_uuid(vm_ref, uuid)
self.assertIsNone(val)
def test_attach_volume_vmdk_invalid(self):
connection_info = {'driver_volume_type': 'vmdk',
'serial': 'volume-fake-id',
'data': {'volume': 'vm-10',
'volume_id': 'volume-fake-id'}}
instance = mock.MagicMock(name='fake-name', vm_state=vm_states.ACTIVE)
path_and_type = ('fake-path', 'ide', 'preallocated')
with contextlib.nested(
mock.patch.object(vm_util, 'get_vm_ref'),
mock.patch.object(self._volumeops, '_get_volume_ref'),
mock.patch.object(vm_util, 'get_vmdk_path_and_adapter_type',
return_value=path_and_type)
) as (get_vm_ref, get_volume_ref, get_vmdk_path_and_adapter_type):
self.assertRaises(exception.Invalid,
self._volumeops._attach_volume_vmdk, connection_info,
instance)
get_vm_ref.assert_called_once_with(self._volumeops._session,
instance)
get_volume_ref.assert_called_once_with(
connection_info['data']['volume'])
self.assertTrue(get_vmdk_path_and_adapter_type.called)
def test_detach_volume_vmdk_invalid(self):
connection_info = {'driver_volume_type': 'vmdk',
'serial': 'volume-fake-id',
'data': {'volume': 'vm-10',
'volume_id': 'volume-fake-id'}}
instance = mock.MagicMock(name='fake-name', vm_state=vm_states.ACTIVE)
path_and_type = ('fake-path', 'ide', 'preallocated')
with contextlib.nested(
mock.patch.object(vm_util, 'get_vm_ref',
return_value=mock.sentinel.vm_ref),
mock.patch.object(self._volumeops, '_get_volume_ref'),
mock.patch.object(self._volumeops,
'_get_vmdk_backed_disk_device'),
mock.patch.object(vm_util, 'get_vmdk_path_and_adapter_type',
return_value=path_and_type)
) as (get_vm_ref, get_volume_ref, get_vmdk_backed_disk_device,
get_vmdk_path_and_adapter_type):
self.assertRaises(exception.Invalid,
self._volumeops._detach_volume_vmdk, connection_info,
instance)
get_vm_ref.assert_called_once_with(self._volumeops._session,
instance)
get_volume_ref.assert_called_once_with(
connection_info['data']['volume'])
get_vmdk_backed_disk_device.assert_called_once_with(
mock.sentinel.vm_ref, connection_info['data'])
self.assertTrue(get_vmdk_path_and_adapter_type.called)

View File

@ -20,9 +20,11 @@ Management class for Storage-related functions (attach, detach, etc).
from oslo.config import cfg
from oslo.vmware import vim_util as vutil
from nova.compute import vm_states
from nova import exception
from nova.i18n import _, _LI
from nova.openstack.common import log as logging
from nova.virt.vmwareapi import constants
from nova.virt.vmwareapi import vim_util
from nova.virt.vmwareapi import vm_util
@ -315,21 +317,23 @@ class VMwareVolumeOps(object):
LOG.debug("_attach_volume_vmdk: %s", connection_info,
instance=instance)
data = connection_info['data']
# Get volume details from volume ref
volume_ref = self._get_volume_ref(data['volume'])
volume_device = self._get_vmdk_base_volume_device(volume_ref)
volume_vmdk_path = volume_device.backing.fileName
# Get details required for adding disk device such as
# adapter_type, disk_type
hw_devices = self._session._call_method(vim_util,
'get_dynamic_property',
vm_ref, 'VirtualMachine',
volume_ref, 'VirtualMachine',
'config.hardware.device')
(vmdk_file_path, adapter_type,
(volume_vmdk_path, adapter_type,
disk_type) = vm_util.get_vmdk_path_and_adapter_type(hw_devices)
# IDE does not support disk hotplug
if (instance.vm_state == vm_states.ACTIVE and
adapter_type == constants.ADAPTER_TYPE_IDE):
msg = _('%s does not support disk hotplug.') % adapter_type
raise exception.Invalid(msg)
# Attach the disk to virtual machine instance
self.attach_disk_to_vm(vm_ref, instance, adapter_type,
disk_type, vmdk_path=volume_vmdk_path)
@ -408,7 +412,8 @@ class VMwareVolumeOps(object):
compute_res, compute_res._type,
'resourcePool')
def _consolidate_vmdk_volume(self, instance, vm_ref, device, volume_ref):
def _consolidate_vmdk_volume(self, instance, vm_ref, device, volume_ref,
adapter_type=None, disk_type=None):
"""Consolidate volume backing VMDK files if needed.
The volume's VMDK file attached to an instance can be moved by SDRS
@ -455,15 +460,7 @@ class VMwareVolumeOps(object):
# Delete the original disk from the volume_ref
self.detach_disk_from_vm(volume_ref, instance, original_device,
destroy_disk=True)
# Attach the current disk to the volume_ref
# Get details required for adding disk device such as
# adapter_type, disk_type
hw_devices = self._session._call_method(vim_util,
'get_dynamic_property',
volume_ref, 'VirtualMachine',
'config.hardware.device')
(vmdk_file_path, adapter_type,
disk_type) = vm_util.get_vmdk_path_and_adapter_type(hw_devices)
# Attach the current volume to the volume_ref
self.attach_disk_to_vm(volume_ref, instance,
adapter_type, disk_type,
@ -491,12 +488,28 @@ class VMwareVolumeOps(object):
LOG.debug("_detach_volume_vmdk: %s", connection_info,
instance=instance)
data = connection_info['data']
volume_ref = self._get_volume_ref(data['volume'])
device = self._get_vmdk_backed_disk_device(vm_ref, data)
# Get the volume ref
volume_ref = self._get_volume_ref(data['volume'])
self._consolidate_vmdk_volume(instance, vm_ref, device, volume_ref)
# Get details required for adding disk device such as
# adapter_type, disk_type
hw_devices = self._session._call_method(vim_util,
'get_dynamic_property',
volume_ref, 'VirtualMachine',
'config.hardware.device')
(vmdk_file_path, adapter_type,
disk_type) = vm_util.get_vmdk_path_and_adapter_type(hw_devices)
# IDE does not support disk hotplug
if (instance.vm_state == vm_states.ACTIVE and
adapter_type == constants.ADAPTER_TYPE_IDE):
msg = _('%s does not support disk hotplug.') % adapter_type
raise exception.Invalid(msg)
self._consolidate_vmdk_volume(instance, vm_ref, device, volume_ref,
adapter_type=adapter_type,
disk_type=disk_type)
self.detach_disk_from_vm(vm_ref, instance, device)
LOG.debug("Detached VMDK: %s", connection_info, instance=instance)