1. Patchew
  2. Libvirt
  3. View series

[PATCH libvirt v1] conf: verify for duplicate hostdevs

Shalini Chellathurai Saroja posted 1 patch 2 years, 10 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210618104612.279981-1-shalini@linux.ibm.com
src/conf/domain_conf.c                        |  2 +-
src/conf/domain_conf.h                        |  2 +
src/conf/domain_validate.c                    | 21 ++++++++++
src/libvirt_private.syms                      |  1 +
.../hostdev-mdev-duplicate.err                |  1 +
.../hostdev-mdev-duplicate.xml                | 41 +++++++++++++++++++
.../hostdev-pci-duplicate.err                 |  1 +
.../hostdev-pci-duplicate.xml                 | 40 ++++++++++++++++++
.../hostdev-scsi-duplicate.err                |  1 +
.../hostdev-scsi-duplicate.xml                | 40 ++++++++++++++++++
.../hostdev-usb-duplicate.err                 |  1 +
.../hostdev-usb-duplicate.xml                 | 40 ++++++++++++++++++
tests/qemuxml2argvtest.c                      |  8 ++++
13 files changed, 198 insertions(+), 1 deletion(-)
create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-duplicate.err
create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-duplicate.xml
create mode 100644 tests/qemuxml2argvdata/hostdev-pci-duplicate.err
create mode 100644 tests/qemuxml2argvdata/hostdev-pci-duplicate.xml
create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-duplicate.err
create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-duplicate.xml
create mode 100644 tests/qemuxml2argvdata/hostdev-usb-duplicate.err
create mode 100644 tests/qemuxml2argvdata/hostdev-usb-duplicate.xml
[PATCH libvirt v1] conf: verify for duplicate hostdevs
Posted by Shalini Chellathurai Saroja 2 years, 10 months ago 
It is possible to define/edit(in shut off state) a domain XML with
same hostdev device repeated more than once, as shown below. This
behavior is not expected. So, this patch fixes it.

vser1:
<domain type='kvm'>
[...]
  <devices>
 [...]
    <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-ccw'>
      <source>
        <address uuid='8e782fea-e5f4-45fa-a0f9-024cf66e5009'/>
      </source>
      <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0005'/>
    </hostdev>
    <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-ccw'>
      <source>
        <address uuid='8e782fea-e5f4-45fa-a0f9-024cf66e5009'/>
      </source>
      <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0006'/>
    </hostdev>
[...]
  </devices>
</domain>

$ virsh define vser1
Domain 'vser1' defined from vser1

Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
---
 src/conf/domain_conf.c                        |  2 +-
 src/conf/domain_conf.h                        |  2 +
 src/conf/domain_validate.c                    | 21 ++++++++++
 src/libvirt_private.syms                      |  1 +
 .../hostdev-mdev-duplicate.err                |  1 +
 .../hostdev-mdev-duplicate.xml                | 41 +++++++++++++++++++
 .../hostdev-pci-duplicate.err                 |  1 +
 .../hostdev-pci-duplicate.xml                 | 40 ++++++++++++++++++
 .../hostdev-scsi-duplicate.err                |  1 +
 .../hostdev-scsi-duplicate.xml                | 40 ++++++++++++++++++
 .../hostdev-usb-duplicate.err                 |  1 +
 .../hostdev-usb-duplicate.xml                 | 40 ++++++++++++++++++
 tests/qemuxml2argvtest.c                      |  8 ++++
 13 files changed, 198 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-duplicate.err
 create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-duplicate.xml
 create mode 100644 tests/qemuxml2argvdata/hostdev-pci-duplicate.err
 create mode 100644 tests/qemuxml2argvdata/hostdev-pci-duplicate.xml
 create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-duplicate.err
 create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-duplicate.xml
 create mode 100644 tests/qemuxml2argvdata/hostdev-usb-duplicate.err
 create mode 100644 tests/qemuxml2argvdata/hostdev-usb-duplicate.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f65509d8..5746f69e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15369,7 +15369,7 @@ virDomainHostdevMatchCaps(virDomainHostdevDef *a,
 }
 
 
-static int
+int
 virDomainHostdevMatch(virDomainHostdevDef *a,
                       virDomainHostdevDef *b)
 {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index f706c498..4d9d499b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3590,6 +3590,8 @@ virDomainHostdevDef *
 virDomainHostdevRemove(virDomainDef *def, size_t i);
 int virDomainHostdevFind(virDomainDef *def, virDomainHostdevDef *match,
                          virDomainHostdevDef **found);
+int virDomainHostdevMatch(virDomainHostdevDef *a,
+                          virDomainHostdevDef *b);
 
 virDomainGraphicsListenDef *
 virDomainGraphicsGetListen(virDomainGraphicsDef *def, size_t i);
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 2124d25d..df2ab473 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -1068,7 +1068,25 @@ virDomainDefDuplicateDiskInfoValidate(const virDomainDef *def)
     return 0;
 }
 
