On Mon, Apr 13, 2015 at 16:33:51 +0200, Martin Kletzander wrote:
It already had a virMutex inside, so this is just a cleanup.
It also probably fixes a potential deadlock too.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/datatypes.c | 12 ++---------- src/datatypes.h | 12 +++++------- src/libvirt-host.c | 18 +++++++++--------- src/util/virerror.c | 18 +++++++++--------- 4 files changed, 25 insertions(+), 35 deletions(-)
...
diff --git a/src/datatypes.h b/src/datatypes.h index 4973b07..9e19c55 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -1,7 +1,7 @@ /* * datatypes.h: management of structs for public data types * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc.
Yay, it's 2015 already? We are going to get hoverboards this year!
* * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -328,7 +328,7 @@ struct _virConnectCloseCallbackData { * Internal structure associated to a connection */ struct _virConnect { - virObject object; + virObjectLockable object; /* All the variables from here, until the 'lock' declaration * are setup at time of connection open, and never changed * since. Thus no need to lock when accessing them
The lock delcaration is no longer here, so you should mention the comment you've changed below.
@@ -352,12 +352,10 @@ struct _virConnect { void * privateData;
/* - * The lock mutex must be acquired before accessing/changing - * any of members following this point, or changing the ref - * count of any virDomain/virNetwork object associated with - * this connection + * Object lock must be acquired before accessing/changing any of + * members following this point, or changing the ref count of any + * virDomain/virNetwork object associated with this connection. */ - virMutex lock;
/* Per-connection error. */ virError err; /* the last error */
...
diff --git a/src/libvirt-host.c b/src/libvirt-host.c index b4dc13e..03bee1f 100644 --- a/src/libvirt-host.c +++ b/src/libvirt-host.c
...
@@ -1288,15 +1288,15 @@ virConnectUnregisterCloseCallback(virConnectPtr conn, conn->closeCallback->freeCallback(conn->closeCallback->opaque); conn->closeCallback->freeCallback = NULL;
- virObjectUnref(conn); virObjectUnlock(conn->closeCallback); - virMutexUnlock(&conn->lock); + virObjectUnlock(conn); + virObjectUnref(conn);
So this is the only semantic change in this patch. I'm surprised it didn't bite us at some point actually, since the connection dispose code is/was locking conn->lock. Perhaps it didn't ever order in the wrong way.
return 0;
error: virObjectUnlock(conn->closeCallback); - virMutexUnlock(&conn->lock); + virObjectUnlock(conn); virDispatchError(conn); return -1; }
ACK with the comment updated. Peter