On 07/15/2014 06:11 AM, lvroyce@linux.vnet.ibm.com wrote:

From: Royce Lv <lvroyce@linux.vnet.ibm.com>

Image file probe will be used in identify image file os info and generate reasonable configuration for it. This will be useful when import image and create a vm from it.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/README.md | 9 ++++++--- src/kimchi/exception.py | 4 ++++ src/kimchi/i18n.py | 4 ++++ src/kimchi/imageinfo.py | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 src/kimchi/imageinfo.py

diff --git a/docs/README.md b/docs/README.md index ab03918..24537e1 100644 --- a/docs/README.md +++ b/docs/README.md @@ -54,7 +54,8 @@ Install Dependencies qemu-kvm python-psutil python-ethtool sos \ python-ipaddr python-lxml nfs-utils \ iscsi-initiator-utils libxslt pyparted nginx \ - policycoreutils-python + policycoreutils-python python-libguestfs \ + libguestfs-tools # If using RHEL6, install the following additional packages: $ sudo yum install python-unittest2 python-ordereddict # Restart libvirt to allow configuration changes to take effect @@ -76,7 +77,8 @@ for more information on how to configure your system to access this repository. python-pam python-m2crypto python-jsonschema \ qemu-kvm libtool python-psutil python-ethtool \ sosreport python-ipaddr python-lxml nfs-common \ - open-iscsi lvm2 xsltproc python-parted nginx firewalld + open-iscsi lvm2 xsltproc python-parted nginx \ + firewalld python-guestfs libguestfs-tools

Packages version requirement: python-jsonschema >= 1.3.0 @@ -90,7 +92,8 @@ for more information on how to configure your system to access this repository. python-pam python-M2Crypto python-jsonschema \ rpm-build kvm python-psutil python-ethtool \ python-ipaddr python-lxml nfs-client open-iscsi \ - libxslt-tools python-xml python-parted + libxslt-tools python-xml python-parted \ + python-libguestfs guestfs-tools

You need to update spec files and DEBIAN/control.in to add this new dependency

Packages version requirement: python-psutil >= 0.6.0 diff --git a/src/kimchi/exception.py b/src/kimchi/exception.py index fcf60cc..6b4c913 100644 --- a/src/kimchi/exception.py +++ b/src/kimchi/exception.py @@ -89,5 +89,9 @@ class IsoFormatError(KimchiException): pass

+class ImageFormatError(KimchiException): + pass + + class TimeoutExpired(KimchiException): pass diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 91684b6..daeeed8 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -61,6 +61,10 @@ messages = { "'%(user)s' to the ISO path group, or (not recommended) 'chmod -R o+x 'path_to_iso'." "Details: %(err)s" ),

+ "KCHIMG0001E": _("Error occurs when probing image os information."), + "KCHIMG0002E": _("No os information found in given image."),

Use "OS" (in caps lock) instead of "os"

+ "KCHIMG0003E": _("Unable to find/read image file %(filename)s"), + "KCHVM0001E": _("Virtual machine %(name)s already exists"), "KCHVM0002E": _("Virtual machine %(name)s does not exist"), "KCHVM0003E": _("Unable to rename virtual machine %(name)s. The name %(new_name)s already exists or it is not powered off."), diff --git a/src/kimchi/imageinfo.py b/src/kimchi/imageinfo.py new file mode 100644 index 0000000..26a9480 --- /dev/null +++ b/src/kimchi/imageinfo.py @@ -0,0 +1,48 @@ +# +# Kimchi +# +# Copyright IBM Corp, 2013 +# +# 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 + +import os +import sys +import guestfs + +from kimchi.exception import ImageFormatError + +def probe_image(image_path): + g = guestfs.GuestFS(python_return_dict=True) + g.add_drive_opts(image_path, readonly=1) + g.launch() + if not os.access(image_path, os.R_OK): + raise ImageFormatError("KCHIMG0003E", {'filename': image_path}) + try: + roots = g.inspect_os() + except: + raise ImageFormatError("KCHIMG0001E") + if len(roots) == 0: + raise ImageFormatError("KCHIMG0002E") + + for root in roots: + version = "%d.%d" % (g.inspect_get_major_version(root), + g.inspect_get_minor_version(root)) + distro = "%s" % (g.inspect_get_distro(root)) + + return (distro, version) + + +if __name__ == '__main__': + print probe_image(sys.argv[1])

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 07/15/2014 06:11 AM, lvroyce@linux.vnet.ibm.com wrote:

From: Royce Lv <lvroyce@linux.vnet.ibm.com>

Add 'base' to 'disks' param to create template, so that we can support create template from image.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 3 ++- src/kimchi/API.json | 8 +++++++- src/kimchi/i18n.py | 1 + 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/docs/API.md b/docs/API.md index 4f51dd0..a261762 100644 --- a/docs/API.md +++ b/docs/API.md @@ -174,7 +174,7 @@ Represents a snapshot of the Virtual Machine's primary monitor. * cpus *(optional)*: The number of CPUs assigned to the VM. Default is 1. * memory *(optional)*: The amount of memory assigned to the VM. Default is 1024M. - * cdrom *(required)*: A volume name or URI to an ISO image. + * cdrom *(optional)*: A volume name or URI to an ISO image. * storagepool *(optional)*: URI of the storagepool. Default is '/storagepools/default' * networks *(optional)*: list of networks will be assigned to the new VM. @@ -183,6 +183,7 @@ Represents a snapshot of the Virtual Machine's primary monitor. (either *size* or *volume* must be specified): * index: The device index * size: The device size in GB + * base: Base image of this disk

* graphics *(optional)*: The graphics paramenters of this template * type: The type of graphics. It can be VNC or spice or None. diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 6d1324c..60deeb4 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -362,7 +362,6 @@ "description": "Path for cdrom", "type": "string", "pattern": "^((/)|(http)[s]?:|[t]?(ftp)[s]?:)+.*$", - "required": true, "error": "KCHTMPL0014E" }, "disks": { @@ -381,7 +380,14 @@ "type": "number", "minimum": 1, "error": "KCHTMPL0022E" + }, + "base": { + "description": "Base image of the disk", + "type": "string", + "pattern": "^/.+$", + "error": "KCHTMPL0023E" } + } }, "minItems": 1, diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index daeeed8..7a7b675 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -125,6 +125,7 @@ messages = { "KCHTMPL0020E": _("Unable to create template due error: %(err)s"), "KCHTMPL0021E": _("Unable to delete template due error: %(err)s"), "KCHTMPL0022E": _("Disk size must be greater than 1GB."), + "KCHTMPL0023E": _("Template base image must be a valid local image file"),

"KCHPOOL0001E": _("Storage pool %(name)s already exists"), "KCHPOOL0002E": _("Storage pool %(name)s does not exist"),

On 07/15/2014 06:11 AM, lvroyce@linux.vnet.ibm.com wrote:

From: Royce Lv <lvroyce@linux.vnet.ibm.com>

Multiple files modification for change 'cdrom' to optional param.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/control/templates.py | 2 +- src/kimchi/i18n.py | 2 +- src/kimchi/mockmodel.py | 27 +++++++++++++++++++++------ src/kimchi/model/templates.py | 13 ++++++++----- src/kimchi/vmtemplate.py | 12 ++++++++++-- tests/test_rest.py | 2 +- 6 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/src/kimchi/control/templates.py b/src/kimchi/control/templates.py index a535960..e16cc1a 100644 --- a/src/kimchi/control/templates.py +++ b/src/kimchi/control/templates.py @@ -47,7 +47,7 @@ class Template(Resource): 'os_version': self.info['os_version'], 'cpus': self.info['cpus'], 'memory': self.info['memory'], - 'cdrom': self.info['cdrom'], + 'cdrom': self.info.get('cdrom', None), 'disks': self.info['disks'], 'storagepool': self.info['storagepool'], 'networks': self.info['networks'], diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 7a7b675..3ba1e23 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -118,7 +118,7 @@ messages = { "KCHTMPL0013E": _("Amount of memory (MB) must be an integer greater than 512"), "KCHTMPL0014E": _("Template CDROM must be a local or remote ISO file"), "KCHTMPL0015E": _("Invalid storage pool URI %(value)s specified for template"), - "KCHTMPL0016E": _("Specify an ISO image as CDROM to create a template"), + "KCHTMPL0016E": _("Specify an ISO image as CDROM or a base image to create a template"), "KCHTMPL0017E": _("All networks for the template must be specified in a list."), "KCHTMPL0018E": _("Must specify a volume to a template, when storage pool is iscsi or scsi"), "KCHTMPL0019E": _("The volume: %(volume)s in not in storage pool %(pool)s"), diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index a0920e0..cea3ce4 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -200,8 +200,10 @@ class MockModel(object): index += 1

cdrom = "hd" + string.ascii_lowercase[index + 1] - cdrom_params = {'dev': cdrom, 'path': t_info['cdrom'], 'type': 'cdrom'} - vm.storagedevices[cdrom] = MockVMStorageDevice(cdrom_params) + if t_info.get('cdrom'): + cdrom_params = { + 'dev': cdrom, 'path': t_info['cdrom'], 'type': 'cdrom'} + vm.storagedevices[cdrom] = MockVMStorageDevice(cdrom_params)

self._mock_vms[name] = vm return name @@ -237,11 +239,24 @@ class MockModel(object):

def templates_create(self, params): name = params.get('name', '').strip() + iso = params.get('cdrom') if not name: - iso = params['cdrom'] - iso_name = os.path.splitext(iso[iso.rfind('/') + 1:])[0] - name = iso_name + str(int(time.time() * 1000)) - params['name'] = name + if iso: + iso_name = os.path.splitext(iso[iso.rfind('/') + 1:])[0] + name = iso_name + str(int(time.time() * 1000)) + params['name'] = name + else: + raise InvalidParameter("KCHTMPL0008E") + + base_tmpl = False + if not iso: + if 'disks' in params: + for d in params['disks']: + if 'base' in d: + base_tmpl = True + break + if not base_tmpl: + raise InvalidParameter("KCHTMPL0016E")

if name in self._mock_templates: raise InvalidOperation("KCHTMPL0001E", {'name': name}) diff --git a/src/kimchi/model/templates.py b/src/kimchi/model/templates.py index 60f4de5..0e07a78 100644 --- a/src/kimchi/model/templates.py +++ b/src/kimchi/model/templates.py @@ -40,9 +40,9 @@ class TemplatesModel(object):

def create(self, params): name = params.get('name', '').strip() - iso = params['cdrom'] + iso = params.get('cdrom') # check search permission - if iso.startswith('/') and os.path.isfile(iso): + if iso and iso.startswith('/') and os.path.isfile(iso): user = UserTests().probe_user() ret, excp = probe_file_permission_as_user(iso, user) if ret is False: @@ -51,9 +51,12 @@ class TemplatesModel(object): 'err': excp})

if not name: - iso_name = os.path.splitext(iso[iso.rfind('/') + 1:])[0] - name = iso_name + str(int(time.time() * 1000)) - params['name'] = name + if iso: + iso_name = os.path.splitext(iso[iso.rfind('/') + 1:])[0] + name = iso_name + str(int(time.time() * 1000)) + params['name'] = name + else: + raise InvalidParameter('KCHTMPL0008E')

conn = self.conn.get() pool_uri = params.get(u'storagepool', '') diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py index 8d5217a..91617d9 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -54,6 +54,14 @@ class VMTemplate(object): iso_distro = iso_version = 'unknown' iso = args.get('cdrom', '')

+ # if ISO not specified and base disk image specified, + # prevent cdrom from filling automatically + if len(iso) == 0 and 'disks' in args: + for d in args['disks']: + if 'base' in d: + args['cdrom'] = '' + break +

The same I commented in the previous patch. This code should not be needed. osinfo.py should not set/change the cdrom value I checked the code and see; params['cdrom'] = isolinks.get(distro, {}).get(version, '') And isolinks is a dict of remote ISOs The pre-configured remote ISOs should come through JSON files under /distros Seems this code is old and should be removed from osinfo.py

if scan and len(iso) > 0: iso_distro, iso_version = self.get_iso_info(iso) if not iso.startswith('/'): @@ -414,8 +422,8 @@ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/>

# validate iso integrity # FIXME when we support multiples cdrom devices - iso = self.info['cdrom'] - if not (os.path.isfile(iso) or check_url_path(iso)): + iso = self.info.get('cdrom') + if iso and not (os.path.isfile(iso) or check_url_path(iso)): invalid['cdrom'] = [iso]

self.info['invalid'] = invalid diff --git a/tests/test_rest.py b/tests/test_rest.py index ad8fc72..5d84e85 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -1068,7 +1068,7 @@ class RestTests(unittest.TestCase): self.assertEquals(200, resp.status) self.assertEquals(0, len(json.loads(resp.read())))

- # Create a template without cdrom fails with 400 + # Create a template without cdrom and disk specified fails with 400 t = {'name': 'test', 'os_distro': 'ImagineOS', 'os_version': '1.0', 'memory': 1024, 'cpus': 1, 'storagepool': '/storagepools/alt'}