On 2012年07月31日 01:00, Eric Blake wrote:
On 07/24/2012 08:52 PM, Osier Yang wrote:
Or folder the functions into conf/domain_conf.[ch], conf/storage_conf.[ch], etc?
Where is the source data structure defined? If the data structure that is used to list of domains/networks/storage pools|volumes is defined in conf/*_conf.h, then the function that operates on that data structure to produce the output list should be defined in conf/*_conf.c. If the data structure is defined in ${driver}.h, then the function that operates on it should be defined in ${driver}.c. (I think for all examples in this current patchset, the answer is *_conf.c)
Well. You are a bit late (may be on vacation). virdomainlist.[ch] is already there. I'd want to see a 3rd opinion on this, if finally we want them be foldered into *_conf.c, I will be fine with it. We need to get rid of the existed virdomainlist.[ch] first.
My recollection was that Peter originally added virdomainlist.c to solve a link error - our use of virGetDomain() caused a link failure when trying to build the LXC driver. LXC requires several functions from domain_conf.c, but apparently was not linking against the souce for datatypes.c, which made virGetDomain() the first instance of a linking problem.
But in the meantime, we have refactored the Makefile, and how LXC is linked in the first place. I'm wondering if Daniel's commit 284143b makes it possible to use virGetDomain() directly from domain_conf.c in the first place. If that is the case, then the answer is 'get rid of virdomainlist.c/virobjectlist.c, and stick everything in the appropriate foo_conf.c instead'. If we still get a link failure for LXC in spite of using virGetDomain() directly from domain_conf.c, then it might be worth investigating that link failure first.
I tried, virGetDomain can be used without including virdomainlist.h, seems Daniel's commit fixed the link error.
Yeah, that doesn't quite answer the question (one file virobjectlist.c shared among multiple objects, vs. one file per object type), but hopefully it helps in figuring out a way forward.
I think your opinion is to destroy virdomainlist.h then. Assuming it as a 3rd opinion, I will go this way, and rebase the whole patch set. Thanks! Regards, Osier