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

[Christy Perez]

Tested-by: Christy Perez <christy at 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 at linux.vnet.ibm.com wrote:
> From: Royce Lv <lvroyce at 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 at 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)
>