[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] util: generate correct macvtap name



On 03/28/2016 06:43 AM, Shanzhi Yu wrote:
in commit 370608b, bitmap of in-used macvtap devices was introduced.
if there is already macvtap device created not by libvirt,
virNetDevMacVLanCreateWithVPortProfile will failed after try
MACVLAN_MAX_ID times call virNetDevMacVLanReleaseID

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

NACK.

This is a bonafide bug, but not the right fix. Your patch would go back and pick up the unused "macvtap0" name rather than trying macvtap2, but once macvtap0 was in use, the next call (for yet another macvtap device) would attempt to use macvtap2 (since macvtap0 and macvtap1 were in use) and would then end up looping in the same manner as it did without your patch (MACVLAN_MAX_ID times == 8191) and then still reporting a failure.

The proper fix is to call virBitmapNextClearBit() (inside virNetDevMacVLanReserveID()) with the most recently failed id (or -1) rather than with 0. This is what I've done in the following patch:

https://www.redhat.com/archives/libvir-list/2016-March/msg01286.html



Signed-off-by: Shanzhi Yu <shyu redhat com>
---
  src/util/virnetdevmacvlan.c | 16 +++++++++-------
  1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 2409113..973363e 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -988,7 +988,8 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested,
          MACVTAP_NAME_PREFIX : MACVLAN_NAME_PREFIX;
      const char *pattern = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
          MACVTAP_NAME_PATTERN : MACVLAN_NAME_PATTERN;
-    int rc, reservedID = -1;
+    int rc = -1;
+    int reservedID = 0;
      char ifname[IFNAMSIZ];
      int retries, do_retry = 0;
      uint32_t macvtapMode;
@@ -1072,16 +1073,17 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested,
      retries = MACVLAN_MAX_ID;
      while (!ifnameCreated && retries) {
          virMutexLock(&virNetDevMacVLanCreateMutex);
-        reservedID = virNetDevMacVLanReserveID(reservedID, flags, false, true);
-        if (reservedID < 0) {
-            virMutexUnlock(&virNetDevMacVLanCreateMutex);
-            return -1;
-        }
          snprintf(ifname, sizeof(ifname), pattern, reservedID);
          rc = virNetDevMacVLanCreate(ifname, type, macaddress, linkdev,
                                      macvtapMode, &do_retry);
          if (rc < 0) {
-            virNetDevMacVLanReleaseID(reservedID, flags);
+            reservedID++;
+            reservedID = virNetDevMacVLanReserveID(reservedID, flags, false, true);
+            if (reservedID < 0) {
+                virMutexUnlock(&virNetDevMacVLanCreateMutex);
+                return -1;
+            }
+
              virMutexUnlock(&virNetDevMacVLanCreateMutex);
              if (!do_retry)
                  return -1;


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]