On Wed, Mar 13, 2019 at 09:54:31PM -0600, Suyang Chen wrote:
move all the def->cputune 'period must be in range' and 'quota must be in range' errors into two macros and called in virDomainDefCputuneValidate function and have it called from virDomainDefValidateInternal function
I'll reformulate ^this a tiny bit to indicate we also moved the checks from the 'parse' phase into the 'validation' phase. For future reference, make sure you start a sentence with a capital letter and end it with a period ;).
Reason: Two macros maybe easier to debug and change in the future
Solve the first two small tasks in bitsizedtask: "Move validation checks out of domain XML parsing"
This doesn't provide much information and in fact should be dropped from the commit message.
Signed-off-by: Suyang Chen <dawson0xff@gmail.com> --- src/conf/domain_conf.c | 109 +++++++++++++++-------------------------- 1 file changed, 40 insertions(+), 69 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 356a9ae56c..c159cffa05 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6565,6 +6565,43 @@ virDomainDefLifecycleActionValidate(const virDomainDef *def) return 0; }
+#define CPUTUNE_VALIDATE_PERIOD(name) \ + if (def->cputune.name > 0 && \ + (def->cputune.name < 1000 || def->cputune.name > 1000000)) { \ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \ + _("Value of cputune '%s' must be in range " \ + "[1000, 1000000]"), #name); \ + return -1; \ + } + +#define CPUTUNE_VALIDATE_QUOTA(name) \ + if (def->cputune.name > 0 && \ + (def->cputune.name < 1000 || \ + def->cputune.name > 18446744073709551LL)) { \ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \ + _("Value of cputune '%s' must be in range " \ + "[1000, 18446744073709551]"), #name); \ + return -1; \ + } +
2 macros are fine, maybe even more readable. One thing though, although we don't enforce this and often forget, the preference for writing preprocesor macros is by using do-while(0), which has a neat side effect that you can reliably call the macro as a function. So, I'll convert them before merging. Reviewed-by: Erik Skultety <eskultet@redhat.com>