[PATCH v2 1/1] qemu_command: tidy up qemuBuildHostdevCommandLine loop

[Daniel Henrique Barboza <danielhb413@xxxxxxxxx>]

The current 'for' loop with 5 consecutive 'ifs' inside
qemuBuildHostdevCommandLine can be a bit smarter:

- all 5 'ifs' fails if hostdev->mode is not equal to
VIR_DOMAIN_HOSTDEV_MODE_SUBSYS. This check can be moved to the
start of the loop, failing to the next element immediately
in case it fails;

- all 5 'ifs' checks for a specific subsys->type to build the proper
command line argument (virHostdevIsSCSIDevice and virHostdevIsMdevDevice
do that but within a helper). Problem is that the code will keep
checking for matches even if one was already found, and there is
no way a hostdev will fit more than one 'if' (i.e. a hostdev can't
have 2+ different types). This means that a SUBSYS_TYPE_USB will
create its command line argument in the first 'if', then all other
conditionals will surely fail but will end up being checked anyway.

All of this can be avoided by moving the hostdev->mode comparing
to the start of the loop and using a switch statement with
subsys->type to execute the proper code for a given hostdev
type.

Suggested-by: Ján Tomko <jtomko@xxxxxxxxxx>
Signed-off-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>
---
 src/qemu/qemu_command.c | 48 +++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index cbf25d5f07..9c3471256c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5463,23 +5463,31 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
     for (i = 0; i < def->nhostdevs; i++) {
         virDomainHostdevDefPtr hostdev = def->hostdevs[i];
         virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys;
+        virDomainHostdevSubsysSCSIPtr scsisrc;
+        virDomainHostdevSubsysMediatedDevPtr mdevsrc;
         VIR_AUTOFREE(char *) devstr = NULL;
+        VIR_AUTOFREE(char *) drvstr = NULL;
+        VIR_AUTOFREE(char *) vhostfdName = NULL;
+        unsigned int bootIndex;
+        int backend, vhostfd = -1;
 
-        /* USB */
-        if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
-            subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
+        if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
+            continue;
 
+        switch (subsys->type) {
+        /* USB */
+        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
             virCommandAddArg(cmd, "-device");
             if (!(devstr =
                   qemuBuildUSBHostdevDevStr(def, hostdev, qemuCaps)))
                 return -1;
             virCommandAddArg(cmd, devstr);
-        }
+
+            break;
 
         /* PCI */
-        if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
-            subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
-            int backend = subsys->u.pci.backend;
+        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
+            backend = subsys->u.pci.backend;
 
             if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
                 if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) {
@@ -5490,7 +5498,7 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
                 }
             }
 
-            unsigned int bootIndex = hostdev->info->bootIndex;
+            bootIndex = hostdev->info->bootIndex;
 
             /* bootNet will be non-0 if boot order was set and no other
              * net devices were encountered
@@ -5508,13 +5516,12 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
             if (!devstr)
                 return -1;
             virCommandAddArg(cmd, devstr);
-        }
+
+            break;
 
         /* SCSI */
-        if (virHostdevIsSCSIDevice(hostdev)) {
-            virDomainHostdevSubsysSCSIPtr scsisrc =
-                &hostdev->source.subsys.u.scsi;
-            VIR_AUTOFREE(char *) drvstr = NULL;
+        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
+            scsisrc = &hostdev->source.subsys.u.scsi;
 
             if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
                 virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc =
@@ -5537,15 +5544,13 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
             if (!(devstr = qemuBuildSCSIHostdevDevStr(def, hostdev)))
                 return -1;
             virCommandAddArg(cmd, devstr);
-        }
+
+            break;
 
         /* SCSI_host */
-        if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
-            subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST) {
+        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST:
             if (hostdev->source.subsys.u.scsi_host.protocol ==
                 VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_HOST_PROTOCOL_TYPE_VHOST) {
-                VIR_AUTOFREE(char *) vhostfdName = NULL;
-                int vhostfd = -1;
 
                 if (virSCSIVHostOpenVhostSCSI(&vhostfd) < 0)
                     return -1;
@@ -5567,11 +5572,12 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
 
                 virCommandAddArg(cmd, devstr);
             }
-        }
+
+            break;
 
         /* MDEV */
-        if (virHostdevIsMdevDevice(hostdev)) {
-            virDomainHostdevSubsysMediatedDevPtr mdevsrc = &subsys->u.mdev;
+       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
+            mdevsrc = &subsys->u.mdev;
 
             switch ((virMediatedDeviceModelType) mdevsrc->model) {
             case VIR_MDEV_MODEL_TYPE_VFIO_PCI:
-- 
2.21.0

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