Re: [Kimchi-devel] [PATCH 2/2 v4] guest-storage-add: removing "Storage Name" backend support

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(a)linux.vnet.ibm.com&gt; > --- > 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(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel