[libvirt] [PATCHv2 15/21] conf: Add separate defaults addition and validation for XML parsing

[Peter Krempa]

On 03/07/13 18:34, Laine Stump wrote:
> On 03/06/2013 10:37 AM, Peter Krempa wrote:
>> This patch adds instrumentation that will ultimately allow to split out
>> filling of defaults and input validation from the XML parser to separate
>> functions.
>>
>> With this patch, after the XML is parsed, a callback to the driver is
>> issued requesing to fill and validate driver specific details of the
>> configuration. This allows to use sensible defaults and checks on a per
>> driver basis at the time the XML is parsed.
>>
>> Three callback pointers are exposed in the new virDomainXMLConf object:
>> * virDeviceDefAdjustCallback - called for a single device parsed and
>>                                 for every single device in a domain
>>                                 config. A virDomainDeviceDefPtr is
>>                                 passed along with the domain
>>                                 definition and virCaps.
>> * virDomainDefAdjustCallback - called if a domain definition is parsed
>>                                 device specific callbacks were called.
>>                                 A virDomainDefPtr is passed along with
>>                                 virCaps. There are two callbacks of this
>>                                 kind, one called before the devices are
>>                                 parsed and one after.
>>
>> Errors may be reported in those callbacks resulting in a XML parsing
>> failure.
>>
>> Additionally internal filler and checker functions are added that are
>> meant to be used forseparating the validation and defaults assignment
>> code from the parser function.
>
> Just repeating what I said in another mail on a different (and probably
> buried) thread yesterday:
>
>     https://www.redhat.com/archives/libvir-list/2013-March/msg00249.html
>
> I think that not all validation should be done in these separate
> callbacks. Only validation that requires more information than is
> available in the currently-in-process element (e.g., needs to know
> hypervisor-specific or host environment-specific information, or
> something about a completely different part of the domain xml) should be
> done externally; a good indicator of this is if a restriction can't be
> reasonably included in the RNG, then it probably needs to be validated
> in the external callback function.
>
> If, on the other hand, it is specified in the RNG, and applies to all
> situations (different hypervisor, hypervisor binary, host, etc), then it
> should simply be part of the basic parser.

I agree in the general with this. My approach isn't to put all the stuff 
into the driver callbacks. On the other hand, I'd like to keep the 
parser function tidy and the checks don't help in this effort. That's 
why I created verification functions (see patch 21) that is there to do 
all the common checking stuff.

This approach might also help Michal with his "normalization" API he is 
planing.

Peter