[libvirt] [PATCHv4 6/9] conf: Enforce ranges on cputune variables

[Peter Krempa]

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