On 03/25/2013 02:25 PM, Laine Stump wrote:
>> + if (xmlconf && xmlconf->config.devicesConfigCallback) {
>> + ret = xmlconf->config.devicesConfigCallback(dev, def, caps,
>> + xmlconf->config.priv);
>
> The implementation of these functions in the hypervisor drivers has been
> changed to xxxPostParsexxx, but you're still calling it
> devicesConfigCallback in the struct that's setup. I think this struct
> should have the members called xxxPostParsexxx as well. This is
> especially confusing because the parser is parsing a domain config (so
> we have the "config for the parser of the config" - it makes it a bit
> difficult to follow whether we're talking about the domain config or
> parser config sometimes.).
>
>> +
>> +typedef struct _virDomainDefParserConfig virDomainDefParserConfig;
>> +typedef virDomainDefParserConfig *virDomainDefParserConfigPtr;
>> +struct _virDomainDefParserConfig {
>> + virDomainDefPostParseCallback domainConfigCallback;
>> + virDomainDeviceDefPostParseCallback devicesConfigCallback;
>
> Yeah, here's the definition. I think domainConfigCallback should be
> called domainPostParseCallback and devicesConfigCallback should be
> called devicesPostParseCallback. That would help quite a lot to
> un-confuse me.
>
> (an aside - I'm still trying to think of something that's less confusing
> than "conf" for xmlconf and virDomainXMLConfPtr - this dual usage (for
> both domain config that we're parsing, and the parser config) requires
> too much concentration from my brain :-). I haven't come up with a
> better idea yet, though...)
Maybe virDomainXMLOptionPtr. Leave 'conf' for items related to parsing
a user's libvirtd.conf and qemu.conf files, and 'xmlopt' instead of
'xmlconf' for items related to options that the driver passed in for
impacting xml parsing/validation.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list