11:09 a.m.
From: Aline Manera <alinefm(a)br.ibm.com>
Aline Manera (6):
isoinfo: Add ignore_list paramter to main program
isoinfo: Use absolute path only for local ISO files
Move IsoFormatError() from isoinfo.py to exception.py
Move ISO path validation to IsoImage()
isoinfo: Move _probe_iso() to IsoImage()
pep8 cleanup for isoinfo.py
Makefile.am | 1 +
src/kimchi/exception.py | 4 ++
src/kimchi/isoinfo.py | 138 +++++++++++++++++++++++-----------------------
src/kimchi/model.py | 11 ++--
src/kimchi/scan.py | 5 +-
src/kimchi/vmtemplate.py | 8 ++-
6 files changed, 87 insertions(+), 80 deletions(-)
--
1.7.10.4
11:09 a.m.
New subject: [PATCH 1/6] isoinfo: Add ignore_list paramter to main program
From: Aline Manera <alinefm(a)br.ibm.com>
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>
---
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..8f4738c 100644
--- a/src/kimchi/isoinfo.py
+++ b/src/kimchi/isoinfo.py
@@ -340,5 +340,5 @@ if __name__ == '__main__':
iso_list = []
def updater(iso_info):
iso_list.append(iso_info)
- probe_iso(None, dict(path=sys.argv[1], updater=updater))
+ probe_iso(None, dict(path=sys.argv[1], updater=updater, ignore_list=[]))
print iso_list
--
1.7.10.4
11:09 a.m.
New subject: [PATCH 2/6] isoinfo: Use absolute path only for local ISO files
From: Aline Manera <alinefm(a)br.ibm.com>
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>
---
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 8f4738c..59cdd45 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):
--
1.7.10.4
11:09 a.m.
New subject: [PATCH 3/6] Move IsoFormatError() from isoinfo.py to exception.py
From: Aline Manera <alinefm(a)br.ibm.com>
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>
---
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
11:09 a.m.
New subject: [PATCH 4/6] Move ISO path validation to IsoImage()
From: Aline Manera <alinefm(a)br.ibm.com>
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>
---
src/kimchi/isoinfo.py | 55 +++++++++++++++++++++++++++----------------------
1 file changed, 30 insertions(+), 25 deletions(-)
diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py
index 7b7fa78..dd26204 100644
--- a/src/kimchi/isoinfo.py
+++ b/src/kimchi/isoinfo.py
@@ -134,13 +134,37 @@ 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, remote = None):
self.path = path
self.volume_id = None
self.bootable = False
- self.remote = remote
+
+ if remote is not None:
+ self.remote = remote
+ else:
+ self.remote = self._is_iso_remote()
+
self._scan()
+ def _is_iso_remote(self):
+ if os.path.isfile(self.path):
+ return False
+
+ if self._check_url_path():
+ return True
+
+ raise IsoFormatError('ISO %s does not exist' % self.path)
+
+ def _check_url_path(self):
+ try:
+ code = urllib2.urlopen(self.path).getcode()
+ if code != 200:
+ return False
+ except (urllib2.HTTPError, ValueError):
+ return False
+
+ return True
+
def _unpack(self, s, data):
return s.unpack(data[:s.size])
@@ -246,7 +270,7 @@ class Matcher(object):
return self.lastmatch.group(num)
-def _probe_iso(fname, remote = False):
+def _probe_iso(fname, remote = None):
try:
iso = IsoImage(fname, remote)
except Exception, e:
@@ -298,39 +322,20 @@ def probe_iso(status_helper, params):
continue
iso = os.path.join(root, name)
try:
- ret = _probe_iso(iso)
+ ret = _probe_iso(iso, False)
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__':
--
1.7.10.4
11:09 a.m.
New subject: [PATCH 5/6] isoinfo: Move _probe_iso() to IsoImage()
From: Aline Manera <alinefm(a)br.ibm.com>
That way all operations related to IsoImage() object will be in this class.
Signed-off-by: Aline Manera <alinefm(a)br.ibm.com>
---
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
11:09 a.m.
New subject: [PATCH 6/6] pep8 cleanup for isoinfo.py
From: Aline Manera <alinefm(a)br.ibm.com>
This patch cleans up pep8 style issue in isoinfo.py
Signed-off-by: Aline Manera <alinefm(a)br.ibm.com>
---
Makefile.am | 1 +
src/kimchi/isoinfo.py | 29 +++++++++++++++++------------
2 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 0fd92c8..9ed9aec 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -43,6 +43,7 @@ PEP8_WHITELIST = \
src/kimchi/cachebust.py \
src/kimchi/config.py.in \
src/kimchi/disks.py \
+ src/kimchi/isoinfo.py \
src/kimchi/root.py \
src/kimchi/server.py \
plugins/__init__.py \
diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py
index 6f3ad7d..1a0b359 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 kimchi_log
+
iso_dir = [
##
# Portions of this data from libosinfo: http://libosinfo.org/
@@ -134,7 +135,7 @@ class IsoImage(object):
EL_TORITO_VALIDATION_ENTRY = struct.Struct("=BBH24sHBB")
EL_TORITO_BOOT_ENTRY = struct.Struct("=BBHBBHL20x")
- def __init__(self, path, remote = None):
+ def __init__(self, path, remote=None):
self.path = path
self.volume_id = None
self.bootable = False
@@ -180,8 +181,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')
@@ -192,9 +193,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):
@@ -214,7 +215,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
@@ -242,8 +244,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:
@@ -253,7 +255,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:
@@ -325,13 +328,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, ignore_list=[]))
print iso_list
--
1.7.10.4
2:21 p.m.
New subject: [PATCH 1/6] isoinfo: Add ignore_list paramter to main program
Reviewed-By: Ramon Medeiros <ramonn(a)br.ibm.com>
2:21 p.m.
New subject: [PATCH 2/6] isoinfo: Use absolute path only for local ISO files
Reviewed-By: Ramon Medeiros <ramonn(a)br.ibm.com>
2:22 p.m.
New subject: [PATCH 3/6] Move IsoFormatError() from isoinfo.py to exception.py
Reviewed-By: Ramon Medeiros <ramonn(a)br.ibm.com>
2:25 p.m.
New subject: [PATCH 4/6] Move ISO path validation to IsoImage()
On 12/30/2013 03:09 PM, Aline Manera wrote:
From: Aline Manera <alinefm(a)br.ibm.com>
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>
---
src/kimchi/isoinfo.py | 55 +++++++++++++++++++++++++++----------------------
1 file changed, 30 insertions(+), 25 deletions(-)
diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py
index 7b7fa78..dd26204 100644
--- a/src/kimchi/isoinfo.py
+++ b/src/kimchi/isoinfo.py
@@ -134,13 +134,37 @@ 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, remote = None):
self.path = path
self.volume_id = None
self.bootable = False
- self.remote = remote
+
+ if remote is not None:
+ self.remote = remote
+ else:
+ self.remote = self._is_iso_remote()
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
+
self._scan()
+ def _is_iso_remote(self):
+ if os.path.isfile(self.path):
+ return False
+
+ if self._check_url_path():
+ return True
+
+ raise IsoFormatError('ISO %s does not exist' % self.path)
+
+ def _check_url_path(self):
+ try:
+ code = urllib2.urlopen(self.path).getcode()
+ if code != 200:
+ return False
+ except (urllib2.HTTPError, ValueError):
+ return False
+
+ return True
+
def _unpack(self, s, data):
return s.unpack(data[:s.size])
@@ -246,7 +270,7 @@ class Matcher(object):
return self.lastmatch.group(num)
-def _probe_iso(fname, remote = False):
+def _probe_iso(fname, remote = None):
try:
iso = IsoImage(fname, remote)
except Exception, e:
@@ -298,39 +322,20 @@ def probe_iso(status_helper, params):
continue
iso = os.path.join(root, name)
try:
- ret = _probe_iso(iso)
+ ret = _probe_iso(iso, False)
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__':
--
Ramon Nunes Medeiros
Software Engineer - Linux Technology Center Brazil
IBM Systems & Technology Group
Phone : +55 19 2132 7878
ramonn(a)br.ibm.com
2:26 p.m.
New subject: [PATCH 5/6] isoinfo: Move _probe_iso() to IsoImage()
Reviewed-By: Ramon Medeiros <ramonn(a)br.ibm.com>
2:26 p.m.
New subject: [PATCH 6/6] pep8 cleanup for isoinfo.py
Reviewed-By: Ramon Medeiros <ramonn(a)br.ibm.com>
8:24 p.m.
New subject: [PATCH 1/6] isoinfo: Add ignore_list paramter to main program
On 2013年12月31日 01:09, Aline Manera wrote:
From: Aline Manera <alinefm(a)br.ibm.com>
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>
---
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..8f4738c 100644
--- a/src/kimchi/isoinfo.py
+++ b/src/kimchi/isoinfo.py
@@ -340,5 +340,5 @@ if __name__ == '__main__':
iso_list = []
def updater(iso_info):
iso_list.append(iso_info)
- probe_iso(None, dict(path=sys.argv[1], updater=updater))
+ probe_iso(None, dict(path=sys.argv[1], updater=updater, ignore_list=[]))
print iso_list
Without affecting the upper layer,
What about change :
def probe_iso(status_helper, params):
loc = params['path'].encode("utf-8")
updater = params['updater']
ignore = False
ignore_list = params.get('ignore_list', [])
8:55 p.m.
New subject: [PATCH 2/6] isoinfo: Use absolute path only for local ISO files
Reviewed-by: Royce Lv<lvroyce(a)linux.vnet.ibm.com>
On 2013年12月31日 01:09, Aline Manera wrote:
From: Aline Manera <alinefm(a)br.ibm.com>
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>
---
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 8f4738c..59cdd45 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):
8:57 p.m.
New subject: [PATCH 3/6] Move IsoFormatError() from isoinfo.py to exception.py
Reviewed-by: Royce Lv<lvroyce(a)linux.vnet.ibm.com>
On 2013年12月31日 01:09, Aline Manera wrote:
From: Aline Manera <alinefm(a)br.ibm.com>
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>
---
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
12:44 a.m.
New subject: [PATCH 4/6] Move ISO path validation to IsoImage()
On 2013年12月31日 01:09, Aline Manera wrote:
From: Aline Manera <alinefm(a)br.ibm.com>
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>
---
src/kimchi/isoinfo.py | 55 +++++++++++++++++++++++++++----------------------
1 file changed, 30 insertions(+), 25 deletions(-)
diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py
index 7b7fa78..dd26204 100644
--- a/src/kimchi/isoinfo.py
+++ b/src/kimchi/isoinfo.py
@@ -134,13 +134,37 @@ 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, remote = None):
self.path = path
self.volume_id = None
self.bootable = False
- self.remote = remote
+
+ if remote is not None:
+ self.remote = remote
+ else:
+ self.remote = self._is_iso_remote()
+
self._scan()
+ def _is_iso_remote(self):
+ if os.path.isfile(self.path):
+ return False
+
+ if self._check_url_path():
+ return True
+
+ raise IsoFormatError('ISO %s does not exist' % self.path)
+
+ def _check_url_path(self):
+ try:
+ code = urllib2.urlopen(self.path).getcode()
+ if code != 200:
+ return False
+ except (urllib2.HTTPError, ValueError):
+ return False
+
+ return True
I kind of want this func moved outside for reusage of other
module.
+
def _unpack(self, s, data):
return s.unpack(data[:s.size])
@@ -246,7 +270,7 @@ class Matcher(object):
return self.lastmatch.group(num)
-def _probe_iso(fname, remote = False):
+def _probe_iso(fname, remote = None):
try:
iso = IsoImage(fname, remote)
except Exception, e:
@@ -298,39 +322,20 @@ def probe_iso(status_helper, params):
continue
iso = os.path.join(root, name)
try:
- ret = _probe_iso(iso)
+ ret = _probe_iso(iso, False)
update_result(iso, ret)
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.
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)
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.
if __name__ == '__main__':
1:15 a.m.
New subject: [PATCH 5/6] isoinfo: Move _probe_iso() to IsoImage()
Reviewed-by: Royce Lv<lvroyce(a)linux.vnet.ibm.com>
On 2013年12月31日 01:09, Aline Manera wrote:
From: Aline Manera <alinefm(a)br.ibm.com>
That way all operations related to IsoImage() object will be in this class.
Signed-off-by: Aline Manera <alinefm(a)br.ibm.com>
---
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)
You really read my mind!
-
-
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:37 p.m.
New subject: [PATCH 1/6] isoinfo: Add ignore_list paramter to main program
On 12/31/2013 12:24 AM, Royce Lv wrote:
On 2013年12月31日 01:09, Aline Manera wrote:
> From: Aline Manera <alinefm(a)br.ibm.com>
>
> 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>
> ---
> 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..8f4738c 100644
> --- a/src/kimchi/isoinfo.py
> +++ b/src/kimchi/isoinfo.py
> @@ -340,5 +340,5 @@ if __name__ == '__main__':
> iso_list = []
> def updater(iso_info):
> iso_list.append(iso_info)
> - probe_iso(None, dict(path=sys.argv[1], updater=updater))
> + probe_iso(None, dict(path=sys.argv[1], updater=updater,
> ignore_list=[]))
> print iso_list
Without affecting the upper layer,
What about change :
def probe_iso(status_helper, params):
loc = params['path'].encode("utf-8")
updater = params['updater']
ignore = False
ignore_list = params.get('ignore_list', [])
Ack.
1:40 p.m.
New subject: [PATCH 4/6] Move ISO path validation to IsoImage()
On 12/30/2013 06:25 PM, Ramon Medeiros wrote:
On 12/30/2013 03:09 PM, Aline Manera wrote:
> From: Aline Manera <alinefm(a)br.ibm.com>
>
> 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>
> ---
> src/kimchi/isoinfo.py | 55
> +++++++++++++++++++++++++++----------------------
> 1 file changed, 30 insertions(+), 25 deletions(-)
>
> diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py
> index 7b7fa78..dd26204 100644
> --- a/src/kimchi/isoinfo.py
> +++ b/src/kimchi/isoinfo.py
> @@ -134,13 +134,37 @@ 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, remote = None):
> self.path = path
> self.volume_id = None
> self.bootable = False
> - self.remote = remote
> +
> + if remote is not None:
> + self.remote = remote
> + else:
> + self.remote = self._is_iso_remote()
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
ack
> +
> self._scan()
>
> + def _is_iso_remote(self):
> + if os.path.isfile(self.path):
> + return False
> +
> + if self._check_url_path():
> + return True
> +
> + raise IsoFormatError('ISO %s does not exist' % self.path)
> +
> + def _check_url_path(self):
> + try:
> + code = urllib2.urlopen(self.path).getcode()
> + if code != 200:
> + return False
> + except (urllib2.HTTPError, ValueError):
> + return False
> +
> + return True
> +
> def _unpack(self, s, data):
> return s.unpack(data[:s.size])
>
> @@ -246,7 +270,7 @@ class Matcher(object):
> return self.lastmatch.group(num)
>
>
> -def _probe_iso(fname, remote = False):
> +def _probe_iso(fname, remote = None):
> try:
> iso = IsoImage(fname, remote)
> except Exception, e:
> @@ -298,39 +322,20 @@ def probe_iso(status_helper, params):
> continue
> iso = os.path.join(root, name)
> try:
> - ret = _probe_iso(iso)
> + ret = _probe_iso(iso, False)
> 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__':
1:44 p.m.
New subject: [PATCH 4/6] Move ISO path validation to IsoImage()
On 12/31/2013 04:44 AM, Royce Lv wrote:
On 2013年12月31日 01:09, Aline Manera wrote:
> From: Aline Manera <alinefm(a)br.ibm.com>
>
> 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>
> ---
> src/kimchi/isoinfo.py | 55
> +++++++++++++++++++++++++++----------------------
> 1 file changed, 30 insertions(+), 25 deletions(-)
>
> diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py
> index 7b7fa78..dd26204 100644
> --- a/src/kimchi/isoinfo.py
> +++ b/src/kimchi/isoinfo.py
> @@ -134,13 +134,37 @@ 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, remote = None):
> self.path = path
> self.volume_id = None
> self.bootable = False
> - self.remote = remote
> +
> + if remote is not None:
> + self.remote = remote
> + else:
> + self.remote = self._is_iso_remote()
> +
> self._scan()
>
> + def _is_iso_remote(self):
> + if os.path.isfile(self.path):
> + return False
> +
> + if self._check_url_path():
> + return True
> +
> + raise IsoFormatError('ISO %s does not exist' % self.path)
> +
> + def _check_url_path(self):
> + try:
> + code = urllib2.urlopen(self.path).getcode()
> + if code != 200:
> + return False
> + except (urllib2.HTTPError, ValueError):
> + return False
> +
> + return True
I kind of want this func moved outside for reusage of other module.
It isn't used in other place because that I moved it to to IsoImage()
> +
> def _unpack(self, s, data):
> return s.unpack(data[:s.size])
>
> @@ -246,7 +270,7 @@ class Matcher(object):
> return self.lastmatch.group(num)
>
>
> -def _probe_iso(fname, remote = False):
> +def _probe_iso(fname, remote = None):
> try:
> iso = IsoImage(fname, remote)
> except Exception, e:
> @@ -298,39 +322,20 @@ def probe_iso(status_helper, params):
> continue
> iso = os.path.join(root, name)
> try:
> - ret = _probe_iso(iso)
> + ret = _probe_iso(iso, False)
> update_result(iso, ret)
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.
In this case, I passed 'remote' parameter as we have already checked if
it is local
if os.path.isdir(loc):
....
> 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)
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.
>
>
> if __name__ == '__main__':
_______________________________________________
Kimchi-devel mailing list
Kimchi-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel
8:19 p.m.
New subject: [PATCH 4/6] Move ISO path validation to IsoImage()
On 2014年01月03日 03:44, Aline Manera wrote:
On 12/31/2013 04:44 AM, Royce Lv wrote:
> On 2013年12月31日 01:09, Aline Manera wrote:
>> From: Aline Manera <alinefm(a)br.ibm.com>
>>
>> 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>
>> ---
>> src/kimchi/isoinfo.py | 55
>> +++++++++++++++++++++++++++----------------------
>> 1 file changed, 30 insertions(+), 25 deletions(-)
>>
>> diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py
>> index 7b7fa78..dd26204 100644
>> --- a/src/kimchi/isoinfo.py
>> +++ b/src/kimchi/isoinfo.py
>> @@ -134,13 +134,37 @@ 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, remote = None):
>> self.path = path
>> self.volume_id = None
>> self.bootable = False
>> - self.remote = remote
>> +
>> + if remote is not None:
>> + self.remote = remote
>> + else:
>> + self.remote = self._is_iso_remote()
>> +
>> self._scan()
>>
>> + def _is_iso_remote(self):
>> + if os.path.isfile(self.path):
>> + return False
>> +
>> + if self._check_url_path():
>> + return True
>> +
>> + raise IsoFormatError('ISO %s does not exist' % self.path)
>> +
>> + def _check_url_path(self):
>> + try:
>> + code = urllib2.urlopen(self.path).getcode()
>> + if code != 200:
>> + return False
>> + except (urllib2.HTTPError, ValueError):
>> + return False
>> +
>> + return True
> I kind of want this func moved outside for reusage of other module.
It isn't used in other place because that I moved it to to IsoImage()
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.
>> +
>> def _unpack(self, s, data):
>> return s.unpack(data[:s.size])
>>
>> @@ -246,7 +270,7 @@ class Matcher(object):
>> return self.lastmatch.group(num)
>>
>>
>> -def _probe_iso(fname, remote = False):
>> +def _probe_iso(fname, remote = None):
>> try:
>> iso = IsoImage(fname, remote)
>> except Exception, e:
>> @@ -298,39 +322,20 @@ def probe_iso(status_helper, params):
>> continue
>> iso = os.path.join(root, name)
>> try:
>> - ret = _probe_iso(iso)
>> + ret = _probe_iso(iso, False)
>> update_result(iso, ret)
> 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.
In this case, I passed 'remote' parameter as we have already checked
if it is local
if os.path.isdir(loc):
....
>> 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)
> 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.
>>
>>
>> if __name__ == '__main__':
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
8:08 a.m.
New subject: [PATCH 4/6] Move ISO path validation to IsoImage()
On 01/03/2014 12:19 AM, Royce Lv wrote:
On 2014年01月03日 03:44, Aline Manera wrote:
> On 12/31/2013 04:44 AM, Royce Lv wrote:
>> On 2013年12月31日 01:09, Aline Manera wrote:
>>> From: Aline Manera <alinefm(a)br.ibm.com>
>>>
>>> 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>
>>> ---
>>> src/kimchi/isoinfo.py | 55
>>> +++++++++++++++++++++++++++----------------------
>>> 1 file changed, 30 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py
>>> index 7b7fa78..dd26204 100644
>>> --- a/src/kimchi/isoinfo.py
>>> +++ b/src/kimchi/isoinfo.py
>>> @@ -134,13 +134,37 @@ 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, remote = None):
>>> self.path = path
>>> self.volume_id = None
>>> self.bootable = False
>>> - self.remote = remote
>>> +
>>> + if remote is not None:
>>> + self.remote = remote
>>> + else:
>>> + self.remote = self._is_iso_remote()
>>> +
>>> self._scan()
>>>
>>> + def _is_iso_remote(self):
>>> + if os.path.isfile(self.path):
>>> + return False
>>> +
>>> + if self._check_url_path():
>>> + return True
>>> +
>>> + raise IsoFormatError('ISO %s does not exist' % self.path)
>>> +
>>> + def _check_url_path(self):
>>> + try:
>>> + code = urllib2.urlopen(self.path).getcode()
>>> + if code != 200:
>>> + return False
>>> + except (urllib2.HTTPError, ValueError):
>>> + return False
>>> +
>>> + return True
>> I kind of want this func moved outside for reusage of other module.
>
> It isn't used in other place because that I moved it to to IsoImage()
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.
Got it. I will do it in V3.
>
>>> +
>>> def _unpack(self, s, data):
>>> return s.unpack(data[:s.size])
>>>
>>> @@ -246,7 +270,7 @@ class Matcher(object):
>>> return self.lastmatch.group(num)
>>>
>>>
>>> -def _probe_iso(fname, remote = False):
>>> +def _probe_iso(fname, remote = None):
>>> try:
>>> iso = IsoImage(fname, remote)
>>> except Exception, e:
>>> @@ -298,39 +322,20 @@ def probe_iso(status_helper, params):
>>> continue
>>> iso = os.path.join(root, name)
>>> try:
>>> - ret = _probe_iso(iso)
>>> + ret = _probe_iso(iso, False)
>>> update_result(iso, ret)
>> 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.
>
> In this case, I passed 'remote' parameter as we have already checked
> if it is local
>
> if os.path.isdir(loc):
> ....
>
>
>>> 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)
>> 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.
>>>
>>>
>>> if __name__ == '__main__':
>>
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel(a)ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
3765
days inactive
3769
days old
22 comments
3 participants
participants (3)
-
Aline Manera -
Ramon Medeiros -
Royce Lv