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 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. > --- > > 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(+) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index fde88b2..5a59e3f 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -2499,6 +2499,43 @@ virDomainDefPostParseInternal(virDomainDefPtr def, > return -1; > } > > + /* enforce range checks for cputune values */ > + /* these are not represented in the XML schema, but are documented */ > + if (def->cputune.period > 0 && > + (def->cputune.period < 1000 || def->cputune.period > 1000000)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("Value of cputune period must be in range " > + "[1000, 1000000]")); > + return -1; > + } > + > + if (def->cputune.emulator_period > 0 && > + (def->cputune.emulator_period < 1000 || > + def->cputune.emulator_period > 1000000)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("Value of cputune emulator_period must be in range " > + "[1000, 1000000]")); > + return -1; > + } > + > + if (def->cputune.quota > 0 && > + (def->cputune.quota < 1000 || > + def->cputune.quota > 18446744073709551)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("Value of cputune quota must be in range " > + "[1000, 18446744073709551]")); > + return -1; > + } > + > + if (def->cputune.emulator_quota > 0 && > + (def->cputune.emulator_quota < 1000 || > + def->cputune.emulator_quota > 18446744073709551)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("Value of cputune emulator_quota must be in range " > + "[1000, 18446744073709551]")); > + return -1; > + } > + > return 0; > } > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list