Fix snapshot failure with VMwareVCDriver

The snapshot operation was failing because it calls
VirtualDiskManager.CopyVirtualDisk with a destination disk spec, which
is not supported when called through VC. The fix is to not supply a spec
when calling through VC, in which case the disk is consolidated and
copied without type transformation.

While the fix is to use spec-less CopyVirtualDisk in VC, doing so in ESX
too will result in unintended transformation (because it ESX will
default to busLogic/preallocated), so the use of the spec was retained
in ESX-mode instead.

Another issue found and fixed is that the name of the snapshot image is
incorrectly set when uploading to glance.

The following tempest tests are now unbroken against the VC and ESX
nodes:
  tempest.api.compute.images.
     test_images_oneserver.ImagesOneServerTest{JSON,XML}.
        test_create_delete_image
        test_create_second_image_when_first_image_is_being_saved
        test_delete_image_of_another_tenant
     test_list_image_filters
  tempest.api.compute.test_authorization.AuthorizationTestJSON
  tempest.api.compute.test_authorization.AuthorizationTestXML

Fixes bug 1184807

Change-Id: I7ec49859fb2842cc02cdc87a6434aa58612a2964
This commit is contained in:
Vui Lam 2013-08-05 00:37:47 -07:00
parent 4d3689baef
commit 61bfac8881
4 changed files with 54 additions and 11 deletions

View File

