openstack
/
nova
Code Issues Proposed changes

refactor: move down ``dev_number`` in xenapi 

This patch resolves the TODO marked by John Garbutt.
Refactored xenapi.volumeops by moving ``dev_number`` var from
``attach_volume`` to ``_attach_volume_to_vm`` method.

Change-Id: I413c4272fe575661774297afdb429bbb0a5b8211
This commit is contained in:
Maciej Szankin 2016-12-05 13:19:23 -06:00
parent 1ac100b778
commit 5aee29399b
2 changed files with 11 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
nova/tests/unit/virt/xenapi/test_volumeops.py
View File

@ -204,8 +204,8 @@ class AttachVolumeTestCase(VolumeOpsTestBase):
self.ops.attach_volume({}, "instance_name", "/dev/xvda") self.ops.attach_volume({}, "instance_name", "/dev/xvda")
mock_attach.assert_called_once_with({}, "vm_ref", "instance_name", 0, mock_attach.assert_called_once_with({}, "vm_ref", "instance_name",
True) '/dev/xvda', True)
@mock.patch.object(volumeops.VolumeOps, "_attach_volume") @mock.patch.object(volumeops.VolumeOps, "_attach_volume")
@mock.patch.object(vm_utils, "vm_ref_or_raise") @mock.patch.object(vm_utils, "vm_ref_or_raise")
@ -214,8 +214,8 @@ class AttachVolumeTestCase(VolumeOpsTestBase):
self.ops.attach_volume({}, "instance_name", "/dev/xvda", False) self.ops.attach_volume({}, "instance_name", "/dev/xvda", False)
mock_attach.assert_called_once_with({}, "vm_ref", "instance_name", 0, mock_attach.assert_called_once_with({}, "vm_ref", "instance_name",
False) '/dev/xvda', False)
@mock.patch.object(volumeops.VolumeOps, "_attach_volume") @mock.patch.object(volumeops.VolumeOps, "_attach_volume")
def test_attach_volume_default_hotplug_connect_volume(self, mock_attach): def test_attach_volume_default_hotplug_connect_volume(self, mock_attach):
@ -386,7 +386,7 @@ class AttachVolumeTestCase(VolumeOpsTestBase):
mock_shutdown.return_value = False mock_shutdown.return_value = False
with mock.patch.object(self.session.VBD, "plug") as mock_plug: with mock.patch.object(self.session.VBD, "plug") as mock_plug:
self.ops._attach_volume_to_vm("vdi", "vm", "name", 2, True) self.ops._attach_volume_to_vm("vdi", "vm", "name", '/dev/2', True)
mock_plug.assert_called_once_with("vbd", "vm") mock_plug.assert_called_once_with("vbd", "vm")
mock_vbd.assert_called_once_with(self.session, "vm", "vdi", 2, mock_vbd.assert_called_once_with(self.session, "vm", "vdi", 2,
@ -400,7 +400,7 @@ class AttachVolumeTestCase(VolumeOpsTestBase):
mock_shutdown.return_value = True mock_shutdown.return_value = True
with mock.patch.object(self.session.VBD, "plug") as mock_plug: with mock.patch.object(self.session.VBD, "plug") as mock_plug:
self.ops._attach_volume_to_vm("vdi", "vm", "name", 2, True) self.ops._attach_volume_to_vm("vdi", "vm", "name", '/dev/2', True)
self.assertFalse(mock_plug.called) self.assertFalse(mock_plug.called)
mock_vbd.assert_called_once_with(self.session, "vm", "vdi", 2, mock_vbd.assert_called_once_with(self.session, "vm", "vdi", 2,
@ -413,7 +413,7 @@ class AttachVolumeTestCase(VolumeOpsTestBase):
mock_vbd.return_value = "vbd" mock_vbd.return_value = "vbd"
with mock.patch.object(self.session.VBD, "plug") as mock_plug: with mock.patch.object(self.session.VBD, "plug") as mock_plug:
self.ops._attach_volume_to_vm("vdi", "vm", "name", 2, False) self.ops._attach_volume_to_vm("vdi", "vm", "name", '/dev/2', False)
self.assertFalse(mock_plug.called) self.assertFalse(mock_plug.called)
mock_vbd.assert_called_once_with(self.session, "vm", "vdi", 2, mock_vbd.assert_called_once_with(self.session, "vm", "vdi", 2,

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

@ -39,12 +39,9 @@ class VolumeOps(object):
def attach_volume(self, connection_info, instance_name, mountpoint, def attach_volume(self, connection_info, instance_name, mountpoint,
hotplug=True): hotplug=True):
"""Attach volume to VM instance.""" """Attach volume to VM instance."""
# TODO(johngarbutt) move this into _attach_volume_to_vm
dev_number = volume_utils.get_device_number(mountpoint)
vm_ref = vm_utils.vm_ref_or_raise(self._session, instance_name) vm_ref = vm_utils.vm_ref_or_raise(self._session, instance_name)
return self._attach_volume(connection_info, vm_ref, return self._attach_volume(connection_info, vm_ref,
instance_name, dev_number, hotplug) instance_name, mountpoint, hotplug)
def connect_volume(self, connection_info): def connect_volume(self, connection_info):
"""Attach volume to hypervisor, but not the VM.""" """Attach volume to hypervisor, but not the VM."""
@ -112,11 +109,13 @@ class VolumeOps(object):
return vdi_ref return vdi_ref
def _attach_volume_to_vm(self, vdi_ref, vm_ref, instance_name, dev_number, def _attach_volume_to_vm(self, vdi_ref, vm_ref, instance_name, mountpoint,
hotplug): hotplug):
LOG.debug('Attach_volume vdi: %(vdi_ref)s vm: %(vm_ref)s', LOG.debug('Attach_volume vdi: %(vdi_ref)s vm: %(vm_ref)s',
{'vdi_ref': vdi_ref, 'vm_ref': vm_ref}) {'vdi_ref': vdi_ref, 'vm_ref': vm_ref})
dev_number = volume_utils.get_device_number(mountpoint)
# osvol is added to the vbd so we can spot which vbds are volumes # osvol is added to the vbd so we can spot which vbds are volumes
vbd_ref = vm_utils.create_vbd(self._session, vm_ref, vdi_ref, vbd_ref = vm_utils.create_vbd(self._session, vm_ref, vdi_ref,
dev_number, bootable=False, dev_number, bootable=False,