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

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

 



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@xxxxxxxxxx>
> ---
>  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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux