[PATCH v5 1/3] Fix Key Error when editing CD ROM path
lxml only accepts strings as parameters when looking up xml elements. The port for remote ISO was being passed in as an int, and a KeyError was thrown. This patch just casts the ints to strings for the lookups. Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- src/kimchi/model/vmstorages.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index b5311db..e5be17c 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -67,7 +67,7 @@ def _get_storage_xml(params, ignore_source=False): # Working with url paths if src_type == 'network': output = urlparse.urlparse(params.get('path')) - port = output.port or socket.getservbyname(output.scheme) + port = str(output.port or socket.getservbyname(output.scheme)) host = E.host(name=output.hostname, port=port) source = E.source(protocol=output.scheme, name=output.path) source.append(host) -- 1.9.3
Currently the entire ISO is grabbed, which times out. Use httplib to connect to the server and just get the head of the http object. Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- src/kimchi/utils.py | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index 9ed8802..0977b9f 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -28,9 +28,10 @@ import traceback import urllib2 import xml.etree.ElementTree as ET +from httplib import HTTPConnection, HTTPException from multiprocessing import Process, Queue from threading import Timer - +from urlparse import urlparse from cherrypy.lib.reprconf import Parser from kimchi.asynctask import AsyncTask @@ -140,12 +141,25 @@ def import_module(module_name): def check_url_path(path): try: - code = urllib2.urlopen(path).getcode() + code = '' + parse_result = urlparse(path) + server_name = parse_result.netloc + urlpath = parse_result.path + if not urlpath: + # Just a server, as with a repo. + code = urllib2.urlopen(path).getcode() + else: + # socket.gaierror could be raised, + # which is a child class of IOError + conn = HTTPConnection(server_name, timeout=15) + # Don't try to get the whole file: + conn.request('HEAD', path) + code = conn.getresponse().status + conn.close() if code != 200: return False - except (urllib2.URLError, ValueError): + except (urllib2.URLError, HTTPException, IOError, ValueError): return False - return True -- 1.9.3
Adding tests to ensure that updating an existing CD ROM can be done when using remote ISOs. This is to test for regression for the libxml KeyError seen when even saving the same CD ROM ISO as the update. Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- tests/test_model.py | 23 ++++++++++++++++++++++- tests/utils.py | 20 +++++++++++++++++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/tests/test_model.py b/tests/test_model.py index 92b34b6..7fd0f00 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -23,6 +23,7 @@ import platform import psutil import pwd +import re import shutil import tempfile import threading @@ -285,7 +286,7 @@ def _attach_disk(expect_bus='virtio'): self.assertEquals(u'disk', disk_info['type']) inst.vmstorage_delete(vm_name, disk) - # Specify pool and path at sametime will fail + # Specifying pool and path at same time will fail disk_args = {"type": "disk", "pool": pool, "vol": vol, @@ -361,6 +362,11 @@ def test_vm_cdrom(self): self.assertRaises(InvalidParameter, inst.vmstorage_update, vm_name, cdrom_dev, {'path': wrong_iso_path}) + # Make sure CD ROM still exists after failure + cd_info = inst.vmstorage_lookup(vm_name, cdrom_dev) + self.assertEquals(u'cdrom', cd_info['type']) + self.assertEquals(iso_path, cd_info['path']) + # update path of existing cd with existent iso of shutoff vm inst.vmstorage_update(vm_name, cdrom_dev, {'path': iso_path2}) cdrom_info = inst.vmstorage_lookup(vm_name, cdrom_dev) @@ -387,6 +393,21 @@ def test_vm_cdrom(self): storage_list = inst.vmstorages_get_list(vm_name) self.assertEquals(prev_count, len(storage_list)) + # Create a new cdrom using a remote iso + valid_remote_iso_path = utils.get_remote_iso_path() + cdrom_args = {"type": "cdrom", + "path": valid_remote_iso_path} + cdrom_dev = inst.vmstorages_create(vm_name, cdrom_args) + storage_list = inst.vmstorages_get_list(vm_name) + self.assertEquals(prev_count + 1, len(storage_list)) + + # Update remote-backed cdrom with the same ISO + inst.vmstorage_update(vm_name, cdrom_dev, + {'path': valid_remote_iso_path}) + cdrom_info = inst.vmstorage_lookup(vm_name, cdrom_dev) + cur_cdrom_path = re.sub(":80/", '/', cdrom_info['path']) + self.assertEquals(valid_remote_iso_path, cur_cdrom_path) + @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_vm_storage_provisioning(self): inst = model.Model(objstore_loc=self.tmp_store) diff --git a/tests/utils.py b/tests/utils.py index 30c9cae..7f9570e 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -21,19 +21,20 @@ import base64 import cherrypy import httplib +import json import os import socket import sys import threading import unittest - from contextlib import closing from lxml import etree import kimchi.mockmodel import kimchi.server +from kimchi.config import paths from kimchi.exception import OperationFailed _ports = {} @@ -146,6 +147,23 @@ def request(host, port, path, data=None, method='GET', headers=None): conn = httplib.HTTPSConnection(host, port) return _request(conn, path, data, method, headers) +def get_remote_iso_path(): + """ + Get a remote iso with the right arch from the distro files shipped + with kimchi. + """ + host_arch = os.uname()[4] + remote_path = '' + with open(os.path.join(paths.conf_dir, 'distros.d', 'fedora.json'))\ + as fedora_isos: + # Get a list of dicts + json_isos_list = json.load(fedora_isos) + for iso in json_isos_list: + if (iso.get('os_arch')) == host_arch: + remote_path = iso.get('path') + break + + return remote_path def patch_auth(sudo=True): """ -- 1.9.3
participants (2)
-
Aline Manera -
Christy Perez