10:02 a.m.
Use the recently implemented VIR_LOCK_GUARD and VIR_WITH_MUTEX_LOCK_GUARD
to simplify mutex management.
Tim Wiederhake (10):
virnetdaemon: Use automatic mutex management
bridge_driver: Use automatic mutex management
node_device_driver: Use automatic mutex management
interface_backend_netcf: Use automatic mutex management
node_device_udev: Use automatic mutex management
qemu_agent: Use automatic mutex management
vbox_common: Use automatic mutex management
datatypes: Use automatic mutex management
ch_monitor: Use automatic mutex management
virportallocator: Use automatic mutex management
src/ch/ch_monitor.c | 55 +++----
src/datatypes.c | 63 +++-----
src/interface/interface_backend_netcf.c | 192 +++++++++++-------------
src/network/bridge_driver.c | 59 +++-----
src/network/bridge_driver_linux.c | 4 +-
src/node_device/node_device_driver.c | 12 +-
src/node_device/node_device_udev.c | 76 +++++-----
src/qemu/qemu_agent.c | 25 ++-
src/rpc/virnetdaemon.c | 144 ++++++------------
src/util/virportallocator.c | 93 +++++-------
src/vbox/vbox_common.c | 31 ++--
11 files changed, 302 insertions(+), 452 deletions(-)
--
2.31.1
10:02 a.m.
New subject: [libvirt PATCH 01/10] virnetdaemon: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/rpc/virnetdaemon.c | 144 +++++++++++++----------------------------
1 file changed, 45 insertions(+), 99 deletions(-)
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index c946e4a92a..1f32bae35f 100644
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -171,20 +171,14 @@ int
virNetDaemonAddServer(virNetDaemon *dmn,
virNetServer *srv)
{
- int ret = -1;
const char *serverName = virNetServerGetName(srv);
-
- virObjectLock(dmn);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
if (virHashAddEntry(dmn->servers, serverName, srv) < 0)
- goto cleanup;
+ return -1;
virObjectRef(srv);
-
- ret = 0;
- cleanup:
- virObjectUnlock(dmn);
- return ret;
+ return 0;
}
@@ -192,11 +186,8 @@ virNetServer *
virNetDaemonGetServer(virNetDaemon *dmn,
const char *serverName)
{
- virNetServer *srv = NULL;
-
- virObjectLock(dmn);
- srv = virObjectRef(virHashLookup(dmn->servers, serverName));
- virObjectUnlock(dmn);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
+ virNetServer *srv = virObjectRef(virHashLookup(dmn->servers, serverName));
if (!srv) {
virReportError(VIR_ERR_NO_SERVER,
@@ -210,13 +201,9 @@ bool
virNetDaemonHasServer(virNetDaemon *dmn,
const char *serverName)
{
- void *ent;
-
- virObjectLock(dmn);
- ent = virHashLookup(dmn->servers, serverName);
- virObjectUnlock(dmn);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
- return ent != NULL;
+ return virHashLookup(dmn->servers, serverName) != NULL;
}
@@ -252,26 +239,19 @@ ssize_t
virNetDaemonGetServers(virNetDaemon *dmn,
virNetServer ***servers)
{
+ VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
struct collectData data = { servers, 0 };
- ssize_t ret = -1;
*servers = NULL;
- virObjectLock(dmn);
-
if (virHashForEach(dmn->servers, collectServers, &data) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot get all servers from daemon"));
- goto cleanup;
+ virObjectListFreeCount(*servers, data.nservers);
+ return -1;
}
- ret = data.nservers;
-
- cleanup:
- if (ret < 0)
- virObjectListFreeCount(*servers, data.nservers);
- virObjectUnlock(dmn);
- return ret;
+ return data.nservers;
}
@@ -387,48 +367,39 @@ virNetDaemonPreExecRestart(virNetDaemon *dmn)
g_autoptr(virJSONValue) object = virJSONValueNewObject();
g_autoptr(virJSONValue) srvObj = virJSONValueNewObject();
g_autofree virHashKeyValuePair *srvArray = NULL;
-
- virObjectLock(dmn);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
if (!(srvArray = virHashGetItems(dmn->servers, NULL, true)))
- goto error;
+ return NULL;
for (i = 0; srvArray[i].key; i++) {
virNetServer *server = virHashLookup(dmn->servers, srvArray[i].key);
g_autoptr(virJSONValue) srvJSON = NULL;
if (!server)
- goto error;
+ return NULL;
srvJSON = virNetServerPreExecRestart(server);
if (!srvJSON)
- goto error;
+ return NULL;
if (virJSONValueObjectAppend(srvObj, srvArray[i].key, &srvJSON) < 0)
- goto error;
+ return NULL;
}
- virObjectUnlock(dmn);
-
if (virJSONValueObjectAppend(object, "servers", &srvObj) < 0)
return NULL;
return g_steal_pointer(&object);
-
- error:
- virObjectUnlock(dmn);
- return NULL;
}
bool
virNetDaemonIsPrivileged(virNetDaemon *dmn)
{
- bool priv;
- virObjectLock(dmn);
- priv = dmn->privileged;
- virObjectUnlock(dmn);
- return priv;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
+
+ return dmn->privileged;
}
@@ -436,11 +407,9 @@ void
virNetDaemonAutoShutdown(virNetDaemon *dmn,
unsigned int timeout)
{
- virObjectLock(dmn);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
dmn->autoShutdownTimeout = timeout;
-
- virObjectUnlock(dmn);
}
@@ -507,7 +476,8 @@ virNetDaemonCallInhibit(virNetDaemon *dmn,
void
virNetDaemonAddShutdownInhibition(virNetDaemon *dmn)
{
- virObjectLock(dmn);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
+
dmn->autoShutdownInhibitions++;
VIR_DEBUG("dmn=%p inhibitions=%zu", dmn, dmn->autoShutdownInhibitions);
@@ -520,15 +490,14 @@ virNetDaemonAddShutdownInhibition(virNetDaemon *dmn)
_("Virtual machines need to be saved"),
"delay");
#endif
-
- virObjectUnlock(dmn);
}
void
virNetDaemonRemoveShutdownInhibition(virNetDaemon *dmn)
{
- virObjectLock(dmn);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
+
dmn->autoShutdownInhibitions--;
VIR_DEBUG("dmn=%p inhibitions=%zu", dmn, dmn->autoShutdownInhibitions);
@@ -537,8 +506,6 @@ virNetDaemonRemoveShutdownInhibition(virNetDaemon *dmn)
VIR_DEBUG("Closing inhibit FD %d", dmn->autoShutdownInhibitFd);
VIR_FORCE_CLOSE(dmn->autoShutdownInhibitFd);
}
-
- virObjectUnlock(dmn);
}
@@ -648,13 +615,12 @@ virNetDaemonAddSignalHandler(virNetDaemon *dmn,
virNetDaemonSignalFunc func,
void *opaque)
{
- virNetDaemonSignal *sigdata = NULL;
+ g_autofree virNetDaemonSignal *sigdata = NULL;
struct sigaction sig_action;
-
- virObjectLock(dmn);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
if (virNetDaemonSignalSetup(dmn) < 0)
- goto error;
+ return -1;
VIR_EXPAND_N(dmn->signals, dmn->nsignals, 1);
@@ -671,15 +637,8 @@ virNetDaemonAddSignalHandler(virNetDaemon *dmn,
sigaction(signum, &sig_action, &sigdata->oldaction);
- dmn->signals[dmn->nsignals-1] = sigdata;
-
- virObjectUnlock(dmn);
+ dmn->signals[dmn->nsignals-1] = g_steal_pointer(&sigdata);
return 0;
-
- error:
- VIR_FREE(sigdata);
- virObjectUnlock(dmn);
- return -1;
}
#else /* WIN32 */
@@ -703,15 +662,12 @@ virNetDaemonAutoShutdownTimer(int timerid G_GNUC_UNUSED,
void *opaque)
{
virNetDaemon *dmn = opaque;
-
- virObjectLock(dmn);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
if (!dmn->autoShutdownInhibitions) {
VIR_DEBUG("Automatic shutdown triggered");
dmn->quit = true;
}
-
- virObjectUnlock(dmn);
}
static int
@@ -730,9 +686,9 @@ void
virNetDaemonUpdateServices(virNetDaemon *dmn,
bool enabled)
{
- virObjectLock(dmn);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
+
virHashForEach(dmn->servers, daemonServerUpdateServices, &enabled);
- virObjectUnlock(dmn);
}
static int
@@ -764,19 +720,17 @@ daemonShutdownWait(void *opaque)
bool graceful = false;
virHashForEach(dmn->servers, daemonServerShutdownWait, NULL);
- if (dmn->shutdownWaitCb && dmn->shutdownWaitCb() < 0)
- goto finish;
+ if (!dmn->shutdownWaitCb || dmn->shutdownWaitCb() >= 0) {
+ if (dmn->stateStopThread)
+ virThreadJoin(dmn->stateStopThread);
- if (dmn->stateStopThread)
- virThreadJoin(dmn->stateStopThread);
-
- graceful = true;
+ graceful = true;
+ }
- finish:
- virObjectLock(dmn);
- dmn->graceful = graceful;
- virEventUpdateTimeout(dmn->finishTimer, 0);
- virObjectUnlock(dmn);
+ VIR_WITH_OBJECT_LOCK_GUARD(dmn) {
+ dmn->graceful = graceful;
+ virEventUpdateTimeout(dmn->finishTimer, 0);
+ }
}
static void
@@ -784,10 +738,9 @@ virNetDaemonFinishTimer(int timerid G_GNUC_UNUSED,
void *opaque)
{
virNetDaemon *dmn = opaque;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
- virObjectLock(dmn);
dmn->finished = true;
- virObjectUnlock(dmn);
}
void
@@ -896,36 +849,31 @@ void
virNetDaemonSetStateStopWorkerThread(virNetDaemon *dmn,
virThread **thr)
{
- virObjectLock(dmn);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
VIR_DEBUG("Setting state stop worker thread on dmn=%p to thr=%p", dmn,
thr);
dmn->stateStopThread = g_steal_pointer(thr);
- virObjectUnlock(dmn);
}
void
virNetDaemonQuit(virNetDaemon *dmn)
{
- virObjectLock(dmn);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
VIR_DEBUG("Quit requested %p", dmn);
dmn->quit = true;
-
- virObjectUnlock(dmn);
}
void
virNetDaemonQuitExecRestart(virNetDaemon *dmn)
{
- virObjectLock(dmn);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
VIR_DEBUG("Exec-restart requested %p", dmn);
dmn->quit = true;
dmn->execRestart = true;
-
- virObjectUnlock(dmn);
}
@@ -969,10 +917,8 @@ virNetDaemonSetShutdownCallbacks(virNetDaemon *dmn,
virNetDaemonShutdownCallback prepareCb,
virNetDaemonShutdownCallback waitCb)
{
- virObjectLock(dmn);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
dmn->shutdownPrepareCb = prepareCb;
dmn->shutdownWaitCb = waitCb;
-
- virObjectUnlock(dmn);
}
--
2.31.1
10:02 a.m.
New subject: [libvirt PATCH 02/10] bridge_driver: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/network/bridge_driver.c | 59 +++++++++++++------------------
src/network/bridge_driver_linux.c | 4 +--
2 files changed, 25 insertions(+), 38 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 67c5a111b3..6113b67248 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -457,13 +457,11 @@ networkUpdateState(virNetworkObj *obj,
g_autoptr(dnsmasqCaps) dnsmasq_caps = networkGetDnsmasqCaps(driver);
virMacMap *macmap;
g_autofree char *macMapFile = NULL;
- int ret = -1;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(obj);
+
+ if (!virNetworkObjIsActive(obj))
+ return 0;
- virObjectLock(obj);
- if (!virNetworkObjIsActive(obj)) {
- ret = 0;
- goto cleanup;
- }
def = virNetworkObjGetDef(obj);
switch ((virNetworkForwardType) def->forward.type) {
@@ -477,10 +475,10 @@ networkUpdateState(virNetworkObj *obj,
if (!(macMapFile = virMacMapFileName(driver->dnsmasqStateDir,
def->bridge)))
- goto cleanup;
+ return -1;
if (!(macmap = virMacMapNew(macMapFile)))
- goto cleanup;
+ return -1;
virNetworkObjSetMacMap(obj, macmap);
@@ -509,7 +507,7 @@ networkUpdateState(virNetworkObj *obj,
case VIR_NETWORK_FORWARD_LAST:
default:
virReportEnumRangeError(virNetworkForwardType, def->forward.type);
- goto cleanup;
+ return -1;
}
virNetworkObjPortForEach(obj, networkUpdatePort, obj);
@@ -525,10 +523,7 @@ networkUpdateState(virNetworkObj *obj,
virNetworkObjSetDnsmasqPid(obj, dnsmasqPid);
}
- ret = 0;
- cleanup:
- virObjectUnlock(obj);
- return ret;
+ return 0;
}
@@ -536,19 +531,19 @@ static int
networkAutostartConfig(virNetworkObj *obj,
void *opaque)
{
+ VIR_LOCK_GUARD lock = virObjectLockGuard(obj);
virNetworkDriverState *driver = opaque;
- int ret = -1;
- virObjectLock(obj);
- if (virNetworkObjIsAutostart(obj) &&
- !virNetworkObjIsActive(obj) &&
- networkStartNetwork(driver, obj) < 0)
- goto cleanup;
+ if (!virNetworkObjIsAutostart(obj))
+ return 0;
- ret = 0;
- cleanup:
- virObjectUnlock(obj);
- return ret;
+ if (virNetworkObjIsActive(obj))
+ return 0;
+
+ if (networkStartNetwork(driver, obj) >= 0)
+ return 0;
+
+ return -1;
}
@@ -1650,11 +1645,10 @@ static int
networkRefreshDaemonsHelper(virNetworkObj *obj,
void *opaque)
{
- virNetworkDef *def;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(obj);
virNetworkDriverState *driver = opaque;
+ virNetworkDef *def = virNetworkObjGetDef(obj);
- virObjectLock(obj);
- def = virNetworkObjGetDef(obj);
if (virNetworkObjIsActive(obj)) {
switch ((virNetworkForwardType) def->forward.type) {
case VIR_NETWORK_FORWARD_NONE:
@@ -1679,12 +1673,10 @@ networkRefreshDaemonsHelper(virNetworkObj *obj,
case VIR_NETWORK_FORWARD_LAST:
default:
virReportEnumRangeError(virNetworkForwardType, def->forward.type);
- goto cleanup;
+ return 0;
}
}
- cleanup:
- virObjectUnlock(obj);
return 0;
}
@@ -1706,10 +1698,9 @@ static int
networkReloadFirewallRulesHelper(virNetworkObj *obj,
void *opaque G_GNUC_UNUSED)
{
- virNetworkDef *def;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(obj);
+ virNetworkDef *def = virNetworkObjGetDef(obj);
- virObjectLock(obj);
- def = virNetworkObjGetDef(obj);
if (virNetworkObjIsActive(obj)) {
switch ((virNetworkForwardType) def->forward.type) {
case VIR_NETWORK_FORWARD_NONE:
@@ -1735,12 +1726,10 @@ networkReloadFirewallRulesHelper(virNetworkObj *obj,
case VIR_NETWORK_FORWARD_LAST:
default:
virReportEnumRangeError(virNetworkForwardType, def->forward.type);
- goto cleanup;
+ return 0;
}
}
- cleanup:
- virObjectUnlock(obj);
return 0;
}
diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
index 34982706be..98d2a33a1d 100644
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -87,9 +87,9 @@ static int
networkHasRunningNetworksWithFWHelper(virNetworkObj *obj,
void *opaque)
{
+ VIR_LOCK_GUARD lock = virObjectLockGuard(obj);
bool *activeWithFW = opaque;
- virObjectLock(obj);
if (virNetworkObjIsActive(obj)) {
virNetworkDef *def = virNetworkObjGetDef(obj);
@@ -111,8 +111,6 @@ networkHasRunningNetworksWithFWHelper(virNetworkObj *obj,
}
}
- virObjectUnlock(obj);
-
/*
* terminate ForEach early once we find an active network that
* adds Firewall rules (return status is ignored)
--
2.31.1
10:02 a.m.
New subject: [libvirt PATCH 03/10] node_device_driver: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/node_device/node_device_driver.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index 130c1d4023..1eb452b989 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1072,14 +1072,10 @@ static bool
matchDeviceAddress(virNodeDeviceObj *obj,
const void *opaque)
{
- g_autofree char *addr = NULL;
- bool want = false;
-
- virObjectLock(obj);
- addr = nodeDeviceObjFormatAddress(obj);
- want = STREQ_NULLABLE(addr, opaque);
- virObjectUnlock(obj);
- return want;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(obj);
+ g_autofree char *addr = nodeDeviceObjFormatAddress(obj);
+
+ return STREQ_NULLABLE(addr, opaque);
}
--
2.31.1
10:02 a.m.
New subject: [libvirt PATCH 04/10] interface_backend_netcf: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/interface/interface_backend_netcf.c | 192 +++++++++++-------------
1 file changed, 86 insertions(+), 106 deletions(-)
diff --git a/src/interface/interface_backend_netcf.c
b/src/interface/interface_backend_netcf.c
index 92698ee769..d625964421 100644
--- a/src/interface/interface_backend_netcf.c
+++ b/src/interface/interface_backend_netcf.c
@@ -155,31 +155,26 @@ netcfStateCleanup(void)
static int
netcfStateReload(void)
{
- int ret = -1;
-
if (!driver)
return 0;
- virObjectLock(driver);
- ncf_close(driver->netcf);
- if (ncf_init(&driver->netcf, NULL) != 0) {
- /* this isn't a good situation, because we can't shut down the
- * driver as there may still be connections to it. If we set
- * the netcf handle to NULL, any subsequent calls to netcf
- * will just fail rather than causing a crash. Not ideal, but
- * livable (since this should never happen).
- */
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("failed to re-init netcf"));
- driver->netcf = NULL;
- goto cleanup;
+ VIR_WITH_OBJECT_LOCK_GUARD(driver) {
+ ncf_close(driver->netcf);
+ if (ncf_init(&driver->netcf, NULL) != 0) {
+ /* this isn't a good situation, because we can't shut down the
+ * driver as there may still be connections to it. If we set
+ * the netcf handle to NULL, any subsequent calls to netcf
+ * will just fail rather than causing a crash. Not ideal, but
+ * livable (since this should never happen).
+ */
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to re-init netcf"));
+ driver->netcf = NULL;
+ return -1;
+ }
}
- ret = 0;
- cleanup:
- virObjectUnlock(driver);
-
- return ret;
+ return 0;
}
@@ -528,66 +523,68 @@ static int netcfConnectListInterfacesImpl(virConnectPtr conn,
static int netcfConnectNumOfInterfaces(virConnectPtr conn)
{
- int count;
+ int count = -1;
if (virConnectNumOfInterfacesEnsureACL(conn) < 0)
return -1;
- virObjectLock(driver);
- count = netcfConnectNumOfInterfacesImpl(conn,
- NETCF_IFACE_ACTIVE,
- virConnectNumOfInterfacesCheckACL);
- virObjectUnlock(driver);
+ VIR_WITH_OBJECT_LOCK_GUARD(driver) {
+ count = netcfConnectNumOfInterfacesImpl(conn,
+ NETCF_IFACE_ACTIVE,
+ virConnectNumOfInterfacesCheckACL);
+ }
+
return count;
}
static int netcfConnectListInterfaces(virConnectPtr conn, char **const names, int
nnames)
{
- int count;
+ int count = -1;
if (virConnectListInterfacesEnsureACL(conn) < 0)
return -1;
- virObjectLock(driver);
- count = netcfConnectListInterfacesImpl(conn,
- NETCF_IFACE_ACTIVE,
- names, nnames,
- virConnectListInterfacesCheckACL);
- virObjectUnlock(driver);
- return count;
+ VIR_WITH_OBJECT_LOCK_GUARD(driver) {
+ count = netcfConnectListInterfacesImpl(conn,
+ NETCF_IFACE_ACTIVE,
+ names, nnames,
+ virConnectListInterfacesCheckACL);
+ }
+ return count;
}
static int netcfConnectNumOfDefinedInterfaces(virConnectPtr conn)
{
- int count;
+ int count = -1;
if (virConnectNumOfDefinedInterfacesEnsureACL(conn) < 0)
return -1;
- virObjectLock(driver);
- count = netcfConnectNumOfInterfacesImpl(conn,
- NETCF_IFACE_INACTIVE,
- virConnectNumOfDefinedInterfacesCheckACL);
- virObjectUnlock(driver);
+ VIR_WITH_OBJECT_LOCK_GUARD(driver) {
+ count = netcfConnectNumOfInterfacesImpl(conn,
+ NETCF_IFACE_INACTIVE,
+
virConnectNumOfDefinedInterfacesCheckACL);
+ }
+
return count;
}
static int netcfConnectListDefinedInterfaces(virConnectPtr conn, char **const names, int
nnames)
{
- int count;
+ int count = -1;
if (virConnectListDefinedInterfacesEnsureACL(conn) < 0)
return -1;
- virObjectLock(driver);
- count = netcfConnectListInterfacesImpl(conn,
- NETCF_IFACE_INACTIVE,
- names, nnames,
- virConnectListDefinedInterfacesCheckACL);
- virObjectUnlock(driver);
- return count;
+ VIR_WITH_OBJECT_LOCK_GUARD(driver) {
+ count = netcfConnectListInterfacesImpl(conn,
+ NETCF_IFACE_INACTIVE,
+ names, nnames,
+ virConnectListDefinedInterfacesCheckACL);
+ }
+ return count;
}
#define MATCH(FLAG) (flags & (FLAG))
@@ -731,8 +728,8 @@ static virInterfacePtr netcfInterfaceLookupByName(virConnectPtr conn,
struct netcf_if *iface;
virInterfacePtr ret = NULL;
g_autoptr(virInterfaceDef) def = NULL;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(driver);
- virObjectLock(driver);
iface = ncf_lookup_by_name(driver->netcf, name);
if (!iface) {
const char *errmsg, *details;
@@ -759,7 +756,6 @@ static virInterfacePtr netcfInterfaceLookupByName(virConnectPtr conn,
cleanup:
ncf_if_free(iface);
- virObjectUnlock(driver);
return ret;
}
@@ -770,8 +766,8 @@ static virInterfacePtr netcfInterfaceLookupByMACString(virConnectPtr
conn,
int niface;
virInterfacePtr ret = NULL;
g_autoptr(virInterfaceDef) def = NULL;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(driver);
- virObjectLock(driver);
niface = ncf_lookup_by_mac_string(driver->netcf, macstr, 1, &iface);
if (niface < 0) {
@@ -806,7 +802,6 @@ static virInterfacePtr netcfInterfaceLookupByMACString(virConnectPtr
conn,
cleanup:
ncf_if_free(iface);
- virObjectUnlock(driver);
return ret;
}
@@ -818,11 +813,10 @@ static char *netcfInterfaceGetXMLDesc(virInterfacePtr ifinfo,
g_autoptr(virInterfaceDef) ifacedef = NULL;
char *ret = NULL;
bool active;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(driver);
virCheckFlags(VIR_INTERFACE_XML_INACTIVE, NULL);
- virObjectLock(driver);
-
iface = interfaceDriverGetNetcfIF(driver->netcf, ifinfo);
if (!iface) {
/* helper already reported error */
@@ -865,7 +859,6 @@ static char *netcfInterfaceGetXMLDesc(virInterfacePtr ifinfo,
cleanup:
ncf_if_free(iface);
VIR_FREE(xmlstr);
- virObjectUnlock(driver);
return ret;
}
@@ -877,11 +870,10 @@ static virInterfacePtr netcfInterfaceDefineXML(virConnectPtr conn,
char *xmlstr = NULL;
g_autoptr(virInterfaceDef) ifacedef = NULL;
virInterfacePtr ret = NULL;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(driver);
virCheckFlags(VIR_INTERFACE_DEFINE_VALIDATE, NULL);
- virObjectLock(driver);
-
ifacedef = virInterfaceDefParseString(xml, flags);
if (!ifacedef) {
/* error was already reported */
@@ -913,7 +905,6 @@ static virInterfacePtr netcfInterfaceDefineXML(virConnectPtr conn,
cleanup:
ncf_if_free(iface);
VIR_FREE(xmlstr);
- virObjectUnlock(driver);
return ret;
}
@@ -922,8 +913,7 @@ static int netcfInterfaceUndefine(virInterfacePtr ifinfo)
struct netcf_if *iface = NULL;
g_autoptr(virInterfaceDef) def = NULL;
int ret = -1;
-
- virObjectLock(driver);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(driver);
iface = interfaceDriverGetNetcfIF(driver->netcf, ifinfo);
if (!iface) {
@@ -951,7 +941,6 @@ static int netcfInterfaceUndefine(virInterfacePtr ifinfo)
cleanup:
ncf_if_free(iface);
- virObjectUnlock(driver);
return ret;
}
@@ -962,11 +951,10 @@ static int netcfInterfaceCreate(virInterfacePtr ifinfo,
g_autoptr(virInterfaceDef) def = NULL;
int ret = -1;
bool active;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(driver);
virCheckFlags(0, -1);
- virObjectLock(driver);
-
iface = interfaceDriverGetNetcfIF(driver->netcf, ifinfo);
if (!iface) {
/* helper already reported error */
@@ -1002,7 +990,6 @@ static int netcfInterfaceCreate(virInterfacePtr ifinfo,
cleanup:
ncf_if_free(iface);
- virObjectUnlock(driver);
return ret;
}
@@ -1013,11 +1000,10 @@ static int netcfInterfaceDestroy(virInterfacePtr ifinfo,
g_autoptr(virInterfaceDef) def = NULL;
int ret = -1;
bool active;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(driver);
virCheckFlags(0, -1);
- virObjectLock(driver);
-
iface = interfaceDriverGetNetcfIF(driver->netcf, ifinfo);
if (!iface) {
/* helper already reported error */
@@ -1053,7 +1039,6 @@ static int netcfInterfaceDestroy(virInterfacePtr ifinfo,
cleanup:
ncf_if_free(iface);
- virObjectUnlock(driver);
return ret;
}
@@ -1063,8 +1048,7 @@ static int netcfInterfaceIsActive(virInterfacePtr ifinfo)
g_autoptr(virInterfaceDef) def = NULL;
int ret = -1;
bool active;
-
- virObjectLock(driver);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(driver);
iface = interfaceDriverGetNetcfIF(driver->netcf, ifinfo);
if (!iface) {
@@ -1085,82 +1069,78 @@ static int netcfInterfaceIsActive(virInterfacePtr ifinfo)
cleanup:
ncf_if_free(iface);
- virObjectUnlock(driver);
return ret;
}
static int netcfInterfaceChangeBegin(virConnectPtr conn, unsigned int flags)
{
- int ret;
+ int ret = -1;
virCheckFlags(0, -1); /* currently flags must be 0 */
if (virInterfaceChangeBeginEnsureACL(conn) < 0)
return -1;
- virObjectLock(driver);
-
- ret = ncf_change_begin(driver->netcf, 0);
- if (ret < 0) {
- const char *errmsg, *details;
- int errcode = ncf_error(driver->netcf, &errmsg, &details);
- virReportError(netcf_to_vir_err(errcode),
- _("failed to begin transaction: %s%s%s"),
- errmsg, details ? " - " : "",
- NULLSTR_EMPTY(details));
+ VIR_WITH_OBJECT_LOCK_GUARD(driver) {
+ ret = ncf_change_begin(driver->netcf, 0);
+ if (ret < 0) {
+ const char *errmsg, *details;
+ int errcode = ncf_error(driver->netcf, &errmsg, &details);
+ virReportError(netcf_to_vir_err(errcode),
+ _("failed to begin transaction: %s%s%s"),
+ errmsg, details ? " - " : "",
+ NULLSTR_EMPTY(details));
+ }
}
- virObjectUnlock(driver);
return ret;
}
static int netcfInterfaceChangeCommit(virConnectPtr conn, unsigned int flags)
{
- int ret;
+ int ret = -1;
virCheckFlags(0, -1); /* currently flags must be 0 */
if (virInterfaceChangeCommitEnsureACL(conn) < 0)
return -1;
- virObjectLock(driver);
-
- ret = ncf_change_commit(driver->netcf, 0);
- if (ret < 0) {
- const char *errmsg, *details;
- int errcode = ncf_error(driver->netcf, &errmsg, &details);
- virReportError(netcf_to_vir_err(errcode),
- _("failed to commit transaction: %s%s%s"),
- errmsg, details ? " - " : "",
- NULLSTR_EMPTY(details));
+ VIR_WITH_OBJECT_LOCK_GUARD(driver) {
+ ret = ncf_change_commit(driver->netcf, 0);
+ if (ret < 0) {
+ const char *errmsg, *details;
+ int errcode = ncf_error(driver->netcf, &errmsg, &details);
+ virReportError(netcf_to_vir_err(errcode),
+ _("failed to commit transaction: %s%s%s"),
+ errmsg, details ? " - " : "",
+ NULLSTR_EMPTY(details));
+ }
}
- virObjectUnlock(driver);
return ret;
}
static int netcfInterfaceChangeRollback(virConnectPtr conn, unsigned int flags)
{
- int ret;
+ int ret = -1;
virCheckFlags(0, -1); /* currently flags must be 0 */
if (virInterfaceChangeRollbackEnsureACL(conn) < 0)
return -1;
- virObjectLock(driver);
-
- ret = ncf_change_rollback(driver->netcf, 0);
- if (ret < 0) {
- const char *errmsg, *details;
- int errcode = ncf_error(driver->netcf, &errmsg, &details);
- virReportError(netcf_to_vir_err(errcode),
- _("failed to rollback transaction: %s%s%s"),
- errmsg, details ? " - " : "",
- NULLSTR_EMPTY(details));
+ VIR_WITH_OBJECT_LOCK_GUARD(driver) {
+ ret = ncf_change_rollback(driver->netcf, 0);
+ if (ret < 0) {
+ const char *errmsg, *details;
+ int errcode = ncf_error(driver->netcf, &errmsg, &details);
+ virReportError(netcf_to_vir_err(errcode),
+ _("failed to rollback transaction: %s%s%s"),
+ errmsg, details ? " - " : "",
+ NULLSTR_EMPTY(details));
+ }
}
- virObjectUnlock(driver);
return ret;
}
--
2.31.1
10:02 a.m.
New subject: [libvirt PATCH 05/10] node_device_udev: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/node_device/node_device_udev.c | 76 ++++++++++++++----------------
1 file changed, 35 insertions(+), 41 deletions(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index b3e8103dae..b68dc12158 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1688,10 +1688,10 @@ nodeStateCleanup(void)
priv = driver->privateData;
if (priv) {
- virObjectLock(priv);
- priv->threadQuit = true;
- virCondSignal(&priv->threadCond);
- virObjectUnlock(priv);
+ VIR_WITH_OBJECT_LOCK_GUARD(priv) {
+ priv->threadQuit = true;
+ virCondSignal(&priv->threadCond);
+ }
if (priv->initThread) {
virThreadJoin(priv->initThread);
g_clear_pointer(&priv->initThread, g_free);
@@ -1807,24 +1807,21 @@ udevEventHandleThread(void *opaque G_GNUC_UNUSED)
/* continue rather than break from the loop on non-fatal errors */
while (1) {
- virObjectLock(priv);
- while (!priv->dataReady && !priv->threadQuit) {
- if (virCondWait(&priv->threadCond, &priv->parent.lock)) {
- virReportSystemError(errno, "%s",
- _("handler failed to wait on
condition"));
- virObjectUnlock(priv);
- return;
+ VIR_WITH_OBJECT_LOCK_GUARD(priv) {
+ while (!priv->dataReady && !priv->threadQuit) {
+ if (virCondWait(&priv->threadCond, &priv->parent.lock)) {
+ virReportSystemError(errno, "%s",
+ _("handler failed to wait on
condition"));
+ return;
+ }
}
- }
- if (priv->threadQuit) {
- virObjectUnlock(priv);
- return;
- }
+ if (priv->threadQuit)
+ return;
- errno = 0;
- device = udev_monitor_receive_device(priv->udev_monitor);
- virObjectUnlock(priv);
+ errno = 0;
+ device = udev_monitor_receive_device(priv->udev_monitor);
+ }
if (!device) {
if (errno == 0) {
@@ -1848,9 +1845,9 @@ udevEventHandleThread(void *opaque G_GNUC_UNUSED)
/* Trying to move the reset of the @priv->dataReady flag to
* after the udev_monitor_receive_device wouldn't help much
* due to event mgmt and scheduler timing. */
- virObjectLock(priv);
- priv->dataReady = false;
- virObjectUnlock(priv);
+ VIR_WITH_OBJECT_LOCK_GUARD(priv) {
+ priv->dataReady = false;
+ }
continue;
}
@@ -1873,8 +1870,7 @@ udevEventHandleCallback(int watch G_GNUC_UNUSED,
void *data G_GNUC_UNUSED)
{
udevEventData *priv = driver->privateData;
-
- virObjectLock(priv);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(priv);
if (!udevEventMonitorSanityCheck(priv, fd))
priv->threadQuit = true;
@@ -1882,7 +1878,6 @@ udevEventHandleCallback(int watch G_GNUC_UNUSED,
priv->dataReady = true;
virCondSignal(&priv->threadCond);
- virObjectUnlock(priv);
}
@@ -1897,18 +1892,17 @@ udevGetDMIData(virNodeDevCapSystem *syscap)
virNodeDevCapSystemHardware *hardware = &syscap->hardware;
virNodeDevCapSystemFirmware *firmware = &syscap->firmware;
- virObjectLock(priv);
- udev = udev_monitor_get_udev(priv->udev_monitor);
+ VIR_WITH_OBJECT_LOCK_GUARD(priv) {
+ udev = udev_monitor_get_udev(priv->udev_monitor);
- device = udev_device_new_from_syspath(udev, DMI_DEVPATH);
- if (device == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to get udev device for syspath
'%s'"),
- DMI_DEVPATH);
- virObjectUnlock(priv);
- return;
+ device = udev_device_new_from_syspath(udev, DMI_DEVPATH);
+ if (device == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to get udev device for syspath
'%s'"),
+ DMI_DEVPATH);
+ return;
+ }
}
- virObjectUnlock(priv);
if (udevGetStringSysfsAttr(device, "product_name",
&syscap->product_name) < 0)
@@ -2002,12 +1996,12 @@ nodeStateInitializeEnumerate(void *opaque)
return;
error:
- virObjectLock(priv);
- ignore_value(virEventRemoveHandle(priv->watch));
- priv->watch = -1;
- priv->threadQuit = true;
- virCondSignal(&priv->threadCond);
- virObjectUnlock(priv);
+ VIR_WITH_OBJECT_LOCK_GUARD(priv) {
+ ignore_value(virEventRemoveHandle(priv->watch));
+ priv->watch = -1;
+ priv->threadQuit = true;
+ virCondSignal(&priv->threadCond);
+ }
goto cleanup;
}
--
2.31.1
10:02 a.m.
New subject: [libvirt PATCH 06/10] qemu_agent: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/qemu/qemu_agent.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 4909fb336f..4156cb0dca 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -679,9 +679,9 @@ qemuAgentNotifyClose(qemuAgent *agent)
VIR_DEBUG("agent=%p", agent);
- virObjectLock(agent);
- qemuAgentNotifyCloseLocked(agent);
- virObjectUnlock(agent);
+ VIR_WITH_OBJECT_LOCK_GUARD(agent) {
+ qemuAgentNotifyCloseLocked(agent);
+ }
}
@@ -692,17 +692,16 @@ void qemuAgentClose(qemuAgent *agent)
VIR_DEBUG("agent=%p", agent);
- virObjectLock(agent);
+ VIR_WITH_OBJECT_LOCK_GUARD(agent) {
+ if (agent->socket) {
+ qemuAgentUnregister(agent);
+ g_clear_pointer(&agent->socket, g_object_unref);
+ agent->fd = -1;
+ }
- if (agent->socket) {
- qemuAgentUnregister(agent);
- g_clear_pointer(&agent->socket, g_object_unref);
- agent->fd = -1;
+ qemuAgentNotifyCloseLocked(agent);
}
- qemuAgentNotifyCloseLocked(agent);
- virObjectUnlock(agent);
-
virObjectUnref(agent);
}
@@ -1125,7 +1124,7 @@ qemuAgentMakeStringsArray(const char **strings, unsigned int len)
void qemuAgentNotifyEvent(qemuAgent *agent,
qemuAgentEvent event)
{
- virObjectLock(agent);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(agent);
VIR_DEBUG("agent=%p event=%d await_event=%d", agent, event,
agent->await_event);
if (agent->await_event == event) {
@@ -1136,8 +1135,6 @@ void qemuAgentNotifyEvent(qemuAgent *agent,
virCondSignal(&agent->notify);
}
}
-
- virObjectUnlock(agent);
}
VIR_ENUM_DECL(qemuAgentShutdownMode);
--
2.31.1
10:02 a.m.
New subject: [libvirt PATCH 07/10] vbox_common: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/vbox/vbox_common.c | 31 +++++++++++++------------------
1 file changed, 13 insertions(+), 18 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index acd18494d3..03f55f2f1e 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -602,11 +602,10 @@ vboxDomainSave(virDomainPtr dom, const char *path G_GNUC_UNUSED)
static int vboxConnectGetVersion(virConnectPtr conn, unsigned long *version)
{
struct _vboxDriver *data = conn->privateData;
- VIR_DEBUG("%s: in vboxGetVersion", conn->driver->name);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(data);
- virObjectLock(data);
+ VIR_DEBUG("%s: in vboxGetVersion", conn->driver->name);
*version = data->version;
- virObjectUnlock(data);
return 0;
}
@@ -664,16 +663,12 @@ vboxConnectGetMaxVcpus(virConnectPtr conn, const char *type
G_GNUC_UNUSED)
static char *vboxConnectGetCapabilities(virConnectPtr conn)
{
struct _vboxDriver *data = conn->privateData;
- char *ret = NULL;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(data);
if (!data->vboxObj)
- return ret;
-
- virObjectLock(data);
- ret = virCapabilitiesFormatXML(data->caps);
- virObjectUnlock(data);
+ return NULL;
- return ret;
+ return virCapabilitiesFormatXML(data->caps);
}
static int vboxConnectListDomains(virConnectPtr conn, int *ids, int nids)
@@ -7568,13 +7563,13 @@ vboxNodeGetFreePages(virConnectPtr conn,
unsigned int flags)
{
struct _vboxDriver *driver = conn->privateData;
- int lastCell;
+ int lastCell = -1;
virCheckFlags(0, -1);
- virObjectLock(driver);
- lastCell = virCapabilitiesHostNUMAGetMaxNode(driver->caps->host.numa);
- virObjectUnlock(driver);
+ VIR_WITH_OBJECT_LOCK_GUARD(driver) {
+ lastCell = virCapabilitiesHostNUMAGetMaxNode(driver->caps->host.numa);
+ }
return virHostMemGetFreePages(npages, pages, startCell,
cellCount, lastCell, counts);
@@ -7590,14 +7585,14 @@ vboxNodeAllocPages(virConnectPtr conn G_GNUC_UNUSED,
unsigned int flags)
{
struct _vboxDriver *driver = conn->privateData;
- int lastCell;
+ int lastCell = -1;
bool add = !(flags & VIR_NODE_ALLOC_PAGES_SET);
virCheckFlags(VIR_NODE_ALLOC_PAGES_SET, -1);
- virObjectLock(driver);
- lastCell = virCapabilitiesHostNUMAGetMaxNode(driver->caps->host.numa);
- virObjectUnlock(driver);
+ VIR_WITH_OBJECT_LOCK_GUARD(driver) {
+ lastCell = virCapabilitiesHostNUMAGetMaxNode(driver->caps->host.numa);
+ }
return virHostMemAllocPages(npages, pageSizes, pageCounts,
startCell, cellCount, lastCell, add);
--
2.31.1
10:02 a.m.
New subject: [libvirt PATCH 08/10] datatypes: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/datatypes.c | 63 +++++++++++++++----------------------------------
1 file changed, 19 insertions(+), 44 deletions(-)
diff --git a/src/datatypes.c b/src/datatypes.c
index aa614612f9..da8a9970f1 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -217,71 +217,56 @@ void virConnectCloseCallbackDataRegister(virConnectCloseCallbackData
*closeData,
void *opaque,
virFreeCallback freecb)
{
- virObjectLock(closeData);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(closeData);
if (closeData->callback != NULL) {
- VIR_WARN("Attempt to register callback on armed"
- " close callback object %p", closeData);
- goto cleanup;
+ VIR_WARN("Attempt to register callback on armed close callback object
%p",
+ closeData);
+ return;
}
closeData->conn = virObjectRef(conn);
closeData->callback = cb;
closeData->opaque = opaque;
closeData->freeCallback = freecb;
-
- cleanup:
-
- virObjectUnlock(closeData);
}
void virConnectCloseCallbackDataUnregister(virConnectCloseCallbackData *closeData,
virConnectCloseFunc cb)
{
- virObjectLock(closeData);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(closeData);
if (closeData->callback != cb) {
- VIR_WARN("Attempt to unregister different callback on "
- " close callback object %p", closeData);
- goto cleanup;
+ VIR_WARN("Attempt to unregister different callback on close callback object
%p",
+ closeData);
+ return;
}
virConnectCloseCallbackDataReset(closeData);
closeData->callback = NULL;
-
- cleanup:
-
- virObjectUnlock(closeData);
}
void virConnectCloseCallbackDataCall(virConnectCloseCallbackData *closeData,
int reason)
{
- virObjectLock(closeData);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(closeData);
if (!closeData->conn)
- goto exit;
+ return;
VIR_DEBUG("Triggering connection close callback %p reason=%d, opaque=%p",
closeData->callback, reason, closeData->opaque);
closeData->callback(closeData->conn, reason, closeData->opaque);
virConnectCloseCallbackDataReset(closeData);
-
- exit:
- virObjectUnlock(closeData);
}
virConnectCloseFunc
virConnectCloseCallbackDataGetCallback(virConnectCloseCallbackData *closeData)
{
- virConnectCloseFunc cb;
-
- virObjectLock(closeData);
- cb = closeData->callback;
- virObjectUnlock(closeData);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(closeData);
- return cb;
+ return closeData->callback;
}
/**
@@ -1128,10 +1113,9 @@ static void
virAdmConnectCloseCallbackDataDispose(void *obj)
{
virAdmConnectCloseCallbackData *cb_data = obj;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(cb_data);
- virObjectLock(cb_data);
virAdmConnectCloseCallbackDataReset(cb_data);
- virObjectUnlock(cb_data);
}
void
@@ -1150,20 +1134,16 @@ int
virAdmConnectCloseCallbackDataUnregister(virAdmConnectCloseCallbackData *cbdata,
virAdmConnectCloseFunc cb)
{
- int ret = -1;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(cbdata);
- virObjectLock(cbdata);
if (cbdata->callback != cb) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("A different callback was requested"));
- goto cleanup;
+ return -1;
}
virAdmConnectCloseCallbackDataReset(cbdata);
- ret = 0;
- cleanup:
- virObjectUnlock(cbdata);
- return ret;
+ return 0;
}
int
@@ -1173,14 +1153,12 @@
virAdmConnectCloseCallbackDataRegister(virAdmConnectCloseCallbackData *cbdata,
void *opaque,
virFreeCallback freecb)
{
- int ret = -1;
-
- virObjectLock(cbdata);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(cbdata);
if (cbdata->callback) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("A close callback is already registered"));
- goto cleanup;
+ return -1;
}
cbdata->conn = virObjectRef(conn);
@@ -1188,10 +1166,7 @@
virAdmConnectCloseCallbackDataRegister(virAdmConnectCloseCallbackData *cbdata,
cbdata->opaque = opaque;
cbdata->freeCallback = freecb;
- ret = 0;
- cleanup:
- virObjectUnlock(conn->closeCallback);
- return ret;
+ return 0;
}
virAdmServerPtr
--
2.31.1
3:54 a.m.
New subject: [libvirt PATCH 08/10] datatypes: Use automatic mutex management
On 3/25/22 16:02, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/datatypes.c | 63 +++++++++++++++----------------------------------
1 file changed, 19 insertions(+), 44 deletions(-)
diff --git a/src/datatypes.c b/src/datatypes.c
index aa614612f9..da8a9970f1 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
int
@@ -1173,14 +1153,12 @@
virAdmConnectCloseCallbackDataRegister(virAdmConnectCloseCallbackData *cbdata,
void *opaque,
virFreeCallback freecb)
{
- int ret = -1;
-
- virObjectLock(cbdata);
Here, @cbdata used to be locked ...
+ VIR_LOCK_GUARD lock = virObjectLockGuard(cbdata);
if (cbdata->callback) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("A close callback is already registered"));
- goto cleanup;
+ return -1;
}
cbdata->conn = virObjectRef(conn);
@@ -1188,10 +1166,7 @@
virAdmConnectCloseCallbackDataRegister(virAdmConnectCloseCallbackData *cbdata,
cbdata->opaque = opaque;
cbdata->freeCallback = freecb;
- ret = 0;
- cleanup:
- virObjectUnlock(conn->closeCallback);
... but ere conn->closeCallback was unlocked. I believe your change is
correct and in fact fixes this problem. But can't really verify because
this function is never called.
- return ret;
+ return 0;
}
virAdmServerPtr
Michal
10:02 a.m.
New subject: [libvirt PATCH 09/10] ch_monitor: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/ch/ch_monitor.c | 55 ++++++++++++++++++++-------------------------
1 file changed, 24 insertions(+), 31 deletions(-)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 84085b7991..2c6b83a1b5 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -648,14 +648,13 @@ virCHMonitorCurlPerform(CURL *handle)
int
virCHMonitorPutNoContent(virCHMonitor *mon, const char *endpoint)
{
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mon);
g_autofree char *url = NULL;
int responseCode = 0;
int ret = -1;
url = g_strdup_printf("%s/%s", URL_ROOT, endpoint);
- virObjectLock(mon);
-
/* reset all options of a libcurl session handle at first */
curl_easy_reset(mon->handle);
@@ -666,8 +665,6 @@ virCHMonitorPutNoContent(virCHMonitor *mon, const char *endpoint)
responseCode = virCHMonitorCurlPerform(mon->handle);
- virObjectUnlock(mon);
-
if (responseCode == 200 || responseCode == 204)
ret = 0;
@@ -707,26 +704,24 @@ virCHMonitorGet(virCHMonitor *mon, const char *endpoint,
virJSONValue **response
url = g_strdup_printf("%s/%s", URL_ROOT, endpoint);
- virObjectLock(mon);
+ VIR_WITH_OBJECT_LOCK_GUARD(mon) {
+ /* reset all options of a libcurl session handle at first */
+ curl_easy_reset(mon->handle);
- /* reset all options of a libcurl session handle at first */
- curl_easy_reset(mon->handle);
+ curl_easy_setopt(mon->handle, CURLOPT_UNIX_SOCKET_PATH, mon->socketpath);
+ curl_easy_setopt(mon->handle, CURLOPT_URL, url);
- curl_easy_setopt(mon->handle, CURLOPT_UNIX_SOCKET_PATH, mon->socketpath);
- curl_easy_setopt(mon->handle, CURLOPT_URL, url);
+ if (response) {
+ headers = curl_slist_append(headers, "Accept: application/json");
+ headers = curl_slist_append(headers, "Content-Type:
application/json");
+ curl_easy_setopt(mon->handle, CURLOPT_HTTPHEADER, headers);
+ curl_easy_setopt(mon->handle, CURLOPT_WRITEFUNCTION, curl_callback);
+ curl_easy_setopt(mon->handle, CURLOPT_WRITEDATA, (void *)&data);
+ }
- if (response) {
- headers = curl_slist_append(headers, "Accept: application/json");
- headers = curl_slist_append(headers, "Content-Type:
application/json");
- curl_easy_setopt(mon->handle, CURLOPT_HTTPHEADER, headers);
- curl_easy_setopt(mon->handle, CURLOPT_WRITEFUNCTION, curl_callback);
- curl_easy_setopt(mon->handle, CURLOPT_WRITEDATA, (void *)&data);
+ responseCode = virCHMonitorCurlPerform(mon->handle);
}
- responseCode = virCHMonitorCurlPerform(mon->handle);
-
- virObjectUnlock(mon);
-
if (responseCode == 200 || responseCode == 204) {
if (response) {
data.content = g_realloc(data.content, data.size + 1);
@@ -863,20 +858,18 @@ virCHMonitorCreateVM(virCHMonitor *mon,
nnicindexes, nicindexes) != 0)
return -1;
- virObjectLock(mon);
-
- /* reset all options of a libcurl session handle at first */
- curl_easy_reset(mon->handle);
-
- curl_easy_setopt(mon->handle, CURLOPT_UNIX_SOCKET_PATH, mon->socketpath);
- curl_easy_setopt(mon->handle, CURLOPT_URL, url);
- curl_easy_setopt(mon->handle, CURLOPT_CUSTOMREQUEST, "PUT");
- curl_easy_setopt(mon->handle, CURLOPT_HTTPHEADER, headers);
- curl_easy_setopt(mon->handle, CURLOPT_POSTFIELDS, payload);
+ VIR_WITH_OBJECT_LOCK_GUARD(mon) {
+ /* reset all options of a libcurl session handle at first */
+ curl_easy_reset(mon->handle);
- responseCode = virCHMonitorCurlPerform(mon->handle);
+ curl_easy_setopt(mon->handle, CURLOPT_UNIX_SOCKET_PATH, mon->socketpath);
+ curl_easy_setopt(mon->handle, CURLOPT_URL, url);
+ curl_easy_setopt(mon->handle, CURLOPT_CUSTOMREQUEST, "PUT");
+ curl_easy_setopt(mon->handle, CURLOPT_HTTPHEADER, headers);
+ curl_easy_setopt(mon->handle, CURLOPT_POSTFIELDS, payload);
- virObjectUnlock(mon);
+ responseCode = virCHMonitorCurlPerform(mon->handle);
+ }
if (responseCode == 200 || responseCode == 204)
ret = 0;
--
2.31.1
10:02 a.m.
New subject: [libvirt PATCH 10/10] virportallocator: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/util/virportallocator.c | 93 ++++++++++++++++---------------------
1 file changed, 40 insertions(+), 53 deletions(-)
diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c
index 5e63470848..44edd8f99a 100644
--- a/src/util/virportallocator.c
+++ b/src/util/virportallocator.c
@@ -204,7 +204,6 @@ int
virPortAllocatorAcquire(const virPortAllocatorRange *range,
unsigned short *port)
{
- int ret = -1;
size_t i;
virPortAllocator *pa = virPortAllocatorGet();
@@ -213,44 +212,39 @@ virPortAllocatorAcquire(const virPortAllocatorRange *range,
if (!pa)
return -1;
- virObjectLock(pa);
-
- for (i = range->start; i <= range->end && !*port; i++) {
- bool used = false, v6used = false;
-
- if (virBitmapIsBitSet(pa->bitmap, i))
- continue;
-
- if (virPortAllocatorBindToPort(&v6used, i, AF_INET6) < 0 ||
- virPortAllocatorBindToPort(&used, i, AF_INET) < 0)
- goto cleanup;
-
- if (!used && !v6used) {
- /* Add port to bitmap of reserved ports */
- if (virBitmapSetBit(pa->bitmap, i) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to reserve port %zu"), i);
- goto cleanup;
+ VIR_WITH_OBJECT_LOCK_GUARD(pa) {
+ for (i = range->start; i <= range->end; i++) {
+ bool used = false, v6used = false;
+
+ if (virBitmapIsBitSet(pa->bitmap, i))
+ continue;
+
+ if (virPortAllocatorBindToPort(&v6used, i, AF_INET6) < 0 ||
+ virPortAllocatorBindToPort(&used, i, AF_INET) < 0)
+ return -1;
+
+ if (!used && !v6used) {
+ /* Add port to bitmap of reserved ports */
+ if (virBitmapSetBit(pa->bitmap, i) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to reserve port %zu"), i);
+ return -1;
+ }
+ *port = i;
+ return 0;
}
- *port = i;
- ret = 0;
}
}
- if (*port == 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Unable to find an unused port in range '%s'
(%d-%d)"),
- range->name, range->start, range->end);
- }
- cleanup:
- virObjectUnlock(pa);
- return ret;
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to find an unused port in range '%s'
(%d-%d)"),
+ range->name, range->start, range->end);
+ return -1;
}
int
virPortAllocatorRelease(unsigned short port)
{
- int ret = -1;
virPortAllocator *pa = virPortAllocatorGet();
if (!pa)
@@ -259,25 +253,21 @@ virPortAllocatorRelease(unsigned short port)
if (!port)
return 0;
- virObjectLock(pa);
-
- if (virBitmapClearBit(pa->bitmap, port) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to release port %d"),
- port);
- goto cleanup;
+ VIR_WITH_OBJECT_LOCK_GUARD(pa) {
+ if (virBitmapClearBit(pa->bitmap, port) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to release port %d"),
+ port);
+ return -1;
+ }
}
- ret = 0;
- cleanup:
- virObjectUnlock(pa);
- return ret;
+ return 0;
}
int
virPortAllocatorSetUsed(unsigned short port)
{
- int ret = -1;
virPortAllocator *pa = virPortAllocatorGet();
if (!pa)
@@ -286,17 +276,14 @@ virPortAllocatorSetUsed(unsigned short port)
if (!port)
return 0;
- virObjectLock(pa);
-
- if (virBitmapIsBitSet(pa->bitmap, port) ||
- virBitmapSetBit(pa->bitmap, port) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to reserve port %d"), port);
- goto cleanup;
+ VIR_WITH_OBJECT_LOCK_GUARD(pa) {
+ if (virBitmapIsBitSet(pa->bitmap, port) ||
+ virBitmapSetBit(pa->bitmap, port) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to reserve port %d"), port);
+ return -1;
+ }
}
- ret = 0;
- cleanup:
- virObjectUnlock(pa);
- return ret;
+ return 0;
}
--
2.31.1
9:30 a.m.
ping
On Fri, 2022-03-25 at 16:02 +0100, Tim Wiederhake wrote:
Use the recently implemented VIR_LOCK_GUARD and
VIR_WITH_MUTEX_LOCK_GUARD
to simplify mutex management.
Tim Wiederhake (10):
virnetdaemon: Use automatic mutex management
bridge_driver: Use automatic mutex management
node_device_driver: Use automatic mutex management
interface_backend_netcf: Use automatic mutex management
node_device_udev: Use automatic mutex management
qemu_agent: Use automatic mutex management
vbox_common: Use automatic mutex management
datatypes: Use automatic mutex management
ch_monitor: Use automatic mutex management
virportallocator: Use automatic mutex management
src/ch/ch_monitor.c | 55 +++----
src/datatypes.c | 63 +++-----
src/interface/interface_backend_netcf.c | 192 +++++++++++-----------
--
src/network/bridge_driver.c | 59 +++-----
src/network/bridge_driver_linux.c | 4 +-
src/node_device/node_device_driver.c | 12 +-
src/node_device/node_device_udev.c | 76 +++++-----
src/qemu/qemu_agent.c | 25 ++-
src/rpc/virnetdaemon.c | 144 ++++++------------
src/util/virportallocator.c | 93 +++++-------
src/vbox/vbox_common.c | 31 ++--
11 files changed, 302 insertions(+), 452 deletions(-)
--
2.31.1
3:54 a.m.
On 3/25/22 16:02, Tim Wiederhake wrote:
Use the recently implemented VIR_LOCK_GUARD and
VIR_WITH_MUTEX_LOCK_GUARD
to simplify mutex management.
Tim Wiederhake (10):
virnetdaemon: Use automatic mutex management
bridge_driver: Use automatic mutex management
node_device_driver: Use automatic mutex management
interface_backend_netcf: Use automatic mutex management
node_device_udev: Use automatic mutex management
qemu_agent: Use automatic mutex management
vbox_common: Use automatic mutex management
datatypes: Use automatic mutex management
ch_monitor: Use automatic mutex management
virportallocator: Use automatic mutex management
src/ch/ch_monitor.c | 55 +++----
src/datatypes.c | 63 +++-----
src/interface/interface_backend_netcf.c | 192 +++++++++++-------------
src/network/bridge_driver.c | 59 +++-----
src/network/bridge_driver_linux.c | 4 +-
src/node_device/node_device_driver.c | 12 +-
src/node_device/node_device_udev.c | 76 +++++-----
src/qemu/qemu_agent.c | 25 ++-
src/rpc/virnetdaemon.c | 144 ++++++------------
src/util/virportallocator.c | 93 +++++-------
src/vbox/vbox_common.c | 31 ++--
11 files changed, 302 insertions(+), 452 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
957
days inactive
968
days old
13 comments
2 participants
participants (2)
-
Michal Prívozník -
Tim Wiederhake