[PATCH] Reject cdrom update when protocol is not supported by libvirt

From: Royce Lv <lvroyce@linux.vnet.ibm.com> Based on Christy's patchset: [Kimchi-devel] [PATCH v3 1/3] Fix Key Error when editing CD ROM path When remote iso protocol is not supported by libvirt, even if we work around by qemu command, it will mess up device list logic. So reject this use case. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/i18n.py | 1 + src/kimchi/model/vmstorages.py | 5 ++++- tests/test_model.py | 13 +++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 2eae7e8..f5ef275 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -255,6 +255,7 @@ messages = { "KCHVMSTOR0015E": _("Cannot lookup disk path information by given pool/volume: %(error)s"), "KCHVMSTOR0016E": _("Volume already been used by other vm"), "KCHVMSTOR0017E": _("Only one of path or pool/volume can be specified to add a new virtual machine disk"), + "KCHVMSTOR0018E": _("Remote ISO update/attach is not supported by your libvirt version"), "KCHREPOS0001E": _("YUM Repository ID must be one word only string."), "KCHREPOS0002E": _("Repository URL must be an http://, ftp:// or file:// URL."), diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index e5be17c..7dc8800 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -29,6 +29,7 @@ from lxml.builder import E from kimchi.exception import InvalidOperation, InvalidParameter, NotFoundError from kimchi.exception import OperationFailed +from kimchi.model.config import CapabilitiesModel from kimchi.model.vms import DOM_STATE_MAP, VMModel from kimchi.model.storagevolumes import StorageVolumeModel from kimchi.model.utils import get_vm_config_flag @@ -38,7 +39,7 @@ from kimchi.vmdisks import get_device_xml, get_vm_disk, get_vm_disk_list from kimchi.vmdisks import DEV_TYPE_SRC_ATTR_MAP HOTPLUG_TYPE = ['scsi', 'virtio'] - +caps = CapabilitiesModel() def _get_device_bus(dev_type, dom): try: @@ -69,6 +70,8 @@ def _get_storage_xml(params, ignore_source=False): output = urlparse.urlparse(params.get('path')) port = str(output.port or socket.getservbyname(output.scheme)) host = E.host(name=output.hostname, port=port) + if output.scheme not in caps.libvirt_stream_protocols: + raise InvalidOperation("KCHVMSTOR0018E") source = E.source(protocol=output.scheme, name=output.path) source.append(host) disk.append(source) diff --git a/tests/test_model.py b/tests/test_model.py index 7fd0f00..dc56676 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -29,6 +29,7 @@ import tempfile import threading import time import unittest +import urlparse import uuid @@ -38,6 +39,7 @@ import utils from kimchi import netinfo from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import NotFoundError, OperationFailed +from kimchi.featuretests import FeatureTests from kimchi.iscsi import TargetClient from kimchi.model import model from kimchi.rollbackcontext import RollbackContext @@ -397,11 +399,22 @@ class ModelTests(unittest.TestCase): valid_remote_iso_path = utils.get_remote_iso_path() cdrom_args = {"type": "cdrom", "path": valid_remote_iso_path} + protocol = urlparse.urlparse(valid_remote_iso_path).scheme + if not FeatureTests.libvirt_supports_iso_stream(protocol): + self.assertRaises(InvalidOperation, inst.vmstorages_create, + vm_name, cdrom_args) + return + 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 + protocol = urlparse.urlparse(valid_remote_iso_path).scheme + if not FeatureTests.libvirt_supports_iso_stream(protocol): + self.assertRaises(InvalidOperation, inst.vmstorage_update, + vm_name, cdrom_dev, {'path': valid_remote_iso_path}) + return inst.vmstorage_update(vm_name, cdrom_dev, {'path': valid_remote_iso_path}) cdrom_info = inst.vmstorage_lookup(vm_name, cdrom_dev) -- 1.8.3.2

