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

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.