[PATCH 0/5] cdrom: support add and query cdrom

From: Royce Lv <lvroyce@linux.vnet.ibm.com> Add support for cdrom query and add a new cdrom: list: GET /vms/vm1/cdroms --> [{u'path': u'/home/royce/isorepo/Fedora-19-x86_64-DVD.iso', u'name': u'hdc'}] add: POST /vms/vm1/cdroms {'name':'hde', 'path': 'path-of-your-iso'} get detial: GET /vms/vm1/cdroms/hdc -->{u'path': u'/home/royce/isorepo/Fedora-19-x86_64-DVD.iso', u'name': u'hdc'} REF: https://github.com/kimchi-project/kimchi/wiki/customize-VM NOTE: Change media will be covered in cdrom update patch. This only covers query and add a new cdrom device. Royce Lv (5): cdrom: Update API.md for add cdrom and query cdrom cdrom: update controller cdrom: Add convinient functions to deal with xml parse cdrom: update model.py cdrom: Add model test to check cdrom operations docs/API.md | 15 ++++++++ src/kimchi/API.json | 16 +++++++++ src/kimchi/control/vms.py | 22 ++++++++++++ src/kimchi/model.py | 90 +++++++++++++++++++++++++++++++++++++++++++++-- src/kimchi/xmlutils.py | 5 +++ tests/test_model.py | 29 +++++++++++++++ 6 files changed, 175 insertions(+), 2 deletions(-) -- 1.8.1.2

From: Royce Lv <lvroyce@linux.vnet.ibm.com> Add cdrom sub-collection CDROMs and sub-resource CDROM to vm resource. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/API.md b/docs/API.md index 0013e86..f352e9e 100644 --- a/docs/API.md +++ b/docs/API.md @@ -103,6 +103,21 @@ Represents a snapshot of the Virtual Machine's primary monitor. * **GET**: Redirect to the latest screenshot of a Virtual Machine in PNG format + +### Sub-collection: Virtual Machine CDROMs +**URI:** /vms/*:name*/cdroms +#* **GET**: Retrieve a summarized list of all CDROMs of specified virtual machine +* **POST**: Attatch a new CDROM to specified virtual machine. + * name : The name of CDROM in VM. + * path *(optional)*: Path of ISO to insert in the CDROM. + + +### Sub-resource: Virtual Machine CDROM +**URI:** /vms/*:name*/cdroms/*:name* +#* **GET**: Retrieve cdrom information + * path: Path of ISO img. + + ### Collection: Templates **URI:** /templates -- 1.8.1.2

On 01/06/2014 07:32 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add cdrom sub-collection CDROMs and sub-resource CDROM to vm resource.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/docs/API.md b/docs/API.md index 0013e86..f352e9e 100644 --- a/docs/API.md +++ b/docs/API.md @@ -103,6 +103,21 @@ Represents a snapshot of the Virtual Machine's primary monitor.
* **GET**: Redirect to the latest screenshot of a Virtual Machine in PNG format
+ +### Sub-collection: Virtual Machine CDROMs +**URI:** /vms/*:name*/cdroms
Maybe we can use /vms/<name>/storages That way we can use the same API to add/remove different kind of storages to the vm: cdrom, scsi disk, ide disk, etc What do you think? /vms/<name>/storages/<storage-id> in which 'storage-id' is a unique value: cdrom1, cdrom2 or even the device it points to - hdc, sda
+#* **GET**: Retrieve a summarized list of all CDROMs of specified virtual machine +* **POST**: Attatch a new CDROM to specified virtual machine. + * name : The name of CDROM in VM. + * path *(optional)*: Path of ISO to insert in the CDROM. + + +### Sub-resource: Virtual Machine CDROM +**URI:** /vms/*:name*/cdroms/*:name* +#* **GET**: Retrieve cdrom information + * path: Path of ISO img. + + ### Collection: Templates
**URI:** /templates

