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.