From: Royce Lv <lvroyce@linux.vnet.ibm.com> UI just accept local ISO as cdrom attachment before, enable remote ISO link for it. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_storage_add.main.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/js/src/kimchi.guest_storage_add.main.js b/ui/js/src/kimchi.guest_storage_add.main.js index 1f4c4bd..a8c5acb 100644 --- a/ui/js/src/kimchi.guest_storage_add.main.js +++ b/ui/js/src/kimchi.guest_storage_add.main.js @@ -109,7 +109,7 @@ kimchi.guest_storage_add_main = function() { }); var validateCDROM = function(settings) { - if (/^(\/.*)+$/.test(settings['path'])) + if (/^((https|http|ftp|ftps|tftp|\/).*)+$/.test(settings['path'])) return true; else { kimchi.message.error.code('KCHVMSTOR0001E'); -- 1.8.3.2

Tested-by: Christy Perez <christy@linux.vnet.ibm.com> According to the test output, update isn't supported by my libvirt version (libvirt-1.1.3.5-2.fc20.x86_64 on Fedora 20). I tested this with copy/paste against v4 of my patchset, since git am wasn't going to work with my updates. Regards, - Christy On 08/12/2014 04:53 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Based on Christy's patchset: [Kimchi-devel] [PATCH v3 1/3] Fix Key Error when editing CD ROM path
When remote iso protocol is not supported by libvirt, even if we work around by qemu command, it will mess up device list logic. So reject this use case.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/i18n.py | 1 + src/kimchi/model/vmstorages.py | 5 ++++- tests/test_model.py | 13 +++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 2eae7e8..f5ef275 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -255,6 +255,7 @@ messages = { "KCHVMSTOR0015E": _("Cannot lookup disk path information by given pool/volume: %(error)s"), "KCHVMSTOR0016E": _("Volume already been used by other vm"), "KCHVMSTOR0017E": _("Only one of path or pool/volume can be specified to add a new virtual machine disk"), + "KCHVMSTOR0018E": _("Remote ISO update/attach is not supported by your libvirt version"),
"KCHREPOS0001E": _("YUM Repository ID must be one word only string."), "KCHREPOS0002E": _("Repository URL must be an http://, ftp:// or file:// URL."), diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index e5be17c..7dc8800 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -29,6 +29,7 @@ from lxml.builder import E
from kimchi.exception import InvalidOperation, InvalidParameter, NotFoundError from kimchi.exception import OperationFailed +from kimchi.model.config import CapabilitiesModel from kimchi.model.vms import DOM_STATE_MAP, VMModel from kimchi.model.storagevolumes import StorageVolumeModel from kimchi.model.utils import get_vm_config_flag @@ -38,7 +39,7 @@ from kimchi.vmdisks import get_device_xml, get_vm_disk, get_vm_disk_list from kimchi.vmdisks import DEV_TYPE_SRC_ATTR_MAP
HOTPLUG_TYPE = ['scsi', 'virtio'] - +caps = CapabilitiesModel()
def _get_device_bus(dev_type, dom): try: @@ -69,6 +70,8 @@ def _get_storage_xml(params, ignore_source=False): output = urlparse.urlparse(params.get('path')) port = str(output.port or socket.getservbyname(output.scheme)) host = E.host(name=output.hostname, port=port) + if output.scheme not in caps.libvirt_stream_protocols: + raise InvalidOperation("KCHVMSTOR0018E") source = E.source(protocol=output.scheme, name=output.path) source.append(host) disk.append(source) diff --git a/tests/test_model.py b/tests/test_model.py index 7fd0f00..dc56676 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -29,6 +29,7 @@ import tempfile import threading import time import unittest +import urlparse import uuid
@@ -38,6 +39,7 @@ import utils from kimchi import netinfo from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import NotFoundError, OperationFailed +from kimchi.featuretests import FeatureTests from kimchi.iscsi import TargetClient from kimchi.model import model from kimchi.rollbackcontext import RollbackContext @@ -397,11 +399,22 @@ class ModelTests(unittest.TestCase): valid_remote_iso_path = utils.get_remote_iso_path() cdrom_args = {"type": "cdrom", "path": valid_remote_iso_path} + protocol = urlparse.urlparse(valid_remote_iso_path).scheme + if not FeatureTests.libvirt_supports_iso_stream(protocol): + self.assertRaises(InvalidOperation, inst.vmstorages_create, + vm_name, cdrom_args) + return + 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 + protocol = urlparse.urlparse(valid_remote_iso_path).scheme + if not FeatureTests.libvirt_supports_iso_stream(protocol): + self.assertRaises(InvalidOperation, inst.vmstorage_update, + vm_name, cdrom_dev, {'path': valid_remote_iso_path}) + return inst.vmstorage_update(vm_name, cdrom_dev, {'path': valid_remote_iso_path}) cdrom_info = inst.vmstorage_lookup(vm_name, cdrom_dev)

On 12-08-2014 06:53, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Based on Christy's patchset: [Kimchi-devel] [PATCH v3 1/3] Fix Key Error when editing CD ROM path
When remote iso protocol is not supported by libvirt, even if we work around by qemu command, it will mess up device list logic. So reject this use case.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> Please run "make check-local" to fix code formatting.
participants (3)
-
Christy Perez
-
Crístian Viana
-
lvroyce@linux.vnet.ibm.com