From 355e724387c04b644f4149472b827bda9377827c Mon Sep 17 00:00:00 2001 From: John Garbutt Date: Mon, 10 Feb 2014 22:13:22 -0700 Subject: [PATCH] Move vbd plug/unplug into session object To make it easier not to miss the VBD plug/unplug workaround I have moved this into the xenapi object, overriding the regular method. Change-Id: Ia0e677824b2bf700c5fb73a75c335f74a6ebb6a2 --- nova/tests/virt/xenapi/client/test_objects.py | 29 +++++++++++++++++++ nova/tests/virt/xenapi/test_vm_utils.py | 13 ++++----- nova/tests/virt/xenapi/test_volume_utils.py | 25 ---------------- nova/tests/virt/xenapi/test_volumeops.py | 3 +- nova/virt/xenapi/client/objects.py | 22 ++++++++++++++ nova/virt/xenapi/fake.py | 2 ++ nova/virt/xenapi/vm_utils.py | 4 +-- nova/virt/xenapi/volume_utils.py | 21 -------------- nova/virt/xenapi/volumeops.py | 2 +- 9 files changed, 63 insertions(+), 58 deletions(-) diff --git a/nova/tests/virt/xenapi/client/test_objects.py b/nova/tests/virt/xenapi/client/test_objects.py index 33283e995f2e..65d40b66ff9d 100644 --- a/nova/tests/virt/xenapi/client/test_objects.py +++ b/nova/tests/virt/xenapi/client/test_objects.py @@ -16,6 +16,7 @@ import mock from nova.tests.virt.xenapi import stubs +from nova import utils from nova.virt.xenapi.client import objects @@ -89,3 +90,31 @@ class ObjectsTestCase(stubs.XenAPITestBaseNoDB): pool = objects.Pool(self.session) pool.get_X("ref") self.session.call_xenapi.assert_called_once_with("pool.get_X", "ref") + + +class VBDTestCase(stubs.XenAPITestBaseNoDB): + def setUp(self): + super(VBDTestCase, self).setUp() + self.session = mock.Mock() + self.session.VBD = objects.VBD(self.session) + + def test_plug(self): + self.session.VBD.plug("vbd_ref", "vm_ref") + self.session.call_xenapi.assert_called_once_with("VBD.plug", "vbd_ref") + + @mock.patch.object(utils, 'synchronized') + def test_vbd_plug_check_synchronized(self, mock_synchronized): + session = mock.Mock() + self.session.VBD.plug("vbd_ref", "vm_ref") + mock_synchronized.assert_called_once_with("xenapi-vbd-vm_ref") + + def test_unplug(self): + self.session.VBD.unplug("vbd_ref", "vm_ref") + self.session.call_xenapi.assert_called_once_with("VBD.unplug", + "vbd_ref") + + @mock.patch.object(utils, 'synchronized') + def test_vbd_plug_check_synchronized(self, mock_synchronized): + session = mock.Mock() + self.session.VBD.unplug("vbd_ref", "vm_ref") + mock_synchronized.assert_called_once_with("xenapi-vbd-vm_ref") diff --git a/nova/tests/virt/xenapi/test_vm_utils.py b/nova/tests/virt/xenapi/test_vm_utils.py index dc0c66817727..2c61464f233c 100644 --- a/nova/tests/virt/xenapi/test_vm_utils.py +++ b/nova/tests/virt/xenapi/test_vm_utils.py @@ -874,7 +874,7 @@ class CreateVBDTestCase(VMUtilsTestBase): class UnplugVbdTestCase(VMUtilsTestBase): @mock.patch.object(greenthread, 'sleep') def test_unplug_vbd_works(self, mock_sleep): - session = mock.Mock() + session = _get_fake_session() vbd_ref = "vbd_ref" vm_ref = 'vm_ref' @@ -884,7 +884,7 @@ class UnplugVbdTestCase(VMUtilsTestBase): self.assertEqual(0, mock_sleep.call_count) def test_unplug_vbd_raises_unexpected_error(self): - session = mock.Mock() + session = _get_fake_session() vbd_ref = "vbd_ref" vm_ref = 'vm_ref' session.call_xenapi.side_effect = test.TestingException() @@ -1728,16 +1728,13 @@ class VDIAttachedHere(VMUtilsTestBase): @mock.patch.object(vm_utils, 'destroy_vbd') @mock.patch.object(vm_utils, '_get_this_vm_ref') @mock.patch.object(vm_utils, 'create_vbd') - @mock.patch.object(volume_utils, 'vbd_plug') @mock.patch.object(vm_utils, '_remap_vbd_dev') @mock.patch.object(vm_utils, '_wait_for_device') @mock.patch.object(utils, 'execute') - @mock.patch.object(vm_utils, 'unplug_vbd') - def test_sync_called(self, mock_unplug_vbd, mock_execute, - mock_wait_for_device, mock_remap_vbd_dev, - mock_vbd_plug, mock_create_vbd, + def test_sync_called(self, mock_execute, mock_wait_for_device, + mock_remap_vbd_dev, mock_create_vbd, mock_get_this_vm_ref, mock_destroy_vbd): - session = mock.Mock() + session = _get_fake_session() with vm_utils.vdi_attached_here(session, 'vdi_ref'): pass mock_execute.assert_called_with('sync', run_as_root=True) diff --git a/nova/tests/virt/xenapi/test_volume_utils.py b/nova/tests/virt/xenapi/test_volume_utils.py index 13c83b67bb84..1ac7d91ac1b8 100644 --- a/nova/tests/virt/xenapi/test_volume_utils.py +++ b/nova/tests/virt/xenapi/test_volume_utils.py @@ -18,34 +18,9 @@ import mock from eventlet import greenthread from nova.tests.virt.xenapi import stubs -from nova import utils from nova.virt.xenapi import volume_utils -class CallXenAPIHelpersTestCase(stubs.XenAPITestBaseNoDB): - def test_vbd_plug(self): - session = mock.Mock() - volume_utils.vbd_plug(session, "vbd_ref", "vm_ref:123") - session.call_xenapi.assert_called_once_with("VBD.plug", "vbd_ref") - - @mock.patch.object(utils, 'synchronized') - def test_vbd_plug_check_synchronized(self, mock_synchronized): - session = mock.Mock() - volume_utils.vbd_plug(session, "vbd_ref", "vm_ref:123") - mock_synchronized.assert_called_once_with("xenapi-events-vm_ref:123") - - def test_vbd_unplug(self): - session = mock.Mock() - volume_utils.vbd_unplug(session, "vbd_ref", "vm_ref:123") - session.call_xenapi.assert_called_once_with("VBD.unplug", "vbd_ref") - - @mock.patch.object(utils, 'synchronized') - def test_vbd_unplug_check_synchronized(self, mock_synchronized): - session = mock.Mock() - volume_utils.vbd_unplug(session, "vbd_ref", "vm_ref:123") - mock_synchronized.assert_called_once_with("xenapi-events-vm_ref:123") - - class SROps(stubs.XenAPITestBaseNoDB): def test_find_sr_valid_uuid(self): self.session = mock.Mock() diff --git a/nova/tests/virt/xenapi/test_volumeops.py b/nova/tests/virt/xenapi/test_volumeops.py index d79469e8eb9c..87bd9ab73b8b 100644 --- a/nova/tests/virt/xenapi/test_volumeops.py +++ b/nova/tests/virt/xenapi/test_volumeops.py @@ -128,6 +128,7 @@ class VolumeAttachTestCase(test.NoDBTestCase): self.mox.StubOutWithMock(volumeops.volume_utils, 'introduce_vdi') self.mox.StubOutWithMock(volumeops.vm_utils, 'create_vbd') self.mox.StubOutWithMock(volumeops.vm_utils, 'is_vm_shutdown') + self.mox.StubOutWithMock(ops._session.VBD, 'plug') self.mox.StubOutWithMock(ops._session, 'call_xenapi') instance_name = 'instance_1' @@ -159,7 +160,7 @@ class VolumeAttachTestCase(test.NoDBTestCase): volumeops.vm_utils.is_vm_shutdown(session, vm_ref).AndReturn(not vm_running) if plugged: - ops._session.call_xenapi("VBD.plug", vbd_ref) + ops._session.VBD.plug(vbd_ref, vm_ref) ops._session.call_xenapi("VDI.get_uuid", vdi_ref).AndReturn(vdi_uuid) diff --git a/nova/virt/xenapi/client/objects.py b/nova/virt/xenapi/client/objects.py index 1dfc2a8e57d5..a358d41b8e8c 100644 --- a/nova/virt/xenapi/client/objects.py +++ b/nova/virt/xenapi/client/objects.py @@ -12,6 +12,8 @@ # License for the specific language governing permissions and limitations # under the License. +from nova import utils + class XenAPISessionObject(object): """Wrapper to make calling and mocking the session easier @@ -71,6 +73,26 @@ class VBD(XenAPISessionObject): def __init__(self, session): super(VBD, self).__init__(session, "VBD") + def plug(self, vbd_ref, vm_ref): + @utils.synchronized('xenapi-vbd-' + vm_ref) + def synchronized_plug(): + self._call_method("plug", vbd_ref) + + # NOTE(johngarbutt) we need to ensure there is only ever one + # VBD.unplug or VBD.plug happening at once per VM + # due to a bug in XenServer 6.1 and 6.2 + synchronized_plug() + + def unplug(self, vbd_ref, vm_ref): + @utils.synchronized('xenapi-vbd-' + vm_ref) + def synchronized_unplug(): + self._call_method("unplug", vbd_ref) + + # NOTE(johngarbutt) we need to ensure there is only ever one + # VBD.unplug or VBD.plug happening at once per VM + # due to a bug in XenServer 6.1 and 6.2 + synchronized_unplug() + class VDI(XenAPISessionObject): """Virtual disk image.""" diff --git a/nova/virt/xenapi/fake.py b/nova/virt/xenapi/fake.py index e852bfd4448b..506d49c9c6c8 100644 --- a/nova/virt/xenapi/fake.py +++ b/nova/virt/xenapi/fake.py @@ -63,6 +63,7 @@ from nova.openstack.common import jsonutils from nova.openstack.common import log as logging from nova.openstack.common import timeutils from nova.openstack.common import units +from nova.virt.xenapi.client import session as xenapi_session _CLASSES = ['host', 'network', 'session', 'pool', 'SR', 'VBD', @@ -472,6 +473,7 @@ class SessionBase(object): def __init__(self, uri): self._session = None + xenapi_session.apply_session_helpers(self) def pool_get_default_SR(self, _1, pool_ref): return _db_content['pool'].values()[0]['default-SR'] diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index 9789a752c88b..d31b3cd65336 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -426,7 +426,7 @@ def unplug_vbd(session, vbd_ref, this_vm_ref): if num_attempt > 1: greenthread.sleep(1) - volume_utils.vbd_unplug(session, vbd_ref, this_vm_ref) + session.VBD.unplug(vbd_ref, this_vm_ref) return except session.XenAPI.Failure as exc: err = len(exc.details) > 0 and exc.details[0] @@ -2178,7 +2178,7 @@ def vdi_attached_here(session, vdi_ref, read_only=False): read_only=read_only, bootable=False) try: LOG.debug(_('Plugging VBD %s ... '), vbd_ref) - volume_utils.vbd_plug(session, vbd_ref, this_vm_ref) + session.VBD.plug(vbd_ref, this_vm_ref) try: LOG.debug(_('Plugging VBD %s done.'), vbd_ref) orig_dev = session.call_xenapi("VBD.get_device", vbd_ref) diff --git a/nova/virt/xenapi/volume_utils.py b/nova/virt/xenapi/volume_utils.py index bc88f0b8efb9..bd9b8be7952f 100644 --- a/nova/virt/xenapi/volume_utils.py +++ b/nova/virt/xenapi/volume_utils.py @@ -26,7 +26,6 @@ from oslo.config import cfg from nova.openstack.common.gettextutils import _ from nova.openstack.common import log as logging -from nova import utils xenapi_volume_utils_opts = [ cfg.IntOpt('introduce_vdi_retry_wait', @@ -312,23 +311,3 @@ def _get_target_port(iscsi_string): return iscsi_string.split(':')[1] return CONF.xenserver.target_port - - -def vbd_plug(session, vbd_ref, vm_ref): - @utils.synchronized('xenapi-events-' + vm_ref) - def synchronized_plug(): - session.call_xenapi("VBD.plug", vbd_ref) - - # NOTE(johngarbutt) we need to ensure there is only ever one VBD.plug - # happening at once per VM due to a bug in XenServer 6.1 and 6.2 - synchronized_plug() - - -def vbd_unplug(session, vbd_ref, vm_ref): - @utils.synchronized('xenapi-events-' + vm_ref) - def synchronized_unplug(): - session.call_xenapi("VBD.unplug", vbd_ref) - - # NOTE(johngarbutt) we need to ensure there is only ever one VBD.unplug - # happening at once per VM due to a bug in XenServer 6.1 and 6.2 - synchronized_unplug() diff --git a/nova/virt/xenapi/volumeops.py b/nova/virt/xenapi/volumeops.py index bf50c78e2551..83e6a45e4d0d 100644 --- a/nova/virt/xenapi/volumeops.py +++ b/nova/virt/xenapi/volumeops.py @@ -111,7 +111,7 @@ class VolumeOps(object): running = not vm_utils.is_vm_shutdown(self._session, vm_ref) if hotplug and running: - volume_utils.vbd_plug(self._session, vbd_ref, vm_ref) + self._session.VBD.plug(vbd_ref, vm_ref) vdi_uuid = self._session.call_xenapi("VDI.get_uuid", vdi_ref) return (sr_uuid, vdi_uuid)