Re: [libvirt] [libvirt PATCHv8 1/1] add DHCP snooping

On 03/30/2012 03:07 PM, David L Stevens wrote: This is the 2nd version to patch on top of this v8. Some more testing revealed a locking problem that I now fixed. Cleanup some parts of the DHCP snooping v8 code addressing - naming consistency - memory leaks - if-before-free not being necessary - separate shutdown function (for avoiding a valgrind reporting memory leak) - a compilation error ("%d", strlen()) - pass NULL for a pointer rather than 0 - use common exits where possible - make 'make syntax-check' pass - due to a locking bug resulting in a deadlock reworked the locking and introduced a reference counter for the SnoopReq that must be held while the 'req' variable is accessed; it resulred in finer grained locking with the virNWFilterSnoopLock() --- po/POTFILES.in | 1 src/nwfilter/nwfilter_dhcpsnoop.c | 360 +++++++++++++++++++++++++------------- src/nwfilter/nwfilter_dhcpsnoop.h | 1 src/nwfilter/nwfilter_driver.c | 6 4 files changed, 246 insertions(+), 122 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_dhcpsnoop.c +++ libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.c @@ -59,6 +59,7 @@ static struct { int LeaseFD; int nLeases; /* number of active leases */ int wLeases; /* number of written leases */ + int nThreads; /* number of running threads */ /* thread management */ virHashTablePtr SnoopReqs; virHashTablePtr IfnameToKey; @@ -67,23 +68,21 @@ static struct { virMutex ActiveLock; } virNWFilterSnoopState = { .LeaseFD = -1, - .SnoopLock = { .lock=PTHREAD_MUTEX_INITIALIZER }, - .ActiveLock = { .lock=PTHREAD_MUTEX_INITIALIZER }, }; #define virNWFilterSnoopLock() \ { virMutexLock(&virNWFilterSnoopState.SnoopLock); } #define virNWFilterSnoopUnlock() \ { virMutexUnlock(&virNWFilterSnoopState.SnoopLock); } -#define virNWFilterSnoopLockActive() \ +#define virNWFilterSnoopActiveLock() \ { virMutexLock(&virNWFilterSnoopState.ActiveLock); } -#define virNWFilterSnoopUnlockActive() \ +#define virNWFilterSnoopActiveUnlock() \ { virMutexUnlock(&virNWFilterSnoopState.ActiveLock); } static char * virNWFilterSnoopActivate(virThreadPtr thread) { - char *key, *data; + char *key; unsigned long threadID = (unsigned long int)thread->thread; if (virAsprintf(&key, "0x%0*lX", (int)sizeof(threadID)*2, threadID) < 0) { @@ -91,16 +90,11 @@ virNWFilterSnoopActivate(virThreadPtr th return NULL; } - virNWFilterSnoopLockActive(); - data = strdup("1"); - if (data == NULL) { - virReportOOMError(); - VIR_FREE(key); - } else if (virHashAddEntry(virNWFilterSnoopState.Active, key, data)) { + virNWFilterSnoopActiveLock(); + if (virHashAddEntry(virNWFilterSnoopState.Active, key, (void *)0x1)) { VIR_FREE(key); - VIR_FREE(data); } - virNWFilterSnoopUnlockActive(); + virNWFilterSnoopActiveUnlock(); return key; } @@ -110,10 +104,10 @@ virNWFilterSnoopCancel(char **ThreadKey) if (*ThreadKey == NULL) return; - virNWFilterSnoopLockActive(); + virNWFilterSnoopActiveLock(); (void) virHashRemoveEntry(virNWFilterSnoopState.Active, *ThreadKey); - *ThreadKey = NULL; - virNWFilterSnoopUnlockActive(); + VIR_FREE(*ThreadKey); + virNWFilterSnoopActiveUnlock(); } static bool @@ -123,15 +117,18 @@ virNWFilterSnoopIsActive(char *ThreadKey if (ThreadKey == NULL) return 0; - virNWFilterSnoopLockActive(); + + virNWFilterSnoopActiveLock(); entry = virHashLookup(virNWFilterSnoopState.Active, ThreadKey); - virNWFilterSnoopUnlockActive(); + virNWFilterSnoopActiveUnlock(); + return entry != NULL; } #define VIR_IFKEY_LEN ((VIR_UUID_STRING_BUFLEN) + (VIR_MAC_STRING_BUFLEN)) struct virNWFilterSnoopReq { + unsigned int refctr; virNWFilterTechDriverPtr techdriver; const char *ifname; int ifindex; @@ -168,6 +165,7 @@ static void virNWFilterSnoopUpdateLease( time_t timeout); static struct virNWFilterSnoopReq *virNWFilterSnoopNewReq(const char *ifkey); +static void virNWFilterSnoopReqRelease(void *req0, const void *name ATTRIBUTE_UNUSED); static void virNWFilterSnoopLeaseFileOpen(void); static void virNWFilterSnoopLeaseFileClose(void); @@ -176,6 +174,56 @@ static void virNWFilterSnoopLeaseFileSav static void virNWFilterSnoopLeaseFileRestore(struct virNWFilterSnoopReq *req); static void virNWFilterSnoopLeaseFileRefresh(void); +static void +virNWFilterSnoopReqGet(struct virNWFilterSnoopReq *req) +{ + req->refctr++; +} + +static struct virNWFilterSnoopReq * +virNWFilterSnoopReqGetByKey(const char *ifkey) +{ + struct virNWFilterSnoopReq *req; + + virNWFilterSnoopLock(); + + req = virHashLookup(virNWFilterSnoopState.SnoopReqs, ifkey); + if (req) + virNWFilterSnoopReqGet(req); + + virNWFilterSnoopUnlock(); + + return req; +} + +static void +virNWFilterSnoopReqPut(struct virNWFilterSnoopReq *req) +{ + if (!req) + return; + + virNWFilterSnoopLock() + + req->refctr--; + + if (req->refctr == 0) { + /* + * delete the request: + * - if we don't find it on the global list anymore + * we would keep the request: + * - if we still have a valid lease, keep the req for restarts + */ + if (virHashLookup(virNWFilterSnoopState.SnoopReqs, req->ifkey) != req) { + virNWFilterSnoopReqRelease(req, NULL); + } else if (!req->start || req->start->Timeout < time(0)) { + (void) virHashRemoveEntry(virNWFilterSnoopState.SnoopReqs, + req->ifkey); + } + } + + virNWFilterSnoopUnlock(); +} + /* * virNWFilterSnoopListAdd - add an IP lease to a list */ @@ -297,7 +345,11 @@ virNWFilterSnoopLeaseAdd(struct virNWFil return; } virNWFilterSnoopTimerAdd(pl); + + virNWFilterSnoopLock(); virNWFilterSnoopState.nLeases++; + virNWFilterSnoopUnlock(); + if (update_leasefile) virNWFilterSnoopLeaseFileSave(pl); } @@ -361,7 +413,10 @@ virNWFilterSnoopLeaseDel(struct virNWFil _("virNWFilterSnoopListDel failed")); } VIR_FREE(ipl); + + virNWFilterSnoopLock(); virNWFilterSnoopState.nLeases--; + virNWFilterSnoopUnlock(); } /* @@ -624,6 +679,9 @@ virNWFilterSnoopReqFree(struct virNWFilt if (!req) return; + if (req->refctr != 0) + return; + /* free all leases */ for (ipl = req->start; ipl; ipl = req->start) virNWFilterSnoopLeaseDel(req, ipl->IPAddress, 0); @@ -633,6 +691,7 @@ virNWFilterSnoopReqFree(struct virNWFilt VIR_FREE(req->linkdev); VIR_FREE(req->filtername); virNWFilterHashTableFree(req->vars); + VIR_FREE(req); } @@ -645,43 +704,38 @@ virNWFilterDHCPSnoop(void *req0) struct virNWFilterSnoopEthHdr *packet; int ifindex; int errcount; - char *threadkey; + char *threadkey = NULL; virThread thread; + /* the thread starting us increased the reference counter for the req */ + handle = virNWFilterSnoopDHCPOpen(req->ifname); if (!handle) - return; + goto exit; virThreadSelf(&thread); req->threadkey = virNWFilterSnoopActivate(&thread); threadkey = strdup(req->threadkey); if (threadkey == NULL) { virReportOOMError(); - pcap_close(handle); - return; + goto exit; } ifindex = if_nametoindex(req->ifname); - virNWFilterSnoopLock(); virNWFilterSnoopLeaseFileRestore(req); - virNWFilterSnoopUnlock(); errcount = 0; while (1) { int rv; - virNWFilterSnoopLock(); virNWFilterSnoopTimerRun(req); - virNWFilterSnoopUnlock(); rv = pcap_next_ex(handle, &hdr, (const u_char **)&packet); - if (!virNWFilterSnoopIsActive(threadkey)) { - VIR_FREE(threadkey); - pcap_close(handle); - return; - } + if (!virNWFilterSnoopIsActive(threadkey)) + goto exit; + if (rv < 0) { if (virNetDevValidateConfig(req->ifname, NULL, ifindex) <= 0) break; @@ -697,20 +751,31 @@ virNWFilterDHCPSnoop(void *req0) continue; } errcount = 0; - if (rv) { - virNWFilterSnoopLock(); + if (rv) virNWFilterSnoopDHCPDecode(req, packet, hdr->caplen); - virNWFilterSnoopUnlock(); - } } virNWFilterSnoopCancel(&req->threadkey); + + virNWFilterSnoopLock(); + (void) virHashRemoveEntry(virNWFilterSnoopState.IfnameToKey, req->ifname); + + virNWFilterSnoopUnlock(); + VIR_FREE(req->ifname); - /* if we still have a valid lease, keep the req for restarts */ - if (!req->start || req->start->Timeout < time(0)) - (void) virHashRemoveEntry(virNWFilterSnoopState.SnoopReqs, req->ifkey); + +exit: + virNWFilterSnoopReqPut(req); + VIR_FREE(threadkey); - pcap_close(handle); + + if (handle) + pcap_close(handle); + + virNWFilterSnoopLock(); + virNWFilterSnoopState.nThreads--; + virNWFilterSnoopUnlock(); + return; } @@ -740,22 +805,23 @@ virNWFilterDHCPSnoopReq(virNWFilterTechD virThread thread; virNWFilterSnoopIFKeyFMT(ifkey, vmuuid, macaddr); - virNWFilterSnoopLock(); - req = virHashLookup(virNWFilterSnoopState.SnoopReqs, ifkey); - isnewreq = req == NULL; + + req = virNWFilterSnoopReqGetByKey(ifkey); + isnewreq = (req == NULL); if (!isnewreq) { if (req->threadkey) { - virNWFilterSnoopUnlock(); + virNWFilterSnoopReqPut(req); return 0; } } else { req = virNWFilterSnoopNewReq(ifkey); - if (!req) { - virNWFilterSnoopUnlock(); + if (!req) return -1; - } + + virNWFilterSnoopReqGet(req); } + req->driver = driver; req->techdriver = techdriver; req->ifindex = if_nametoindex(ifname); req->linkdev = linkdev ? strdup(linkdev) : NULL; @@ -763,11 +829,10 @@ virNWFilterDHCPSnoopReq(virNWFilterTechD req->ifname = strdup(ifname); memcpy(req->macaddr, macaddr, sizeof(req->macaddr)); req->filtername = strdup(filtername); + if (req->ifname == NULL || req->filtername == NULL) { - virNWFilterSnoopUnlock(); - virNWFilterSnoopReqFree(req); virReportOOMError(); - return -1; + goto exit_snoopreqput; } if (techdriver->applyDHCPOnlyRules(req->ifname, req->macaddr, NULL, @@ -778,20 +843,18 @@ virNWFilterDHCPSnoopReq(virNWFilterTechD req->vars = virNWFilterHashTableCreate(0); if (!req->vars) { - virNWFilterSnoopUnlock(); - virNWFilterSnoopReqFree(req); virReportOOMError(); - return -1; + goto exit_snoopreqput; } + if (virNWFilterHashTablePutAll(filterparams, req->vars)) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("virNWFilterDHCPSnoopReq: can't copy variables" " on if %s"), ifkey); - virNWFilterSnoopUnlock(); - virNWFilterSnoopReqFree(req); - return -1; + goto exit_snoopreqput; } - req->driver = driver; + + virNWFilterSnoopLock(); if (virHashAddEntry(virNWFilterSnoopState.IfnameToKey, ifname, req->ifkey)) { @@ -799,9 +862,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechD _("virNWFilterDHCPSnoopReq ifname map failed" " on interface \"%s\" key \"%s\""), ifname, ifkey); - virNWFilterSnoopUnlock(); - virNWFilterSnoopReqFree(req); - return -1; + goto exit_snoopunlock; } if (isnewreq && virHashAddEntry(virNWFilterSnoopState.SnoopReqs, ifkey, req)) { @@ -810,23 +871,30 @@ virNWFilterDHCPSnoopReq(virNWFilterTechD " interface \"%s\" ifkey \"%s\""), ifname, ifkey); (void) virHashRemoveEntry(virNWFilterSnoopState.IfnameToKey, ifname); - virNWFilterSnoopUnlock(); - virNWFilterSnoopReqFree(req); - return -1; + goto exit_snoopunlock; } - virNWFilterSnoopUnlock(); + + virNWFilterSnoopState.nThreads++; + if (virThreadCreate(&thread, false, virNWFilterDHCPSnoop, req) != 0) { - virNWFilterSnoopLock(); + virNWFilterSnoopState.nThreads--; (void) virHashRemoveEntry(virNWFilterSnoopState.IfnameToKey, ifname); - (void) virHashRemoveEntry(virNWFilterSnoopState.SnoopReqs, ifkey); - virNWFilterSnoopUnlock(); - virNWFilterSnoopReqFree(req); virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("virNWFilterDHCPSnoopReq virThreadCreate " "failed on interface \"%s\""), ifname); - return -1; + goto exit_snoopunlock; } + + virNWFilterSnoopUnlock(); + return 0; + +exit_snoopunlock: + virNWFilterSnoopUnlock(); +exit_snoopreqput: + virNWFilterSnoopReqPut(req); + + return -1; } /* @@ -842,6 +910,7 @@ virNWFilterSnoopReqRelease(void *req0, c if (req->threadkey) virNWFilterSnoopCancel(&req->threadkey); + virNWFilterSnoopReqFree(req); } @@ -863,13 +932,21 @@ virNWFilterSnoopLeaseFileOpen(void) int virNWFilterDHCPSnoopInit(void) { - if (virNWFilterSnoopState.SnoopReqs) + if (virNWFilterSnoopState.SnoopReqs) { return 0; + } + + if (virMutexInitRecursive(&virNWFilterSnoopState.SnoopLock) < 0) + return -1; + if (virMutexInit(&virNWFilterSnoopState.ActiveLock) < 0) + return -1; virNWFilterSnoopLock(); + virNWFilterSnoopState.IfnameToKey = virHashCreate(0, NULL); virNWFilterSnoopState.SnoopReqs = virHashCreate(0, virNWFilterSnoopReqRelease); + if (!virNWFilterSnoopState.IfnameToKey || !virNWFilterSnoopState.SnoopReqs) { virNWFilterSnoopUnlock(); @@ -881,29 +958,28 @@ virNWFilterDHCPSnoopInit(void) virNWFilterSnoopUnlock(); - virNWFilterSnoopLockActive(); - virNWFilterSnoopState.Active = virHashCreate(0, 0); + virNWFilterSnoopActiveLock(); + + virNWFilterSnoopState.Active = virHashCreate(0, NULL); if (!virNWFilterSnoopState.Active) { - virNWFilterSnoopUnlockActive(); + virNWFilterSnoopActiveUnlock(); virReportOOMError(); goto errexit; } - virNWFilterSnoopUnlockActive(); + virNWFilterSnoopActiveUnlock(); + return 0; errexit: - if (virNWFilterSnoopState.IfnameToKey) { - virHashFree(virNWFilterSnoopState.IfnameToKey); - virNWFilterSnoopState.IfnameToKey = 0; - } - if (virNWFilterSnoopState.SnoopReqs) { - virHashFree(virNWFilterSnoopState.SnoopReqs); - virNWFilterSnoopState.SnoopReqs = 0; - } - if (virNWFilterSnoopState.Active) { - virHashFree(virNWFilterSnoopState.Active); - virNWFilterSnoopState.Active = 0; - } + virHashFree(virNWFilterSnoopState.IfnameToKey); + virNWFilterSnoopState.IfnameToKey = NULL; + + virHashFree(virNWFilterSnoopState.SnoopReqs); + virNWFilterSnoopState.SnoopReqs = NULL; + + virHashFree(virNWFilterSnoopState.Active); + virNWFilterSnoopState.Active = NULL; + return -1; } @@ -913,64 +989,87 @@ virNWFilterDHCPSnoopEnd(const char *ifna char *ifkey = NULL; virNWFilterSnoopLock(); - if (!virNWFilterSnoopState.SnoopReqs) { - virNWFilterSnoopUnlock(); - return; - } + if (!virNWFilterSnoopState.SnoopReqs) + goto cleanup; if (ifname) { - ifkey = (char *)virHashLookup(virNWFilterSnoopState.IfnameToKey,ifname); + ifkey = (char *)virHashLookup(virNWFilterSnoopState.IfnameToKey, + ifname); (void) virHashRemoveEntry(virNWFilterSnoopState.IfnameToKey, ifname); if (!ifkey) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("ifname \"%s\" not in key map"), ifname); - virNWFilterSnoopUnlock(); - return; + goto cleanup; } } if (ifkey) { struct virNWFilterSnoopReq *req; - req = virHashLookup(virNWFilterSnoopState.SnoopReqs, ifkey); + req = virNWFilterSnoopReqGetByKey(ifkey); if (!req) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("ifkey \"%s\" has no req"), ifkey); - virNWFilterSnoopUnlock(); - return; - } - if (!req->start || req->start->Timeout < time(0)) { - (void) virHashRemoveEntry(virNWFilterSnoopState.SnoopReqs, - req->ifkey); - virNWFilterSnoopUnlock(); - return; + goto cleanup; } /* keep valid lease req; drop interface association */ virNWFilterSnoopCancel(&req->threadkey); + VIR_FREE(req->ifname); + + virNWFilterSnoopReqPut(req); } else { /* free all of them */ virNWFilterSnoopLeaseFileClose(); virHashFree(virNWFilterSnoopState.IfnameToKey); virHashFree(virNWFilterSnoopState.SnoopReqs); - virNWFilterSnoopState.IfnameToKey = virHashCreate(0, 0); + virNWFilterSnoopState.IfnameToKey = virHashCreate(0, NULL); if (!virNWFilterSnoopState.IfnameToKey) { - virNWFilterSnoopUnlock(); virReportOOMError(); - return; + goto cleanup; } virNWFilterSnoopState.SnoopReqs = virHashCreate(0, virNWFilterSnoopReqRelease); if (!virNWFilterSnoopState.SnoopReqs) { virHashFree(virNWFilterSnoopState.IfnameToKey); - virNWFilterSnoopUnlock(); virReportOOMError(); - return; + goto cleanup; } virNWFilterSnoopLeaseFileLoad(); } + +cleanup: virNWFilterSnoopUnlock(); } +void +virNWFilterDHCPSnoopShutdown(void) +{ + /* + * free the SnoopReqs before the 'Active' hash since this will already + * clear some of the key / value pairs in the Active hash. + */ + + virNWFilterSnoopLock(); + + virNWFilterSnoopLeaseFileClose(); + virHashFree(virNWFilterSnoopState.IfnameToKey); + virHashFree(virNWFilterSnoopState.SnoopReqs); + + virNWFilterSnoopUnlock(); + + virNWFilterSnoopActiveLock(); + virHashFree(virNWFilterSnoopState.Active); + virNWFilterSnoopActiveUnlock(); + + while (1) { + if (virNWFilterSnoopState.nThreads == 0) + break; + + VIR_WARN("Waiting for snooping threads to terminate\n"); + usleep(1000 * 1000); + } +} + static int virNWFilterSnoopLeaseFileWrite(int lfd, const char *ifkey, struct virNWFilterSnoopIPLease *ipl) @@ -990,7 +1089,7 @@ virNWFilterSnoopLeaseFileWrite(int lfd, /* time intf ip dhcpserver */ snprintf(lbuf, sizeof(lbuf), "%u %s %s %s\n", ipl->Timeout, ifkey, ipstr, dhcpstr); - if (write(lfd, lbuf, strlen(lbuf)) < 0) { + if (safewrite(lfd, lbuf, strlen(lbuf)) != strlen(lbuf)) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("lease file write failed: %s"), strerror(errno)); @@ -1005,14 +1104,20 @@ virNWFilterSnoopLeaseFileSave(struct vir { struct virNWFilterSnoopReq *req = ipl->SnoopReq; + virNWFilterSnoopLock(); + if (virNWFilterSnoopState.LeaseFD < 0) - virNWFilterSnoopLeaseFileOpen(); + virNWFilterSnoopLeaseFileOpen(); if (virNWFilterSnoopLeaseFileWrite(virNWFilterSnoopState.LeaseFD, - req->ifkey, ipl) < 0) - return; + req->ifkey, ipl) < 0) { + virNWFilterSnoopUnlock(); + return; + } /* keep dead leases at < ~95% of file size */ - if (++virNWFilterSnoopState.wLeases >= virNWFilterSnoopState.nLeases*20) + if (++virNWFilterSnoopState.wLeases >= virNWFilterSnoopState.nLeases * 20) virNWFilterSnoopLeaseFileLoad(); /* load & refresh lease file */ + + virNWFilterSnoopUnlock(); } static struct virNWFilterSnoopReq * @@ -1023,14 +1128,17 @@ virNWFilterSnoopNewReq(const char *ifkey if (ifkey == NULL || strlen(ifkey) != VIR_IFKEY_LEN-1) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("virNWFilterSnoopNewReq called with invalid " - "key \"%s\" (%d)"), - ifkey ? ifkey : "", strlen(ifkey)); + "key \"%s\" (%u)"), + ifkey ? ifkey : "", + (unsigned int)strlen(ifkey)); return NULL; } + if (VIR_ALLOC(req) < 0) { virReportOOMError(); return NULL; } + if (virStrcpyStatic(req->ifkey, ifkey) == NULL) VIR_FREE(req); @@ -1077,6 +1185,9 @@ virNWFilterSnoopLeaseFileRefresh(void) TMPLEASEFILE, strerror(errno)); return; } + + virNWFilterSnoopLock(); + if (virNWFilterSnoopState.SnoopReqs) virHashForEach(virNWFilterSnoopState.SnoopReqs, virNWFilterSnoopSaveIter, (void *)&tfd); @@ -1089,6 +1200,8 @@ virNWFilterSnoopLeaseFileRefresh(void) } virNWFilterSnoopState.wLeases = 0; virNWFilterSnoopLeaseFileOpen(); + + virNWFilterSnoopUnlock(); } @@ -1101,7 +1214,7 @@ virNWFilterSnoopLeaseFileLoad(void) struct virNWFilterSnoopReq *req; time_t now; FILE *fp; - int ln = 0; + int ln = 0, tmp; fp = fopen(LEASEFILE, "r"); time(&now); @@ -1123,13 +1236,20 @@ virNWFilterSnoopLeaseFileLoad(void) } if (ipl.Timeout && ipl.Timeout < now) continue; - req = virHashLookup(virNWFilterSnoopState.SnoopReqs, ifkey); + req = virNWFilterSnoopReqGetByKey(ifkey); if (!req) { req = virNWFilterSnoopNewReq(ifkey); if (!req) break; - if (virHashAddEntry(virNWFilterSnoopState.SnoopReqs, ifkey, req)) { - virNWFilterSnoopReqFree(req); + + virNWFilterSnoopReqGet(req); + + virNWFilterSnoopLock(); + tmp = virHashAddEntry(virNWFilterSnoopState.SnoopReqs, ifkey, req); + virNWFilterSnoopUnlock(); + + if (tmp) { + virNWFilterSnoopReqPut(req); virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("virNWFilterSnoopLeaseFileLoad req add" " failed on interface \"%s\""), ifkey); @@ -1141,6 +1261,7 @@ virNWFilterSnoopLeaseFileLoad(void) virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("line %d corrupt ipaddr \"%s\""), ln, ipstr); + virNWFilterSnoopReqPut(req); continue; } (void) inet_pton(AF_INET, srvstr, &ipl.IPServer); @@ -1150,10 +1271,13 @@ virNWFilterSnoopLeaseFileLoad(void) virNWFilterSnoopLeaseAdd(&ipl, 0); else virNWFilterSnoopLeaseDel(req, ipl.IPAddress, 0); + + virNWFilterSnoopReqPut(req); } if (fp != NULL) (void) fclose(fp); virNWFilterSnoopLeaseFileRefresh(); + } static void @@ -1192,6 +1316,6 @@ virNWFilterDHCPSnoopReq(virNWFilterTechD virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("libvirt was not compiled " "with libpcap and \"ip_learning='dhcp'\" requires" " it.")); - return 1; + return -1; } #endif /* HAVE_LIBPCAP */ Index: libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.h =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_dhcpsnoop.h +++ libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.h @@ -25,6 +25,7 @@ #define __NWFILTER_DHCPSNOOP_H int virNWFilterDHCPSnoopInit(void); +void virNWFilterDHCPSnoopShutdown(void); int virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, const char *ifname, const char *linkdev, Index: libvirt-acl/src/nwfilter/nwfilter_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_driver.c @@ -130,7 +130,7 @@ alloc_err_exit: conf_init_err: virNWFilterTechDriversShutdown(); - virNWFilterDHCPSnoopEnd(0); + virNWFilterDHCPSnoopShutdown(); virNWFilterLearnShutdown(); return -1; @@ -153,7 +153,7 @@ nwfilterDriverReload(void) { conn = virConnectOpen("qemu:///system"); if (conn) { - virNWFilterDHCPSnoopEnd(0); + virNWFilterDHCPSnoopEnd(NULL); /* shut down all threads -- they will be restarted if necessary */ virNWFilterLearnThreadsTerminate(true); @@ -208,7 +208,7 @@ nwfilterDriverShutdown(void) { virNWFilterConfLayerShutdown(); virNWFilterTechDriversShutdown(); - virNWFilterDHCPSnoopEnd(0); + virNWFilterDHCPSnoopShutdown(); virNWFilterLearnShutdown(); nwfilterDriverLock(driverState); Index: libvirt-acl/po/POTFILES.in =================================================================== --- libvirt-acl.orig/po/POTFILES.in +++ libvirt-acl/po/POTFILES.in @@ -52,7 +52,6 @@ src/node_device/node_device_hal.c src/node_device/node_device_linux_sysfs.c src/node_device/node_device_udev.c src/nodeinfo.c -src/nwfilter/nwfilter_dhcpsnoop.c src/nwfilter/nwfilter_driver.c src/nwfilter/nwfilter_ebiptables_driver.c src/nwfilter/nwfilter_gentech_driver.c