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

Re: [libvirt] [PATCH RFC 0/9] Introduce virPoolObj[Table]Ptr data/API's



On 02/11/2017 05:29 PM, John Ferlan wrote:
> Based of current top of git commit id '5620c6095'... I imagine things
> will change while this is on-list, so if there is the desire to create
> your own branch that's your starting point!

Yeah, they had changed. Sorry for ignoring this for a long time. I'm
gonna review it but obviously, the master had diverged too much to apply
these cleanly -> a rebased version needs to be sent again.

> 
> This is perhaps more than an RFC considering I've added comments
> already (at least one reason why the insertions count is high). Still
> it's not really complete and ready for prime time, but I figured I'd
> put it out there for feedback.
> 
> Essentially I've tried to follow the domainobj 'model' by creating
> an API to manage the "objects" various drivers have the need to manage.
> Currently there are 4 drivers using linked lists of some sort and 4
> using hash tables in some manner. This implementation modifies the
> linked list abusers to use a common hash table.
> 
> The common hash table will have uuid/name accessors. The preference
> remains uuid for uniqueness, but there are a couple of tables (node
> device, nwfilter) that don't have a uuid and thus name is unique
> enough for them.
> 
> The genesis for this was recent patches to the storage driver that
> were essentially copying many lines of code and just changing the
> names to do the same thing. As I looked deeper at that - I found
> a mish-mash of object management throughout the drivers, so this
> is my attempt to unify that.
> 
> The "goal" is to have a common look and feel to the API's so there's
> less thought over how a specific implementation does things. Each
> trip into the driver and exit can be handled in a common manner
> especially with respect to locks/refcnt's.

Agreed. Ideally we can have just one implementation shared among all the
drivers. But let me check the patches how you get there.

> 
> Since I was splitting things apart - I also split out the object
> code/data *_conf.{c,h} into separate vir*obj.{c,h} files. I took
> some liberties with adjusting some names, modifying some other things
> along the way since the type A personality took over to try and
> follow more current coding conventions while I was in the middle
> of changing things.

Fair enough.

> 
> While the ultimate goal is to finish with the domain code - that's
> just too busy in order to make adjustments right now. Still at least
> that code does use a similar hash table model - so while it will be
> painful - it can be done later if/when this is accepted (and perhaps
> when we know there's a slow period for posting changes).

Yeah, this is going to be tricky. But if we review quickly we might be
lucky to have as few conflicts as possible. Having said that, ACKs in my
review should be read as "this change makes sense" not "green to push".

Michal


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