[Kimchi-devel] [PATCHv2] Reject cdrom update when protocol is not supported by libvirt

[Royce Lv]

On 2014年08月14日 08:54, Aline Manera wrote:
> The test is failing for me and my libvirt has support for iso streaming:
>
> ======================================================================
> ERROR: test_vm_cdrom (test_model.ModelTests)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File "test_model.py", line 409, in test_vm_cdrom
> cdrom_dev = inst.vmstorages_create(vm_name, cdrom_args)
> File "/home/alinefm/kimchi/src/kimchi/model/vmstorages.py", line 185, 
> in create
> dev_xml = _get_storage_xml(params)
> File "/home/alinefm/kimchi/src/kimchi/model/vmstorages.py", line 75, 
> in _get_storage_xml
> raise InvalidOperation("KCHVMSTOR0018E")
> InvalidOperation: KCHVMSTOR0018E: Remote ISO update/attach is not 
> supported by your libvirt version
>
> ----------------------------------------------------------------------
>
> I guess it is because CapabilitiesModel() depends on cherrypy server 
> to run the FeatureTests on start up and the test_model.py does not 
> start the server, only uses the model instance.
> To do those tests we need to start the server or find other approach 
> to the tests.
Yeah, when I tested I had a kimchid running too... I need to figure out 
another way...
>
> On 08/13/2014 06:42 AM, lvroyce at linux.vnet.ibm.com wrote:
>> From: Royce Lv <lvroyce at linux.vnet.ibm.com>
>>
>> v1>v2, fix pep8.
>> 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 at linux.vnet.ibm.com>
>> ---
>> src/kimchi/i18n.py | 1 +
>> src/kimchi/model/vmstorages.py | 4 ++++
>> tests/test_model.py | 16 +++++++++++++++-
>> 3 files changed, 20 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..72bdccc 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,6 +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):
>> @@ -69,6 +71,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..ef0a0e6 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,13 +399,25 @@ 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})
>> + {'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)
>