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

Daniel Henrique Barboza danielhb413 at gmail.com
Wed Oct 2 12:37:38 UTC 2019


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.
This can be avoided by adding 'continue' statements inside the first
4 conditionals.

Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
---

Arguably safe for freeze since it's trivial, but not a deal
breaker if postponed. I am fine with both.


 src/qemu/qemu_command.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8c979fdf65..0357481dd1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5471,20 +5471,23 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
         virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys;
         VIR_AUTOFREE(char *) devstr = NULL;
 
+        if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
+            continue;
+
         /* USB */
-        if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
-            subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
+        if (subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
 
             virCommandAddArg(cmd, "-device");
             if (!(devstr =
                   qemuBuildUSBHostdevDevStr(def, hostdev, qemuCaps)))
                 return -1;
             virCommandAddArg(cmd, devstr);
+
+            continue;
         }
 
         /* PCI */
-        if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
-            subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
+        if (subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
             int backend = subsys->u.pci.backend;
 
             if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
@@ -5514,6 +5517,8 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
             if (!devstr)
                 return -1;
             virCommandAddArg(cmd, devstr);
+
+            continue;
         }
 
         /* SCSI */
@@ -5543,11 +5548,12 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
             if (!(devstr = qemuBuildSCSIHostdevDevStr(def, hostdev)))
                 return -1;
             virCommandAddArg(cmd, devstr);
+
+            continue;
         }
 
         /* SCSI_host */
-        if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
-            subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST) {
+        if (subsys->type == 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;
@@ -5573,6 +5579,8 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
 
                 virCommandAddArg(cmd, devstr);
             }
+
+            continue;
         }
 
         /* MDEV */
-- 
2.21.0




More information about the libvir-list mailing list