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@xxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list