Re: [libvirt] [PATCHv4 6/9] conf: Enforce ranges on cputune variables
On 03/26/13 01:19, Laine Stump wrote:
On 03/15/2013 11:26 AM, Peter Krempa wrote:
> The limits are documented at
> http://libvirt.org/formatdomain.html#elementsCPUTuning . Enforce them
> when going through XML parsing in addition to being enforced by the API.
What's the rationale for doing this validation during the post-parse
rather than just doing it as the cputune elements are being parsed? They
I wanted to keep the parser clean of stuff that can't or isn't
represented in the XML schema. In this case, this information is
"enforced" by the docs and probably also could be represented in the
schema. Thus it might be worth moving this to the parser.
don't depend on any device that might be modified during a
post-parse
callback (or any other unrelated part of the domain). My opinion is that
a separate post-parse validation should only be done if:
1) the validation depends on hypervisor (in which case it will be done
in a hypervisor-specific callback)
2) the validation depends on some other element of the domain object (in
which case it would be done in virDomainDefPostParseInternal, as you've
done here)
or
2a) the validation depends on some other element of the domain that
could be changed by a hypervisor-specific post-parse validation function.
Doing it in a separate function when none of the above is true just has
the effect of spreading out the parsing of a single element into
multiple places, making it more difficult to understand and maintain the
code.
I agree on those points and I'll move that stuff to the parser.
> ---
>
> Notes:
> Version 4:
> - changed error from VIR_ERR_XML_ERROR to VIR_ERR_CONFIG_UNSUPPORTED
> Version 3:
> - new in series
>
> src/conf/domain_conf.c | 37 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
Peter