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

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

 



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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]