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(a)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;