src/conf/domain_conf.c | 109 +++++++++++++++-------------------------- 1 file changed, 40 insertions(+), 69 deletions(-)
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
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"
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; \
+ }
+
+static int
+virDomainDefCputuneValidate(const virDomainDef *def)
+{
+ CPUTUNE_VALIDATE_PERIOD(period)
+ CPUTUNE_VALIDATE_PERIOD(global_period)
+ CPUTUNE_VALIDATE_PERIOD(emulator_period)
+ CPUTUNE_VALIDATE_PERIOD(iothread_period)
+
+ CPUTUNE_VALIDATE_QUOTA(quota)
+ CPUTUNE_VALIDATE_QUOTA(global_quota)
+ CPUTUNE_VALIDATE_QUOTA(emulator_quota)
+ CPUTUNE_VALIDATE_QUOTA(iothread_quota)
+
+ return 0;
+}
+
+#undef CPUTUNE_VALIDATE_PERIOD
+#undef CPUTUNE_VALIDATE_QUOTA
static int
virDomainDefMemtuneValidate(const virDomainDef *def)
@@ -6685,6 +6722,9 @@ virDomainDefValidateInternal(const virDomainDef *def,
if (virDomainDefOSValidate(def, xmlopt) < 0)
return -1;
+ if (virDomainDefCputuneValidate(def) < 0)
+ return -1;
+
return 0;
}
@@ -19673,14 +19713,6 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
}
- 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]"));
- goto error;
- }
-
if (virXPathLongLong("string(./cputune/quota[1])", ctxt,
&def->cputune.quota) < -1) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -19688,15 +19720,6 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
}
- if (def->cputune.quota > 0 &&
- (def->cputune.quota < 1000 ||
- def->cputune.quota > 18446744073709551LL)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Value of cputune quota must be in range "
- "[1000, 18446744073709551]"));
- goto error;
- }
-
if (virXPathULongLong("string(./cputune/global_period[1])", ctxt,
&def->cputune.global_period) < -1) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -19704,14 +19727,6 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
}
- if (def->cputune.global_period > 0 &&
- (def->cputune.global_period < 1000 || def->cputune.global_period > 1000000)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Value of cputune global period must be in range "
- "[1000, 1000000]"));
- goto error;
- }
-
if (virXPathLongLong("string(./cputune/global_quota[1])", ctxt,
&def->cputune.global_quota) < -1) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -19719,15 +19734,6 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
}
- if (def->cputune.global_quota > 0 &&
- (def->cputune.global_quota < 1000 ||
- def->cputune.global_quota > 18446744073709551LL)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Value of cputune global quota must be in range "
- "[1000, 18446744073709551]"));
- goto error;
- }
-
if (virXPathULongLong("string(./cputune/emulator_period[1])", ctxt,
&def->cputune.emulator_period) < -1) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -19735,15 +19741,6 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
}
- 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]"));
- goto error;
- }
-
if (virXPathLongLong("string(./cputune/emulator_quota[1])", ctxt,
&def->cputune.emulator_quota) < -1) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -19751,14 +19748,6 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
}
- if (def->cputune.emulator_quota > 0 &&
- (def->cputune.emulator_quota < 1000 ||
- def->cputune.emulator_quota > 18446744073709551LL)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Value of cputune emulator_quota must be in range "
- "[1000, 18446744073709551]"));
- goto error;
- }
if (virXPathULongLong("string(./cputune/iothread_period[1])", ctxt,
&def->cputune.iothread_period) < -1) {
@@ -19767,15 +19756,6 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
}
- if (def->cputune.iothread_period > 0 &&
- (def->cputune.iothread_period < 1000 ||
- def->cputune.iothread_period > 1000000)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Value of cputune iothread_period must be in range "
- "[1000, 1000000]"));
- goto error;
- }
-
if (virXPathLongLong("string(./cputune/iothread_quota[1])", ctxt,
&def->cputune.iothread_quota) < -1) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -19783,15 +19763,6 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
}
- if (def->cputune.iothread_quota > 0 &&
- (def->cputune.iothread_quota < 1000 ||
- def->cputune.iothread_quota > 18446744073709551LL)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Value of cputune iothread_quota must be in range "
- "[1000, 18446744073709551]"));
- goto error;
- }
-
if ((n = virXPathNodeSet("./cputune/vcpupin", ctxt, &nodes)) < 0)
goto error;
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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> -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.