diff --git a/nova/tests/virt/vmwareapi/db_fakes.py b/nova/tests/virt/vmwareapi/db_fakes.py index d7f21547465d..24ce9562562f 100644 --- a/nova/tests/virt/vmwareapi/db_fakes.py +++ b/nova/tests/virt/vmwareapi/db_fakes.py @@ -82,6 +82,7 @@ def stub_out_db_instance_api(stubs): 'mac_addresses': [{'address': values['mac_address']}], 'root_gb': type_data['root_gb'], 'node': values['node'], + 'metadata': [] } return base_options diff --git a/nova/tests/virt/vmwareapi/test_configdrive.py b/nova/tests/virt/vmwareapi/test_configdrive.py old mode 100644 new mode 100755 index f89d9f81ff31..37bcd2888abf --- a/nova/tests/virt/vmwareapi/test_configdrive.py +++ b/nova/tests/virt/vmwareapi/test_configdrive.py @@ -75,7 +75,8 @@ class ConfigDriveTestCase(test.TestCase): 'reservation_id': 'r-3t8muvr0', 'id': 1, 'uuid': 'fake-uuid', - 'node': self.node_name} + 'node': self.node_name, + 'metadata': []} class FakeInstanceMetadata(object): def __init__(self, instance, content=None, extra_md=None): diff --git a/nova/tests/virt/vmwareapi/test_vmwareapi_vmops.py b/nova/tests/virt/vmwareapi/test_vmwareapi_vmops.py old mode 100644 new mode 100755 index 0b69b8218a3a..d57c939bffd7 --- a/nova/tests/virt/vmwareapi/test_vmwareapi_vmops.py +++ b/nova/tests/virt/vmwareapi/test_vmwareapi_vmops.py @@ -76,3 +76,30 @@ class VMwareVMOpsTestCase(test.TestCase): self.flags(network_api_class='nova.network.quantumv2.api.API') ops = vmops.VMwareVMOps(None, None, None) self.assertTrue(ops._is_neutron) + + def test_use_linked_clone_override_nf(self): + value = vmops.VMwareVMOps.decide_linked_clone(None, False) + self.assertFalse(value, "No overrides present but still overridden!") + + def test_use_linked_clone_override_nt(self): + value = vmops.VMwareVMOps.decide_linked_clone(None, True) + self.assertTrue(value, "No overrides present but still overridden!") + + def test_use_linked_clone_override_ny(self): + value = vmops.VMwareVMOps.decide_linked_clone(None, "yes") + self.assertTrue(value, "No overrides present but still overridden!") + + def test_use_linked_clone_override_ft(self): + value = vmops.VMwareVMOps.decide_linked_clone(False, True) + self.assertFalse(value, + "image level metadata failed to override global") + + def test_use_linked_clone_override_nt(self): + value = vmops.VMwareVMOps.decide_linked_clone("no", True) + self.assertFalse(value, + "image level metadata failed to override global") + + def test_use_linked_clone_override_yf(self): + value = vmops.VMwareVMOps.decide_linked_clone("yes", False) + self.assertTrue(value, + "image level metadata failed to override global") diff --git a/nova/utils.py b/nova/utils.py old mode 100644 new mode 100755 index f1c35792364a..1ea2c3645bc6 --- a/nova/utils.py +++ b/nova/utils.py @@ -50,6 +50,7 @@ from nova.openstack.common import lockutils from nova.openstack.common import log as logging from nova.openstack.common import processutils from nova.openstack.common.rpc import common as rpc_common +from nova.openstack.common import strutils from nova.openstack.common import timeutils notify_decorator = 'nova.notifications.notify_decorator' @@ -1285,3 +1286,10 @@ def get_image_from_system_metadata(system_meta): image_meta['properties'] = properties return image_meta + + +def get_boolean(value): + if isinstance(value, bool): + return value + else: + return strutils.bool_from_string(value) diff --git a/nova/virt/vmwareapi/driver.py b/nova/virt/vmwareapi/driver.py index 44b9b9ec20c4..8d6f1fb6ed3c 100644 --- a/nova/virt/vmwareapi/driver.py +++ b/nova/virt/vmwareapi/driver.py @@ -159,6 +159,10 @@ class VMwareESXDriver(driver.ComputeDriver): self._host = host.Host(self._session) self._host_state = None + #TODO(hartsocks): back-off into a configuration test module. + if CONF.vmware.use_linked_clone is None: + raise error_util.UseLinkedCloneConfigurationFault() + @property def host_state(self): if not self._host_state: diff --git a/nova/virt/vmwareapi/error_util.py b/nova/virt/vmwareapi/error_util.py index 17b2d649213c..7e8bda58f8e4 100644 --- a/nova/virt/vmwareapi/error_util.py +++ b/nova/virt/vmwareapi/error_util.py @@ -18,6 +18,7 @@ """ Exception classes and SOAP response error checking module. """ +from nova import exception from nova.openstack.common.gettextutils import _ @@ -96,3 +97,25 @@ class FaultCheckers(object): raise VimFaultException(fault_list, Exception(_("Error(s) %s " "occurred in the call to RetrievePropertiesEx") % exc_msg_list)) + + +class VMwareDriverException(exception.NovaException): + """Base class for all exceptions raised by the VMware Driver. + + All exceptions raised by the VMwareAPI drivers should raise + an exception descended from this class as a root. This will + allow the driver to potentially trap problems related to its + own internal configuration before halting the nova-compute + node. + """ + msg_fmt = _("VMware Driver fault.") + + +class VMwareDriverConfigurationException(VMwareDriverException): + """Base class for all configuration exceptions. + """ + msg_fmt = _("VMware Driver configuration fault.") + + +class UseLinkedCloneConfigurationFault(VMwareDriverConfigurationException): + msg_fmt = _("No default value for use_linked_clone found.") diff --git a/nova/virt/vmwareapi/vmops.py b/nova/virt/vmwareapi/vmops.py old mode 100644 new mode 100755 index 30ff756bc75c..a7c0723f2b07 --- a/nova/virt/vmwareapi/vmops.py +++ b/nova/virt/vmwareapi/vmops.py @@ -73,6 +73,7 @@ VMWARE_POWER_STATES = { 'suspended': power_state.SUSPENDED} VMWARE_PREFIX = 'vmware' +VMWARE_LINKED_CLONE = 'vmware_linked_clone' RESIZE_TOTAL_STEPS = 4 @@ -156,6 +157,10 @@ class VMwareVMOps(object): data_store_ref = ds[0] data_store_name = ds[1] + #TODO(hartsocks): this pattern is confusing, reimplement as methods + # The use of nested functions in this file makes for a confusing and + # hard to maintain file. At some future date, refactor this method to + # be a full-fledged method. This will also make unit testing easier. def _get_image_properties(): """ Get the Size of the flat vmdk file that is there on the storage @@ -173,11 +178,18 @@ class VMwareVMOps(object): "preallocated") # Get the network card type from the image properties. vif_model = image_properties.get("hw_vif_model", "VirtualE1000") + + # Fetch the image_linked_clone data here. It is retrieved + # with the above network based API call. To retrieve it + # later will necessitate additional network calls using the + # identical method. Consider this a cache. + image_linked_clone = image_properties.get(VMWARE_LINKED_CLONE) + return (vmdk_file_size_in_kb, os_type, adapter_type, disk_type, - vif_model) + vif_model, image_linked_clone) (vmdk_file_size_in_kb, os_type, adapter_type, - disk_type, vif_model) = _get_image_properties() + disk_type, vif_model, image_linked_clone) = _get_image_properties() vm_folder_ref = self._get_vmfolder_ref() node_mo_id = vm_util.get_mo_id_from_instance(instance) @@ -362,7 +374,13 @@ class VMwareVMOps(object): self._default_root_device, block_device_info) if not ebs_root: - linked_clone = CONF.vmware.use_linked_clone + # this logic allows for instances or images to decide + # for themselves which strategy is best for them. + + linked_clone = VMwareVMOps.decide_linked_clone( + image_linked_clone, + CONF.vmware.use_linked_clone + ) if linked_clone: upload_folder = self._instance_path_base upload_name = instance['image_ref'] @@ -516,6 +534,52 @@ class VMwareVMOps(object): "cdrom %(file_path)s"), {'instance_name': instance_name, 'file_path': file_path}) + @staticmethod + def decide_linked_clone(image_linked_clone, global_linked_clone): + """Explicit decision logic: whether to use linked clone on a vmdk. + + This is *override* logic not boolean logic. + + 1. let the image over-ride if set at all + 2. default to the global setting + + In math terms, I need to allow: + glance image to override global config. + + That is g vs c. "g" for glance. "c" for Config. + + So, I need g=True vs c=False to be True. + And, I need g=False vs c=True to be False. + And, I need g=None vs c=True to be True. + + Some images maybe independently best tuned for use_linked_clone=True + saving datastorage space. Alternatively a whole OpenStack install may + be tuned to performance use_linked_clone=False but a single image + in this environment may be best configured to save storage space and + set use_linked_clone=True only for itself. + + The point is: let each layer of control override the layer beneath it. + + rationale: + For technical discussion on the clone strategies and their trade-offs + see: https://www.vmware.com/support/ws5/doc/ws_clone_typeofclone.html + + :param image_linked_clone: boolean or string or None + :param global_linked_clone: boolean or string or None + :return: Boolean + """ + + value = None + + # Consider the values in order of override. + if image_linked_clone is not None: + value = image_linked_clone + else: + # this will never be not-set by this point. + value = global_linked_clone + + return utils.get_boolean(value) + def snapshot(self, context, instance, snapshot_name, update_task_state): """Create snapshot from a running VM instance.