Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 02/10/2014 10:52 AM, Rodrigo Trujillo wrote:

This patch changes API.md with new storage sub-collection/sub-resource information.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- docs/API.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)

diff --git a/docs/API.md b/docs/API.md index 7f0628d..fb5ec17 100644 --- a/docs/API.md +++ b/docs/API.md @@ -117,6 +117,28 @@ 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 storages +**URI:** /vms/*:name*/storages +* **GET**: Retrieve a summarized list of all storages of specified guest +* **POST**: Attach a new storage to specified virtual machine. + Simulate add new cdrom + * dev : The name of the storage in the vm. + * type : The type of the storage (cdrom, disk). + * path : Path of cdrom iso or disk. + +### Sub-resource: storage +**URI:** /vms/*:name*/storages/*:dev* +* **GET**: Retrieve storage information + * dev : The name of the storage in the vm. + * type : The type of the storage (cdrom, disk). + * path : Path of cdrom iso or disk. +* **PUT**: Update storage information + * path: Path of cdrom iso or disk. +* **DELETE**: Remove the storage. Simulate eject a cdrom + + + ### Collection: Templates

**URI:** /templates

On 02/10/2014 10:52 AM, Rodrigo Trujillo wrote:

This patch adds the control classes for guest storages

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/API.json | 33 ++++++++++++++++++++++++++ src/kimchi/control/vm/storages.py | 49 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 src/kimchi/control/vm/storages.py

diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 842fb11..861c96e 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -290,6 +290,39 @@ }, "additionalProperties": false }, + "storage_create": { + "type": "object", + "properties": { + "dev": { + "description": "The storage device name", + "type": "string", + "pattern": "^hd[b-z]$", + "required": true

The dev can be optinal. We can choose one if user does not provide one

+ }, + "type": { + "description": "The storage type", + "type": "string", + "pattern": "^[cdrom|disk]$", + "required": true + }, + "path": { + "description": "Path of iso image file or disk mount point", + "type": "string", + "pattern": "^((/)|(http)[s]?:|[t]?(ftp)[s]?:)+.*$"

The path is a required parameter.

+ } + } + }, + "storage_update": { + "type": "object", + "properties": { + "path": { + "description": "Path of iso image file or disk mount point", + "type": "string", + "pattern": "^((/)|(http)[s]?:|[t]?(ftp)[s]?:)+.*$", + "required": true + } + } + }, "template_update": { "type": "object", "properties": { diff --git a/src/kimchi/control/vm/storages.py b/src/kimchi/control/vm/storages.py new file mode 100644 index 0000000..8878d6e --- /dev/null +++ b/src/kimchi/control/vm/storages.py @@ -0,0 +1,49 @@ +# +# Project Kimchi +# +# Copyright IBM, Corp. 2013 +# +# Authors: +# ShaoHe Feng <shaohef@linux.vnet.ibm.com> +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +from kimchi.control.base import Collection, Resource +from kimchi.control.utils import UrlSubNode + + +@UrlSubNode("storages") +class Storages(Collection): + def __init__(self, model, vm): + super(Storages, self).__init__(model) + self.resource = Storage + self.vm = vm + self.resource_args = [self.vm, ] + self.model_args = [self.vm, ] + + +class Storage(Resource): + def __init__(self, model, vm, ident): + super(Storage, self).__init__(model, ident) + self.vm = vm + self.ident = ident + self.info = {} + self.model_args = [self.vm, self.ident] + self.uri_fmt = '/vms/%s/storages/%s' + self.update_params = ['path'] + + @property + def data(self): + return self.info

On 02/10/2014 06:11 PM, Aline Manera wrote:

On 02/10/2014 10:52 AM, Rodrigo Trujillo wrote:

...

This patch adds the CREATE, LOOKUP, UPDATE and DELETE functionalities to guest storage devices support.

Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/model/vms.py | 167 +++++++++++++++++++++++++++++++++++++++++++++++- src/kimchi/xmlutils.py | 5 ++ 2 files changed, 171 insertions(+), 1 deletion(-)

diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 4623e28..069ab7c 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -20,8 +20,11 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA

+import libvirt import os +import socket import time +import urlparse import uuid from xml.etree import ElementTree

@@ -36,7 +39,8 @@ from kimchi.model.config import CapabilitiesModel from kimchi.model.templates import TemplateModel from kimchi.model.utils import get_vm_name from kimchi.screenshot import VMScreenshot -from kimchi.utils import run_setfacl_set_attr, template_name_from_uri +from kimchi.utils import kimchi_log, run_setfacl_set_attr +from kimchi.utils import template_name_from_uri

DOM_STATE_MAP = {0: 'nostate', @@ -47,6 +51,10 @@ DOM_STATE_MAP = {0: 'nostate', 5: 'shutoff', 6: 'crashed'}

+DEV_TYPE_SRC_ATTR_MAP = {'file': 'file', + 'block': 'dev', + 'dir': 'dir'} + GUESTS_STATS_INTERVAL = 5 VM_STATIC_UPDATE_PARAMS = {'name': './name'} VM_LIVE_UPDATE_PARAMS = {} @@ -474,3 +482,160 @@ class LibvirtVMScreenshot(VMScreenshot): stream.finish() finally: os.close(fd) + + +class StoragesModel(object): + def __init__(self, **kargs): + self.conn = kargs['conn'] + + def _get_storage_xml(self, params):

...

+ storage = """ + <disk type='%(src_type)s' device='%(dev_type)s'> + <driver name='qemu' type='raw'/>%(source)s + <target dev='%(dev)s' bus='ide'/> + </disk> + """

You can use lxml to build the xml entry I think there are some code examples upstream

...

+ info = {} + info['dev'] = params.get('dev') + info['dev_type'] = params.get('type') + info['src_type'] = params.get('src_type') + + # Working with url paths + if info['src_type'] == 'network': + net = {} + output = urlparse.urlparse(params.get('path')) + net['protocol'] = output.scheme + net['port'] = output.port or socket.getservbyname(output.scheme) + net['hostname'] = output.hostname + net['url_path'] = output.path + info['source'] = """ + <source protocol='%(protocol)s' name='%(url_path)s'> + <host name='%(hostname)s' port='%(port)s'/> + </source>""" % net + else: + # Fixing source attribute + info['source'] = """ + <source %s='%s'/>""" % (DEV_TYPE_SRC_ATTR_MAP[info['src_type']], + params.get('path')) + return storage % info + + def create(self, vm_name, params): + #TODO: Check if device name is already use + path = params.get('path') + + # Checking if path exist, if not url + if path.startswith('/'): + if not os.path.exists(path): + msg = "Path specified for device is not valide" + raise InvalidParameter(msg) + elif path.endswith('.iso') or os.path.isfile(path): + params['src_type'] = 'file' + elif os.path.isdir(path): + params['src_type'] = 'dir' + else: + params['src_type'] = 'block' + else: + params['src_type'] = 'network' + + # Add device to VM + dev_xml = self._get_storage_xml(params) + try: + conn = self.conn.get() + dom = conn.lookupByName(vm_name) + dom.attachDeviceFlags(dev_xml, libvirt.VIR_DOMAIN_AFFECT_CURRENT) + except Exception as e: + msg = 'Was not possible to attach storage device: %s' % e.message + kimchi_log.error(msg) + raise OperationFailed(e.message) + return params['dev'] + + + def get_list(self, dev): + dom = VMModel.get_vm(dev, self.conn) + xml = dom.XMLDesc(0) + device_xml = xmlutils.xml_get_child(xml, './devices') + storages = self._parse_vm_disks(device_xml, ['disk', 'cdrom']) + return storages + + def _parse_vm_disks(self, xml_str, filter_list): + # xml_str: xml_str of device with all devices + # filter_list: List of which device type to retrieve + root = ElementTree.fromstring(xml_str) + ret = [] + for opt in filter_list: + xpath = ".disk/[@device='%s']" % opt + for disk in root.findall(xpath): + name = disk.find('./target').attrib['dev'] + ret.append(name) + return ret + + +class StorageModel(object): + def __init__(self, **kargs): + self.conn = kargs['conn'] + self.kargs = kargs + + def _get_device_xml(self, vm_name, dev_name): + # Get VM xml and then devices xml + dom = VMModel.get_vm(vm_name, self.conn) + device_xml = xmlutils.xml_get_child(dom.XMLDesc(0), './devices') + dev_root = ElementTree.fromstring(device_xml) + xpath = "./disk/target[@dev='%s']/.." % dev_name + return dev_root.find(xpath)

why did you do in 2 steps?

xpath = "./devices/disk/target[@dev=%s]" % de

...

+ + def lookup(self, vm_name, dev_name): + # Retrieve disk xml and format return dict + disk = self._get_device_xml(vm_name, dev_name) + if disk is not None: + source = disk.find('./source') + path = "" + if source is not None: + src_type = disk.attrib.get('type') + if src_type == 'network': + host = source.find('./host') + path = source.attrib.get('protocol') + '://' +\ + host.attrib.get('name') + ':' +\ + host.attrib.get('port') + source.attrib.get('name') + else: + path = source.attrib.get( + DEV_TYPE_SRC_ATTR_MAP[src_type]) + dev_type = disk.attrib['device'] + return { 'dev': dev_name, + 'type': dev_type, + 'path': path} + else: + msg = 'The storage device "%s" does not exist in the guest "%s"' % ( + dev_name,vm_name) + raise NotFoundError(msg) +

Just a suggestion: In a if/else block statement, try to put first the smaller block and return after it That way we avoid one level indentation in the bigger block

Example:

if disk is None: reutrn NotFoundError()

source = disk.find('./source') ...

Agree

...

+ def delete(self, vm_name, dev_name): + # Get storage device xml + disk = self._get_device_xml(vm_name, dev_name) + if disk is None: + msg = 'The storage device "%s" does not exist in the guest "%s"' % ( + dev_name,vm_name) + raise NotFoundError(msg) + try: + conn = self.conn.get() + dom = conn.lookupByName(vm_name) + dom.detachDeviceFlags(ElementTree.tostring(disk), + libvirt.VIR_DOMAIN_AFFECT_CURRENT) + except Exception as e: + msg = 'Was not possible to detach storage device: %s' % e.message + kimchi_log.error(msg) + raise OperationFailed(e.message) + + def update(self, vm_name, dev_name, params): + try: + info = self.lookup(vm_name, dev_name) + self.delete(vm_name, dev_name) + info.update(params) + kargs = {'conn': self.conn} + stgModel = StoragesModel(**kargs) + dev_name = stgModel.create(vm_name, info) + return dev_name + except Exception as e: + raise + msg = 'Was not possible to update storage device: %s' % e.message + kimchi_log.error(msg) + raise OperationFailed(e.message)

In case of failure you should recreate the old instance. Otherwise, the vm will loose a cdrom/disk I suggest:

try: # create new storage except: # raise error

# delete old storage

I cannot create before delete, because storage devices are going to have the same name, and then conflict. I am going to improve this code.

...

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")

_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel