From f936594d9e37abf4d8c22c2545a9b67ed8f25c90 Mon Sep 17 00:00:00 2001 From: Johannes Erdfelt Date: Fri, 9 Mar 2012 21:02:38 +0000 Subject: [PATCH] Reduce duplicated code in xenapi Reduce three implementations of creating VBDs to one. Use helper functions in other places. Change-Id: I34ea03fc1fb3cb5d5d343cffa28da5bf3dff888b --- nova/tests/test_xenapi.py | 8 +-- nova/virt/xenapi/vm_utils.py | 90 +++++++++++++------------------- nova/virt/xenapi/vmops.py | 40 +++++++------- nova/virt/xenapi/volume_utils.py | 24 --------- nova/virt/xenapi/volumeops.py | 14 ++--- 5 files changed, 63 insertions(+), 113 deletions(-) diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py index e7357b028162..dd8f143e94b3 100644 --- a/nova/tests/test_xenapi.py +++ b/nova/tests/test_xenapi.py @@ -1201,10 +1201,10 @@ class XenAPIAutoDiskConfigTestCase(test.TestCase): @classmethod def fake_create_vbd(cls, session, vm_ref, vdi_ref, userdevice, - bootable=True): + vbd_type='disk', read_only=False, bootable=True): pass - self.stubs.Set(volume_utils.VolumeHelper, + self.stubs.Set(vm_utils.VMHelper, "create_vbd", fake_create_vbd) @@ -1294,10 +1294,10 @@ class XenAPIGenerateLocal(test.TestCase): @classmethod def fake_create_vbd(cls, session, vm_ref, vdi_ref, userdevice, - bootable=True): + vbd_type='disk', read_only=False, bootable=True): pass - self.stubs.Set(volume_utils.VolumeHelper, + self.stubs.Set(vm_utils.VMHelper, "create_vbd", fake_create_vbd) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index da4c3ca88e25..d45b0df778f8 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -229,30 +229,6 @@ class VMHelper(xenapi.HelperBase): host)) return host_free_mem >= mem - @classmethod - def create_cd_vbd(cls, session, vm_ref, vdi_ref, userdevice, bootable): - """Create a VBD record. Returns a Deferred that gives the new - VBD reference specific to CDRom devices.""" - vbd_rec = {} - vbd_rec['VM'] = vm_ref - vbd_rec['VDI'] = vdi_ref - vbd_rec['userdevice'] = str(userdevice) - vbd_rec['bootable'] = bootable - vbd_rec['mode'] = 'RO' - vbd_rec['type'] = 'CD' - vbd_rec['unpluggable'] = True - vbd_rec['empty'] = False - vbd_rec['other_config'] = {} - vbd_rec['qos_algorithm_type'] = '' - vbd_rec['qos_algorithm_params'] = {} - vbd_rec['qos_supported_algorithms'] = [] - LOG.debug(_('Creating a CDROM-specific VBD for VM %(vm_ref)s,' - ' VDI %(vdi_ref)s ... ') % locals()) - vbd_ref = session.call_xenapi('VBD.create', vbd_rec) - LOG.debug(_('Created a CDROM-specific VBD %(vbd_ref)s ' - ' for VM %(vm_ref)s, VDI %(vdi_ref)s.') % locals()) - return vbd_ref - @classmethod def find_vbd_by_number(cls, session, vm_ref, number): """Get the VBD reference from the device number""" @@ -289,6 +265,30 @@ class VMHelper(xenapi.HelperBase): raise volume_utils.StorageError( _('Unable to destroy VBD %s') % vbd_ref) + @classmethod + def create_vbd(cls, session, vm_ref, vdi_ref, userdevice, + vbd_type='disk', read_only=False, bootable=False): + """Create a VBD record and returns its reference.""" + vbd_rec = {} + vbd_rec['VM'] = vm_ref + vbd_rec['VDI'] = vdi_ref + vbd_rec['userdevice'] = str(userdevice) + vbd_rec['bootable'] = bootable + vbd_rec['mode'] = read_only and 'RO' or 'RW' + vbd_rec['type'] = vbd_type + vbd_rec['unpluggable'] = True + vbd_rec['empty'] = False + vbd_rec['other_config'] = {} + vbd_rec['qos_algorithm_type'] = '' + vbd_rec['qos_algorithm_params'] = {} + vbd_rec['qos_supported_algorithms'] = [] + LOG.debug(_('Creating %(vbd_type)s-type VBD for VM %(vm_ref)s,' + ' VDI %(vdi_ref)s ... ') % locals()) + vbd_ref = session.call_xenapi('VBD.create', vbd_rec) + LOG.debug(_('Created VBD %(vbd_ref)s for VM %(vm_ref)s,' + ' VDI %(vdi_ref)s.') % locals()) + return vbd_ref + @classmethod def destroy_vdi(cls, session, vdi_ref): try: @@ -530,8 +530,8 @@ class VMHelper(xenapi.HelperBase): run_as_root=True) # 4. Create VBD between instance VM and swap VDI - volume_utils.VolumeHelper.create_vbd( - session, vm_ref, vdi_ref, userdevice, bootable=False) + cls.create_vbd(session, vm_ref, vdi_ref, userdevice, + bootable=False) except Exception: with utils.save_and_reraise_exception(): cls.destroy_vdi(session, vdi_ref) @@ -867,7 +867,7 @@ class VMHelper(xenapi.HelperBase): filename = session.call_plugin('glance', fn, args) # Remove the VDI as it is not needed anymore. - session.call_xenapi("VDI.destroy", vdi_ref) + cls.destroy_vdi(session, vdi_ref) LOG.debug(_("Kernel/Ramdisk VDI %s destroyed"), vdi_ref) return [dict(vdi_type=ImageType.to_string(image_type), vdi_uuid=None, @@ -1454,22 +1454,10 @@ def _wait_for_device(dev): @contextlib.contextmanager def vdi_attached_here(session, vdi_ref, read_only=False): this_vm_ref = get_this_vm_ref(session) - vbd_rec = {} - vbd_rec['VM'] = this_vm_ref - vbd_rec['VDI'] = vdi_ref - vbd_rec['userdevice'] = 'autodetect' - vbd_rec['bootable'] = False - vbd_rec['mode'] = read_only and 'RO' or 'RW' - vbd_rec['type'] = 'disk' - vbd_rec['unpluggable'] = True - vbd_rec['empty'] = False - vbd_rec['other_config'] = {} - vbd_rec['qos_algorithm_type'] = '' - vbd_rec['qos_algorithm_params'] = {} - vbd_rec['qos_supported_algorithms'] = [] - LOG.debug(_('Creating VBD for VDI %s ... '), vdi_ref) - vbd_ref = session.call_xenapi("VBD.create", vbd_rec) - LOG.debug(_('Creating VBD for VDI %s done.'), vdi_ref) + + vbd_ref = VMHelper.create_vbd(session, this_vm_ref, vdi_ref, + 'autodetect', read_only=read_only, + bootable=False) try: LOG.debug(_('Plugging VBD %s ... '), vbd_ref) session.call_xenapi("VBD.plug", vbd_ref) @@ -1491,7 +1479,11 @@ def vdi_attached_here(session, vdi_ref, read_only=False): LOG.debug(_('Destroying VBD for VDI %s ... '), vdi_ref) vbd_unplug_with_retry(session, vbd_ref) finally: - ignore_failure(session.call_xenapi, "VBD.destroy", vbd_ref) + try: + VMHelper.destroy_vbd(session, vbd_ref) + except volume_utils.StorageError: + # destroy_vbd() will log error + pass LOG.debug(_('Destroying VBD for VDI %s done.'), vdi_ref) @@ -1502,7 +1494,7 @@ def vbd_unplug_with_retry(session, vbd_ref): should be dead.""" while True: try: - session.call_xenapi("VBD.unplug", vbd_ref) + VMHelper.unplug_vbd(session, vbd_ref) LOG.debug(_('VBD.unplug successful first time.')) return except VMHelper.XenAPI.Failure, e: @@ -1521,14 +1513,6 @@ def vbd_unplug_with_retry(session, vbd_ref): return -def ignore_failure(func, *args, **kwargs): - try: - return func(*args, **kwargs) - except VMHelper.XenAPI.Failure, e: - LOG.error(_('Ignoring XenAPI.Failure %s'), e) - return None - - def get_this_vm_uuid(): with file('/sys/hypervisor/uuid') as f: return f.readline().strip() diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index aada50965466..506c0fcdb667 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -446,16 +446,16 @@ class VMOps(object): "install") cd_vdi_ref = first_vdi_ref - first_vdi_ref = VMHelper.fetch_blank_disk(session=self._session, - instance_type_id=instance.instance_type_id) + first_vdi_ref = VMHelper.fetch_blank_disk(self._session, + instance.instance_type_id) - VolumeHelper.create_vbd(session=self._session, vm_ref=vm_ref, - vdi_ref=first_vdi_ref, userdevice=userdevice, bootable=False) + VMHelper.create_vbd(self._session, vm_ref, first_vdi_ref, + userdevice, bootable=False) # device 1 reserved for rescue disk and we've used '0' userdevice = 2 - VMHelper.create_cd_vbd(session=self._session, vm_ref=vm_ref, - vdi_ref=cd_vdi_ref, userdevice=userdevice, bootable=True) + VMHelper.create_vbd(self._session, vm_ref, cd_vdi_ref, + userdevice, vbd_type='CD', bootable=True) # set user device to next free value userdevice += 1 @@ -466,13 +466,12 @@ class VMOps(object): " resize partition...") % locals()) instance_type = db.instance_type_get(ctx, instance.instance_type_id) - VMHelper.auto_configure_disk(session=self._session, - vdi_ref=first_vdi_ref, - new_gb=instance_type['root_gb']) + VMHelper.auto_configure_disk(self._session, + first_vdi_ref, + instance_type['root_gb']) - VolumeHelper.create_vbd(session=self._session, vm_ref=vm_ref, - vdi_ref=first_vdi_ref, - userdevice=userdevice, bootable=True) + VMHelper.create_vbd(self._session, vm_ref, first_vdi_ref, + userdevice, bootable=True) # set user device to next free value # userdevice 1 is reserved for rescue and we've used '0' @@ -482,16 +481,14 @@ class VMOps(object): swap_mb = instance_type['swap'] generate_swap = swap_mb and FLAGS.xenapi_generate_swap if generate_swap: - VMHelper.generate_swap(session=self._session, instance=instance, - vm_ref=vm_ref, userdevice=userdevice, - swap_mb=swap_mb) + VMHelper.generate_swap(self._session, instance, + vm_ref, userdevice, swap_mb) userdevice += 1 ephemeral_gb = instance_type['ephemeral_gb'] if ephemeral_gb: VMHelper.generate_ephemeral(self._session, instance, - vm_ref, userdevice, - ephemeral_gb) + vm_ref, userdevice, ephemeral_gb) userdevice += 1 # Attach any other disks @@ -504,9 +501,8 @@ class VMOps(object): VMHelper.destroy_vdi(self._session, vdi_ref) continue - VolumeHelper.create_vbd(session=self._session, vm_ref=vm_ref, - vdi_ref=vdi_ref, userdevice=userdevice, - bootable=False) + VMHelper.create_vbd(self._session, vm_ref, vdi_ref, + userdevice, bootable=False) userdevice += 1 def _configure_instance(self, ctx, instance, vm_ref, @@ -1078,8 +1074,8 @@ class VMOps(object): vdi_ref = self._session.call_xenapi("VBD.get_record", vbd_ref)["VDI"] - return VolumeHelper.create_vbd(self._session, rescue_vm_ref, vdi_ref, - 1, False) + return VMHelper.create_vbd(self._session, rescue_vm_ref, vdi_ref, + 1, bootable=False) def _shutdown_rescue(self, rescue_vm_ref): """Shutdown a rescue instance.""" diff --git a/nova/virt/xenapi/volume_utils.py b/nova/virt/xenapi/volume_utils.py index dfd6af6641ba..3bc1ab13e698 100644 --- a/nova/virt/xenapi/volume_utils.py +++ b/nova/virt/xenapi/volume_utils.py @@ -186,30 +186,6 @@ class VolumeHelper(xenapi.HelperBase): raise StorageError(_('Unable to find SR from VBD %s') % vbd_ref) return sr_ref - @classmethod - def create_vbd(cls, session, vm_ref, vdi_ref, userdevice, bootable): - """Create a VBD record. Returns a Deferred that gives the new - VBD reference.""" - vbd_rec = {} - vbd_rec['VM'] = vm_ref - vbd_rec['VDI'] = vdi_ref - vbd_rec['userdevice'] = str(userdevice) - vbd_rec['bootable'] = bootable - vbd_rec['mode'] = 'RW' - vbd_rec['type'] = 'disk' - vbd_rec['unpluggable'] = True - vbd_rec['empty'] = False - vbd_rec['other_config'] = {} - vbd_rec['qos_algorithm_type'] = '' - vbd_rec['qos_algorithm_params'] = {} - vbd_rec['qos_supported_algorithms'] = [] - LOG.debug(_('Creating VBD for VM %(vm_ref)s,' - ' VDI %(vdi_ref)s ... ') % locals()) - vbd_ref = session.call_xenapi('VBD.create', vbd_rec) - LOG.debug(_('Created VBD %(vbd_ref)s for VM %(vm_ref)s,' - ' VDI %(vdi_ref)s.') % locals()) - return vbd_ref - @classmethod def create_pbd(cls, session, sr_ref, params): pbd_rec = {} diff --git a/nova/virt/xenapi/volumeops.py b/nova/virt/xenapi/volumeops.py index fb82432c80b6..2f3aafb71a81 100644 --- a/nova/virt/xenapi/volumeops.py +++ b/nova/virt/xenapi/volumeops.py @@ -63,11 +63,7 @@ class VolumeOps(object): if vdi_ref is None: raise exception.Error(_('Could not find VDI ref')) - try: - self._session.call_xenapi("VDI.destroy", vdi_ref) - except self.XenAPI.Failure, exc: - LOG.exception(exc) - raise volume_utils.StorageError(_('Error destroying VDI')) + vm_utils.VMHelper.destroy_vdi(vdi_ref) def create_sr(self, label, params): LOG.debug(_("Creating SR %s") % label) @@ -184,11 +180,9 @@ class VolumeOps(object): dev_number = volume_utils.VolumeHelper.mountpoint_to_number(mountpoint) try: - vbd_ref = volume_utils.VolumeHelper.create_vbd(self._session, - vm_ref, - vdi_ref, - dev_number, - False) + vbd_ref = vm_utils.VMHelper.create_vbd(self._session, vm_ref, + vdi_ref, dev_number, + bootable=False) except self.XenAPI.Failure, exc: LOG.exception(exc) self.forget_sr(uuid)