[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:36:30 UTC 2014


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




More information about the Kimchi-devel mailing list