From: Aline Manera <alinefm(a)br.ibm.com&gt; ignore_list parameter was added to avoid displaying duplicated results during deep scan. So isoinfo main program must be updated accordingly or it will raise the following error: $ python kimchi/isoinfo.py http://localhost/Fedora-Live-Desktop-x86_64-19-1.iso Traceback (most recent call last): File "kimchi/isoinfo.py", line 343, in <module> probe_iso(None, dict(path=sys.argv[1], updater=updater)) File "kimchi/isoinfo.py", line 283, in probe_iso ignore_list = params['ignore_list'] KeyError: 'ignore_list' Signed-off-by: Aline Manera <alinefm(a)br.ibm.com&gt; --- src/kimchi/isoinfo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py index f76fd90..3d57533 100644 --- a/src/kimchi/isoinfo.py +++ b/src/kimchi/isoinfo.py @@ -280,7 +280,7 @@ def probe_iso(status_helper, params): loc = params['path'].encode("utf-8") updater = params['updater'] ignore = False - ignore_list = params['ignore_list'] + ignore_list = params.get('ignore_list', []) def update_result(iso, ret): if ret != ('unknown', 'unknown'):

From: Aline Manera <alinefm(a)br.ibm.com&gt; When running isoinfo main program to probe a remote ISO file, it returned a wrong path: $ python kimchi/isoinfo.py http://localhost/Fedora-Live-Desktop-x86_64-19-1.iso [{'path': '/home/alinefm/kimchi/src/http:/localhost/Fedora-Live-Desktop-x86_64-19-1.iso', 'version': '19', 'distro': 'fedora'}] So verify the ISO is a local one and only use abspath() in this case. Otherwise, use the path entried by user. Signed-off-by: Aline Manera <alinefm(a)br.ibm.com&gt; --- src/kimchi/isoinfo.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py index 3d57533..121241a 100644 --- a/src/kimchi/isoinfo.py +++ b/src/kimchi/isoinfo.py @@ -284,8 +284,8 @@ def probe_iso(status_helper, params): def update_result(iso, ret): if ret != ('unknown', 'unknown'): - iso = os.path.abspath(iso) - updater({'path': iso, 'distro': ret[0], 'version': ret[1]}) + path = os.path.abspath(iso) if os.path.isfile(iso) else iso + updater({'path': path, 'distro': ret[0], 'version': ret[1]}) if os.path.isdir(loc): for root, dirs, files in os.walk(loc):

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 df9619f..bff0a18 100644 --- a/src/kimchi/exception.py +++ b/src/kimchi/exception.py @@ -39,3 +39,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 121241a..7d919a0 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 a21fcf7..f48f3d3 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.iscsi import TargetClient from kimchi.networkxml import to_network_xml @@ -1184,7 +1184,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 5d31f2a..1720384 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

From: Aline Manera <alinefm(a)br.ibm.com&gt; That way we will have a single point to check ISO path validation. And the user doesn't need to care about it as IsoImage() will check if the ISO path is local, remote or invalid. Signed-off-by: Aline Manera <alinefm(a)br.ibm.com&gt; --- src/kimchi/isoinfo.py | 42 ++++++++++++++++-------------------------- src/kimchi/utils.py | 12 ++++++++++++ 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py index 7d919a0..f93ff0e 100644 --- a/src/kimchi/isoinfo.py +++ b/src/kimchi/isoinfo.py @@ -29,7 +29,7 @@ import urllib2 from kimchi.exception import IsoFormatError -from kimchi.utils import kimchi_log +from kimchi.utils import check_url_path, kimchi_log iso_dir = [ ## @@ -134,13 +134,22 @@ class IsoImage(object): EL_TORITO_VALIDATION_ENTRY = struct.Struct("=BBH24sHBB") EL_TORITO_BOOT_ENTRY = struct.Struct("=BBHBBHL20x") - def __init__(self, path, remote = False): + def __init__(self, path): self.path = path + self.remote = self._is_iso_remote() self.volume_id = None self.bootable = False - self.remote = remote self._scan() + def _is_iso_remote(self): + if os.path.isfile(self.path): + return False + + if check_url_path(self.path): + return True + + raise IsoFormatError('ISO %s does not exist' % self.path) + def _unpack(self, s, data): return s.unpack(data[:s.size]) @@ -246,9 +255,9 @@ class Matcher(object): return self.lastmatch.group(num) -def _probe_iso(fname, remote = False): +def _probe_iso(fname): try: - iso = IsoImage(fname, remote) + iso = IsoImage(fname) except Exception, e: kimchi_log.warning("probe_iso: Error processing ISO image: %s\n%s" % (fname, e)) @@ -302,35 +311,16 @@ def probe_iso(status_helper, params): update_result(iso, ret) except: continue - elif os.path.isfile(loc): - ret = _probe_iso(loc, False) - update_result(loc, ret) else: - ret = _probe_iso(loc, True) + ret = _probe_iso(loc) update_result(loc, ret) if status_helper != None: status_helper('', True) -def _check_url_path(path): - try: - code = urllib2.urlopen(path).getcode() - if code != 200: - return False - except (urllib2.HTTPError, ValueError): - return False - - return True def probe_one(iso): - if os.path.isfile(iso): - remote = False - elif _check_url_path(iso): - remote = True - else: - raise IsoFormatError('ISO %s does not exist' % iso) - - return _probe_iso(iso, remote) + return _probe_iso(iso) if __name__ == '__main__': diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index b6d84fd..af245c6 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -23,6 +23,7 @@ import cherrypy import os +import urllib2 from cherrypy.lib.reprconf import Parser @@ -84,3 +85,14 @@ def import_class(class_path): def import_module(module_name): return __import__(module_name, globals(), locals(), ['']) + + +def check_url_path(path): + try: + code = urllib2.urlopen(path).getcode() + if code != 200: + return False + except (urllib2.URLError, ValueError): + return False + + return True

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 f93ff0e..a992a68 100644 --- a/src/kimchi/isoinfo.py +++ b/src/kimchi/isoinfo.py @@ -150,6 +150,26 @@ class IsoImage(object): raise IsoFormatError('ISO %s does not exist' % self.path) + 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]) @@ -255,33 +275,6 @@ class Matcher(object): return self.lastmatch.group(num) -def _probe_iso(fname): - try: - iso = IsoImage(fname) - 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'] @@ -307,22 +300,20 @@ def probe_iso(status_helper, params): continue iso = os.path.join(root, name) try: - ret = _probe_iso(iso) + 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 f48f3d3..7115583 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 @@ -66,6 +65,7 @@ from kimchi.exception import InvalidOperation, InvalidParameter, IsoFormatError from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.featuretests import FeatureTests from kimchi.iscsi import TargetClient +from kimchi.isoinfo import IsoImage from kimchi.networkxml import to_network_xml from kimchi.objectstore import ObjectStore from kimchi.scan import Scanner @@ -1182,7 +1182,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 1720384..69af6ec 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)