On 2014年01月11日 00:42, Aline Manera wrote:
On 01/06/2014 07:32 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add cdrom sub-collection CDROMs and sub-resource CDROM to vm resource.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/docs/API.md b/docs/API.md index 0013e86..f352e9e 100644 --- a/docs/API.md +++ b/docs/API.md @@ -103,6 +103,21 @@ Represents a snapshot of the Virtual Machine's primary monitor.
* **GET**: Redirect to the latest screenshot of a Virtual Machine in PNG format
+ +### Sub-collection: Virtual Machine CDROMs +**URI:** /vms/*:name*/cdroms
Maybe we can use /vms/<name>/storages
That way we can use the same API to add/remove different kind of storages to the vm: cdrom, scsi disk, ide disk, etc
What do you think?
/vms/<name>/storages/<storage-id> in which 'storage-id' is a unique value: cdrom1, cdrom2 or even the device it points to - hdc, sda Good point, they share the same implementation, only difference is type.
+#* **GET**: Retrieve a summarized list of all CDROMs of specified virtual machine +* **POST**: Attatch a new CDROM to specified virtual machine. + * name : The name of CDROM in VM. + * path *(optional)*: Path of ISO to insert in the CDROM. + + +### Sub-resource: Virtual Machine CDROM +**URI:** /vms/*:name*/cdroms/*:name* +#* **GET**: Retrieve cdrom information + * path: Path of ISO img. + + ### Collection: Templates
**URI:** /templates

On 01/06/2014 07:32 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add cdrom sub-collection CDROMs and sub-resource CDROM to vm resource.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/docs/API.md b/docs/API.md index 0013e86..f352e9e 100644 --- a/docs/API.md +++ b/docs/API.md @@ -103,6 +103,21 @@ Represents a snapshot of the Virtual Machine's primary monitor.
* **GET**: Redirect to the latest screenshot of a Virtual Machine in PNG format
+ +### Sub-collection: Virtual Machine CDROMs +**URI:** /vms/*:name*/cdroms +#* **GET**: Retrieve a summarized list of all CDROMs of specified virtual machine +* **POST**: Attatch a new CDROM to specified virtual machine. Typo Attach
+ * name : The name of CDROM in VM. + * path *(optional)*: Path of ISO to insert in the CDROM. + + +### Sub-resource: Virtual Machine CDROM +**URI:** /vms/*:name*/cdroms/*:name* +#* **GET**: Retrieve cdrom information + * path: Path of ISO img. + + ### Collection: Templates
**URI:** /templates

On 2014年01月13日 19:59, Rodrigo Trujillo wrote:
On 01/06/2014 07:32 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add cdrom sub-collection CDROMs and sub-resource CDROM to vm resource.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/docs/API.md b/docs/API.md index 0013e86..f352e9e 100644 --- a/docs/API.md +++ b/docs/API.md @@ -103,6 +103,21 @@ Represents a snapshot of the Virtual Machine's primary monitor.
* **GET**: Redirect to the latest screenshot of a Virtual Machine in PNG format
+ +### Sub-collection: Virtual Machine CDROMs +**URI:** /vms/*:name*/cdroms +#* **GET**: Retrieve a summarized list of all CDROMs of specified virtual machine +* **POST**: Attatch a new CDROM to specified virtual machine. Typo Attach ACK
+ * name : The name of CDROM in VM. + * path *(optional)*: Path of ISO to insert in the CDROM. + + +### Sub-resource: Virtual Machine CDROM +**URI:** /vms/*:name*/cdroms/*:name* +#* **GET**: Retrieve cdrom information + * path: Path of ISO img. + + ### Collection: Templates
**URI:** /templates

