From 78e74acdcac9ada430b822afa6b5e1dadae575e8 Mon Sep 17 00:00:00 2001

Message-Id: <78e74acdcac9ada430b822afa6b5e1dadae575e8.1378475168.git.jdenemar@redhat.com>

From: Michal Privoznik <mprivozn@redhat.com>

Date: Tue, 3 Sep 2013 15:18:46 +0200

Subject: [PATCH] qemu: Handle huge number of queues correctly

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

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().

(cherry picked from commit 1dc5dea7d605d3b02c64940fc3566a7765dce3c3)

---

 src/qemu/qemu_command.c | 10 +++++++---

 src/util/virnetdevtap.c |  5 +++--

 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c

index 6dfd141..6e9657f 100644

--- a/src/qemu/qemu_command.c

+++ b/src/qemu/qemu_command.c

@@ -405,7 +405,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);

@@ -7215,6 +7215,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;

@@ -7242,6 +7244,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;

     }

@@ -7303,13 +7307,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 42e8dfe..fb173e3 100644

--- 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;

 }

-- 

1.8.3.2