diff --git a/nova/tests/virt/xenapi/test_volume_utils.py b/nova/tests/virt/xenapi/test_volume_utils.py index ca48a0c3a23b..051e5dcc63e5 100644 --- a/nova/tests/virt/xenapi/test_volume_utils.py +++ b/nova/tests/virt/xenapi/test_volume_utils.py @@ -18,6 +18,7 @@ import mock from eventlet import greenthread from nova import exception +from nova import test from nova.tests.virt.xenapi import stubs from nova.virt.xenapi import volume_utils @@ -155,3 +156,51 @@ class ParseVolumeInfoTestCase(stubs.XenAPITestBaseNoDB): exception.StorageError, volume_utils.get_device_number, 'dev/sd') + + +class FindVBDTestCase(stubs.XenAPITestBaseNoDB): + def test_find_vbd_by_number_works(self): + session = mock.Mock() + session.VM.get_VBDs.return_value = ["a", "b"] + session.VBD.get_userdevice.return_value = "1" + + result = volume_utils.find_vbd_by_number(session, "vm_ref", 1) + + self.assertEqual("a", result) + session.VM.get_VBDs.assert_called_once_with("vm_ref") + session.VBD.get_userdevice.assert_called_once_with("a") + + def test_find_vbd_by_number_no_matches(self): + session = mock.Mock() + session.VM.get_VBDs.return_value = ["a", "b"] + session.VBD.get_userdevice.return_value = "3" + + result = volume_utils.find_vbd_by_number(session, "vm_ref", 1) + + self.assertIsNone(result) + session.VM.get_VBDs.assert_called_once_with("vm_ref") + expected = [mock.call("a"), mock.call("b")] + self.assertEqual(expected, + session.VBD.get_userdevice.call_args_list) + + def test_find_vbd_by_number_no_vbds(self): + session = mock.Mock() + session.VM.get_VBDs.return_value = [] + + result = volume_utils.find_vbd_by_number(session, "vm_ref", 1) + + self.assertIsNone(result) + session.VM.get_VBDs.assert_called_once_with("vm_ref") + self.assertFalse(session.VBD.get_userdevice.called) + + def test_find_vbd_by_number_ignores_exception(self): + session = mock.Mock() + session.XenAPI.Failure = test.TestingException + session.VM.get_VBDs.return_value = ["a"] + session.VBD.get_userdevice.side_effect = test.TestingException + + result = volume_utils.find_vbd_by_number(session, "vm_ref", 1) + + self.assertIsNone(result) + session.VM.get_VBDs.assert_called_once_with("vm_ref") + session.VBD.get_userdevice.assert_called_once_with("a") diff --git a/nova/tests/virt/xenapi/test_volumeops.py b/nova/tests/virt/xenapi/test_volumeops.py index 8f42438b481c..496b8ba74b2e 100644 --- a/nova/tests/virt/xenapi/test_volumeops.py +++ b/nova/tests/virt/xenapi/test_volumeops.py @@ -43,7 +43,7 @@ class VolumeDetachTestCase(VolumeOpsTestBase): ops = volumeops.VolumeOps('session') self.mox.StubOutWithMock(volumeops.vm_utils, 'lookup') - self.mox.StubOutWithMock(volumeops.vm_utils, 'find_vbd_by_number') + self.mox.StubOutWithMock(volumeops.volume_utils, 'find_vbd_by_number') self.mox.StubOutWithMock(volumeops.vm_utils, 'is_vm_shutdown') self.mox.StubOutWithMock(volumeops.vm_utils, 'unplug_vbd') self.mox.StubOutWithMock(volumeops.vm_utils, 'destroy_vbd') @@ -57,7 +57,7 @@ class VolumeDetachTestCase(VolumeOpsTestBase): volumeops.volume_utils.get_device_number('mountpoint').AndReturn( 'devnumber') - volumeops.vm_utils.find_vbd_by_number( + volumeops.volume_utils.find_vbd_by_number( 'session', 'vmref', 'devnumber').AndReturn('vbdref') volumeops.vm_utils.is_vm_shutdown('session', 'vmref').AndReturn( @@ -84,7 +84,7 @@ class VolumeDetachTestCase(VolumeOpsTestBase): ['find_sr_from_vbd', 'destroy_vbd'], registered_calls) @mock.patch.object(volumeops.VolumeOps, "_detach_vbds_and_srs") - @mock.patch.object(vm_utils, "find_vbd_by_number") + @mock.patch.object(volume_utils, "find_vbd_by_number") @mock.patch.object(vm_utils, "vm_ref_or_raise") def test_detach_volume(self, mock_vm, mock_vbd, mock_detach): mock_vm.return_value = "vm_ref" @@ -97,19 +97,19 @@ class VolumeDetachTestCase(VolumeOpsTestBase): mock_detach.assert_called_once_with("vm_ref", ["vbd_ref"]) @mock.patch.object(volumeops.VolumeOps, "_detach_vbds_and_srs") - @mock.patch.object(vm_utils, "find_vbd_by_number") + @mock.patch.object(volume_utils, "find_vbd_by_number") @mock.patch.object(vm_utils, "vm_ref_or_raise") def test_detach_volume_skips_error_skip_attach(self, mock_vm, mock_vbd, mock_detach): mock_vm.return_value = "vm_ref" - mock_vbd.side_effect = exception.StorageError(reason="") + mock_vbd.return_value = None self.ops.detach_volume({}, "name", "/dev/xvdd") self.assertFalse(mock_detach.called) @mock.patch.object(volumeops.VolumeOps, "_detach_vbds_and_srs") - @mock.patch.object(vm_utils, "find_vbd_by_number") + @mock.patch.object(volume_utils, "find_vbd_by_number") @mock.patch.object(vm_utils, "vm_ref_or_raise") def test_detach_volume_raises(self, mock_vm, mock_vbd, mock_detach): diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index c5bcac4e02c0..0f7015b12a15 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -391,22 +391,6 @@ def is_enough_free_mem(session, instance): return host_free_mem >= mem -def find_vbd_by_number(session, vm_ref, number): - """Get the VBD reference from the device number.""" - vbd_refs = session.call_xenapi("VM.get_VBDs", vm_ref) - if vbd_refs: - for vbd_ref in vbd_refs: - try: - user_device = session.call_xenapi("VBD.get_userdevice", - vbd_ref) - if user_device == str(number): - return vbd_ref - except session.XenAPI.Failure as exc: - LOG.exception(exc) - raise exception.StorageError( - reason=_('VBD not found in instance %s') % vm_ref) - - def _should_retry_unplug_vbd(err): # Retry if unplug failed with DEVICE_DETACH_REJECTED # For reasons which we don't understand, diff --git a/nova/virt/xenapi/volume_utils.py b/nova/virt/xenapi/volume_utils.py index d089896dc1a9..ae9a87ac0db4 100644 --- a/nova/virt/xenapi/volume_utils.py +++ b/nova/virt/xenapi/volume_utils.py @@ -300,3 +300,18 @@ def _get_target_port(iscsi_string): return iscsi_string.split(':')[1] return CONF.xenserver.target_port + + +def find_vbd_by_number(session, vm_ref, dev_number): + """Get the VBD reference from the device number.""" + vbd_refs = session.VM.get_VBDs(vm_ref) + requested_device = str(dev_number) + if vbd_refs: + for vbd_ref in vbd_refs: + try: + user_device = session.VBD.get_userdevice(vbd_ref) + if user_device == requested_device: + return vbd_ref + except session.XenAPI.Failure: + msg = "Error looking up VBD %s for %s" % (vbd_ref, vm_ref) + LOG.debug(msg, exc_info=True) diff --git a/nova/virt/xenapi/volumeops.py b/nova/virt/xenapi/volumeops.py index e5ffc2942d36..73e32005fad5 100644 --- a/nova/virt/xenapi/volumeops.py +++ b/nova/virt/xenapi/volumeops.py @@ -133,7 +133,11 @@ class VolumeOps(object): {'instance_name': instance_name, 'mountpoint': mountpoint}) vm_ref = vm_utils.vm_ref_or_raise(self._session, instance_name) - vbd_ref = self._find_vbd(vm_ref, mountpoint) + + device_number = volume_utils.get_device_number(mountpoint) + vbd_ref = volume_utils.find_vbd_by_number(self._session, vm_ref, + device_number) + if vbd_ref is None: # NOTE(sirp): If we don't find the VBD then it must have been # detached previously. @@ -146,14 +150,6 @@ class VolumeOps(object): {'instance_name': instance_name, 'mountpoint': mountpoint}) - def _find_vbd(self, vm_ref, mountpoint): - device_number = volume_utils.get_device_number(mountpoint) - try: - return vm_utils.find_vbd_by_number( - self._session, vm_ref, device_number) - except exception.StorageError: - return - def _detach_vbds_and_srs(self, vm_ref, vbd_refs): is_vm_shutdown = vm_utils.is_vm_shutdown(self._session, vm_ref)