[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 1/8] util: Rename virObjectLockRead to virObjectRWLockRead



On 08/01/2017 02:05 AM, John Ferlan wrote:
> Since the class it represents is based on virObjectRWLockableClass
> and in order to make sure we diffentiate lest anyone somehow

^^ couple of typos

> believes they could use virObjectLockRead for a virObjectLockableClass,
> let's rename the API to use the RW in the name. Besides the RW locks
> refer to pthread_rwlock_{init|rdlock|wrlock|unlock|destroy} while the
> other locks refer to pthread_mutex_{init|lock|unlock|destroy}.

Firstly, this is just an internal implementation. We often rename APIs
for us to use. Secondly, this is because pthreads are C API with no
'object' reference. So they have to have two unlock functions for two
different objects.

> 
> Signed-off-by: John Ferlan <jferlan redhat com>
> ---
>  src/conf/virdomainobjlist.c | 18 +++++++++---------
>  src/libvirt_private.syms    |  2 +-
>  src/util/virobject.c        | 11 ++++++++---
>  src/util/virobject.h        |  2 +-
>  4 files changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
> index 1d027a4..9bc6603 100644
> --- a/src/conf/virdomainobjlist.c
> +++ b/src/conf/virdomainobjlist.c
> @@ -118,7 +118,7 @@ virDomainObjListFindByIDInternal(virDomainObjListPtr doms,
>                                   bool ref)
>  {
>      virDomainObjPtr obj;
> -    virObjectLockRead(doms);
> +    virObjectRWLockRead(doms);

If we are going to do this (which I'm no fan of), then we should also
turn virObjectLock() into virObjectLockableLock(). For the consistency
sake. Moreover, as I stated in discussion to v1 (not sure if you've read
it before sending this series), I quite like that I'm able to type
virObjectLock, hit shortcut for bringing up completion list and chose
'virObjectLock', 'virObjectLockRead', (or even) 'virObjectLockWrite'.
With this patch I'm no longer able to do that so easily.

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]