From: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com&gt; This patch changes API.md with new storage sub-collection/sub-resource information. Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com&gt; --- docs/API.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/API.md b/docs/API.md index 48a293f..b1a49cd 100644 --- a/docs/API.md +++ b/docs/API.md @@ -115,6 +115,27 @@ 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 or virtual drive to specified virtual machine. + * 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 -- 1.8.3.1

Hello Royce, On 02/12/2014 01:46 AM, Royce Lv wrote: I agree that the "eject" function is better represented by a PUT with blank path. I'll make the required adjustments to reflect this. About the removal of a CDROM device, I agree that it is kind of rare too. But shouldn't we support it anyway? I am a little worried about removing this support just to later in the road someone miss it and then ask to add it again. I've read your comments about providing support to CDROM only and forget about hard disks. I haven't participated in the RFC or any discussion about the design of this feature, but at this point, and I beg your pardon if I sound "lazy", it is easier and faster to just leave hard disk support in this contribution than to chop the patches and make it CDROM only. If we realize that the hard disk support provided in these patches are incomplete, we can just complement it with further patches in the future. How does that sound to you?

On 2014年02月12日 19:24, Daniel H Barboza wrote: OK I'm OK with inner logic imcomplete if we do not declare officially support it, we can complete it in the future. I mean at least from the rest API we have to prevent users to call it, if wrongly used, that will make actual damage to their system.

From: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com&gt; This patch adds the control classes for guest storages Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com&gt; API.json: adding virtio device pattern Signed-off-by: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com&gt; --- 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 f025661..8ea9de7 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -296,6 +296,39 @@ }, "additionalProperties": false }, + "storages_create": { + "type": "object", + "properties": { + "dev": { + "description": "The storage device name", + "type": "string", + "pattern": "^hd[b-z]|vd[a-z]$" + }, + "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]?:)+.*$", + "required": true + } + } + }, + "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..953620f --- /dev/null +++ b/src/kimchi/control/vm/storages.py @@ -0,0 +1,49 @@ +# +# Project Kimchi +# +# Copyright IBM, Corp. 2013 +# +# Authors: +# ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt; +# +# 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 -- 1.8.3.1

From: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com&gt; This patch adds the CREATE, LOOKUP, UPDATE and DELETE functionalities to guest storage devices support. Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com&gt; Using lxml and objectify to parse the XML info. Pep8 adjustments in model/vms.py Signed-off-by: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com&gt; --- src/kimchi/model/vms.py | 184 ++++++++++++++++++++++++++++++++++++++++++++++-- src/kimchi/xmlutils.py | 5 ++ 2 files changed, 185 insertions(+), 4 deletions(-) diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 1244eeb..3c2a661 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -20,13 +20,17 @@ # 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 string import time +import urlparse import uuid -from xml.etree import ElementTree -import libvirt from cherrypy.process.plugins import BackgroundTask +from lxml import etree, objectify +from xml.etree import ElementTree from kimchi import vnc from kimchi import xmlutils @@ -36,7 +40,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 +52,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 = {} @@ -222,7 +231,7 @@ class VMModel(object): dom = self.get_vm(name, self.conn) dom = self._static_vm_update(dom, params) self._live_vm_update(dom, params) - return dom.name().decode('utf-8') + return dom.name() def _static_vm_update(self, dom, params): state = DOM_STATE_MAP[dom.info()[0]] @@ -453,3 +462,170 @@ 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> + """ + 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 valid" + 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' + + # Use device name passed or pick next + dev_name = params.get('dev') + if not dev_name: + params['dev'] = self._get_storage_device_name(vm_name) + + # 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_storage_device_name(self, vm_name): + dev_list = [dev for dev in self.get_list(vm_name) + if dev.startswith('hd')] + if len(dev_list) == 0: + return 'hda' + 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 + return 'hd' + string.ascii_lowercase[next_dev_letter_pos] + + def get_list(self, vm_name): + dom = VMModel.get_vm(vm_name, self.conn) + xml = dom.XMLDesc(0) + devices = objectify.fromstring(xml).devices + storages = [disk.target.attrib['dev'] + for disk in devices.xpath("./disk[@device='disk']")] + storages += [disk.target.attrib['dev'] + for disk in devices.xpath("./disk[@device='cdrom']")] + return storages + + +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) + xml = dom.XMLDesc(0) + devices = objectify.fromstring(xml).devices + disk = devices.xpath("./disk/target[@dev='%s']/.." % dev_name) + return disk[0] + + 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 None: + raise NotFoundError('The storage device "%s" does not exist in ' + 'the guest "%s"' % (dev_name, vm_name)) + source = disk.source + path = "" + if source is not None: + src_type = disk.attrib['type'] + if src_type == 'network': + host = source.host + path = (source.attrib['protocol'] + '://' + + host.attrib['name'] + ':' + + host.attrib['port'] + source.attrib['name']) + else: + path = source.attrib[DEV_TYPE_SRC_ATTR_MAP[src_type]] + dev_type = disk.attrib['device'] + return {'dev': dev_name, + 'type': dev_type, + 'path': path} + + def delete(self, vm_name, dev_name): + # Get storage device xml + disk = self._get_device_xml(vm_name, dev_name) + if disk is None: + raise NotFoundError('The storage device "%s" does not exist in ' + 'the guest "%s"' % (dev_name, vm_name)) + try: + conn = self.conn.get() + dom = conn.lookupByName(vm_name) + dom.detachDeviceFlags(etree.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): + info = self.lookup(vm_name, dev_name) + backup_params = info.copy() + try: + self.delete(vm_name, dev_name) + except: + msg = 'Was not possible to update storage device: %s' % e.message + kimchi_log.error(msg) + raise OperationFailed(e.message) + + info.update(params) + kargs = {'conn': self.conn} + stgModel = StoragesModel(**kargs) + try: + dev_name = stgModel.create(vm_name, info) + return dev_name + except Exception as e: + # Restoring previous device + dev_name = stgModel.create(vm_name, backup_params) + msg = 'Was not possible to update storage device: %s' % e.message + kimchi_log.error(msg) + raise OperationFailed(e.message) 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.3.1

On 02/12/2014 07:08 AM, Royce Lv wrote: I tried, but couldn't came up with a code using lxml.builder and/or objectify which is simpler and more readable than this one. If anyone wants to jump in and provide such code, please do :)

Am 12-02-2014 00:38, schrieb Daniel Barboza: The import order is defined as: 1) standard libraries; 2) third party libraries; 3) Kimchi libraries. So libvirt should be on the second group, as it was before, and the XML packages should be on the first group (they're python standard libraries, right?). Top-level elements (like the function "xml_get_child") should have two blank lines between themselves.

And the model test? On 2014年02月12日 10:38, Daniel Barboza wrote: