On 12/31/2013 12:24 AM, Royce Lv wrote: Ack.

From: Aline Manera <alinefm(a)br.ibm.com&gt; All Kimchi exception should be in exception.py So move IsoFormatError() to there and update imports accordingly. Signed-off-by: Aline Manera <alinefm(a)br.ibm.com&gt; --- src/kimchi/exception.py | 4 ++++ src/kimchi/isoinfo.py | 5 +---- src/kimchi/model.py | 6 +++--- src/kimchi/vmtemplate.py | 4 ++-- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/kimchi/exception.py b/src/kimchi/exception.py index d7a2835..8143b05 100644 --- a/src/kimchi/exception.py +++ b/src/kimchi/exception.py @@ -34,3 +34,7 @@ class InvalidParameter(Exception): class InvalidOperation(Exception): pass + + +class IsoFormatError(Exception): + pass diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py index 59cdd45..7b7fa78 100644 --- a/src/kimchi/isoinfo.py +++ b/src/kimchi/isoinfo.py @@ -28,6 +28,7 @@ import sys import urllib2 +from kimchi.exception import IsoFormatError from kimchi.utils import kimchi_log iso_dir = [ @@ -117,10 +118,6 @@ iso_dir = [ ] -class IsoFormatError(Exception): - pass - - class IsoImage(object): """ Scan an iso9660 image to extract the Volume ID and check for boot-ability diff --git a/src/kimchi/model.py b/src/kimchi/model.py index a6790b8..decb889 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -62,8 +62,8 @@ from kimchi import vnc from kimchi import xmlutils from kimchi.asynctask import AsyncTask from kimchi.distroloader import DistroLoader -from kimchi.exception import InvalidOperation, InvalidParameter, MissingParameter -from kimchi.exception import NotFoundError, OperationFailed +from kimchi.exception import InvalidOperation, InvalidParameter, IsoFormatError +from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.featuretests import FeatureTests from kimchi.networkxml import to_network_xml from kimchi.objectstore import ObjectStore @@ -1167,7 +1167,7 @@ class Model(object): try: os_distro, os_version = isoinfo.probe_one(path) bootable = True - except isoinfo.IsoFormatError: + except IsoFormatError: bootable = False res.update( dict(os_distro=os_distro, os_version=os_version, path=path, bootable=bootable)) diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py index dd43faa..8bf3df9 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -29,7 +29,7 @@ import urlparse from kimchi import isoinfo from kimchi import osinfo -from kimchi.exception import InvalidParameter +from kimchi.exception import InvalidParameter, IsoFormatError QEMU_NAMESPACE = "xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'" @@ -64,7 +64,7 @@ class VMTemplate(object): try: iso_distro, iso_version = isoinfo.probe_one(iso) - except isoinfo.IsoFormatError, e: + except IsoFormatError, e: raise InvalidParameter(e) # Fetch defaults based on the os distro and version -- 1.7.10.4

Reviewed-by: Royce Lv&lt;lvroyce(a)linux.vnet.ibm.com&gt; On 2013年12月31日 01:09, Aline Manera wrote:

On 12/30/2013 03:09 PM, Aline Manera wrote: to avoid make to many indentations, you can do: self.remote = remote if remote == None: self.remote = self._is_iso_remote() i think it's more clean -- Ramon Nunes Medeiros Software Engineer - Linux Technology Center Brazil IBM Systems & Technology Group Phone : +55 19 2132 7878 ramonn(a)br.ibm.com

On 2013年12月31日 01:09, Aline Manera wrote: I kind of want this func moved outside for reusage of other module. I think if we want to move remote url probe inside IsoImage class, is there any need to pass 'remote' param outside? We can just dump it. At first we have probe_one and probe_iso originally to serve use cases of just probe one iso and probe all isos under a directory, now that you are refactor this part, I suggest that we dump this probe_one function, and all use probe_iso.

On 2014年01月03日 03:44, Aline Manera wrote: Actually I'm asking it because part I'm doing (change CDROM), I will depend on this function to validate path almost same as remote/local iso except my path doesn't have to be an ISO, so put in utils.py can avoid duplicate code.

From: Aline Manera <alinefm(a)br.ibm.com&gt; That way all operations related to IsoImage() object will be in this class. Signed-off-by: Aline Manera <alinefm(a)br.ibm.com&gt; --- src/kimchi/isoinfo.py | 57 +++++++++++++++++++--------------------------- src/kimchi/model.py | 5 ++-- src/kimchi/scan.py | 5 ++-- src/kimchi/vmtemplate.py | 4 +++- 4 files changed, 33 insertions(+), 38 deletions(-) diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py index dd26204..6f3ad7d 100644 --- a/src/kimchi/isoinfo.py +++ b/src/kimchi/isoinfo.py @@ -165,6 +165,26 @@ class IsoImage(object): return True + def probe(self): + if not self.bootable: + raise IsoFormatError("ISO %s not bootable" % self.path) + + matcher = Matcher(self.volume_id) + + for d, v, regex in iso_dir: + if matcher.search(regex): + distro = d + if hasattr(v, '__call__'): + version = v(matcher) + else: + version = v + return (distro, version) + + kimchi_log.debug("probe_iso: Unable to identify ISO %s with Volume ID: %s" + % (self.path, self.volume_id)) + + return ('unknown', 'unknown') + def _unpack(self, s, data): return s.unpack(data[:s.size]) @@ -270,33 +290,6 @@ class Matcher(object): return self.lastmatch.group(num) -def _probe_iso(fname, remote = None): - try: - iso = IsoImage(fname, remote) - except Exception, e: - kimchi_log.warning("probe_iso: Error processing ISO image: %s\n%s" % - (fname, e)) - raise IsoFormatError(e) - - if not iso.bootable: - raise IsoFormatError("ISO %s not bootable" % fname) - - matcher = Matcher(iso.volume_id) - - for d, v, regex in iso_dir: - if matcher.search(regex): - distro = d - if hasattr(v, '__call__'): - version = v(matcher) - else: - version = v - return (distro, version) - - kimchi_log.debug("probe_iso: Unable to identify ISO %s with Volume ID: %s" - % (fname, iso.volume_id)) - return ('unknown', 'unknown') - - def probe_iso(status_helper, params): loc = params['path'].encode("utf-8") updater = params['updater'] @@ -322,22 +315,20 @@ def probe_iso(status_helper, params): continue iso = os.path.join(root, name) try: - ret = _probe_iso(iso, False) + iso_img = IsoImage(iso) + ret = iso_img.probe() update_result(iso, ret) except: continue else: - ret = _probe_iso(loc) + iso_img = IsoImage(loc) + ret = iso_img.probe() update_result(loc, ret) if status_helper != None: status_helper('', True) -def probe_one(iso): - return _probe_iso(iso) - - if __name__ == '__main__': iso_list = [] def updater(iso_info): diff --git a/src/kimchi/model.py b/src/kimchi/model.py index decb889..3547b73 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -55,7 +55,6 @@ except ImportError: from kimchi import config -from kimchi import isoinfo from kimchi import netinfo from kimchi import network as knetwork from kimchi import vnc @@ -65,6 +64,7 @@ from kimchi.distroloader import DistroLoader from kimchi.exception import InvalidOperation, InvalidParameter, IsoFormatError from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.featuretests import FeatureTests +from kimchi.isoinfo import IsoImage from kimchi.networkxml import to_network_xml from kimchi.objectstore import ObjectStore from kimchi.scan import Scanner @@ -1165,7 +1165,8 @@ class Model(object): path = os.path.join(os.path.dirname(path), os.readlink(path)) os_distro = os_version = 'unknown' try: - os_distro, os_version = isoinfo.probe_one(path) + iso_img = IsoImage(path) + os_distro, os_version = iso_img.probe() bootable = True except IsoFormatError: bootable = False diff --git a/src/kimchi/scan.py b/src/kimchi/scan.py index 27be1da..e192f01 100644 --- a/src/kimchi/scan.py +++ b/src/kimchi/scan.py @@ -29,7 +29,7 @@ import tempfile import time -from kimchi.isoinfo import probe_iso, probe_one +from kimchi.isoinfo import IsoImage, probe_iso from kimchi.utils import kimchi_log @@ -73,7 +73,8 @@ class Scanner(object): duplicates = "%s/%s*" % (params['pool_path'], iso_name) for f in glob.glob(duplicates): - if (iso_info['distro'], iso_info['version']) == probe_one(f): + iso_img = IsoImage(f) + if (iso_info['distro'], iso_info['version']) == iso_img.probe(): return iso_path = iso_name + hashlib.md5(iso_info['path']).hexdigest() + '.iso' diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py index 8bf3df9..12da0d1 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -30,6 +30,7 @@ import urlparse from kimchi import isoinfo from kimchi import osinfo from kimchi.exception import InvalidParameter, IsoFormatError +from kimchi.isoinfo import IsoImage QEMU_NAMESPACE = "xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'" @@ -63,7 +64,8 @@ class VMTemplate(object): self.info.update({'iso_stream': True}) try: - iso_distro, iso_version = isoinfo.probe_one(iso) + iso_img = IsoImage(iso) + iso_distro, iso_version = iso_img.probe() except IsoFormatError, e: raise InvalidParameter(e) -- 1.7.10.4

Reviewed-by: Royce Lv&lt;lvroyce(a)linux.vnet.ibm.com&gt; On 2013年12月31日 01:09, Aline Manera wrote: You really read my mind!