On 07/20/2012 10:24 AM, Osier Yang wrote: > Except objects of domains, domain snapshots, we will also add APIs > to list objects like storage pools, storage vols, network, interface, > etc. And it's deserved to have the small helper functions in a > common file instead of in separate files. > > This patch renames virdomainlist.[ch] to virobjectlist.[ch], and > also renames the macros to filter domain objects more specificly.
I actually started looking at this series down in virConnectListAllNetworks, and was curious why functions specific to virNetwork were put into this new common file. After looking up at a few of the other additions to virobjectlist.c (all the patches with "add helpers (to|for) list" in the subject), it looks like what's ended up in this file are driver-specific functions that just look similar to each other. I don't think that is right. If there are any functions or data structures that are truly reusable *unchanged* by all of the drivers for their listall functions, it makes sense to put those in a common file, but to group together all of these functions just because they superficially look similar is wrong - it's going against all the work that's been done to separate things out on functional boundaries. In short, if a function is specific enough that its name starts with, e.g. virNetwork or virStoragePool, then it should be in that driver's file (or at least the *_conf.c associated with that driver), not in a common location. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
