Use automatic mutex management instead. Signed-off-by: Tim Wiederhake <twiederh@xxxxxxxxxx> --- src/nwfilter/nwfilter_dhcpsnoop.c | 90 +++++++++++-------------------- 1 file changed, 30 insertions(+), 60 deletions(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index c526653bc2..f18f7b80d6 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -85,15 +85,6 @@ struct virNWFilterSnoopState { virMutex activeLock; /* protects Active */ }; -# define virNWFilterSnoopLock() \ - do { \ - virMutexLock(&virNWFilterSnoopState.snoopLock); \ - } while (0) -# define virNWFilterSnoopUnlock() \ - do { \ - virMutexUnlock(&virNWFilterSnoopState.snoopLock); \ - } while (0) - # define VIR_IFKEY_LEN ((VIR_UUID_STRING_BUFLEN) + (VIR_MAC_STRING_BUFLEN)) typedef struct _virNWFilterSnoopReq virNWFilterSnoopReq; @@ -147,7 +138,7 @@ struct _virNWFilterSnoopReq { /* * Note about lock-order: - * 1st: virNWFilterSnoopLock() + * 1st: virNWFilterSnoopState.snoopLock * 2nd: virNWFilterSnoopReqLock(req) * * Rationale: Former protects the SnoopReqs hash, latter its contents @@ -620,16 +611,13 @@ virNWFilterSnoopReqRelease(void *req0) static virNWFilterSnoopReq * virNWFilterSnoopReqGetByIFKey(const char *ifkey) { + VIR_LOCK_GUARD lock = virLockGuardLock(&virNWFilterSnoopState.snoopLock); virNWFilterSnoopReq *req; - virNWFilterSnoopLock(); - req = virHashLookup(virNWFilterSnoopState.snoopReqs, ifkey); if (req) virNWFilterSnoopReqGet(req); - virNWFilterSnoopUnlock(); - return req; } @@ -640,11 +628,11 @@ virNWFilterSnoopReqGetByIFKey(const char *ifkey) static void virNWFilterSnoopReqPut(virNWFilterSnoopReq *req) { + VIR_LOCK_GUARD lock = virLockGuardLock(&virNWFilterSnoopState.snoopLock); + if (!req) return; - virNWFilterSnoopLock(); - if (!!g_atomic_int_dec_and_test(&req->refctr)) { /* * delete the request: @@ -660,8 +648,6 @@ virNWFilterSnoopReqPut(virNWFilterSnoopReq *req) req->ifkey)); } } - - virNWFilterSnoopUnlock(); } /* @@ -1460,20 +1446,19 @@ virNWFilterDHCPSnoopThread(void *req0) } /* while (!error) */ /* protect IfNameToKey */ - virNWFilterSnoopLock(); - - /* protect req->binding->portdevname & req->threadkey */ - virNWFilterSnoopReqLock(req); + VIR_WITH_MUTEX_LOCK_GUARD(&virNWFilterSnoopState.snoopLock) { + /* protect req->binding->portdevname & req->threadkey */ + virNWFilterSnoopReqLock(req); - virNWFilterSnoopCancel(&req->threadkey); + virNWFilterSnoopCancel(&req->threadkey); - ignore_value(virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey, - req->binding->portdevname)); + ignore_value(virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey, + req->binding->portdevname)); - g_clear_pointer(&req->binding->portdevname, g_free); + g_clear_pointer(&req->binding->portdevname, g_free); - virNWFilterSnoopReqUnlock(req); - virNWFilterSnoopUnlock(); + virNWFilterSnoopReqUnlock(req); + } cleanup: virThreadPoolFree(worker); @@ -1511,6 +1496,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriver *techdriver, virThread thread; virNWFilterVarValue *dhcpsrvrs; bool threadPuts = false; + VIR_LOCK_GUARD lock = { NULL }; virNWFilterSnoopIFKeyFMT(ifkey, binding->owneruuid, &binding->mac); @@ -1556,7 +1542,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriver *techdriver, goto exit_snoopreqput; } - virNWFilterSnoopLock(); + lock = virLockGuardLock(&virNWFilterSnoopState.snoopLock); if (virHashAddEntry(virNWFilterSnoopState.ifnameToKey, req->binding->portdevname, @@ -1621,8 +1607,6 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriver *techdriver, virNWFilterSnoopReqUnlock(req); - virNWFilterSnoopUnlock(); - /* do not 'put' the req -- the thread will do this */ return 0; @@ -1634,7 +1618,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriver *techdriver, exit_rem_ifnametokey: virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey, binding->portdevname); exit_snoopunlock: - virNWFilterSnoopUnlock(); + virLockGuardUnlock(&lock); exit_snoopreqput: if (!threadPuts) virNWFilterSnoopReqPut(req); @@ -1695,23 +1679,19 @@ virNWFilterSnoopLeaseFileWrite(int lfd, const char *ifkey, static void virNWFilterSnoopLeaseFileSave(virNWFilterSnoopIPLease *ipl) { + VIR_LOCK_GUARD lock = virLockGuardLock(&virNWFilterSnoopState.snoopLock); virNWFilterSnoopReq *req = ipl->snoopReq; - virNWFilterSnoopLock(); - if (virNWFilterSnoopState.leaseFD < 0) virNWFilterSnoopLeaseFileOpen(); if (virNWFilterSnoopLeaseFileWrite(virNWFilterSnoopState.leaseFD, req->ifkey, ipl) < 0) - goto error; + return; /* keep dead leases at < ~95% of file size */ if (g_atomic_int_add(&virNWFilterSnoopState.wLeases, 1) >= g_atomic_int_get(&virNWFilterSnoopState.nLeases) * 20) virNWFilterSnoopLeaseFileLoad(); /* load & refresh lease file */ - - error: - virNWFilterSnoopUnlock(); } /* @@ -1830,9 +1810,7 @@ virNWFilterSnoopLeaseFileLoad(void) time_t now; FILE *fp; int ln = 0, tmp; - - /* protect the lease file */ - virNWFilterSnoopLock(); + VIR_LOCK_GUARD lock = virLockGuardLock(&virNWFilterSnoopState.snoopLock); fp = fopen(LEASEFILE, "r"); time(&now); @@ -1892,8 +1870,6 @@ virNWFilterSnoopLeaseFileLoad(void) VIR_FORCE_FCLOSE(fp); virNWFilterSnoopLeaseFileRefresh(); - - virNWFilterSnoopUnlock(); } /* @@ -1953,11 +1929,11 @@ virNWFilterSnoopRemAllReqIter(const void *payload, static void virNWFilterSnoopEndThreads(void) { - virNWFilterSnoopLock(); + VIR_LOCK_GUARD lock = virLockGuardLock(&virNWFilterSnoopState.snoopLock); + virHashRemoveSet(virNWFilterSnoopState.snoopReqs, virNWFilterSnoopRemAllReqIter, NULL); - virNWFilterSnoopUnlock(); } int @@ -1996,18 +1972,17 @@ virNWFilterDHCPSnoopInit(void) void virNWFilterDHCPSnoopEnd(const char *ifname) { + VIR_LOCK_GUARD lock = virLockGuardLock(&virNWFilterSnoopState.snoopLock); char *ifkey = NULL; - virNWFilterSnoopLock(); - if (!virNWFilterSnoopState.snoopReqs) - goto cleanup; + return; if (ifname) { ifkey = (char *)virHashLookup(virNWFilterSnoopState.ifnameToKey, ifname); if (!ifkey) - goto cleanup; + return; ignore_value(virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey, ifname)); @@ -2020,7 +1995,7 @@ virNWFilterDHCPSnoopEnd(const char *ifname) if (!req) { virReportError(VIR_ERR_INTERNAL_ERROR, _("ifkey \"%s\" has no req"), ifkey); - goto cleanup; + return; } /* protect req->binding->portdevname & req->threadkey */ @@ -2044,9 +2019,6 @@ virNWFilterDHCPSnoopEnd(const char *ifname) virNWFilterSnoopLeaseFileLoad(); } - - cleanup: - virNWFilterSnoopUnlock(); } void @@ -2055,13 +2027,11 @@ virNWFilterDHCPSnoopShutdown(void) virNWFilterSnoopEndThreads(); virNWFilterSnoopJoinThreads(); - virNWFilterSnoopLock(); - - virNWFilterSnoopLeaseFileClose(); - g_clear_pointer(&virNWFilterSnoopState.ifnameToKey, g_hash_table_unref); - g_clear_pointer(&virNWFilterSnoopState.snoopReqs, g_hash_table_unref); - - virNWFilterSnoopUnlock(); + VIR_WITH_MUTEX_LOCK_GUARD(&virNWFilterSnoopState.snoopLock) { + virNWFilterSnoopLeaseFileClose(); + g_clear_pointer(&virNWFilterSnoopState.ifnameToKey, g_hash_table_unref); + g_clear_pointer(&virNWFilterSnoopState.snoopReqs, g_hash_table_unref); + } VIR_WITH_MUTEX_LOCK_GUARD(&virNWFilterSnoopState.activeLock) { g_clear_pointer(&virNWFilterSnoopState.active, g_hash_table_unref); -- 2.31.1