[libvirt] [PATCH] qemu: Dissolve qemuBuildVhostuserCommandLine in qemuBuildInterfaceCommandLine

Michal Privoznik mprivozn at redhat.com
Fri Nov 2 12:56:17 UTC 2018


https://bugzilla.redhat.com/show_bug.cgi?id=1524230

The qemuBuildVhostuserCommandLine builds command line for
vhostuser type interfaces. It is duplicating some code of the
function it is called from (qemuBuildInterfaceCommandLine)
because of the way it's called. If we merge it into the caller
not only we save a few lines but we also enable checks that we
would have to duplicate otherwise (e.g. QoS availability).

While at it, reorder some VIR_FREE() in
qemuBuildInterfaceCommandLine so that it is easier to track which
variables are freed and which are not.

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/qemu/qemu_command.c | 113 +++++++++++++++++-----------------------
 1 file changed, 48 insertions(+), 65 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6e3ff67660..e338d3172e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8224,34 +8224,24 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
 }
 
 static int
-qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
+qemuInterfaceVhostuserConnect(virQEMUDriverPtr driver,
                               virLogManagerPtr logManager,
                               virSecurityManagerPtr secManager,
                               virCommandPtr cmd,
                               virDomainDefPtr def,
                               virDomainNetDefPtr net,
                               virQEMUCapsPtr qemuCaps,
-                              unsigned int bootindex)
+                              char **chardev)
 {
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
-    char *chardev = NULL;
-    char *netdev = NULL;
-    unsigned int queues = net->driver.virtio.queues;
-    char *nic = NULL;
     int ret = -1;
 
-    if (!qemuDomainSupportsNicdev(def, net)) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       "%s", _("Nicdev support unavailable"));
-        goto cleanup;
-    }
-
     switch ((virDomainChrType)net->data.vhostuser->type) {
     case VIR_DOMAIN_CHR_TYPE_UNIX:
-        if (!(chardev = qemuBuildChrChardevStr(logManager, secManager,
-                                               cmd, cfg, def,
-                                               net->data.vhostuser,
-                                               net->info.alias, qemuCaps, 0)))
+        if (!(*chardev = qemuBuildChrChardevStr(logManager, secManager,
+                                                cmd, cfg, def,
+                                                net->data.vhostuser,
+                                                net->info.alias, qemuCaps, 0)))
             goto cleanup;
         break;
 
@@ -8274,42 +8264,9 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
         goto cleanup;
     }
 
-    if (queues > 1 &&
-        !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE)) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("multi-queue is not supported for vhost-user "
-                         "with this QEMU binary"));
-        goto cleanup;
-    }
-
-    if (!(netdev = qemuBuildHostNetStr(net, driver,
-                                       NULL, 0, NULL, 0)))
-        goto cleanup;
-
-    if (virNetDevOpenvswitchGetVhostuserIfname(net->data.vhostuser->data.nix.path,
-                                               &net->ifname) < 0)
-        goto cleanup;
-
-    virCommandAddArg(cmd, "-chardev");
-    virCommandAddArg(cmd, chardev);
-
-    virCommandAddArg(cmd, "-netdev");
-    virCommandAddArg(cmd, netdev);
-
-    if (!(nic = qemuBuildNicDevStr(def, net, bootindex,
-                                   queues, qemuCaps))) {
-        goto cleanup;
-    }
-
-    virCommandAddArgList(cmd, "-device", nic, NULL);
-
     ret = 0;
  cleanup:
     virObjectUnref(cfg);
-    VIR_FREE(netdev);
-    VIR_FREE(chardev);
-    VIR_FREE(nic);
-
     return ret;
 }
 
@@ -8328,7 +8285,9 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
                               int **nicindexes)
 {
     int ret = -1;
-    char *nic = NULL, *host = NULL;
+    char *nic = NULL;
+    char *host = NULL;
+    char *chardev = NULL;
     int *tapfd = NULL;
     size_t tapfdSize = 0;
     int *vhostfd = NULL;
@@ -8337,6 +8296,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
     char **vhostfdName = NULL;
     virDomainNetType actualType = virDomainNetGetActualType(net);
     virNetDevBandwidthPtr actualBandwidth;
+    bool requireNicdev = false;
     size_t i;
 
 
@@ -8447,9 +8407,24 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
         break;
 
     case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
-        ret = qemuBuildVhostuserCommandLine(driver, logManager, secManager, cmd, def,
-                                            net, qemuCaps, bootindex);
-        goto cleanup;
+        requireNicdev = true;
+
+        if (net->driver.virtio.queues > 1 &&
+            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("multi-queue is not supported for vhost-user "
+                             "with this QEMU binary"));
+            goto cleanup;
+        }
+
+        if (qemuInterfaceVhostuserConnect(driver, logManager, secManager,
+                                          cmd, def, net, qemuCaps, &chardev) < 0)
+            goto cleanup;
+
+        if (virNetDevOpenvswitchGetVhostuserIfname(net->data.vhostuser->data.nix.path,
+                                                   &net->ifname) < 0)
+            goto cleanup;
+
         break;
 
     case VIR_DOMAIN_NET_TYPE_USER:
@@ -8564,6 +8539,9 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
             goto cleanup;
     }
 
+    if (chardev)
+        virCommandAddArgList(cmd, "-chardev", chardev, NULL);
+
     if (!(host = qemuBuildHostNetStr(net, driver,
                                      tapfdName, tapfdSize,
                                      vhostfdName, vhostfdSize)))
@@ -8578,13 +8556,17 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
      */
     if (qemuDomainSupportsNicdev(def, net)) {
         if (!(nic = qemuBuildNicDevStr(def, net, bootindex,
-                                       vhostfdSize, qemuCaps)))
+                                       net->driver.virtio.queues, qemuCaps)))
             goto cleanup;
         virCommandAddArgList(cmd, "-device", nic, NULL);
-    } else {
+    } else if (!requireNicdev) {
         if (!(nic = qemuBuildLegacyNicStr(net)))
             goto cleanup;
         virCommandAddArgList(cmd, "-net", nic, NULL);
+    } else {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       "%s", _("Nicdev support unavailable"));
+        goto cleanup;
     }
 
     ret = 0;
@@ -8595,24 +8577,25 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
         virSetError(saved_err);
         virFreeError(saved_err);
     }
-    for (i = 0; tapfd && i < tapfdSize && tapfd[i] >= 0; i++) {
-        if (ret < 0)
-            VIR_FORCE_CLOSE(tapfd[i]);
-        if (tapfdName)
-            VIR_FREE(tapfdName[i]);
-    }
     for (i = 0; vhostfd && i < vhostfdSize && vhostfd[i] >= 0; i++) {
         if (ret < 0)
             VIR_FORCE_CLOSE(vhostfd[i]);
         if (vhostfdName)
             VIR_FREE(vhostfdName[i]);
     }
-    VIR_FREE(tapfd);
+    VIR_FREE(vhostfdName);
+    for (i = 0; tapfd && i < tapfdSize && tapfd[i] >= 0; i++) {
+        if (ret < 0)
+            VIR_FORCE_CLOSE(tapfd[i]);
+        if (tapfdName)
+            VIR_FREE(tapfdName[i]);
+    }
+    VIR_FREE(tapfdName);
     VIR_FREE(vhostfd);
-    VIR_FREE(nic);
+    VIR_FREE(tapfd);
+    VIR_FREE(chardev);
     VIR_FREE(host);
-    VIR_FREE(tapfdName);
-    VIR_FREE(vhostfdName);
+    VIR_FREE(nic);
     return ret;
 }
 
-- 
2.18.1




More information about the libvir-list mailing list