Reviewed-by: Aline Manera <alinefm(a)linux.vnet.ibm.com>
On 04/13/2014 05:16 AM, lvroyce(a)linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce(a)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(a)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