qemu: Handle huge number of queues correctly

Currently, kernel supports up to 8 queues for a multiqueue tap device.
However, if user tries to enter a huge number (e.g. one million) the tap
allocation fails, as expected. But what is not expected is the log full
of warnings:

    warning : virFileClose:83 : Tried to close invalid fd 0

The problem is, upon error we iterate over an array of FDs (handlers to
queues) and VIR_FORCE_CLOSE() over each item. However, the array is
pre-filled with zeros. Hence, we repeatedly close stdin. Ouch.
But there's more. The queues allocation is done in virNetDevTapCreate()
which cleans up the FDs in case of error. Then, its caller, the
virNetDevTapCreateInBridgePort() iterates over the FD array and tries to
close them too. And so does qemuNetworkIfaceConnect() and
qemuBuildInterfaceCommandLine().

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 459b03e43ecc1cb57272e3491a95207beeef906c..643532ffe4875fa20d8da0a15d435223408c9bcf 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -406,7 +406,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
 cleanup:
     if (ret < 0) {
         size_t i;
-        for (i = 0; i < *tapfdSize; i++)
+        for (i = 0; i < *tapfdSize && tapfd[i] >= 0; i++)
             VIR_FORCE_CLOSE(tapfd[i]);
         if (template_ifname)
             VIR_FREE(net->ifname);
@@ -7338,6 +7338,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
             VIR_ALLOC_N(tapfdName, tapfdSize) < 0)
             goto cleanup;
 
+        memset(tapfd, -1, tapfdSize * sizeof(tapfd[0]));
+
         if (qemuNetworkIfaceConnect(def, conn, driver, net,
                                     qemuCaps, tapfd, &tapfdSize) < 0)
             goto cleanup;
@@ -7365,6 +7367,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
             VIR_ALLOC_N(vhostfdName, vhostfdSize))
             goto cleanup;
 
+        memset(vhostfd, -1, vhostfdSize * sizeof(vhostfd[0]));
+
         if (qemuOpenVhostNet(def, net, qemuCaps, vhostfd, &vhostfdSize) < 0)
             goto cleanup;
     }
@@ -7424,13 +7428,13 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
 cleanup:
     if (ret < 0)
         virDomainConfNWFilterTeardown(net);
-    for (i = 0; tapfd && i < tapfdSize; i++) {
+    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; i++) {
+    for (i = 0; vhostfd && i < vhostfdSize && vhostfd[i] >= 0; i++) {
         if (ret < 0)
             VIR_FORCE_CLOSE(vhostfd[i]);
         if (vhostfdName)

diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 42e8dfe25160e6e58f6beb56517fbbea640f7f8c..fb173e33102702fabbf6bc351e8e328139b3d8fb 100644 (file)
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -445,6 +445,7 @@ int virNetDevTapCreateInBridgePort(const char *brname,
 {
     virMacAddr tapmac;
     char macaddrstr[VIR_MAC_STRING_BUFLEN];
+    size_t i;
 
     if (virNetDevTapCreate(ifname, tapfd, tapfdSize, flags) < 0)
         return -1;
@@ -498,8 +499,8 @@ int virNetDevTapCreateInBridgePort(const char *brname,
     return 0;
 
 error:
-    while (tapfdSize)
-        VIR_FORCE_CLOSE(tapfd[--tapfdSize]);
+    for (i = 0; i < tapfdSize && tapfd[i] >= 0; i++)
+        VIR_FORCE_CLOSE(tapfd[i]);
 
     return -1;
 }