From: Aline Manera <alinefm(a)br.ibm.com&gt; This patch cleans up pep8 style issue in isoinfo.py Signed-off-by: Aline Manera <alinefm(a)br.ibm.com&gt; --- Makefile.am | 1 + src/kimchi/isoinfo.py | 27 ++++++++++++++++----------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/Makefile.am b/Makefile.am index 7ab1bd8..04ad696 100644 --- a/Makefile.am +++ b/Makefile.am @@ -51,6 +51,7 @@ PEP8_WHITELIST = \ src/kimchi/exception.py \ src/kimchi/featuretests.py \ src/kimchi/iscsi.py \ + src/kimchi/isoinfo.py \ src/kimchi/kvmusertests.py \ src/kimchi/rollbackcontext.py \ src/kimchi/root.py \ diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py index a992a68..7e6d733 100644 --- a/src/kimchi/isoinfo.py +++ b/src/kimchi/isoinfo.py @@ -18,7 +18,7 @@ # # 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 +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA import glob import os @@ -31,6 +31,7 @@ import urllib2 from kimchi.exception import IsoFormatError from kimchi.utils import check_url_path, kimchi_log + iso_dir = [ ## # Portions of this data from libosinfo: http://libosinfo.org/ @@ -165,8 +166,8 @@ class IsoImage(object): version = v return (distro, version) - kimchi_log.debug("probe_iso: Unable to identify ISO %s with Volume ID: %s", - self.path, self.volume_id) + msg = "probe_iso: Unable to identify ISO %s with Volume ID: %s" + kimchi_log.debug(msg, self.path, self.volume_id) return ('unknown', 'unknown') @@ -177,9 +178,9 @@ class IsoImage(object): """ Search the Volume Descriptor Table for an El Torito boot record. If found, the boot record will provide a link to a boot catalogue. The - first entry in the boot catalogue is a validation entry. The next entry - contains the default boot entry. The default boot entry will indicate - whether the image is considered bootable. + first entry in the boot catalogue is a validation entry. The next + entry contains the default boot entry. The default boot entry will + indicate whether the image is considered bootable. """ vd_type = -1 for i in xrange(1, 4): @@ -199,7 +200,8 @@ class IsoImage(object): raise IsoFormatError("Invalid El Torito boot record") offset = IsoImage.SECTOR_SIZE * boot_cat - size = IsoImage.EL_TORITO_VALIDATION_ENTRY.size + IsoImage.EL_TORITO_BOOT_ENTRY.size + size = IsoImage.EL_TORITO_VALIDATION_ENTRY.size + \ + IsoImage.EL_TORITO_BOOT_ENTRY.size data = self._get_iso_data(offset, size) fmt = IsoImage.EL_TORITO_VALIDATION_ENTRY @@ -227,8 +229,8 @@ class IsoImage(object): Volume ID from the table """ primary_vol_data = data[0: -1] - (vd_type, vd_ident, vd_ver, - pad0, sys_id, vol_id) = self._unpack(IsoImage.VOL_DESC, primary_vol_data) + info = self._unpack(IsoImage.VOL_DESC, primary_vol_data) + (vd_type, vd_ident, vd_ver, pad0, sys_id, vol_id) = info if vd_type != 1: raise IsoFormatError("Unexpected volume type for primary volume") if vd_ident != 'CD001' or vd_ver != 1: @@ -238,7 +240,8 @@ class IsoImage(object): def _get_iso_data(self, offset, size): if self.remote: request = urllib2.Request(self.path) - request.add_header("range", "bytes=%d-%d" % (offset, offset + size -1)) + range_header = "bytes=%d-%d" % (offset, offset + size - 1) + request.add_header("range", range_header) response = urllib2.urlopen(request) data = response.read() else: @@ -310,13 +313,15 @@ def probe_iso(status_helper, params): ret = iso_img.probe() update_result(loc, ret) - if status_helper != None: + if status_helper is not None: status_helper('', True) if __name__ == '__main__': iso_list = [] + def updater(iso_info): iso_list.append(iso_info) + probe_iso(None, dict(path=sys.argv[1], updater=updater)) print iso_list