[Kimchi-devel] [PATCH 2/2 v4] guest-storage-add: removing "Storage Name" backend support
Royce Lv
lvroyce at linux.vnet.ibm.com
Mon Sep 22 09:38:04 UTC 2014
Reviewed-by: Royce Lv<lvroyce at linux.vnet.ibm.com>
Pls ignore my previous comments, as I am used to started from backend...
On 2014年09月22日 17:36, Royce Lv wrote:
> Just nit below...
> On 2014年09月20日 05:49, Daniel Henrique Barboza wrote:
>> This patch fixes APIs, models and tests to reflect the removal
>> of the "Storage Name" field that was done in the UI.
>>
>> From now on, all device names will be generated automatically.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb at linux.vnet.ibm.com>
>> ---
>> docs/API.md | 1 -
>> src/kimchi/API.json | 6 ------
>> src/kimchi/mockmodel.py | 9 ++-------
>> src/kimchi/model/vmstorages.py | 33 +++++++++++++--------------------
>> tests/test_model.py | 5 -----
>> tests/test_rest.py | 14 ++++++--------
>> 6 files changed, 21 insertions(+), 47 deletions(-)
>>
>> diff --git a/docs/API.md b/docs/API.md
>> index b679ce7..cc438cc 100644
>> --- a/docs/API.md
>> +++ b/docs/API.md
>> @@ -148,7 +148,6 @@ Represents a snapshot of the Virtual Machine's
>> primary monitor.
>> **URI:** /vms/*:name*/storages
>> * **GET**: Retrieve a summarized list of all storages of specified
>> guest
>> * **POST**: Attach a new storage or virtual drive to specified
>> virtual machine.
>> - * dev: The name of the storage in the vm.
>> * type: The type of the storage (currently support 'cdrom' and
>> 'disk').
>> * path: Path of cdrom iso.
>> * pool: Storage pool which disk image file locate in.
>> diff --git a/src/kimchi/API.json b/src/kimchi/API.json
>> index b8604d2..f262e1c 100644
>> --- a/src/kimchi/API.json
>> +++ b/src/kimchi/API.json
>> @@ -486,12 +486,6 @@
>> "type": "object",
>> "error": "KCHVMSTOR0012E",
>> "properties": {
>> - "dev": {
>> - "description": "The storage device name",
>> - "type": "string",
>> - "pattern": "^h|vd[b-z]$",
>> - "error": "KCHVMSTOR0001E"
>> - },
>> "type": {
>> "description": "The storage type",
>> "type": "string",
>> diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
>> index 8b58c36..aa8441e 100644
>> --- a/src/kimchi/mockmodel.py
>> +++ b/src/kimchi/mockmodel.py
>> @@ -801,13 +801,8 @@ class MockModel(object):
>> except Exception as e:
>> raise InvalidParameter("KCHVMSTOR0015E", {'error': e})
>> dom = self._get_vm(vm_name)
>> - dev = params.get('dev', None)
>> - if dev and dev in self.vmstorages_get_list(vm_name):
>> - return OperationFailed(
>> - "KCHVMSTOR0004E", {'dev_name': dev, 'vm_name':
>> vm_name})
>> - if not dev:
>> - index = len(dom.storagedevices.keys()) + 1
>> - params['dev'] = "hd" + string.ascii_lowercase[index]
>> + index = len(dom.storagedevices.keys()) + 1
>> + params['dev'] = "hd" + string.ascii_lowercase[index]
>>
>> vmdev = MockVMStorageDevice(params)
>> dom.storagedevices[params['dev']] = vmdev
>> diff --git a/src/kimchi/model/vmstorages.py
>> b/src/kimchi/model/vmstorages.py
>> index 54e2685..4d6c0b2 100644
>> --- a/src/kimchi/model/vmstorages.py
>> +++ b/src/kimchi/model/vmstorages.py
>> @@ -174,26 +174,19 @@ class VMStoragesModel(object):
>> return params['dev']
>>
>> def _get_storage_device_name(self, vm_name, params):
>> - if params.get('dev') is None:
>> - bus_prefix = PREFIX_MAP[params['bus']]
>> - dev_list = [dev for dev in self.get_list(vm_name)
>> - if dev.startswith(bus_prefix)]
>> - if len(dev_list) == 0:
>> - params['dev'] = bus_prefix + 'a'
>> - else:
>> - dev_list.sort()
>> - last_dev = dev_list.pop()
>> - # TODO: Improve to device names "greater then" hdz
>> - next_dev_letter_pos =\
>> - string.ascii_lowercase.index(last_dev[2]) + 1
>> - params['dev'] =\
>> - bus_prefix +
>> string.ascii_lowercase[next_dev_letter_pos]
>> -
>> - devices = self.get_list(vm_name)
>> - if params['dev'] in devices:
>> - raise OperationFailed(
>> - 'KCHVMSTOR0004E',
> Seems i18n.py needs to be updated to get 'KCHVMSTOR0004E' deleted.
>> - {'dev_name': params['dev'], 'vm_name': vm_name})
>> + bus_prefix = PREFIX_MAP[params['bus']]
>> + dev_list = [dev for dev in self.get_list(vm_name)
>> + if dev.startswith(bus_prefix)]
>> + if len(dev_list) == 0:
>> + params['dev'] = bus_prefix + 'a'
>> + else:
>> + dev_list.sort()
>> + last_dev = dev_list.pop()
>> + # TODO: Improve to device names "greater then" hdz
>> + next_dev_letter_pos =\
>> + string.ascii_lowercase.index(last_dev[2]) + 1
>> + params['dev'] =\
>> + bus_prefix +
>> string.ascii_lowercase[next_dev_letter_pos]
>>
>> def get_list(self, vm_name):
>> dom = VMModel.get_vm(vm_name, self.conn)
>> diff --git a/tests/test_model.py b/tests/test_model.py
>> index ceedc6f..d458478 100644
>> --- a/tests/test_model.py
>> +++ b/tests/test_model.py
>> @@ -356,11 +356,6 @@ class ModelTests(unittest.TestCase):
>> self.assertEquals(u'cdrom', cd_info['type'])
>> self.assertEquals(iso_path, cd_info['path'])
>>
>> - # create a cdrom with existing dev_name
>> - cdrom_args['dev'] = cdrom_dev
>> - self.assertRaises(OperationFailed, inst.vmstorages_create,
>> - vm_name, cdrom_args)
>> -
>> # update path of existing cd with
>> # non existent iso
>> self.assertRaises(InvalidParameter, inst.vmstorage_update,
>> diff --git a/tests/test_rest.py b/tests/test_rest.py
>> index 7778019..7eb6233 100644
>> --- a/tests/test_rest.py
>> +++ b/tests/test_rest.py
>> @@ -535,26 +535,23 @@ class RestTests(unittest.TestCase):
>> self.assertEquals(400, resp.status)
>>
>> # Attach disk with pool and vol specified
>> - req = json.dumps({'dev': 'hdx',
>> - 'type': 'disk',
>> + req = json.dumps({'type': 'disk',
>> 'pool': 'tmp',
>> 'vol': 'attach-volume'})
>> resp = self.request('/vms/test-vm/storages', req, 'POST')
>> self.assertEquals(201, resp.status)
>> cd_info = json.loads(resp.read())
>> - self.assertEquals('hdx', cd_info['dev'])
>> self.assertEquals('disk', cd_info['type'])
>> self.assertEquals('tmp', cd_info['pool'])
>> self.assertEquals('attach-volume', cd_info['vol'])
>>
>> # Attach a cdrom with existent dev name
>> - req = json.dumps({'dev': 'hdk',
>> - 'type': 'cdrom',
>> + req = json.dumps({'type': 'cdrom',
>> 'path': '/tmp/existent.iso'})
>> resp = self.request('/vms/test-vm/storages', req, 'POST')
>> self.assertEquals(201, resp.status)
>> cd_info = json.loads(resp.read())
>> - self.assertEquals('hdk', cd_info['dev'])
>> + cd_dev = cd_info['dev']
>> self.assertEquals('cdrom', cd_info['type'])
>> self.assertEquals('/tmp/existent.iso', cd_info['path'])
>> # Delete the file and cdrom
>> @@ -564,7 +561,7 @@ class RestTests(unittest.TestCase):
>>
>> # Change path of storage cdrom
>> req = json.dumps({'path':
>> 'http://myserver.com/myiso.iso'})
>> - resp = self.request('/vms/test-vm/storages/hdx', req,
>> 'PUT')
>> + resp = self.request('/vms/test-vm/storages/'+cd_dev,
>> req, 'PUT')
>> self.assertEquals(200, resp.status)
>> cd_info = json.loads(resp.read())
>> self.assertEquals('http://myserver.com/myiso.iso',
>> cd_info['path'])
>> @@ -574,7 +571,8 @@ class RestTests(unittest.TestCase):
>> self.assertEquals(4, len(devs))
>>
>> # Detach storage cdrom
>> - resp = self.request('/vms/test-vm/storages/hdx', '{}',
>> 'DELETE')
>> + resp = self.request('/vms/test-vm/storages/'+cd_dev,
>> + '{}', 'DELETE')
>> self.assertEquals(204, resp.status)
>>
>> # Test GET
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
More information about the Kimchi-devel
mailing list