@ -309,7 +309,7 @@ class VMwareAPIVMTestCase(test.TestCase):
network_info=self.network_info,
block_device_info=None)
def test_snapshot(self):
def _test_snapshot(self):
expected_calls = [
{'args': (),
'kwargs':
@ -328,6 +328,9 @@ class VMwareAPIVMTestCase(test.TestCase):
self._check_vm_info(info, power_state.RUNNING)
self.assertIsNone(func_call_matcher.match())
def test_snapshot(self):
self._test_snapshot()
def test_snapshot_non_existent(self):
self._create_instance_in_the_db()
self.assertRaises(exception.InstanceNotFound, self.conn.snapshot,
@ -885,6 +888,7 @@ class VMwareAPIVCDriverTestCase(VMwareAPIVMTestCase):
def setUp(self):
super(VMwareAPIVCDriverTestCase, self).setUp()
cluster_name = 'test_cluster'
cluster_name2 = 'test_cluster2'
self.flags(cluster_name=[cluster_name, cluster_name2],
@ -916,6 +920,7 @@ class VMwareAPIVCDriverTestCase(VMwareAPIVMTestCase):
'[["i686", "vmware", "hvm"], ["x86_64", "vmware", "hvm"]]')
def test_invalid_datastore_regex(self):
# Tests if we raise an exception for Invalid Regular Expression in
# vmware_datastore_regex
self.flags(cluster_name=['test_cluster'], datastore_regex='fake-ds(01',
@ -951,7 +956,6 @@ class VMwareAPIVCDriverTestCase(VMwareAPIVMTestCase):
self.assertEquals(False, self.power_on_called)
def test_get_vnc_console(self):
self._create_instance_in_the_db()
self._create_vm()
fake_vm = vmwareapi_fake._get_objects("VirtualMachine").objects[0]
fake_vm_id = int(fake_vm.obj.value.replace('vm-', ''))
@ -959,3 +963,15 @@ class VMwareAPIVCDriverTestCase(VMwareAPIVMTestCase):
self.assertEquals(vnc_dict['host'], "ha-host")
self.assertEquals(vnc_dict['port'], cfg.CONF.vmware.vnc_port +
fake_vm_id % cfg.CONF.vmware.vnc_port_total)
def test_snapshot(self):
# Ensure VMwareVCVMOps's _get_copy_virtual_disk_spec is getting called
self.mox.StubOutWithMock(vmops.VMwareVCVMOps,
'_get_copy_virtual_disk_spec')
self.conn._vmops._get_copy_virtual_disk_spec(
mox.IgnoreArg(),
mox.IgnoreArg(),
mox.IgnoreArg()).AndReturn(None)
self.mox.ReplayAll()
self._test_snapshot()

View File

@ -497,7 +497,7 @@ class VMwareVCDriver(VMwareESXDriver):
_volumeops = volumeops.VMwareVolumeOps(self._session,
self.dict_mors[node]['cluster_mor'],
vc_support=True)
_vmops = vmops.VMwareVMOps(self._session, self._virtapi,
_vmops = vmops.VMwareVCVMOps(self._session, self._virtapi,
_volumeops,
self.dict_mors[node]['cluster_mor'])
name = self.dict_mors.get(node)['name']

View File

@ -580,7 +580,13 @@ class VMwareVMOps(object):
return utils.get_boolean(value)
def snapshot(self, context, instance, snapshot_name, update_task_state):
def _get_copy_virtual_disk_spec(self, client_factory, adapter_type,
disk_type):
return vm_util.get_copy_virtual_disk_spec(client_factory,
adapter_type,
disk_type)
def snapshot(self, context, instance, image_id, update_task_state):
"""Create snapshot from a running VM instance.
Steps followed are:
@ -672,9 +678,9 @@ class VMwareVMOps(object):
def _copy_vmdk_content():
# Copy the contents of the disk ( or disks, if there were snapshots
# done earlier) to a temporary vmdk file.
copy_spec = vm_util.get_copy_virtual_disk_spec(client_factory,
adapter_type,
disk_type)
copy_spec = self._get_copy_virtual_disk_spec(client_factory,
adapter_type,
disk_type)
LOG.debug(_('Copying disk data before snapshot of the VM'),
instance=instance)
copy_disk_task = self._session._call_method(
@ -697,11 +703,11 @@ class VMwareVMOps(object):
def _upload_vmdk_to_image_repository():
# Upload the contents of -flat.vmdk file which has the disk data.
LOG.debug(_("Uploading image %s") % snapshot_name,
LOG.debug(_("Uploading image %s") % image_id,
instance=instance)
vmware_images.upload_image(
context,
snapshot_name,
image_id,
instance,
os_type=os_type,
adapter_type=adapter_type,
@ -711,7 +717,7 @@ class VMwareVMOps(object):
datastore_name=datastore_name,
cookies=cookies,
file_path="vmware-tmp/%s-flat.vmdk" % random_name)
LOG.debug(_("Uploaded image %s") % snapshot_name,
LOG.debug(_("Uploaded image %s") % image_id,
instance=instance)
update_task_state(task_state=task_states.IMAGE_UPLOADING,
@ -1483,3 +1489,22 @@ class VMwareVMOps(object):
def unplug_vifs(self, instance, network_info):
"""Unplug VIFs from networks."""
pass
class VMwareVCVMOps(VMwareVMOps):
"""Management class for VM-related tasks.
Contains specializations to account for differences in vSphere API behavior
when invoked on Virtual Center instead of ESX host.
"""
def _get_copy_virtual_disk_spec(self, client_factory, adapter_type,
disk_type):
LOG.debug(_("Will copy while retaining adapter type "
"%(adapter_type)s and disk type %(disk_type)s") %
{"disk_type": disk_type,
"adapter_type": adapter_type})
# Passing of the destination copy spec is not supported when
# VirtualDiskManager.CopyVirtualDisk is called on VC. The behavior of a
# spec-less copy is to consolidate to the target disk while keeping its
# disk and adapter type unchanged.

View File

@ -149,10 +149,12 @@ def upload_image(context, image, instance, **kwargs):
kwargs.get("file_path"))
file_size = read_file_handle.get_size()
(image_service, image_id) = glance.get_remote_image_service(context, image)
metadata = image_service.show(context, image_id)
# The properties and other fields that we need to set for the image.
image_metadata = {"disk_format": "vmdk",
"is_public": "false",
"name": kwargs.get("snapshot_name"),
"name": metadata['name'],
"status": "active",
"container_format": "bare",
"size": file_size,