[libvirt] [PATCH 0/3] Introduce RW locks to virDomainObjList

[Pavel Hrdina]

On Sun, Jul 23, 2017 at 10:46:15PM +0200, Michal Privoznik wrote:
> On 07/23/2017 07:21 PM, Pavel Hrdina wrote:
> > On Sun, Jul 23, 2017 at 02:33:49PM +0200, Michal Privoznik wrote:
> >> On 07/23/2017 02:10 PM, John Ferlan wrote:
> >>>
> >>>
> >>> On 07/19/2017 10:31 AM, Michal Privoznik wrote:
> >>>> While this is not that critical (hash tables have O(1) time complexity for
> >>>> lookups), it lays down path towards making virDomainObj using RW locks instead
> >>>> of mutexes. Still, in my limited testing this showed slight improvement.
> >>>>
> >>>> Michal Privoznik (3):
> >>>>   virthread: Introduce virRWLockInitPreferWriter
> >>>>   virobject: Introduce virObjectRWLockable
> >>>>   virdomainobjlist: Use virObjectRWLockable
> >>>>
> >>>>  src/conf/virdomainobjlist.c |  24 ++++----
> >>>>  src/libvirt_private.syms    |   4 ++
> >>>>  src/util/virobject.c        | 144 ++++++++++++++++++++++++++++++++++----------
> >>>>  src/util/virobject.h        |  16 +++++
> >>>>  src/util/virthread.c        |  35 +++++++++++
> >>>>  src/util/virthread.h        |   1 +
> >>>>  6 files changed, 180 insertions(+), 44 deletions(-)
> >>>>
> >>>
> >>> This could be a "next step" in the work I've been doing towards a common
> >>> object:
> >>
> >> Sure. If we have just one common object the change can be done in one
> >> place instead of many. I don't care in what order are the changes merged.
> > 
> > I'm still not sure about the implementation that you are heading to,
> 
> "you" as in John or as in me?

Oh right, I should be exact, John.

> 
> > I would actually prefer something similar to the current
> > virDomainObjList implementation, create a new module in utils called
> > virObjectList and make it somehow generic that it could be used by our
> > objects.  I personally don't like the fact that there will be two new
> > classes, one that enables using the other one.
> 
> Well I think we need two classes. A list of objects is something
> different than objects themselves. You can hardly assign some meaning to
> virDomainObj.lookupByName() or virDomainObjList.startCPUs(). It's
> important to differentiate "data an object is working with" and
> "operations an object can do". The fact that virDomainObjList works with
> virDomainObj doesn't mean it should be in any sense derived. In OOP, if
> class B is derived from class A, it means that B has all the
> attributes/methods that A has, plus something more, e.g. because B is
> more specialized. For instance, assume we have virCarClass. Then,
> virTruckClass or virV8TurboCharged3LClass can both be derived from
> virCarClass. But I can hardly imagine virParkingLotClass doing the same
> thing.
> 
> Michal

Well, I know how all this work. What I meant is that virDomainObj
would be still derived from virObjectLockable or virObjectRWLockable and
there would be virObjectList class which would implement the lookup
functions, addObj, removeObj, and so on.  You would create a new
instance of virObjectList class and fill that instance with domain
objects that you need to list.  The domain object itself doesn't have
to know anything about the virObjectList class.

To use the similar explanation, you have virObjectClass
(virObjectLockableClass) and you have virCarClass (virDomainObjClass)
and virTruckClass (virStoragePoolClass) and there would be
virParkingClass (virObjectListClass).  The virCarClass is not derived
from virVehicleParkableClass (virObjectLookupKeys) in order to have
virParkingClass (virObjectLookupHash) to allow virCarClass to park.
That's what I don't like about the current implementation.

Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170723/fc672bce/attachment-0001.sig>