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

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



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




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