+static int
+virDomainDefDuplicateHostdevInfoValidate(const virDomainDef *def)
+{
+    size_t i;
+    size_t j;
 
+    for (i = 0; i < def->nhostdevs; i++) {
+        for (j = i + 1; j < def->nhostdevs; j++) {
+            if (virDomainHostdevMatch(def->hostdevs[i],
+                                      def->hostdevs[j])) {
+                virReportError(VIR_ERR_XML_ERROR, "%s",
+                    _("Hostdev already exists in the domain configuration"));
+                return -1;
+            }
+        }
+    }
+
+    return 0;
+}
 
 /**
  * virDomainDefDuplicateDriveAddressesValidate:
@@ -1529,6 +1547,9 @@ virDomainDefValidateInternal(const virDomainDef *def,
     if (virDomainDefDuplicateDiskInfoValidate(def) < 0)
         return -1;
 
+    if (virDomainDefDuplicateHostdevInfoValidate(def) < 0)
+        return -1;
+
     if (virDomainDefDuplicateDriveAddressesValidate(def) < 0)
         return -1;
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2efa7876..f0d1b7b4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -457,6 +457,7 @@ virDomainHostdevDefFree;
 virDomainHostdevDefNew;
 virDomainHostdevFind;
 virDomainHostdevInsert;
+virDomainHostdevMatch;
 virDomainHostdevModeTypeToString;
 virDomainHostdevRemove;
 virDomainHostdevSubsysPCIBackendTypeToString;
diff --git a/tests/qemuxml2argvdata/hostdev-mdev-duplicate.err b/tests/qemuxml2argvdata/hostdev-mdev-duplicate.err
new file mode 100644
index 00000000..590afd30
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-mdev-duplicate.err
@@ -0,0 +1 @@
+XML error: Hostdev already exists in the domain configuration
diff --git a/tests/qemuxml2argvdata/hostdev-mdev-duplicate.xml b/tests/qemuxml2argvdata/hostdev-mdev-duplicate.xml
new file mode 100644
index 00000000..1c5e3263
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-mdev-duplicate.xml
@@ -0,0 +1,41 @@
+<domain type='qemu'>
+  <name>QEMUGuest2</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-i386</emulator>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <controller type='ide' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+    </controller>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <audio id='1' type='none'/>
+    <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-pci'>
+      <source>
+        <address uuid='53764d0e-85a0-42b4-af5c-2046b460b1dc'/>
+      </source>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+    </hostdev>
+    <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-pci'>
+      <source>
+        <address uuid='53764d0e-85a0-42b4-af5c-2046b460b1dc'/>
+      </source>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x1'/>
+    </hostdev>
+    <memballoon model='none'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/hostdev-pci-duplicate.err b/tests/qemuxml2argvdata/hostdev-pci-duplicate.err
new file mode 100644
index 00000000..590afd30
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-pci-duplicate.err
@@ -0,0 +1 @@
+XML error: Hostdev already exists in the domain configuration
diff --git a/tests/qemuxml2argvdata/hostdev-pci-duplicate.xml b/tests/qemuxml2argvdata/hostdev-pci-duplicate.xml
new file mode 100644
index 00000000..c744fddf
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-pci-duplicate.xml
@@ -0,0 +1,40 @@
+<domain type='qemu'>
+  <name>QEMUGuest2</name>
+  <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219100</memory>
+  <currentMemory unit='KiB'>219100</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-i386</emulator>
+    <disk type='block' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source dev='/dev/HostVG/QEMUGuest2'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='usb' index='0'/>
+    <controller type='ide' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <hostdev mode='subsystem' type='pci' managed='yes'>
+      <source>
+        <address domain='0x0000' bus='0x06' slot='0x12' function='0x5'/>
+      </source>
+    </hostdev>
+    <hostdev mode='subsystem' type='pci' managed='yes'>
+      <source>
+        <address domain='0x0000' bus='0x06' slot='0x12' function='0x5'/>
+      </source>
+    </hostdev>
+    <memballoon model='none'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/hostdev-scsi-duplicate.err b/tests/qemuxml2argvdata/hostdev-scsi-duplicate.err
new file mode 100644
index 00000000..590afd30
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-scsi-duplicate.err
@@ -0,0 +1 @@
+XML error: Hostdev already exists in the domain configuration
diff --git a/tests/qemuxml2argvdata/hostdev-scsi-duplicate.xml b/tests/qemuxml2argvdata/hostdev-scsi-duplicate.xml
new file mode 100644
index 00000000..ea367de3
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-scsi-duplicate.xml
@@ -0,0 +1,40 @@
+<domain type='qemu'>
+  <name>QEMUGuest2</name>
+  <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219100</memory>
+  <currentMemory unit='KiB'>219100</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-i386</emulator>
+    <disk type='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest2'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='scsi' index='0' model='virtio-scsi'/>
+    <controller type='usb' index='0'/>
+    <controller type='ide' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <hostdev mode='subsystem' type='scsi' managed='yes'>
+      <source>
+        <adapter name='scsi_host0'/>
+        <address bus='0' target='0' unit='0'/>
+      </source>
+    </hostdev>
+    <hostdev mode='subsystem' type='scsi' managed='yes'>
+      <source>
+        <adapter name='scsi_host0'/>
+        <address bus='0' target='0' unit='0'/>
+      </source>
+    </hostdev>
+    <memballoon model='virtio'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/hostdev-usb-duplicate.err b/tests/qemuxml2argvdata/hostdev-usb-duplicate.err
new file mode 100644
index 00000000..590afd30
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-usb-duplicate.err
@@ -0,0 +1 @@
+XML error: Hostdev already exists in the domain configuration
diff --git a/tests/qemuxml2argvdata/hostdev-usb-duplicate.xml b/tests/qemuxml2argvdata/hostdev-usb-duplicate.xml
new file mode 100644
index 00000000..533a9059
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-usb-duplicate.xml
@@ -0,0 +1,40 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-i386</emulator>
+    <disk type='block' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='usb' index='0'/>
+    <controller type='ide' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <hostdev mode='subsystem' type='usb' managed='no'>
+      <source>
+        <address bus='14' device='6'/>
+      </source>
+    </hostdev>
+    <hostdev mode='subsystem' type='usb' managed='no'>
+      <source>
+        <address bus='14' device='6'/>
+      </source>
+    </hostdev>
+    <memballoon model='none'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 7fed871c..b9f96532 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1915,8 +1915,11 @@ mymain(void)
     DO_TEST("hostdev-usb-address", NONE);
     DO_TEST("hostdev-usb-address-device", NONE);
     DO_TEST("hostdev-usb-address-device-boot", NONE);
+    DO_TEST_PARSE_ERROR("hostdev-usb-duplicate", NONE);
     DO_TEST("hostdev-pci-address", QEMU_CAPS_DEVICE_VFIO_PCI);
     DO_TEST("hostdev-pci-address-device", QEMU_CAPS_DEVICE_VFIO_PCI);
+    DO_TEST_PARSE_ERROR("hostdev-pci-duplicate",
+                        QEMU_CAPS_DEVICE_VFIO_PCI);
     DO_TEST("hostdev-vfio",
             QEMU_CAPS_DEVICE_VFIO_PCI);
     DO_TEST("hostdev-vfio-multidomain",
@@ -1927,6 +1930,8 @@ mymain(void)
             QEMU_CAPS_DEVICE_VFIO_PCI);
     DO_TEST_PARSE_ERROR("hostdev-mdev-invalid-target-address",
             QEMU_CAPS_DEVICE_VFIO_PCI);
+    DO_TEST_PARSE_ERROR("hostdev-mdev-duplicate",
+                        QEMU_CAPS_DEVICE_VFIO_PCI);
     DO_TEST_CAPS_LATEST("hostdev-mdev-display-spice-opengl");
     DO_TEST_CAPS_LATEST("hostdev-mdev-display-spice-egl-headless");
     DO_TEST_CAPS_LATEST("hostdev-mdev-display-vnc");
@@ -2856,6 +2861,9 @@ mymain(void)
             QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_VHOST_SCSI,
             QEMU_CAPS_DEVICE_PCIE_ROOT_PORT,
             QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY);
+    DO_TEST_PARSE_ERROR("hostdev-scsi-duplicate",
+                        QEMU_CAPS_VIRTIO_SCSI,
+                        QEMU_CAPS_DEVICE_VHOST_SCSI);
 
     DO_TEST_CAPS_VER("mlock-on", "3.0.0");
     DO_TEST_CAPS_VER("mlock-off", "3.0.0");
-- 
2.30.2
Re: [PATCH libvirt v1] conf: verify for duplicate hostdevs
Posted by Shalini Chellathurai Saroja 2 years, 9 months ago 
ping

On 6/18/21 12:46 PM, Shalini Chellathurai Saroja wrote:
> It is possible to define/edit(in shut off state) a domain XML with
> same hostdev device repeated more than once, as shown below. This
> behavior is not expected. So, this patch fixes it.
>
> vser1:
> <domain type='kvm'>
> [...]
>    <devices>
>   [...]
>      <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-ccw'>
>        <source>
>          <address uuid='8e782fea-e5f4-45fa-a0f9-024cf66e5009'/>
>        </source>
>        <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0005'/>
>      </hostdev>
>      <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-ccw'>
>        <source>
>          <address uuid='8e782fea-e5f4-45fa-a0f9-024cf66e5009'/>
>        </source>
>        <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0006'/>
>      </hostdev>
> [...]
>    </devices>
> </domain>
>
> $ virsh define vser1
> Domain 'vser1' defined from vser1
>
> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
> ---
>   src/conf/domain_conf.c                        |  2 +-
>   src/conf/domain_conf.h                        |  2 +
>   src/conf/domain_validate.c                    | 21 ++++++++++
>   src/libvirt_private.syms                      |  1 +
>   .../hostdev-mdev-duplicate.err                |  1 +
>   .../hostdev-mdev-duplicate.xml                | 41 +++++++++++++++++++
>   .../hostdev-pci-duplicate.err                 |  1 +
>   .../hostdev-pci-duplicate.xml                 | 40 ++++++++++++++++++
>   .../hostdev-scsi-duplicate.err                |  1 +
>   .../hostdev-scsi-duplicate.xml                | 40 ++++++++++++++++++
>   .../hostdev-usb-duplicate.err                 |  1 +
>   .../hostdev-usb-duplicate.xml                 | 40 ++++++++++++++++++
>   tests/qemuxml2argvtest.c                      |  8 ++++
>   13 files changed, 198 insertions(+), 1 deletion(-)
>   create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-duplicate.err
>   create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-duplicate.xml
>   create mode 100644 tests/qemuxml2argvdata/hostdev-pci-duplicate.err
>   create mode 100644 tests/qemuxml2argvdata/hostdev-pci-duplicate.xml
>   create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-duplicate.err
>   create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-duplicate.xml
>   create mode 100644 tests/qemuxml2argvdata/hostdev-usb-duplicate.err
>   create mode 100644 tests/qemuxml2argvdata/hostdev-usb-duplicate.xml
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index f65509d8..5746f69e 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -15369,7 +15369,7 @@ virDomainHostdevMatchCaps(virDomainHostdevDef *a,
>   }
>   
>   
> -static int
> +int
>   virDomainHostdevMatch(virDomainHostdevDef *a,
>                         virDomainHostdevDef *b)
>   {
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index f706c498..4d9d499b 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -3590,6 +3590,8 @@ virDomainHostdevDef *
>   virDomainHostdevRemove(virDomainDef *def, size_t i);
>   int virDomainHostdevFind(virDomainDef *def, virDomainHostdevDef *match,
>                            virDomainHostdevDef **found);
> +int virDomainHostdevMatch(virDomainHostdevDef *a,
> +                          virDomainHostdevDef *b);
>   
>   virDomainGraphicsListenDef *
>   virDomainGraphicsGetListen(virDomainGraphicsDef *def, size_t i);
> diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
> index 2124d25d..df2ab473 100644
> --- a/src/conf/domain_validate.c
> +++ b/src/conf/domain_validate.c
> @@ -1068,7 +1068,25 @@ virDomainDefDuplicateDiskInfoValidate(const virDomainDef *def)
>       return 0;
>   }
>   
> +static int
> +virDomainDefDuplicateHostdevInfoValidate(const virDomainDef *def)
> +{
> +    size_t i;
> +    size_t j;
>   
> +    for (i = 0; i < def->nhostdevs; i++) {
> +        for (j = i + 1; j < def->nhostdevs; j++) {
> +            if (virDomainHostdevMatch(def->hostdevs[i],
> +                                      def->hostdevs[j])) {
> +                virReportError(VIR_ERR_XML_ERROR, "%s",
> +                    _("Hostdev already exists in the domain configuration"));
> +                return -1;
> +            }
> +        }
> +    }
> +
> +    return 0;
> +}
>   
>   /**
>    * virDomainDefDuplicateDriveAddressesValidate:
> @@ -1529,6 +1547,9 @@ virDomainDefValidateInternal(const virDomainDef *def,
>       if (virDomainDefDuplicateDiskInfoValidate(def) < 0)
>           return -1;
>   
> +    if (virDomainDefDuplicateHostdevInfoValidate(def) < 0)
> +        return -1;
> +
>       if (virDomainDefDuplicateDriveAddressesValidate(def) < 0)
>           return -1;
>   
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 2efa7876..f0d1b7b4 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -457,6 +457,7 @@ virDomainHostdevDefFree;
>   virDomainHostdevDefNew;
>   virDomainHostdevFind;
>   virDomainHostdevInsert;
> +virDomainHostdevMatch;
>   virDomainHostdevModeTypeToString;
>   virDomainHostdevRemove;
>   virDomainHostdevSubsysPCIBackendTypeToString;
> diff --git a/tests/qemuxml2argvdata/hostdev-mdev-duplicate.err b/tests/qemuxml2argvdata/hostdev-mdev-duplicate.err
> new file mode 100644
> index 00000000..590afd30
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/hostdev-mdev-duplicate.err
> @@ -0,0 +1 @@
> +XML error: Hostdev already exists in the domain configuration
> diff --git a/tests/qemuxml2argvdata/hostdev-mdev-duplicate.xml b/tests/qemuxml2argvdata/hostdev-mdev-duplicate.xml
> new file mode 100644
> index 00000000..1c5e3263
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/hostdev-mdev-duplicate.xml
> @@ -0,0 +1,41 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest2</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219136</memory>
> +  <currentMemory unit='KiB'>219136</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-i386</emulator>
> +    <controller type='usb' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
> +    </controller>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <controller type='ide' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
> +    </controller>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <audio id='1' type='none'/>
> +    <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-pci'>
> +      <source>
> +        <address uuid='53764d0e-85a0-42b4-af5c-2046b460b1dc'/>
> +      </source>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
> +    </hostdev>
> +    <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-pci'>
> +      <source>
> +        <address uuid='53764d0e-85a0-42b4-af5c-2046b460b1dc'/>
> +      </source>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x1'/>
> +    </hostdev>
> +    <memballoon model='none'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvdata/hostdev-pci-duplicate.err b/tests/qemuxml2argvdata/hostdev-pci-duplicate.err
> new file mode 100644
> index 00000000..590afd30
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/hostdev-pci-duplicate.err
> @@ -0,0 +1 @@
> +XML error: Hostdev already exists in the domain configuration
> diff --git a/tests/qemuxml2argvdata/hostdev-pci-duplicate.xml b/tests/qemuxml2argvdata/hostdev-pci-duplicate.xml
> new file mode 100644
> index 00000000..c744fddf
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/hostdev-pci-duplicate.xml
> @@ -0,0 +1,40 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest2</name>
> +  <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219100</memory>
> +  <currentMemory unit='KiB'>219100</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-i386</emulator>
> +    <disk type='block' device='disk'>
> +      <driver name='qemu' type='raw'/>
> +      <source dev='/dev/HostVG/QEMUGuest2'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <controller type='usb' index='0'/>
> +    <controller type='ide' index='0'/>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <hostdev mode='subsystem' type='pci' managed='yes'>
> +      <source>
> +        <address domain='0x0000' bus='0x06' slot='0x12' function='0x5'/>
> +      </source>
> +    </hostdev>
> +    <hostdev mode='subsystem' type='pci' managed='yes'>
> +      <source>
> +        <address domain='0x0000' bus='0x06' slot='0x12' function='0x5'/>
> +      </source>
> +    </hostdev>
> +    <memballoon model='none'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvdata/hostdev-scsi-duplicate.err b/tests/qemuxml2argvdata/hostdev-scsi-duplicate.err
> new file mode 100644
> index 00000000..590afd30
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/hostdev-scsi-duplicate.err
> @@ -0,0 +1 @@
> +XML error: Hostdev already exists in the domain configuration
> diff --git a/tests/qemuxml2argvdata/hostdev-scsi-duplicate.xml b/tests/qemuxml2argvdata/hostdev-scsi-duplicate.xml
> new file mode 100644
> index 00000000..ea367de3
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/hostdev-scsi-duplicate.xml
> @@ -0,0 +1,40 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest2</name>
> +  <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219100</memory>
> +  <currentMemory unit='KiB'>219100</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-i386</emulator>
> +    <disk type='block' device='disk'>
> +      <source dev='/dev/HostVG/QEMUGuest2'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <controller type='scsi' index='0' model='virtio-scsi'/>
> +    <controller type='usb' index='0'/>
> +    <controller type='ide' index='0'/>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <hostdev mode='subsystem' type='scsi' managed='yes'>
> +      <source>
> +        <adapter name='scsi_host0'/>
> +        <address bus='0' target='0' unit='0'/>
> +      </source>
> +    </hostdev>
> +    <hostdev mode='subsystem' type='scsi' managed='yes'>
> +      <source>
> +        <adapter name='scsi_host0'/>
> +        <address bus='0' target='0' unit='0'/>
> +      </source>
> +    </hostdev>
> +    <memballoon model='virtio'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvdata/hostdev-usb-duplicate.err b/tests/qemuxml2argvdata/hostdev-usb-duplicate.err
> new file mode 100644
> index 00000000..590afd30
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/hostdev-usb-duplicate.err
> @@ -0,0 +1 @@
> +XML error: Hostdev already exists in the domain configuration
> diff --git a/tests/qemuxml2argvdata/hostdev-usb-duplicate.xml b/tests/qemuxml2argvdata/hostdev-usb-duplicate.xml
> new file mode 100644
> index 00000000..533a9059
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/hostdev-usb-duplicate.xml
> @@ -0,0 +1,40 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219136</memory>
> +  <currentMemory unit='KiB'>219136</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-i386</emulator>
> +    <disk type='block' device='disk'>
> +      <driver name='qemu' type='raw'/>
> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <controller type='usb' index='0'/>
> +    <controller type='ide' index='0'/>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <hostdev mode='subsystem' type='usb' managed='no'>
> +      <source>
> +        <address bus='14' device='6'/>
> +      </source>
> +    </hostdev>
> +    <hostdev mode='subsystem' type='usb' managed='no'>
> +      <source>
> +        <address bus='14' device='6'/>
> +      </source>
> +    </hostdev>
> +    <memballoon model='none'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 7fed871c..b9f96532 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -1915,8 +1915,11 @@ mymain(void)
>       DO_TEST("hostdev-usb-address", NONE);
>       DO_TEST("hostdev-usb-address-device", NONE);
>       DO_TEST("hostdev-usb-address-device-boot", NONE);
> +    DO_TEST_PARSE_ERROR("hostdev-usb-duplicate", NONE);
>       DO_TEST("hostdev-pci-address", QEMU_CAPS_DEVICE_VFIO_PCI);
>       DO_TEST("hostdev-pci-address-device", QEMU_CAPS_DEVICE_VFIO_PCI);
> +    DO_TEST_PARSE_ERROR("hostdev-pci-duplicate",
> +                        QEMU_CAPS_DEVICE_VFIO_PCI);
>       DO_TEST("hostdev-vfio",
>               QEMU_CAPS_DEVICE_VFIO_PCI);
>       DO_TEST("hostdev-vfio-multidomain",
> @@ -1927,6 +1930,8 @@ mymain(void)
>               QEMU_CAPS_DEVICE_VFIO_PCI);
>       DO_TEST_PARSE_ERROR("hostdev-mdev-invalid-target-address",
>               QEMU_CAPS_DEVICE_VFIO_PCI);
> +    DO_TEST_PARSE_ERROR("hostdev-mdev-duplicate",
> +                        QEMU_CAPS_DEVICE_VFIO_PCI);
>       DO_TEST_CAPS_LATEST("hostdev-mdev-display-spice-opengl");
>       DO_TEST_CAPS_LATEST("hostdev-mdev-display-spice-egl-headless");
>       DO_TEST_CAPS_LATEST("hostdev-mdev-display-vnc");
> @@ -2856,6 +2861,9 @@ mymain(void)
>               QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_VHOST_SCSI,
>               QEMU_CAPS_DEVICE_PCIE_ROOT_PORT,
>               QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY);
> +    DO_TEST_PARSE_ERROR("hostdev-scsi-duplicate",
> +                        QEMU_CAPS_VIRTIO_SCSI,
> +                        QEMU_CAPS_DEVICE_VHOST_SCSI);
>   
>       DO_TEST_CAPS_VER("mlock-on", "3.0.0");
>       DO_TEST_CAPS_VER("mlock-off", "3.0.0");

-- 
Kind regards
Shalini Chellathurai Saroja
Linux on Z and Virtualization Development
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [PATCH libvirt v1] conf: verify for duplicate hostdevs
Posted by Michal Prívozník 2 years, 9 months ago 
On 6/18/21 12:46 PM, Shalini Chellathurai Saroja wrote:
> It is possible to define/edit(in shut off state) a domain XML with
> same hostdev device repeated more than once, as shown below. This
> behavior is not expected. So, this patch fixes it.
> 
> vser1:
> <domain type='kvm'>
> [...]
>   <devices>
>  [...]
>     <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-ccw'>
>       <source>
>         <address uuid='8e782fea-e5f4-45fa-a0f9-024cf66e5009'/>
>       </source>
>       <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0005'/>
>     </hostdev>
>     <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-ccw'>
>       <source>
>         <address uuid='8e782fea-e5f4-45fa-a0f9-024cf66e5009'/>
>       </source>
>       <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0006'/>
>     </hostdev>
> [...]
>   </devices>
> </domain>
> 
> $ virsh define vser1
> Domain 'vser1' defined from vser1
> 
> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
> ---
>  src/conf/domain_conf.c                        |  2 +-
>  src/conf/domain_conf.h                        |  2 +
>  src/conf/domain_validate.c                    | 21 ++++++++++
>  src/libvirt_private.syms                      |  1 +
>  .../hostdev-mdev-duplicate.err                |  1 +
>  .../hostdev-mdev-duplicate.xml                | 41 +++++++++++++++++++
>  .../hostdev-pci-duplicate.err                 |  1 +
>  .../hostdev-pci-duplicate.xml                 | 40 ++++++++++++++++++
>  .../hostdev-scsi-duplicate.err                |  1 +
>  .../hostdev-scsi-duplicate.xml                | 40 ++++++++++++++++++
>  .../hostdev-usb-duplicate.err                 |  1 +
>  .../hostdev-usb-duplicate.xml                 | 40 ++++++++++++++++++
>  tests/qemuxml2argvtest.c                      |  8 ++++
>  13 files changed, 198 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-duplicate.err
>  create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-duplicate.xml
>  create mode 100644 tests/qemuxml2argvdata/hostdev-pci-duplicate.err
>  create mode 100644 tests/qemuxml2argvdata/hostdev-pci-duplicate.xml
>  create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-duplicate.err
>  create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-duplicate.xml
>  create mode 100644 tests/qemuxml2argvdata/hostdev-usb-duplicate.err
>  create mode 100644 tests/qemuxml2argvdata/hostdev-usb-duplicate.xml
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index f65509d8..5746f69e 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -15369,7 +15369,7 @@ virDomainHostdevMatchCaps(virDomainHostdevDef *a,
>  }
>  
>  
> -static int
> +int
>  virDomainHostdevMatch(virDomainHostdevDef *a,
>                        virDomainHostdevDef *b)
>  {
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index f706c498..4d9d499b 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -3590,6 +3590,8 @@ virDomainHostdevDef *
>  virDomainHostdevRemove(virDomainDef *def, size_t i);
>  int virDomainHostdevFind(virDomainDef *def, virDomainHostdevDef *match,
>                           virDomainHostdevDef **found);
> +int virDomainHostdevMatch(virDomainHostdevDef *a,
> +                          virDomainHostdevDef *b);
>  
>  virDomainGraphicsListenDef *
>  virDomainGraphicsGetListen(virDomainGraphicsDef *def, size_t i);
> diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
> index 2124d25d..df2ab473 100644
> --- a/src/conf/domain_validate.c
> +++ b/src/conf/domain_validate.c
> @@ -1068,7 +1068,25 @@ virDomainDefDuplicateDiskInfoValidate(const virDomainDef *def)
>      return 0;
>  }
>  
> +static int
> +virDomainDefDuplicateHostdevInfoValidate(const virDomainDef *def)
> +{
> +    size_t i;
> +    size_t j;
>  
> +    for (i = 0; i < def->nhostdevs; i++) {
> +        for (j = i + 1; j < def->nhostdevs; j++) {
> +            if (virDomainHostdevMatch(def->hostdevs[i],
> +                                      def->hostdevs[j])) {
> +                virReportError(VIR_ERR_XML_ERROR, "%s",
> +                    _("Hostdev already exists in the domain configuration"));
> +                return -1;
> +            }
> +        }
> +    }
> +
> +    return 0;

So this forbids duplicated hostdevs. While there are some devices that
definitely can not be assigned twice (e.g. PCI devices), I'm not so sure
about some other types (e.g. iSCSI or scsi-host). But one can also argue
that such use case is broken. So let's do the following, I'll merge it
tomorrow, after the release so that we give users the longest window
possible to complain.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal
Re: [PATCH libvirt v1] conf: verify for duplicate hostdevs
Posted by Shalini Chellathurai Saroja 2 years, 9 months ago 
On 6/30/21 4:49 PM, Michal Prívozník wrote:
> On 6/18/21 12:46 PM, Shalini Chellathurai Saroja wrote:
>> It is possible to define/edit(in shut off state) a domain XML with
>> same hostdev device repeated more than once, as shown below. This
>> behavior is not expected. So, this patch fixes it.
>>
>> vser1:
>> <domain type='kvm'>
>> [...]
>>    <devices>
>>   [...]
>>      <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-ccw'>
>>        <source>
>>          <address uuid='8e782fea-e5f4-45fa-a0f9-024cf66e5009'/>
>>        </source>
>>        <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0005'/>
>>      </hostdev>
>>      <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-ccw'>
>>        <source>
>>          <address uuid='8e782fea-e5f4-45fa-a0f9-024cf66e5009'/>
>>        </source>
>>        <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0006'/>
>>      </hostdev>
>> [...]
>>    </devices>
>> </domain>
>>
>> $ virsh define vser1
>> Domain 'vser1' defined from vser1
>>
>> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
>> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
>> ---
>>   src/conf/domain_conf.c                        |  2 +-
>>   src/conf/domain_conf.h                        |  2 +
>>   src/conf/domain_validate.c                    | 21 ++++++++++
>>   src/libvirt_private.syms                      |  1 +
>>   .../hostdev-mdev-duplicate.err                |  1 +
>>   .../hostdev-mdev-duplicate.xml                | 41 +++++++++++++++++++
>>   .../hostdev-pci-duplicate.err                 |  1 +
>>   .../hostdev-pci-duplicate.xml                 | 40 ++++++++++++++++++
>>   .../hostdev-scsi-duplicate.err                |  1 +
>>   .../hostdev-scsi-duplicate.xml                | 40 ++++++++++++++++++
>>   .../hostdev-usb-duplicate.err                 |  1 +
>>   .../hostdev-usb-duplicate.xml                 | 40 ++++++++++++++++++
>>   tests/qemuxml2argvtest.c                      |  8 ++++
>>   13 files changed, 198 insertions(+), 1 deletion(-)
>>   create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-duplicate.err
>>   create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-duplicate.xml
>>   create mode 100644 tests/qemuxml2argvdata/hostdev-pci-duplicate.err
>>   create mode 100644 tests/qemuxml2argvdata/hostdev-pci-duplicate.xml
>>   create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-duplicate.err
>>   create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-duplicate.xml
>>   create mode 100644 tests/qemuxml2argvdata/hostdev-usb-duplicate.err
>>   create mode 100644 tests/qemuxml2argvdata/hostdev-usb-duplicate.xml
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index f65509d8..5746f69e 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -15369,7 +15369,7 @@ virDomainHostdevMatchCaps(virDomainHostdevDef *a,
>>   }
>>   
>>   
>> -static int
>> +int
>>   virDomainHostdevMatch(virDomainHostdevDef *a,
>>                         virDomainHostdevDef *b)
>>   {
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index f706c498..4d9d499b 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -3590,6 +3590,8 @@ virDomainHostdevDef *
>>   virDomainHostdevRemove(virDomainDef *def, size_t i);
>>   int virDomainHostdevFind(virDomainDef *def, virDomainHostdevDef *match,
>>                            virDomainHostdevDef **found);
>> +int virDomainHostdevMatch(virDomainHostdevDef *a,
>> +                          virDomainHostdevDef *b);
>>   
>>   virDomainGraphicsListenDef *
>>   virDomainGraphicsGetListen(virDomainGraphicsDef *def, size_t i);
>> diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
>> index 2124d25d..df2ab473 100644
>> --- a/src/conf/domain_validate.c
>> +++ b/src/conf/domain_validate.c
>> @@ -1068,7 +1068,25 @@ virDomainDefDuplicateDiskInfoValidate(const virDomainDef *def)
>>       return 0;
>>   }
>>   
>> +static int
>> +virDomainDefDuplicateHostdevInfoValidate(const virDomainDef *def)
>> +{
>> +    size_t i;
>> +    size_t j;
>>   
>> +    for (i = 0; i < def->nhostdevs; i++) {
>> +        for (j = i + 1; j < def->nhostdevs; j++) {
>> +            if (virDomainHostdevMatch(def->hostdevs[i],
>> +                                      def->hostdevs[j])) {
>> +                virReportError(VIR_ERR_XML_ERROR, "%s",
>> +                    _("Hostdev already exists in the domain configuration"));
>> +                return -1;
>> +            }
>> +        }
>> +    }
>> +
>> +    return 0;
> So this forbids duplicated hostdevs. While there are some devices that
> definitely can not be assigned twice (e.g. PCI devices), I'm not so sure
> about some other types (e.g. iSCSI or scsi-host). But one can also argue
> that such use case is broken. So let's do the following, I'll merge it
> tomorrow, after the release so that we give users the longest window
> possible to complain.
>
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
ok, thank you Michal.
>
> Michal
>
-- 
Kind regards
Shalini Chellathurai Saroja
Linux on Z and Virtualization Development
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [PATCH libvirt v1] conf: verify for duplicate hostdevs
Posted by Michal Prívozník 2 years, 9 months ago 
On 7/1/21 10:31 AM, Shalini Chellathurai Saroja wrote:
> 
> On 6/30/21 4:49 PM, Michal Prívozník wrote:
>> On 6/18/21 12:46 PM, Shalini Chellathurai Saroja wrote:
>>> It is possible to define/edit(in shut off state) a domain XML with
>>> same hostdev device repeated more than once, as shown below. This
>>> behavior is not expected. So, this patch fixes it.
>>>
>>> vser1:
>>> <domain type='kvm'>
>>> [...]
>>>    <devices>
>>>   [...]
>>>      <hostdev mode='subsystem' type='mdev' managed='no'
>>> model='vfio-ccw'>
>>>        <source>
>>>          <address uuid='8e782fea-e5f4-45fa-a0f9-024cf66e5009'/>
>>>        </source>
>>>        <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0005'/>
>>>      </hostdev>
>>>      <hostdev mode='subsystem' type='mdev' managed='no'
>>> model='vfio-ccw'>
>>>        <source>
>>>          <address uuid='8e782fea-e5f4-45fa-a0f9-024cf66e5009'/>
>>>        </source>
>>>        <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0006'/>
>>>      </hostdev>
>>> [...]
>>>    </devices>
>>> </domain>
>>>
>>> $ virsh define vser1
>>> Domain 'vser1' defined from vser1
>>>
>>> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
>>> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
>>> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
>>> ---
>>>   src/conf/domain_conf.c                        |  2 +-
>>>   src/conf/domain_conf.h                        |  2 +
>>>   src/conf/domain_validate.c                    | 21 ++++++++++
>>>   src/libvirt_private.syms                      |  1 +
>>>   .../hostdev-mdev-duplicate.err                |  1 +
>>>   .../hostdev-mdev-duplicate.xml                | 41 +++++++++++++++++++
>>>   .../hostdev-pci-duplicate.err                 |  1 +
>>>   .../hostdev-pci-duplicate.xml                 | 40 ++++++++++++++++++
>>>   .../hostdev-scsi-duplicate.err                |  1 +
>>>   .../hostdev-scsi-duplicate.xml                | 40 ++++++++++++++++++
>>>   .../hostdev-usb-duplicate.err                 |  1 +
>>>   .../hostdev-usb-duplicate.xml                 | 40 ++++++++++++++++++
>>>   tests/qemuxml2argvtest.c                      |  8 ++++
>>>   13 files changed, 198 insertions(+), 1 deletion(-)
>>>   create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-duplicate.err
>>>   create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-duplicate.xml
>>>   create mode 100644 tests/qemuxml2argvdata/hostdev-pci-duplicate.err
>>>   create mode 100644 tests/qemuxml2argvdata/hostdev-pci-duplicate.xml
>>>   create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-duplicate.err
>>>   create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-duplicate.xml
>>>   create mode 100644 tests/qemuxml2argvdata/hostdev-usb-duplicate.err
>>>   create mode 100644 tests/qemuxml2argvdata/hostdev-usb-duplicate.xml

<snip/>

>> So let's do the following, I'll merge it
>> tomorrow, after the release so that we give users the longest window
>> possible to complain.
>>
>> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> ok, thank you Michal.

Pushed now.

Michal
Re: [PATCH libvirt v1] conf: verify for duplicate hostdevs
Posted by Shalini Chellathurai Saroja 2 years, 9 months ago 
On 7/1/21 4:40 PM, Michal Prívozník wrote:
>>> So let's do the following, I'll merge it
>>> tomorrow, after the release so that we give users the longest window
>>> possible to complain.
>>>
>>> Reviewed-by: Michal Privoznik<mprivozn@redhat.com>
>> ok, thank you Michal.
> Pushed now.
>
> Michal
Thank you Michal.

-- 
Kind regards
Shalini Chellathurai Saroja
Linux on Z and Virtualization Development
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.