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

Re: [libvirt] [PATCH 02/49] list: Rename virdomainlist.[ch] for common use



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.


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