Re: [libvirt] [PATCH 0/3] Introduce RW locks to virDomainObjList
On 07/24/2017 02:33 PM, Pavel Hrdina wrote:
> EXACTLY! This is why I started down this path... Of course I
want far
> too generic for some people's preferences by going with primaryKey and
> secondaryKey type nomenclature, so I was forced by review to use
> UUID/Name which are far less generic, but "tie" together the various
> vir*obj.c consumers which is fine although would seemingly go against
> the premise that ObjList shouldn't be aware of what it's storing. It
> should only care that it's using a some Key rather than a named Key.
You are saying exactly and yet you are introducing new
virObjectLookupKeys which is used instead of virObjectLockable just to
make the new listing virObjectLookupHash to work and that's wrong. It
should work even with virObject.
Pavel
This really isn't the right place for a discussion about the other
series - the merits of that solution should be discussed there...
Still, I must be missing something. Why is it wrong to create a new
object that would have a specific use? virObjectLockable was created at
some point in history and then used as a way to provide locking for a
virObject that only had a @ref. Someone could still create a virObject
as well and just get @refs. With the new objects based on those two
objects you get a few more features that allow for add, search, lookup,
remove. But you call that wrong, which is confusing to me.
It doesn't make much sense to have a hash table with objects that have
no way of being looked up does it? How do you add the object? Even the
virnode* uses a hash table that has objects that have UUID and Name used
for lookup.
A virObjectLockable has a @ref and @lock - there's *nothing* in there
that can be used for a key for a hash table. So what in your opinion
would be the use of an object in a table that cannot be fetched. So
"something" has to create an object that uses the existing
virObjectLockable as a base and allows for it to be build upon in order
to be more useful. A new object class needs to be created and used. If
something still wants to use virObjectLockable and build their own who
knows what in order to manage the virObjectLockable's they still can.
The virObjectLockable isn't going away, it's being extended.
After initial series way back in February:
https://www.redhat.com/archives/libvir-list/2017-February/msg00519.html
I was encouraged to take a more object oriented view, thus the follow up
RFC in April:
https://www.redhat.com/archives/libvir-list/2017-April/msg00321.html
which got no feedback, so in late May it's:
https://www.redhat.com/archives/libvir-list/2017-May/msg01178.html
which got some feedback and a quick v2 in early June:
https://www.redhat.com/archives/libvir-list/2017-June/msg00070.html
that got more feedback, but mainly that the generic primaryKey and
secondaryKey were not favored. So, v3 was generated in mid June that was
less abstract:
https://www.redhat.com/archives/libvir-list/2017-June/msg00916.html
which again gets zero feedback, but apparently has been read.
Still no where in any of this work has it been said using these types of
objects was wrong.
John