nwfilter_dhcpsnoop: Replace virNWFilterSnoopLock macros

Use automatic mutex management instead.

Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This commit is contained in:
Tim Wiederhake 2022-03-16 23:08:47 +01:00
parent f61baec724
commit 5e6442b903

View File

@ -85,15 +85,6 @@ struct virNWFilterSnoopState {
virMutex activeLock; /* protects Active */ 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)) # define VIR_IFKEY_LEN ((VIR_UUID_STRING_BUFLEN) + (VIR_MAC_STRING_BUFLEN))
typedef struct _virNWFilterSnoopReq virNWFilterSnoopReq; typedef struct _virNWFilterSnoopReq virNWFilterSnoopReq;
@ -147,7 +138,7 @@ struct _virNWFilterSnoopReq {
/* /*
* Note about lock-order: * Note about lock-order:
* 1st: virNWFilterSnoopLock() * 1st: virNWFilterSnoopState.snoopLock
* 2nd: virNWFilterSnoopReqLock(req) * 2nd: virNWFilterSnoopReqLock(req)
* *
* Rationale: Former protects the SnoopReqs hash, latter its contents * Rationale: Former protects the SnoopReqs hash, latter its contents
@ -620,16 +611,13 @@ virNWFilterSnoopReqRelease(void *req0)
static virNWFilterSnoopReq * static virNWFilterSnoopReq *
virNWFilterSnoopReqGetByIFKey(const char *ifkey) virNWFilterSnoopReqGetByIFKey(const char *ifkey)
{ {
VIR_LOCK_GUARD lock = virLockGuardLock(&virNWFilterSnoopState.snoopLock);
virNWFilterSnoopReq *req; virNWFilterSnoopReq *req;
virNWFilterSnoopLock();
req = virHashLookup(virNWFilterSnoopState.snoopReqs, ifkey); req = virHashLookup(virNWFilterSnoopState.snoopReqs, ifkey);
if (req) if (req)
virNWFilterSnoopReqGet(req); virNWFilterSnoopReqGet(req);
virNWFilterSnoopUnlock();
return req; return req;
} }
@ -640,11 +628,11 @@ virNWFilterSnoopReqGetByIFKey(const char *ifkey)
static void static void
virNWFilterSnoopReqPut(virNWFilterSnoopReq *req) virNWFilterSnoopReqPut(virNWFilterSnoopReq *req)
{ {
VIR_LOCK_GUARD lock = virLockGuardLock(&virNWFilterSnoopState.snoopLock);
if (!req) if (!req)
return; return;
virNWFilterSnoopLock();
if (!!g_atomic_int_dec_and_test(&req->refctr)) { if (!!g_atomic_int_dec_and_test(&req->refctr)) {
/* /*
* delete the request: * delete the request:
@ -660,8 +648,6 @@ virNWFilterSnoopReqPut(virNWFilterSnoopReq *req)
req->ifkey)); req->ifkey));
} }
} }
virNWFilterSnoopUnlock();
} }
/* /*
@ -1460,20 +1446,19 @@ virNWFilterDHCPSnoopThread(void *req0)
} /* while (!error) */ } /* while (!error) */
/* protect IfNameToKey */ /* protect IfNameToKey */
virNWFilterSnoopLock(); VIR_WITH_MUTEX_LOCK_GUARD(&virNWFilterSnoopState.snoopLock) {
/* protect req->binding->portdevname & req->threadkey */
virNWFilterSnoopReqLock(req);
/* protect req->binding->portdevname & req->threadkey */ virNWFilterSnoopCancel(&req->threadkey);
virNWFilterSnoopReqLock(req);
virNWFilterSnoopCancel(&req->threadkey); ignore_value(virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey,
req->binding->portdevname));
ignore_value(virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey, g_clear_pointer(&req->binding->portdevname, g_free);
req->binding->portdevname));
g_clear_pointer(&req->binding->portdevname, g_free); virNWFilterSnoopReqUnlock(req);
}
virNWFilterSnoopReqUnlock(req);
virNWFilterSnoopUnlock();
cleanup: cleanup:
virThreadPoolFree(worker); virThreadPoolFree(worker);
@ -1556,7 +1541,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriver *techdriver,
goto exit_snoopreqput; goto exit_snoopreqput;
} }
virNWFilterSnoopLock(); virMutexLock(&virNWFilterSnoopState.snoopLock);
if (virHashAddEntry(virNWFilterSnoopState.ifnameToKey, if (virHashAddEntry(virNWFilterSnoopState.ifnameToKey,
req->binding->portdevname, req->binding->portdevname,
@ -1621,7 +1606,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriver *techdriver,
virNWFilterSnoopReqUnlock(req); virNWFilterSnoopReqUnlock(req);
virNWFilterSnoopUnlock(); virMutexUnlock(&virNWFilterSnoopState.snoopLock);
/* do not 'put' the req -- the thread will do this */ /* do not 'put' the req -- the thread will do this */
@ -1634,7 +1619,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriver *techdriver,
exit_rem_ifnametokey: exit_rem_ifnametokey:
virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey, binding->portdevname); virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey, binding->portdevname);
exit_snoopunlock: exit_snoopunlock:
virNWFilterSnoopUnlock(); virMutexUnlock(&virNWFilterSnoopState.snoopLock);
exit_snoopreqput: exit_snoopreqput:
if (!threadPuts) if (!threadPuts)
virNWFilterSnoopReqPut(req); virNWFilterSnoopReqPut(req);
@ -1695,23 +1680,19 @@ virNWFilterSnoopLeaseFileWrite(int lfd, const char *ifkey,
static void static void
virNWFilterSnoopLeaseFileSave(virNWFilterSnoopIPLease *ipl) virNWFilterSnoopLeaseFileSave(virNWFilterSnoopIPLease *ipl)
{ {
VIR_LOCK_GUARD lock = virLockGuardLock(&virNWFilterSnoopState.snoopLock);
virNWFilterSnoopReq *req = ipl->snoopReq; virNWFilterSnoopReq *req = ipl->snoopReq;
virNWFilterSnoopLock();
if (virNWFilterSnoopState.leaseFD < 0) if (virNWFilterSnoopState.leaseFD < 0)
virNWFilterSnoopLeaseFileOpen(); virNWFilterSnoopLeaseFileOpen();
if (virNWFilterSnoopLeaseFileWrite(virNWFilterSnoopState.leaseFD, if (virNWFilterSnoopLeaseFileWrite(virNWFilterSnoopState.leaseFD,
req->ifkey, ipl) < 0) req->ifkey, ipl) < 0)
goto error; return;
/* keep dead leases at < ~95% of file size */ /* keep dead leases at < ~95% of file size */
if (g_atomic_int_add(&virNWFilterSnoopState.wLeases, 1) >= if (g_atomic_int_add(&virNWFilterSnoopState.wLeases, 1) >=
g_atomic_int_get(&virNWFilterSnoopState.nLeases) * 20) g_atomic_int_get(&virNWFilterSnoopState.nLeases) * 20)
virNWFilterSnoopLeaseFileLoad(); /* load & refresh lease file */ virNWFilterSnoopLeaseFileLoad(); /* load & refresh lease file */
error:
virNWFilterSnoopUnlock();
} }
/* /*
@ -1830,9 +1811,7 @@ virNWFilterSnoopLeaseFileLoad(void)
time_t now; time_t now;
FILE *fp; FILE *fp;
int ln = 0, tmp; int ln = 0, tmp;
VIR_LOCK_GUARD lock = virLockGuardLock(&virNWFilterSnoopState.snoopLock);
/* protect the lease file */
virNWFilterSnoopLock();
fp = fopen(LEASEFILE, "r"); fp = fopen(LEASEFILE, "r");
time(&now); time(&now);
@ -1892,8 +1871,6 @@ virNWFilterSnoopLeaseFileLoad(void)
VIR_FORCE_FCLOSE(fp); VIR_FORCE_FCLOSE(fp);
virNWFilterSnoopLeaseFileRefresh(); virNWFilterSnoopLeaseFileRefresh();
virNWFilterSnoopUnlock();
} }
/* /*
@ -1953,11 +1930,11 @@ virNWFilterSnoopRemAllReqIter(const void *payload,
static void static void
virNWFilterSnoopEndThreads(void) virNWFilterSnoopEndThreads(void)
{ {
virNWFilterSnoopLock(); VIR_LOCK_GUARD lock = virLockGuardLock(&virNWFilterSnoopState.snoopLock);
virHashRemoveSet(virNWFilterSnoopState.snoopReqs, virHashRemoveSet(virNWFilterSnoopState.snoopReqs,
virNWFilterSnoopRemAllReqIter, virNWFilterSnoopRemAllReqIter,
NULL); NULL);
virNWFilterSnoopUnlock();
} }
int int
@ -1996,18 +1973,17 @@ virNWFilterDHCPSnoopInit(void)
void void
virNWFilterDHCPSnoopEnd(const char *ifname) virNWFilterDHCPSnoopEnd(const char *ifname)
{ {
VIR_LOCK_GUARD lock = virLockGuardLock(&virNWFilterSnoopState.snoopLock);
char *ifkey = NULL; char *ifkey = NULL;
virNWFilterSnoopLock();
if (!virNWFilterSnoopState.snoopReqs) if (!virNWFilterSnoopState.snoopReqs)
goto cleanup; return;
if (ifname) { if (ifname) {
ifkey = (char *)virHashLookup(virNWFilterSnoopState.ifnameToKey, ifkey = (char *)virHashLookup(virNWFilterSnoopState.ifnameToKey,
ifname); ifname);
if (!ifkey) if (!ifkey)
goto cleanup; return;
ignore_value(virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey, ignore_value(virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey,
ifname)); ifname));
@ -2020,7 +1996,7 @@ virNWFilterDHCPSnoopEnd(const char *ifname)
if (!req) { if (!req) {
virReportError(VIR_ERR_INTERNAL_ERROR, virReportError(VIR_ERR_INTERNAL_ERROR,
_("ifkey \"%s\" has no req"), ifkey); _("ifkey \"%s\" has no req"), ifkey);
goto cleanup; return;
} }
/* protect req->binding->portdevname & req->threadkey */ /* protect req->binding->portdevname & req->threadkey */
@ -2044,9 +2020,6 @@ virNWFilterDHCPSnoopEnd(const char *ifname)
virNWFilterSnoopLeaseFileLoad(); virNWFilterSnoopLeaseFileLoad();
} }
cleanup:
virNWFilterSnoopUnlock();
} }
void void
@ -2055,13 +2028,11 @@ virNWFilterDHCPSnoopShutdown(void)
virNWFilterSnoopEndThreads(); virNWFilterSnoopEndThreads();
virNWFilterSnoopJoinThreads(); virNWFilterSnoopJoinThreads();
virNWFilterSnoopLock(); VIR_WITH_MUTEX_LOCK_GUARD(&virNWFilterSnoopState.snoopLock) {
virNWFilterSnoopLeaseFileClose();
virNWFilterSnoopLeaseFileClose(); g_clear_pointer(&virNWFilterSnoopState.ifnameToKey, g_hash_table_unref);
g_clear_pointer(&virNWFilterSnoopState.ifnameToKey, g_hash_table_unref); g_clear_pointer(&virNWFilterSnoopState.snoopReqs, g_hash_table_unref);
g_clear_pointer(&virNWFilterSnoopState.snoopReqs, g_hash_table_unref); }
virNWFilterSnoopUnlock();
VIR_WITH_MUTEX_LOCK_GUARD(&virNWFilterSnoopState.activeLock) { VIR_WITH_MUTEX_LOCK_GUARD(&virNWFilterSnoopState.activeLock) {
g_clear_pointer(&virNWFilterSnoopState.active, g_hash_table_unref); g_clear_pointer(&virNWFilterSnoopState.active, g_hash_table_unref);