openstack
/
nova-powervm
Code Issues Proposed changes

Support short names for vdisks 

There is a 15-character limit on virtual disk names, imposed by VIOS
(but not by the PowerVM REST API).  There is no such limit for SSP LUs.
The recently-changed disk name generation algorithm allowed vdisk names
to exceed 15 characters, causing errors like:

Failed to perform operation on Volume Group on VIOS with ID 2 in CEC
8247-22L*1234A0A - RMC command for Virtual Disk failed with error -

Backing device names can not exceed "15" characters in length.

This change set adds an optional 'short' parameter to the private
_get_disk_name helper which causes it to limit the generated name to 15
characters.  The localdisk driver is changed to use this new parameter.
Unit tests are improved such that they no longer mock _get_disk_name,
and such that they verify both of its variants.

Change-Id: I9efbb9d904e52f2a74bba7d8e631cd45862aa504
This commit is contained in:
Eric Fried 2015-05-14 17:08:35 -05:00
parent f8ef71301a
commit f249ef3fef
4 changed files with 31 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
nova_powervm/tests/virt/powervm/disk/test_localdisk.py
View File

@ -18,14 +18,12 @@ import mock
from oslo_config import cfg
from nova import exception as nova_exc
from nova import objects
from nova import test
import os
from pypowervm.tests.wrappers.util import pvmhttp
from pypowervm.wrappers import storage as pvm_stor
from pypowervm.wrappers import virtual_io_server as pvm_vios
from nova_powervm.tests.virt import powervm
from nova_powervm.tests.virt.powervm import fixtures as fx
from nova_powervm.virt.powervm.disk import driver as disk_dvr
from nova_powervm.virt.powervm.disk import localdisk as ld
@ -71,25 +69,26 @@ class TestLocalDisk(test.TestCase):
# Tear down mocks
self.mock_vg_uuid_p.stop()
def get_ls(self, adpt):
@staticmethod
def get_ls(adpt):
return ld.LocalStorage({'adapter': adpt, 'host_uuid': 'host_uuid'})
@mock.patch('pypowervm.tasks.storage.upload_new_vdisk')
@mock.patch('nova_powervm.virt.powervm.disk.localdisk.LocalStorage.'
'_get_disk_name')
@mock.patch('nova_powervm.virt.powervm.disk.driver.'
'IterableToFileAdapter')
@mock.patch('nova.image.API')
def test_create_disk_from_image(self, mock_img_api, mock_file_adpt,
mock_get_dname, mock_upload_vdisk):
mock_upload_vdisk):
mock_img = {'id': 'fake_id', 'size': 50}
mock_get_dname.return_value = 'fake_vol'
mock_upload_vdisk.return_value = ('vdisk', None)
inst = mock.Mock()
inst.name = 'Inst Name'
inst.uuid = 'd5065c2c-ac43-3fa6-af32-ea84a3960291'
vdisk = self.get_ls(self.apt).create_disk_from_image(
None, None, mock_img, 20)
None, inst, mock_img, 20)
mock_upload_vdisk.assert_called_with(mock.ANY, mock.ANY, mock.ANY,
mock.ANY, 'fake_vol', 50,
mock.ANY, 'b_Inst_Nam_d506', 50,
d_size=21474836480L)
self.assertEqual('vdisk', vdisk)
@ -182,27 +181,25 @@ class TestLocalDisk(test.TestCase):
self.assertEqual(0, len(mock_wrapper.virtual_disks))
@mock.patch('pypowervm.wrappers.storage.VG')
@mock.patch('nova_powervm.virt.powervm.disk.localdisk.LocalStorage.'
'_get_disk_name')
def test_extend_disk(self, mock_dsk_name, mock_vg):
def test_extend_disk_not_found(self, mock_vg):
local = self.get_ls(self.apt)
inst = objects.Instance(**powervm.TEST_INSTANCE)
inst = mock.Mock()
inst.name = 'Name Of Instance'
inst.uuid = 'd5065c2c-ac43-3fa6-af32-ea84a3960291'
vdisk = mock.Mock(name='vdisk')
vdisk.name = 'disk_name'
vdisk.name = 'NO_MATCH'
resp = mock.Mock(name='response')
resp.virtual_disks = [vdisk]
mock_vg.wrap.return_value = resp
mock_dsk_name.return_value = 'NOMATCH'
self.assertRaises(nova_exc.DiskNotFound, local.extend_disk,
'context', inst, dict(type='boot'), 10)
mock_dsk_name.return_value = 'disk_name'
vdisk.name = 'b_Name_Of__d506'
local.extend_disk('context', inst, dict(type='boot'), 1000)
# Validate the call
self.assertEqual(1, resp.update.call_count)
self.assertEqual(vdisk.capacity, 1000)

18
nova_powervm/virt/powervm/disk/driver.py
View File

@ -97,10 +97,20 @@ class DiskAdapter(object):
return IterableToFileAdapter(chunks)
@staticmethod
def _get_disk_name(disk_type, instance):
"""Generate a name for a virtual disk associated with an instance."""
return pvm_util.sanitize_file_name_for_api(instance.name,
prefix=disk_type + '_')
def _get_disk_name(disk_type, instance, short=False):
"""Generate a name for a virtual disk associated with an instance.
:param disk_type: One of the DiskType enum values.
:param instance: The instance for which the disk is to be created.
:param short: If True, the generated name will be limited to 15
characters (the limit for virtual disk). If False, it
will be limited by the API (79 characters currently).
:return:
"""
prefix = '%s_' % (disk_type[0] if short else disk_type)
base = ('%s_%s' % (instance.name[:8], instance.uuid[:4]) if short
else instance.name)
return pvm_util.sanitize_file_name_for_api(base, prefix=prefix)
@staticmethod
def _get_image_name(image_meta):

4
nova_powervm/virt/powervm/disk/localdisk.py
View File

@ -153,7 +153,7 @@ class LocalStorage(disk_dvr.DiskAdapter):
# Transfer the image
stream = self._get_image_upload(context, image)
vol_name = self._get_disk_name(image_type, instance)
vol_name = self._get_disk_name(image_type, instance, short=True)
# Disk size to API is in bytes. Input from method is in Gb
disk_bytes = self._disk_gb_to_bytes(disk_size, floor=image['size'])
@ -215,7 +215,7 @@ class LocalStorage(disk_dvr.DiskAdapter):
vg_wrap.update()
# Get the disk name based on the instance and type
vol_name = self._get_disk_name(disk_info['type'], instance)
vol_name = self._get_disk_name(disk_info['type'], instance, short=True)
LOG.info(_LI('Extending disk: %s'), vol_name)
try:
_extend()

1
nova_powervm/virt/powervm/disk/ssp.py
View File

@ -180,6 +180,7 @@ class SSPDiskAdapter(disk_drv.DiskAdapter):
image_lu = self._get_or_upload_image_lu(context, img_meta)
boot_lu_name = self._get_disk_name(image_type, instance)
LOG.info(_LI('SSP: Disk name is %s'), boot_lu_name)
ssp, boot_lu = tsk_stg.crt_lu_linked_clone(
self._ssp, self._cluster, image_lu, boot_lu_name, disk_size_gb)