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

[Aline Manera]

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.

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)