diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py index a5b88bd0debd..f858fed25fc1 100644 --- a/nova/tests/test_xenapi.py +++ b/nova/tests/test_xenapi.py @@ -288,6 +288,15 @@ class XenAPIVMTestCase(stubs.XenAPITestBase): self.stubs.Set(vmops.VMOps, 'inject_instance_metadata', fake_inject_instance_metadata) + def fake_safe_copy_vdi(session, sr_ref, instance, vdi_to_copy_ref): + name_label = "fakenamelabel" + disk_type = "fakedisktype" + virtual_size = 777 + return vm_utils.create_vdi( + session, sr_ref, instance, name_label, disk_type, + virtual_size) + self.stubs.Set(vm_utils, '_safe_copy_vdi', fake_safe_copy_vdi) + def tearDown(self): super(XenAPIVMTestCase, self).tearDown() fake_image.FakeImageService_reset() diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index 47c5a1e85d6e..6635558f95ac 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -428,11 +428,63 @@ def get_vdis_for_instance(context, session, instance, name_label, image, image_type) -def _copy_vdi(session, sr_ref, vdi_to_copy_ref): - """Copy a VDI and return the new VDIs reference.""" - vdi_ref = session.call_xenapi('VDI.copy', vdi_to_copy_ref, sr_ref) - LOG.debug(_('Copied VDI %(vdi_ref)s from VDI ' - '%(vdi_to_copy_ref)s on %(sr_ref)s.') % locals()) +@contextlib.contextmanager +def _dummy_vm(session, instance, vdi_ref): + """This creates a temporary VM so that we can snapshot a VDI. + + VDI's can't be snapshotted directly since the API expects a `vm_ref`. To + work around this, we need to create a temporary VM and then map the VDI to + the VM using a temporary VBD. + """ + name_label = "dummy" + vm_ref = create_vm(session, instance, name_label, None, None) + try: + vbd_ref = create_vbd(session, vm_ref, vdi_ref, 'autodetect', + read_only=True) + try: + yield vm_ref + finally: + try: + destroy_vbd(session, vbd_ref) + except volume_utils.StorageError: + # destroy_vbd() will log error + pass + finally: + destroy_vm(session, instance, vm_ref) + + +def _safe_copy_vdi(session, sr_ref, instance, vdi_to_copy_ref): + """Copy a VDI and return the new VDIs reference. + + This function differs from the XenAPI `VDI.copy` call in that the copy is + atomic and isolated, meaning we don't see half-downloaded images. It + accomplishes this by copying the VDI's into a temporary directory and then + atomically renaming them into the SR when the copy is completed. + + The correct long term solution is to fix `VDI.copy` so that it is atomic + and isolated. + """ + with _dummy_vm(session, instance, vdi_to_copy_ref) as vm_ref: + label = "snapshot" + + with snapshot_attached_here( + session, instance, vm_ref, label) as vdi_uuids: + params = {'sr_path': get_sr_path(session), + 'vdi_uuids': vdi_uuids, + 'uuid_stack': _make_uuid_stack()} + + kwargs = {'params': pickle.dumps(params)} + result = session.call_plugin( + 'workarounds', 'safe_copy_vdis', kwargs) + imported_vhds = jsonutils.loads(result) + + root_uuid = imported_vhds['root']['uuid'] + + # TODO(sirp): for safety, we should probably re-scan the SR after every + # call to a dom0 plugin, since there is a possibility that the underlying + # VHDs changed + scan_default_sr(session) + vdi_ref = session.call_xenapi('VDI.get_by_uuid', root_uuid) return vdi_ref @@ -526,22 +578,7 @@ def find_cached_image(session, image_id, sr_ref): """Returns the vdi-ref of the cached image.""" for vdi_ref, vdi_rec in _get_all_vdis_in_sr(session, sr_ref): other_config = vdi_rec['other_config'] - - try: - image_id_match = other_config['image-id'] == image_id - except KeyError: - image_id_match = False - - # NOTE(sirp): `VDI.copy` stores the partially-completed file in the SR. - # In order to avoid these half-baked files, we compare its current size - # to the expected size pulled from the original cache file. - try: - size_match = (other_config['expected_physical_utilisation'] == - vdi_rec['physical_utilisation']) - except KeyError: - size_match = False - - if image_id_match and size_match: + if image_id == other_config.get('image-id'): return vdi_ref return None @@ -764,24 +801,16 @@ def _create_cached_image(context, session, instance, name_label, session.call_xenapi('VDI.add_to_other_config', root_vdi_ref, 'image-id', str(image_id)) - for vdi_type, vdi in vdis.iteritems(): - vdi_ref = session.call_xenapi('VDI.get_by_uuid', - vdi['uuid']) - - vdi_rec = session.call_xenapi('VDI.get_record', vdi_ref) - session.call_xenapi('VDI.add_to_other_config', - vdi_ref, 'expected_physical_utilisation', - vdi_rec['physical_utilisation']) - - if vdi_type == 'swap': - session.call_xenapi('VDI.add_to_other_config', - root_vdi_ref, 'swap-disk', - str(vdi['uuid'])) + swap_vdi = vdis.get('swap') + if swap_vdi: + session.call_xenapi( + 'VDI.add_to_other_config', root_vdi_ref, 'swap-disk', + str(swap_vdi['uuid'])) if FLAGS.use_cow_images and sr_type == 'ext': new_vdi_ref = _clone_vdi(session, root_vdi_ref) else: - new_vdi_ref = _copy_vdi(session, sr_ref, root_vdi_ref) + new_vdi_ref = _safe_copy_vdi(session, sr_ref, instance, root_vdi_ref) # Set the name label for the image we just created and remove image id # field from other-config. @@ -799,7 +828,8 @@ def _create_cached_image(context, session, instance, name_label, swap_disk_uuid = vdi_rec['other_config']['swap-disk'] swap_vdi_ref = session.call_xenapi('VDI.get_by_uuid', swap_disk_uuid) - new_swap_vdi_ref = _copy_vdi(session, sr_ref, swap_vdi_ref) + new_swap_vdi_ref = _safe_copy_vdi( + session, sr_ref, instance, swap_vdi_ref) new_swap_vdi_uuid = session.call_xenapi('VDI.get_uuid', new_swap_vdi_ref) vdis['swap'] = dict(uuid=new_swap_vdi_uuid, file=None) diff --git a/plugins/xenserver/xenapi/contrib/rpmbuild/SPECS/openstack-xen-plugins.spec b/plugins/xenserver/xenapi/contrib/rpmbuild/SPECS/openstack-xen-plugins.spec index 8f44153115c5..63b5e71d3e04 100644 --- a/plugins/xenserver/xenapi/contrib/rpmbuild/SPECS/openstack-xen-plugins.spec +++ b/plugins/xenserver/xenapi/contrib/rpmbuild/SPECS/openstack-xen-plugins.spec @@ -33,6 +33,7 @@ rm -rf $RPM_BUILD_ROOT /etc/xapi.d/plugins/kernel /etc/xapi.d/plugins/migration /etc/xapi.d/plugins/pluginlib_nova.py +/etc/xapi.d/plugins/workarounds /etc/xapi.d/plugins/xenhost /etc/xapi.d/plugins/xenstore.py /etc/xapi.d/plugins/utils.py diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/workarounds b/plugins/xenserver/xenapi/etc/xapi.d/plugins/workarounds new file mode 100755 index 000000000000..6114365392ca --- /dev/null +++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/workarounds @@ -0,0 +1,65 @@ +#!/usr/bin/env python + +# Copyright (c) 2012 Openstack, LLC +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Handle the uploading and downloading of images via Glance.""" + +import cPickle as pickle +try: + import json +except ImportError: + import simplejson as json +import os +import shutil + +import XenAPIPlugin + +import utils + +#FIXME(sirp): should this use pluginlib from 5.6? +from pluginlib_nova import * +configure_logging('hacks') + + +def _copy_vdis(sr_path, staging_path, vdi_uuids): + seq_num = 0 + for vdi_uuid in vdi_uuids: + src = os.path.join(sr_path, "%s.vhd" % vdi_uuid) + dst = os.path.join(staging_path, "%d.vhd" % seq_num) + shutil.copyfile(src, dst) + seq_num += 1 + + +def safe_copy_vdis(session, args): + params = pickle.loads(exists(args, 'params')) + sr_path = params["sr_path"] + vdi_uuids = params["vdi_uuids"] + uuid_stack = params["uuid_stack"] + + staging_path = utils.make_staging_area(sr_path) + try: + _copy_vdis(sr_path, staging_path, vdi_uuids) + imported_vhds = utils.import_vhds(sr_path, staging_path, uuid_stack) + finally: + utils.cleanup_staging_area(staging_path) + + # Right now, it's easier to return a single string via XenAPI, + # so we'll json encode the list of VHDs. + return json.dumps(imported_vhds) + + +if __name__ == '__main__': + XenAPIPlugin.dispatch({'safe_copy_vdis': safe_copy_vdis})