[Kimchi-devel] [PATCH 2/7] Guest disks: Update api definition and error reporting
Aline Manera
alinefm at linux.vnet.ibm.com
Thu Apr 17 01:37:33 UTC 2014
Reviewed-by: Aline Manera <alinefm at linux.vnet.ibm.com>
On 04/13/2014 05:16 AM, lvroyce at linux.vnet.ibm.com wrote:
> From: Royce Lv <lvroyce at linux.vnet.ibm.com>
>
> Modify all errors related to cdrom attachment to be compatible
> with disk attachment, changing error naming and msg reporting.
> Add parameter 'bus' for disk attachment to give
> user choice when kimchi cannot decide what bus to use.
>
> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
> ---
> src/kimchi/API.json | 24 +++++++++++++++---------
> src/kimchi/i18n.py | 27 ++++++++++++++-------------
> src/kimchi/mockmodel.py | 10 +++++-----
> src/kimchi/model/vmstorages.py | 26 ++++++++++++++------------
> 4 files changed, 48 insertions(+), 39 deletions(-)
>
> diff --git a/src/kimchi/API.json b/src/kimchi/API.json
> index 5ca94e3..2e76864 100644
> --- a/src/kimchi/API.json
> +++ b/src/kimchi/API.json
> @@ -384,40 +384,46 @@
> },
> "vmstorages_create": {
> "type": "object",
> - "error": "KCHCDROM0012E",
> + "error": "KCHVMSTOR0012E",
> "properties": {
> "dev": {
> - "description": "The storage (cd-rom) device name",
> + "description": "The storage device name",
> "type": "string",
> - "pattern": "^hd[b-z]$",
> - "error": "KCHCDROM0001E"
> + "pattern": "^h|vd[b-z]$",
> + "error": "KCHVMSTOR0001E"
> },
> "type": {
> "description": "The storage type",
> "type": "string",
> - "pattern": "^cdrom$",
> + "pattern": "^cdrom|disk$",
> "required": true,
> - "error": "KCHCDROM0002E"
> + "error": "KCHVMSTOR0002E"
> },
> "path": {
> "description": "Path of iso image file or disk mount point",
> "type": "string",
> "pattern": "^((/)|(http)[s]?:|[t]?(ftp)[s]?:)+.*$",
> "required": true,
> - "error": "KCHCDROM0003E"
> + "error": "KCHVMSTOR0003E"
> + },
> + "bus": {
> + "description": "Target bus type of attached device",
> + "type": "string",
> + "pattern": "^scsi|ide|virtio$",
> + "error": "KCHVMSTOR0005E"
> }
> }
> },
> "vmstorage_update": {
> "type": "object",
> - "error": "KCHCDROM0013E",
> + "error": "KCHVMSTOR0013E",
> "properties": {
> "path": {
> "description": "Path of iso image file or disk mount point",
> "type": "string",
> "pattern": "^((/)|(http)[s]?:|[t]?(ftp)[s]?:)+.*$",
> "required": true,
> - "error": "KCHCDROM0003E"
> + "error": "KCHVMSTOR0003E"
> }
> }
> },
> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
> index e8491ec..f102166 100644
> --- a/src/kimchi/i18n.py
> +++ b/src/kimchi/i18n.py
> @@ -220,19 +220,20 @@ messages = {
> "KCHUTILS0002E": _("Timeout while running command '%(cmd)s' after %(seconds)s seconds"),
> "KCHUTILS0003E": _("Unable to choose a virutal machine name"),
>
> - "KCHCDROM0001E": _("Invalid CDROM device name"),
> - "KCHCDROM0002E": _("Invalid storage type. Types supported: 'cdrom'"),
> - "KCHCDROM0003E": _("The path '%(value)s' is not valid local/remote path for the device"),
> - "KCHCDROM0004E": _("Device name %(dev_name)s already exists in vm %(vm_name)s"),
> - "KCHCDROM0006E": _("Can't specify a directory for a CDROM device path"),
> - "KCHCDROM0007E": _("The storage device %(dev_name)s does not exist in the guest %(vm_name)s"),
> - "KCHCDROM0008E": _("Error while creating new storage device: %(error)s"),
> - "KCHCDROM0009E": _("Error while updating storage device: %(error)s"),
> - "KCHCDROM0010E": _("Error while removing storage device: %(error)s"),
> - "KCHCDROM0011E": _("Do not support guest CDROM hot plug attachment"),
> - "KCHCDROM0012E": _("Specify type and path to add a new virtual machine disk"),
> - "KCHCDROM0013E": _("Specify path to update virtual machine disk"),
> - "KCHCDROM0014E": _("Controller type %(type)s limitation of %(limit)s devices reached"),
> + "KCHVMSTOR0001E": _("Invalid vm storage device name"),
> + "KCHVMSTOR0002E": _("Invalid storage type. Types supported: 'cdrom', 'disk'"),
> + "KCHVMSTOR0003E": _("The path '%(value)s' is not valid local/remote path for the device"),
> + "KCHVMSTOR0004E": _("Device name %(dev_name)s already exists in vm %(vm_name)s"),
> + "KCHVMSTOR0005E": _("Invalid target device bus type, type supported: 'ide', 'scsi', 'virtio'"),
> + "KCHVMSTOR0006E": _("Just support cdrom path update"),
> + "KCHVMSTOR0007E": _("The storage device %(dev_name)s does not exist in the guest %(vm_name)s"),
> + "KCHVMSTOR0008E": _("Error while creating new storage device: %(error)s"),
> + "KCHVMSTOR0009E": _("Error while updating storage device: %(error)s"),
> + "KCHVMSTOR0010E": _("Error while removing storage device: %(error)s"),
> + "KCHVMSTOR0011E": _("Do not support ide device hot plug"),
> + "KCHVMSTOR0012E": _("Specify type and path to add a new virtual machine disk"),
> + "KCHVMSTOR0013E": _("Specify path to update virtual machine disk"),
> + "KCHVMSTOR0014E": _("Controller type %(type)s limitation of %(limit)s devices reached"),
>
> "KCHREPOS0001E": _("YUM Repository ID must be one word only string."),
> "KCHREPOS0002E": _("Repository URL must be an http://, ftp:// or file:// URL."),
> diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
> index bf17975..1ffbeb8 100644
> --- a/src/kimchi/mockmodel.py
> +++ b/src/kimchi/mockmodel.py
> @@ -647,12 +647,12 @@ class MockModel(object):
> def vmstorages_create(self, vm_name, params):
> path = params.get('path')
> if path.startswith('/') and not os.path.exists(path):
> - raise InvalidParameter("KCHCDROM0003E", {'value': path})
> + raise InvalidParameter("KCHVMSTOR0003E", {'value': path})
>
> dom = self._get_vm(vm_name)
> dev = params.get('dev', None)
> if dev and dev in self.vmstorages_get_list(vm_name):
> - return OperationFailed("KCHCDROM0004E", {'dev_name': dev,
> + return OperationFailed("KCHVMSTOR0004E", {'dev_name': dev,
> 'vm_name': vm_name})
> if not dev:
> index = len(dom.storagedevices.keys()) + 1
> @@ -669,14 +669,14 @@ class MockModel(object):
> def vmstorage_lookup(self, vm_name, dev_name):
> dom = self._get_vm(vm_name)
> if dev_name not in self.vmstorages_get_list(vm_name):
> - raise NotFoundError("KCHCDROM0007E", {'dev_name': dev_name,
> + raise NotFoundError("KCHVMSTOR0007E", {'dev_name': dev_name,
> 'vm_name': vm_name})
> return dom.storagedevices.get(dev_name).info
>
> def vmstorage_delete(self, vm_name, dev_name):
> dom = self._get_vm(vm_name)
> if dev_name not in self.vmstorages_get_list(vm_name):
> - raise NotFoundError("KCHCDROM0007E", {'dev_name': dev_name,
> + raise NotFoundError("KCHVMSTOR0007E", {'dev_name': dev_name,
> 'vm_name': vm_name})
> dom.storagedevices.pop(dev_name)
>
> @@ -685,7 +685,7 @@ class MockModel(object):
> dom = self._get_vm(vm_name)
> dom.storagedevices[dev_name].info.update(params)
> except Exception as e:
> - raise OperationFailed("KCHCDROM0009E", {'error': e.message})
> + raise OperationFailed("KCHVMSTOR0009E", {'error': e.message})
> return dev_name
>
> def vmifaces_create(self, vm, params):
> diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py
> index 1cffa84..c31af24 100644
> --- a/src/kimchi/model/vmstorages.py
> +++ b/src/kimchi/model/vmstorages.py
> @@ -89,15 +89,15 @@ def _check_cdrom_path(path):
>
> cds = re.findall("drive name:\t\t(.*)", content)
> if not cds:
> - raise InvalidParameter("KCHCDROM0003E", {'value': path})
> + raise InvalidParameter("KCHVMSTOR0003E", {'value': path})
>
> drives = [os.path.join('/dev', p) for p in cds[0].split('\t')]
> if path not in drives:
> - raise InvalidParameter("KCHCDROM0003E", {'value': path})
> + raise InvalidParameter("KCHVMSTOR0003E", {'value': path})
>
> src_type = 'block'
> else:
> - raise InvalidParameter("KCHCDROM0003E", {'value': path})
> + raise InvalidParameter("KCHVMSTOR0003E", {'value': path})
> return src_type
>
>
> @@ -122,7 +122,7 @@ class VMStoragesModel(object):
> valid_id.remove((bus_id, unit_id))
> continue
> if not valid_id:
> - raise OperationFailed('KCHCDROM0014E', {'type': 'ide', 'limit': 4})
> + raise OperationFailed('KCHVMSTOR0014E', {'type': 'ide', 'limit': 4})
> else:
> address = {'controller': controller_id,
> 'bus': valid_id[0][0], 'unit': valid_id[0][1]}
> @@ -131,7 +131,7 @@ class VMStoragesModel(object):
> def create(self, vm_name, params):
> dom = VMModel.get_vm(vm_name, self.conn)
> if DOM_STATE_MAP[dom.info()[0]] != 'shutoff':
> - raise InvalidOperation('KCHCDROM0011E')
> + raise InvalidOperation('KCHVMSTOR0011E')
>
> # Use device name passed or pick next
> dev_name = params.get('dev', None)
> @@ -140,7 +140,7 @@ class VMStoragesModel(object):
> else:
> devices = self.get_list(vm_name)
> if dev_name in devices:
> - raise OperationFailed('KCHCDROM0004E', {'dev_name': dev_name,
> + raise OperationFailed('KCHVMSTOR0004E', {'dev_name': dev_name,
> 'vm_name': vm_name})
>
> # Path will never be blank due to API.json verification.
> @@ -156,7 +156,7 @@ class VMStoragesModel(object):
> dom = conn.lookupByName(vm_name)
> dom.attachDeviceFlags(dev_xml, libvirt.VIR_DOMAIN_AFFECT_CURRENT)
> except Exception as e:
> - raise OperationFailed("KCHCDROM0008E", {'error': e.message})
> + raise OperationFailed("KCHVMSTOR0008E", {'error': e.message})
> return params['dev']
>
> def _get_storage_device_name(self, vm_name):
> @@ -190,7 +190,7 @@ class VMStorageModel(object):
> dom = VMModel.get_vm(vm_name, self.conn)
> disk = _get_device_xml(dom, dev_name)
> if disk is None:
> - raise NotFoundError("KCHCDROM0007E", {'dev_name': dev_name,
> + raise NotFoundError("KCHVMSTOR0007E", {'dev_name': dev_name,
> 'vm_name': vm_name})
> path = ""
> dev_bus = 'ide'
> @@ -220,13 +220,13 @@ class VMStorageModel(object):
> dom = VMModel.get_vm(vm_name, self.conn)
> disk = _get_device_xml(dom, dev_name)
> if disk is None:
> - raise NotFoundError("KCHCDROM0007E",
> + raise NotFoundError("KCHVMSTOR0007E",
> {'dev_name': dev_name,
> 'vm_name': vm_name})
>
> dom = VMModel.get_vm(vm_name, self.conn)
> if DOM_STATE_MAP[dom.info()[0]] != 'shutoff':
> - raise InvalidOperation('KCHCDROM0011E')
> + raise InvalidOperation('KCHVMSTOR0011E')
>
> try:
> conn = self.conn.get()
> @@ -234,18 +234,20 @@ class VMStorageModel(object):
> dom.detachDeviceFlags(etree.tostring(disk),
> libvirt.VIR_DOMAIN_AFFECT_CURRENT)
> except Exception as e:
> - raise OperationFailed("KCHCDROM0010E", {'error': e.message})
> + raise OperationFailed("KCHVMSTOR0010E", {'error': e.message})
>
> def update(self, vm_name, dev_name, params):
> params['src_type'] = _check_cdrom_path(params['path'])
> dom = VMModel.get_vm(vm_name, self.conn)
>
> dev_info = self.lookup(vm_name, dev_name)
> + if dev_info['type'] != 'cdrom':
> + raise InvalidOperation("KCHVMSTOR0006E")
> dev_info.update(params)
> xml = _get_storage_xml(dev_info)
>
> try:
> dom.updateDeviceFlags(xml, libvirt.VIR_DOMAIN_AFFECT_CURRENT)
> except Exception as e:
> - raise OperationFailed("KCHCDROM0009E", {'error': e.message})
> + raise OperationFailed("KCHVMSTOR0009E", {'error': e.message})
> return dev_name
More information about the Kimchi-devel
mailing list