1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/2] qemu: Fixes for (non-)transitional virtio devices
  4. View patch

[libvirt] [PATCH 1/2] qemu: Improve validation for virtio input devices

Andrea Bolognani posted 2 patches 6 years, 11 months ago
[libvirt] [PATCH 1/2] qemu: Improve validation for virtio input devices
Posted by Andrea Bolognani 6 years, 11 months ago 
While the parser and schema have to accept all possible models,
virtio-(non-)transitional models are only applicable to
type=passthrough and should be otherwise rejected.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/libvirt_private.syms                      |  1 +
 src/qemu/qemu_domain.c                        | 32 +++++++++++++++++++
 .../virtio-transitional-not-supported.xml     | 11 +++++++
 tests/qemuxml2argvtest.c                      |  4 +++
 4 files changed, 48 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/virtio-transitional-not-supported.xml

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 69643732e0..2645e55988 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -419,6 +419,7 @@ virDomainInputBusTypeToString;
 virDomainInputDefFind;
 virDomainInputDefFree;
 virDomainInputDefGetPath;
+virDomainInputTypeToString;
 virDomainIOMMUModelTypeFromString;
 virDomainIOMMUModelTypeToString;
 virDomainIOThreadIDAdd;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1487268a89..f37e6cf81a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5971,6 +5971,38 @@ qemuDomainDeviceDefValidateInput(const virDomainInputDef *input,
     if (input->bus != VIR_DOMAIN_INPUT_BUS_VIRTIO)
         return 0;
 
+    /* Only type=passthrough supports model=virtio-(non-)transitional */
+    switch ((virDomainInputModel)input->model) {
+    case VIR_DOMAIN_INPUT_MODEL_VIRTIO_TRANSITIONAL:
+    case VIR_DOMAIN_INPUT_MODEL_VIRTIO_NON_TRANSITIONAL:
+        switch ((virDomainInputType)input->type) {
+        case VIR_DOMAIN_INPUT_TYPE_MOUSE:
+        case VIR_DOMAIN_INPUT_TYPE_TABLET:
+        case VIR_DOMAIN_INPUT_TYPE_KBD:
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("virtio (non-)transitional models are not "
+                             "supported for input type=%s"),
+                           virDomainInputTypeToString(input->type));
+            return -1;
+        case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH:
+            break;
+        case VIR_DOMAIN_INPUT_TYPE_LAST:
+        default:
+            virReportEnumRangeError(virDomainInputType,
+                                    input->type);
+            return -1;
+        }
+        break;
+    case VIR_DOMAIN_INPUT_MODEL_VIRTIO:
+    case VIR_DOMAIN_INPUT_MODEL_DEFAULT:
+        break;
+    case VIR_DOMAIN_INPUT_MODEL_LAST:
+    default:
+        virReportEnumRangeError(virDomainInputModel,
+                                input->model);
+        return -1;
+    }
+
     switch ((virDomainInputType)input->type) {
     case VIR_DOMAIN_INPUT_TYPE_MOUSE:
         baseName = "virtio-mouse";
diff --git a/tests/qemuxml2argvdata/virtio-transitional-not-supported.xml b/tests/qemuxml2argvdata/virtio-transitional-not-supported.xml
new file mode 100644
index 0000000000..881d91d5a6
--- /dev/null
+++ b/tests/qemuxml2argvdata/virtio-transitional-not-supported.xml
@@ -0,0 +1,11 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <os>
+    <type arch='x86_64' machine='q35'>hvm</type>
+  </os>
+  <devices>
+    <input type='keyboard' bus='virtio' model='virtio-non-transitional'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index de9eb6abdb..113ad54b9c 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -3001,6 +3001,10 @@ mymain(void)
     DO_TEST_CAPS_VER("virtio-non-transitional", "3.1.0");
     DO_TEST_CAPS_LATEST("virtio-transitional");
     DO_TEST_CAPS_LATEST("virtio-non-transitional");
+    DO_TEST_PARSE_ERROR("virtio-transitional-not-supported",
+                        QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+                        QEMU_CAPS_DEVICE_PCI_BRIDGE,
+                        QEMU_CAPS_DEVICE_IOH3420);
 
     /* Simple headless guests for various architectures */
     DO_TEST_CAPS_ARCH_LATEST("aarch64-virt-headless", "aarch64");
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: Improve validation for virtio input devices
Posted by Cole Robinson 6 years, 11 months ago 
On 3/6/19 7:17 AM, Andrea Bolognani wrote:
> While the parser and schema have to accept all possible models,
> virtio-(non-)transitional models are only applicable to
> type=passthrough and should be otherwise rejected.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  src/libvirt_private.syms                      |  1 +
>  src/qemu/qemu_domain.c                        | 32 +++++++++++++++++++
>  .../virtio-transitional-not-supported.xml     | 11 +++++++
>  tests/qemuxml2argvtest.c                      |  4 +++
>  4 files changed, 48 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/virtio-transitional-not-supported.xml
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 69643732e0..2645e55988 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -419,6 +419,7 @@ virDomainInputBusTypeToString;
>  virDomainInputDefFind;
>  virDomainInputDefFree;
>  virDomainInputDefGetPath;
> +virDomainInputTypeToString;
>  virDomainIOMMUModelTypeFromString;
>  virDomainIOMMUModelTypeToString;
>  virDomainIOThreadIDAdd;
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 1487268a89..f37e6cf81a 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -5971,6 +5971,38 @@ qemuDomainDeviceDefValidateInput(const virDomainInputDef *input,
>      if (input->bus != VIR_DOMAIN_INPUT_BUS_VIRTIO)
>          return 0;
>  
> +    /* Only type=passthrough supports model=virtio-(non-)transitional */
> +    switch ((virDomainInputModel)input->model) {
> +    case VIR_DOMAIN_INPUT_MODEL_VIRTIO_TRANSITIONAL:
> +    case VIR_DOMAIN_INPUT_MODEL_VIRTIO_NON_TRANSITIONAL:
> +        switch ((virDomainInputType)input->type) {
> +        case VIR_DOMAIN_INPUT_TYPE_MOUSE:
> +        case VIR_DOMAIN_INPUT_TYPE_TABLET:
> +        case VIR_DOMAIN_INPUT_TYPE_KBD:
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("virtio (non-)transitional models are not "
> +                             "supported for input type=%s"),
> +                           virDomainInputTypeToString(input->type));
> +            return -1;
> +        case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH:
> +            break;
> +        case VIR_DOMAIN_INPUT_TYPE_LAST:
> +        default:
> +            virReportEnumRangeError(virDomainInputType,
> +                                    input->type);
> +            return -1;
> +        }
> +        break;
> +    case VIR_DOMAIN_INPUT_MODEL_VIRTIO:
> +    case VIR_DOMAIN_INPUT_MODEL_DEFAULT:
> +        break;
> +    case VIR_DOMAIN_INPUT_MODEL_LAST:
> +    default:
> +        virReportEnumRangeError(virDomainInputModel,
> +                                input->model);
> +        return -1;
> +    }
> +
>      switch ((virDomainInputType)input->type) {
>      case VIR_DOMAIN_INPUT_TYPE_MOUSE:
>          baseName = "virtio-mouse";
> diff --git a/tests/qemuxml2argvdata/virtio-transitional-not-supported.xml b/tests/qemuxml2argvdata/virtio-transitional-not-supported.xml
> new file mode 100644
> index 0000000000..881d91d5a6
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/virtio-transitional-not-supported.xml
> @@ -0,0 +1,11 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219136</memory>
> +  <os>
> +    <type arch='x86_64' machine='q35'>hvm</type>
> +  </os>
> +  <devices>
> +    <input type='keyboard' bus='virtio' model='virtio-non-transitional'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index de9eb6abdb..113ad54b9c 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -3001,6 +3001,10 @@ mymain(void)
>      DO_TEST_CAPS_VER("virtio-non-transitional", "3.1.0");
>      DO_TEST_CAPS_LATEST("virtio-transitional");
>      DO_TEST_CAPS_LATEST("virtio-non-transitional");
> +    DO_TEST_PARSE_ERROR("virtio-transitional-not-supported",
> +                        QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
> +                        QEMU_CAPS_DEVICE_PCI_BRIDGE,
> +                        QEMU_CAPS_DEVICE_IOH3420);
>  
>      /* Simple headless guests for various architectures */
>      DO_TEST_CAPS_ARCH_LATEST("aarch64-virt-headless", "aarch64");
> 

I'm thoroughly 'meh' to the idea of adding XML tests to exercise
validation failures like this, but only enough to make this comment

Reviewed-by: Cole Robinson <crobinso@redhat.com>

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: Improve validation for virtio input devices
Posted by Andrea Bolognani 6 years, 11 months ago 
On Fri, 2019-03-08 at 15:05 -0500, Cole Robinson wrote:
> On 3/6/19 7:17 AM, Andrea Bolognani wrote:
> > +    DO_TEST_PARSE_ERROR("virtio-transitional-not-supported",
> > +                        QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
> > +                        QEMU_CAPS_DEVICE_PCI_BRIDGE,
> > +                        QEMU_CAPS_DEVICE_IOH3420);
> 
> I'm thoroughly 'meh' to the idea of adding XML tests to exercise
> validation failures like this, but only enough to make this comment

I respectfully disagree: having test coverage for the negative cases
is very useful to stop regressions from sneaking back in.

Anyway, I've fixed the typo you spotted and pushed the series.
Thanks for the review! :)

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.