From: Royce Lv <lvroyce@linux.vnet.ibm.com> Update controller and API.json for cdrom. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/API.json | 16 ++++++++++++++++ src/kimchi/control/vms.py | 22 ++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 3a3c48f..feb0c82 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -103,6 +103,22 @@ } } }, + "cdroms_create": { + "type": "object", + "properties": { + "name": { + "description": "The name of the new VM", + "type": "string", + "pattern": "^hd[b-z]$", + "required": true + }, + "path": { + "description": "Path of image file inserted to the CDROM", + "type": "string", + "pattern": "^((/)|(http)[s]?:|[t]?(ftp)[s]?:)+.*([.]iso)$" + } + } + }, "networks_create": { "type": "object", "properties": { diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py index d722920..51a18c1 100644 --- a/src/kimchi/control/vms.py +++ b/src/kimchi/control/vms.py @@ -37,6 +37,7 @@ class VM(Resource): super(VM, self).__init__(model, ident) self.update_params = ["name"] self.screenshot = VMScreenShot(model, ident) + self.cdroms = CDROMs(model, ident) self.uri_fmt = '/vms/%s' self.start = self.generate_action_handler('start') self.stop = self.generate_action_handler('stop') @@ -63,3 +64,24 @@ class VMScreenShot(Resource): def get(self): self.lookup() raise internal_redirect(self.info) + + +class CDROMs(Collection): + def __init__(self, model, vm): + super(CDROMs, self).__init__(model) + self.resource = CDROM + self.vm = vm + self.resource_args = [self.vm, ] + self.model_args = [self.vm, ] + + +class CDROM(Resource): + def __init__(self, model, vm, ident): + super(CDROM, self).__init__(model, ident) + self.vm = vm + self.model_args = [self.vm, self.ident] + self.uri_fmt = '/vms/%s/cdroms/%s' + + @property + def data(self): + return self.info -- 1.8.1.2

On 01/06/2014 07:32 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Update controller and API.json for cdrom.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/API.json | 16 ++++++++++++++++ src/kimchi/control/vms.py | 22 ++++++++++++++++++++++ 2 files changed, 38 insertions(+)
diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 3a3c48f..feb0c82 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -103,6 +103,22 @@ } } }, + "cdroms_create": { + "type": "object", + "properties": { + "name": { + "description": "The name of the new VM", Should be "new VM cdrom" or "new CDROM" + "type": "string", + "pattern": "^hd[b-z]$", I do not see reasons to restrict the name of the cdrom device. Because, this name may not reflect the device name inside the host, and user could set the name to "my-ubuntu-iso" for instance, for better management.
+ "required": true + }, + "path": { + "description": "Path of image file inserted to the CDROM", + "type": "string", + "pattern": "^((/)|(http)[s]?:|[t]?(ftp)[s]?:)+.*([.]iso)$" + } + } + }, "networks_create": { "type": "object", "properties": { diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py index d722920..51a18c1 100644 --- a/src/kimchi/control/vms.py +++ b/src/kimchi/control/vms.py @@ -37,6 +37,7 @@ class VM(Resource): super(VM, self).__init__(model, ident) self.update_params = ["name"] self.screenshot = VMScreenShot(model, ident) + self.cdroms = CDROMs(model, ident) self.uri_fmt = '/vms/%s' self.start = self.generate_action_handler('start') self.stop = self.generate_action_handler('stop') @@ -63,3 +64,24 @@ class VMScreenShot(Resource): def get(self): self.lookup() raise internal_redirect(self.info) + + +class CDROMs(Collection): + def __init__(self, model, vm): + super(CDROMs, self).__init__(model) + self.resource = CDROM + self.vm = vm + self.resource_args = [self.vm, ] + self.model_args = [self.vm, ] + + +class CDROM(Resource): + def __init__(self, model, vm, ident): + super(CDROM, self).__init__(model, ident) + self.vm = vm + self.model_args = [self.vm, self.ident] + self.uri_fmt = '/vms/%s/cdroms/%s' + + @property + def data(self): + return self.info

From: Royce Lv <lvroyce@linux.vnet.ibm.com> To format xml used to create cdrom, and to parse and filter cdrom query results, add three helper function to do this. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model.py | 42 ++++++++++++++++++++++++++++++++++++++++++ src/kimchi/xmlutils.py | 5 +++++ 2 files changed, 47 insertions(+) diff --git a/src/kimchi/model.py b/src/kimchi/model.py index a21fcf7..c74c973 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -1672,6 +1672,48 @@ def _get_volume_xml(**kwargs): return xml +def _get_disk_xml(**kwargs): + kwargs['readonly'] = "<readonly/>" \ + if kwargs['target']['type'] == 'cdrom' else None + xml = """ + <disk type='{source[type]}' device='{target[type]}'> + <source file='{source[path]}'/> + <target dev='{target[name]}' bus='{target[bus]}'/> + {readonly} + </disk> + """.format(**kwargs) + return dict(xml=xml) + + +def _parse_vm_disks(xml_str, filter_path, filter_name, filter_val): + # xml_str: xml_str of all disks + # filter_path: xpath of the filter attribute, + # such as './disk' or './disk/target' + # filter_name: relative path of filter attr of disks etree element + # such as: type, bus + # filter_val: expected value of filter name + root = ElementTree.fromstring(xml_str) + ret = [] + xpath = "%s/[@%s='%s']" % (filter_path, filter_name, filter_val) + for disk in root.findall(xpath): + name = disk.find('./target').attrib['dev'] + ret.append(name) + + return ret + + +def _parse_device(xml_str, dev_name): + xpath = "./disk/target[@dev='%s']/.." % dev_name + root = ElementTree.fromstring(xml_str) + ret = [] + disk = root.find(xpath) + if disk: + path = disk.find('./source').attrib['file'] + return dict(name=dev_name, path=path) + else: + raise NotFoundError + + class LibvirtConnection(object): def __init__(self, uri): self.uri = uri diff --git a/src/kimchi/xmlutils.py b/src/kimchi/xmlutils.py index 51ff0ec..176ceb1 100644 --- a/src/kimchi/xmlutils.py +++ b/src/kimchi/xmlutils.py @@ -40,3 +40,8 @@ def xml_item_update(xml, xpath, value): item = root.find(xpath) item.text = value return ElementTree.tostring(root, encoding="utf-8") + +def xml_get_child(xml, xpath): + root = ElementTree.fromstring(xml) + item = root.find(xpath) + return ElementTree.tostring(item, encoding="utf-8") -- 1.8.1.2

On 01/06/2014 07:32 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
To format xml used to create cdrom, and to parse and filter cdrom query results, add three helper function to do this.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model.py | 42 ++++++++++++++++++++++++++++++++++++++++++ src/kimchi/xmlutils.py | 5 +++++ 2 files changed, 47 insertions(+)
diff --git a/src/kimchi/model.py b/src/kimchi/model.py index a21fcf7..c74c973 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -1672,6 +1672,48 @@ def _get_volume_xml(**kwargs): return xml
+def _get_disk_xml(**kwargs): + kwargs['readonly'] = "<readonly/>" \ + if kwargs['target']['type'] == 'cdrom' else None
What about: kargs['readonly'] = None if kargs['target']['type'] == 'cdrom': kwargs['readonly'] = "<readonly/>" that way we avoid breaking lines and make the code easier to read
+ xml = """ + <disk type='{source[type]}' device='{target[type]}'> + <source file='{source[path]}'/> + <target dev='{target[name]}' bus='{target[bus]}'/> + {readonly} + </disk> + """.format(**kwargs) + return dict(xml=xml) + + +def _parse_vm_disks(xml_str, filter_path, filter_name, filter_val): + # xml_str: xml_str of all disks + # filter_path: xpath of the filter attribute, + # such as './disk' or './disk/target' + # filter_name: relative path of filter attr of disks etree element + # such as: type, bus + # filter_val: expected value of filter name + root = ElementTree.fromstring(xml_str) + ret = [] + xpath = "%s/[@%s='%s']" % (filter_path, filter_name, filter_val) + for disk in root.findall(xpath): + name = disk.find('./target').attrib['dev'] + ret.append(name) + + return ret + + +def _parse_device(xml_str, dev_name): + xpath = "./disk/target[@dev='%s']/.." % dev_name + root = ElementTree.fromstring(xml_str) + ret = [] + disk = root.find(xpath) + if disk: + path = disk.find('./source').attrib['file'] + return dict(name=dev_name, path=path) + else: + raise NotFoundError + + class LibvirtConnection(object): def __init__(self, uri): self.uri = uri diff --git a/src/kimchi/xmlutils.py b/src/kimchi/xmlutils.py index 51ff0ec..176ceb1 100644 --- a/src/kimchi/xmlutils.py +++ b/src/kimchi/xmlutils.py @@ -40,3 +40,8 @@ def xml_item_update(xml, xpath, value): item = root.find(xpath) item.text = value return ElementTree.tostring(root, encoding="utf-8") + +def xml_get_child(xml, xpath): + root = ElementTree.fromstring(xml) + item = root.find(xpath) + return ElementTree.tostring(item, encoding="utf-8")

On 01/06/2014 07:32 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
To format xml used to create cdrom, and to parse and filter cdrom query results, add three helper function to do this.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model.py | 42 ++++++++++++++++++++++++++++++++++++++++++ src/kimchi/xmlutils.py | 5 +++++ 2 files changed, 47 insertions(+)
diff --git a/src/kimchi/model.py b/src/kimchi/model.py index a21fcf7..c74c973 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -1672,6 +1672,48 @@ def _get_volume_xml(**kwargs): return xml
+def _get_disk_xml(**kwargs): + kwargs['readonly'] = "<readonly/>" \ + if kwargs['target']['type'] == 'cdrom' else None + xml = """ + <disk type='{source[type]}' device='{target[type]}'> + <source file='{source[path]}'/> + <target dev='{target[name]}' bus='{target[bus]}'/> + {readonly} + </disk> + """.format(**kwargs) + return dict(xml=xml) + + +def _parse_vm_disks(xml_str, filter_path, filter_name, filter_val): + # xml_str: xml_str of all disks + # filter_path: xpath of the filter attribute, + # such as './disk' or './disk/target' + # filter_name: relative path of filter attr of disks etree element + # such as: type, bus + # filter_val: expected value of filter name + root = ElementTree.fromstring(xml_str) + ret = [] + xpath = "%s/[@%s='%s']" % (filter_path, filter_name, filter_val) + for disk in root.findall(xpath): + name = disk.find('./target').attrib['dev'] + ret.append(name) + + return ret + + +def _parse_device(xml_str, dev_name): + xpath = "./disk/target[@dev='%s']/.." % dev_name + root = ElementTree.fromstring(xml_str) + ret = [] + disk = root.find(xpath) + if disk: + path = disk.find('./source').attrib['file'] + return dict(name=dev_name, path=path) + else: + raise NotFoundError You could return a error message here, with the name of the device not found. + + class LibvirtConnection(object): def __init__(self, uri): self.uri = uri diff --git a/src/kimchi/xmlutils.py b/src/kimchi/xmlutils.py index 51ff0ec..176ceb1 100644 --- a/src/kimchi/xmlutils.py +++ b/src/kimchi/xmlutils.py @@ -40,3 +40,8 @@ def xml_item_update(xml, xpath, value): item = root.find(xpath) item.text = value return ElementTree.tostring(root, encoding="utf-8") + +def xml_get_child(xml, xpath): + root = ElementTree.fromstring(xml) + item = root.find(xpath) + return ElementTree.tostring(item, encoding="utf-8")

From: Royce Lv <lvroyce@linux.vnet.ibm.com> Implement create/delete/update cdrom functionality in model.py. As _live_vm_update() is designed to manipulate multiple update params all at once, we construct a dict from cdroms_create to adjust to this usage. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model.py | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/src/kimchi/model.py b/src/kimchi/model.py index c74c973..4dd9eff 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -78,9 +78,9 @@ ISO_POOL_NAME = u'kimchi_isos' GUESTS_STATS_INTERVAL = 5 HOST_STATS_INTERVAL = 1 VM_STATIC_UPDATE_PARAMS = {'name': './name'} -VM_LIVE_UPDATE_PARAMS = {} STORAGE_SOURCES = {'netfs': {'addr': '/pool/source/host/@name', 'path': '/pool/source/dir/@path'}} +DEVICE_FILTER = {'cdrom': {'xml': './devices/disk', 'filter_field': './devices/disk/@device'}} def _uri_to_name(collection, uri): @@ -491,7 +491,51 @@ class Model(object): return dom def _live_vm_update(self, dom, params): - pass + VM_LIVE_UPDATE_PARAMS = { + 'device': {'handler': functools.partial(dom.attachDeviceFlags, + flags=libvirt.VIR_DOMAIN_AFFECT_CURRENT), + 'formatter': _get_disk_xml}} + + for key, val in params.items(): + if key in VM_LIVE_UPDATE_PARAMS: + handler = VM_LIVE_UPDATE_PARAMS[key]['handler'] + param = VM_LIVE_UPDATE_PARAMS[key]['formatter'](**val) + try: + handler(**param) + except libvirt.LibvirtError: + raise OperationFailed("Cannot apply change on vm: %s", e.get_error_message()) + + def cdroms_create(self, vm_name, params): + path = params.get('path') + if not os.path.isfile(path): + raise InvalidParameter("Path specified for CDROM is not a valid file") + dom = self._get_vm(vm_name) + state = Model.dom_state_map[dom.info()[0]] + + if state == 'running': + # CDROM is IDE device, which does not allow hotplug. + raise InvalidOperation("CDROM can just be attatched when vm is shutoff") + if params['name'] in self.cdroms_get_list(vm_name): + raise InvalidParameter("CDROM with this name already exsited") + + cdrom_param = {'source': {'type': 'file', 'path': params['path']}, + 'target': {'name': params['name'], 'type': 'cdrom', 'bus': 'ide'}} + self._live_vm_update(dom, dict(device=cdrom_param)) + return params['name'] + + def cdroms_get_list(self, vm_name): + dom = self._get_vm(vm_name) + xml = dom.XMLDesc(0) + device_xml = xmlutils.xml_get_child(xml, './devices') + cdroms = _parse_vm_disks(device_xml, filter_path='./disk', + filter_name='device', filter_val='cdrom') + return cdroms + + def cdrom_lookup(self, vm_name, dev_name): + dom = self._get_vm(vm_name) + xml = dom.XMLDesc(0) + device_xml = xmlutils.xml_get_child(xml, './devices') + return _parse_device(device_xml, dev_name) def vm_update(self, name, params): dom = self._get_vm(name) -- 1.8.1.2

On 01/06/2014 07:32 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Implement create/delete/update cdrom functionality in model.py. As _live_vm_update() is designed to manipulate multiple update params all at once, we construct a dict from cdroms_create to adjust to this usage.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model.py | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/src/kimchi/model.py b/src/kimchi/model.py index c74c973..4dd9eff 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -78,9 +78,9 @@ ISO_POOL_NAME = u'kimchi_isos' GUESTS_STATS_INTERVAL = 5 HOST_STATS_INTERVAL = 1 VM_STATIC_UPDATE_PARAMS = {'name': './name'} -VM_LIVE_UPDATE_PARAMS = {} STORAGE_SOURCES = {'netfs': {'addr': '/pool/source/host/@name', 'path': '/pool/source/dir/@path'}} +DEVICE_FILTER = {'cdrom': {'xml': './devices/disk', 'filter_field': './devices/disk/@device'}}
def _uri_to_name(collection, uri): @@ -491,7 +491,51 @@ class Model(object): return dom
def _live_vm_update(self, dom, params): - pass + VM_LIVE_UPDATE_PARAMS = { + 'device': {'handler': functools.partial(dom.attachDeviceFlags, + flags=libvirt.VIR_DOMAIN_AFFECT_CURRENT), + 'formatter': _get_disk_xml}} + + for key, val in params.items(): + if key in VM_LIVE_UPDATE_PARAMS: + handler = VM_LIVE_UPDATE_PARAMS[key]['handler'] + param = VM_LIVE_UPDATE_PARAMS[key]['formatter'](**val) + try: + handler(**param) + except libvirt.LibvirtError: + raise OperationFailed("Cannot apply change on vm: %s", e.get_error_message()) + + def cdroms_create(self, vm_name, params):
+ path = params.get('path') + if not os.path.isfile(path): + raise InvalidParameter("Path specified for CDROM is not a valid file")
It can also be a remote ISO
+ dom = self._get_vm(vm_name) + state = Model.dom_state_map[dom.info()[0]] + + if state == 'running': + # CDROM is IDE device, which does not allow hotplug. + raise InvalidOperation("CDROM can just be attatched when vm is shutoff") + if params['name'] in self.cdroms_get_list(vm_name): + raise InvalidParameter("CDROM with this name already exsited") + + cdrom_param = {'source': {'type': 'file', 'path': params['path']}, + 'target': {'name': params['name'], 'type': 'cdrom', 'bus': 'ide'}} + self._live_vm_update(dom, dict(device=cdrom_param)) + return params['name'] + + def cdroms_get_list(self, vm_name): + dom = self._get_vm(vm_name) + xml = dom.XMLDesc(0) + device_xml = xmlutils.xml_get_child(xml, './devices') + cdroms = _parse_vm_disks(device_xml, filter_path='./disk', + filter_name='device', filter_val='cdrom') + return cdroms + + def cdrom_lookup(self, vm_name, dev_name): + dom = self._get_vm(vm_name) + xml = dom.XMLDesc(0) + device_xml = xmlutils.xml_get_child(xml, './devices') + return _parse_device(device_xml, dev_name)
def vm_update(self, name, params): dom = self._get_vm(name)

On 01/06/2014 07:32 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Implement create/delete/update cdrom functionality in model.py. As _live_vm_update() is designed to manipulate multiple update params all at once, we construct a dict from cdroms_create to adjust to this usage.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model.py | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/src/kimchi/model.py b/src/kimchi/model.py index c74c973..4dd9eff 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -78,9 +78,9 @@ ISO_POOL_NAME = u'kimchi_isos' GUESTS_STATS_INTERVAL = 5 HOST_STATS_INTERVAL = 1 VM_STATIC_UPDATE_PARAMS = {'name': './name'} -VM_LIVE_UPDATE_PARAMS = {} STORAGE_SOURCES = {'netfs': {'addr': '/pool/source/host/@name', 'path': '/pool/source/dir/@path'}} +DEVICE_FILTER = {'cdrom': {'xml': './devices/disk', 'filter_field': './devices/disk/@device'}}
def _uri_to_name(collection, uri): @@ -491,7 +491,51 @@ class Model(object): return dom
def _live_vm_update(self, dom, params): - pass + VM_LIVE_UPDATE_PARAMS = { + 'device': {'handler': functools.partial(dom.attachDeviceFlags, + flags=libvirt.VIR_DOMAIN_AFFECT_CURRENT), + 'formatter': _get_disk_xml}} + + for key, val in params.items(): + if key in VM_LIVE_UPDATE_PARAMS: + handler = VM_LIVE_UPDATE_PARAMS[key]['handler'] + param = VM_LIVE_UPDATE_PARAMS[key]['formatter'](**val) + try: + handler(**param) + except libvirt.LibvirtError: You have missed the "as e:" , because you are using "e" below + raise OperationFailed("Cannot apply change on vm: %s", e.get_error_message()) + + def cdroms_create(self, vm_name, params): + path = params.get('path') + if not os.path.isfile(path): + raise InvalidParameter("Path specified for CDROM is not a valid file") + dom = self._get_vm(vm_name) + state = Model.dom_state_map[dom.info()[0]] + + if state == 'running': + # CDROM is IDE device, which does not allow hotplug. + raise InvalidOperation("CDROM can just be attatched when vm is shutoff") Typo: attached + if params['name'] in self.cdroms_get_list(vm_name): + raise InvalidParameter("CDROM with this name already exsited") Typo: exist + + cdrom_param = {'source': {'type': 'file', 'path': params['path']}, + 'target': {'name': params['name'], 'type': 'cdrom', 'bus': 'ide'}} + self._live_vm_update(dom, dict(device=cdrom_param)) + return params['name'] + + def cdroms_get_list(self, vm_name): + dom = self._get_vm(vm_name) + xml = dom.XMLDesc(0) + device_xml = xmlutils.xml_get_child(xml, './devices') + cdroms = _parse_vm_disks(device_xml, filter_path='./disk', + filter_name='device', filter_val='cdrom') + return cdroms + + def cdrom_lookup(self, vm_name, dev_name): + dom = self._get_vm(vm_name) + xml = dom.XMLDesc(0) + device_xml = xmlutils.xml_get_child(xml, './devices') + return _parse_device(device_xml, dev_name)
def vm_update(self, name, params): dom = self._get_vm(name)

From: Royce Lv <lvroyce@linux.vnet.ibm.com> Add model test to validate cdrom create, get list, and get detailed attribute. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- tests/test_model.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/test_model.py b/tests/test_model.py index c689bcc..06ffb3e 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -71,6 +71,35 @@ class ModelTests(unittest.TestCase): self.assertRaises(NotFoundError, inst.vm_lookup, 'nosuchvm') @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') + def test_vm_cdrom(self): + inst = kimchi.model.Model(objstore_loc=self.tmp_store) + + with RollbackContext() as rollback: + params = {'name': 'test', 'disks': []} + inst.templates_create(params) + rollback.prependDefer(inst.template_delete, 'test') + + params = {'name': 'kimchi-vm', 'template': '/templates/test'} + inst.vms_create(params) + rollback.prependDefer(inst.vm_delete, 'kimchi-vm') + + vms = inst.vms_get_list() + self.assertTrue('kimchi-vm' in vms) + + path = '/tmp/kimchi-images/tmpdir' + if not os.path.exists(path): + os.makedirs(path) + + iso = '/tmp/kimchi-images/tmpdir/ubuntu12.04.iso' + iso_gen.construct_fake_iso(iso, True, '12.04', 'ubuntu') + param = dict(name='hde', path=iso) + inst.cdroms_create('kimchi-vm', dict(name='hde', path=iso)) + info = inst.cdroms_get_list('kimchi-vm') + self.assertIn('hde', info) + info = inst.cdrom_lookup('kimchi-vm', 'hde') + self.assertEqual(info['path'], iso) + + @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_vm_lifecycle(self): inst = kimchi.model.Model(objstore_loc=self.tmp_store) -- 1.8.1.2

On 01/06/2014 07:32 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add support for cdrom query and add a new cdrom: list: GET /vms/vm1/cdroms --> [{u'path': u'/home/royce/isorepo/Fedora-19-x86_64-DVD.iso', u'name': u'hdc'}]
I think 'source' and 'dev' are more consistent with libvirt terminology [{u'source': u'/home/royce/isorepo/Fedora-19-x86_64-DVD.iso', u'dev': u'hdc'}]
add: POST /vms/vm1/cdroms {'name':'hde', 'path': 'path-of-your-iso'} get detial: GET /vms/vm1/cdroms/hdc -->{u'path': u'/home/royce/isorepo/Fedora-19-x86_64-DVD.iso', u'name': u'hdc'}
REF: https://github.com/kimchi-project/kimchi/wiki/customize-VM
NOTE: Change media will be covered in cdrom update patch. This only covers query and add a new cdrom device.
Royce Lv (5): cdrom: Update API.md for add cdrom and query cdrom cdrom: update controller cdrom: Add convinient functions to deal with xml parse cdrom: update model.py cdrom: Add model test to check cdrom operations
docs/API.md | 15 ++++++++ src/kimchi/API.json | 16 +++++++++ src/kimchi/control/vms.py | 22 ++++++++++++ src/kimchi/model.py | 90 +++++++++++++++++++++++++++++++++++++++++++++-- src/kimchi/xmlutils.py | 5 +++ tests/test_model.py | 29 +++++++++++++++ 6 files changed, 175 insertions(+), 2 deletions(-)

On 01/06/2014 07:32 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add support for cdrom query and add a new cdrom: list: GET /vms/vm1/cdroms --> [{u'path': u'/home/royce/isorepo/Fedora-19-x86_64-DVD.iso', u'name': u'hdc'}] add: POST /vms/vm1/cdroms {'name':'hde', 'path': 'path-of-your-iso'} get detial: GET /vms/vm1/cdroms/hdc -->{u'path': u'/home/royce/isorepo/Fedora-19-x86_64-DVD.iso', u'name': u'hdc'}
REF: https://github.com/kimchi-project/kimchi/wiki/customize-VM
NOTE: Change media will be covered in cdrom update patch. This only covers query and add a new cdrom device.
Royce Lv (5): cdrom: Update API.md for add cdrom and query cdrom cdrom: update controller cdrom: Add convinient functions to deal with xml parse cdrom: update model.py cdrom: Add model test to check cdrom operations
I am missing the mockmodel code and tests.
docs/API.md | 15 ++++++++ src/kimchi/API.json | 16 +++++++++ src/kimchi/control/vms.py | 22 ++++++++++++ src/kimchi/model.py | 90 +++++++++++++++++++++++++++++++++++++++++++++-- src/kimchi/xmlutils.py | 5 +++ tests/test_model.py | 29 +++++++++++++++ 6 files changed, 175 insertions(+), 2 deletions(-)
participants (4)
-
Aline Manera
-
lvroyce@linux.vnet.ibm.com
-
Rodrigo Trujillo
-
Royce Lv