openstack/nova
Code Issues Proposed changes

xenapi: move find_vbd_by_number into volume utils

Browse Source 
Above function is only used in volumeops, so moved to volume_utils.
Also added some unit tests to cover that function.

Change-Id: I1298dd35d8a41832475c6d6bd908af8f6576be82
This commit is contained in:
John Garbutt
2014-05-17 22:46:30 -04:00
parent 7bde55ae12
commit 7e5b58bfa9
5 changed files with 75 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

49
nova/tests/virt/xenapi/test_volume_utils.py
View File

@@ -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")

12
nova/tests/virt/xenapi/test_volumeops.py
View File

@@ -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):

16
nova/virt/xenapi/vm_utils.py
View File

@@ -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,

15
nova/virt/xenapi/volume_utils.py
View File

@@ -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)

14
nova/virt/xenapi/volumeops.py
View File

@@ -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)