On 12/8/20 11:20 PM, Daniel Henrique Barboza wrote:
This patch creates a new function, virDomainDefBootValidate(), to host the validation of boot menu timeout and rebootTimeout outside of parse time. The checks in virDomainDefParseBootXML() were changed to throw VIR_ERR_XML_ERROR in case of parse error of those values.
In an attempt to alleviate the amount of code being stacked inside domain_conf.c, let's put this new function in a new domain_validate.c file that will be used to place these validations.
Suggested-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- po/POTFILES.in | 1 + src/conf/domain_conf.c | 20 +++++++-------- src/conf/domain_validate.c | 51 ++++++++++++++++++++++++++++++++++++++ src/conf/domain_validate.h | 27 ++++++++++++++++++++ src/conf/meson.build | 1 + tests/qemuxml2argvtest.c | 3 ++- 6 files changed, 92 insertions(+), 11 deletions(-) create mode 100644 src/conf/domain_validate.c create mode 100644 src/conf/domain_validate.h
diff --git a/po/POTFILES.in b/po/POTFILES.in index 9782b2beb4..14636d4b93 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -26,6 +26,7 @@ @SRCDIR@src/conf/domain_capabilities.c @SRCDIR@src/conf/domain_conf.c @SRCDIR@src/conf/domain_event.c +@SRCDIR@src/conf/domain_validate.c @SRCDIR@src/conf/interface_conf.c @SRCDIR@src/conf/netdev_bandwidth_conf.c @SRCDIR@src/conf/netdev_vlan_conf.c diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 66ee658e7b..4a45c76cf1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -33,6 +33,7 @@ #include "datatypes.h" #include "domain_addr.h" #include "domain_conf.h" +#include "domain_validate.h" #include "snapshot_conf.h" #include "viralloc.h" #include "virxml.h" @@ -7344,6 +7345,9 @@ virDomainDefValidateInternal(const virDomainDef *def, if (virDomainDefCputuneValidate(def) < 0) return -1;
+ if (virDomainDefBootValidate(def) < 0) + return -1; + if (virDomainNumaDefValidate(def->numa) < 0) return -1;
@@ -18867,11 +18871,9 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt,
tmp = virXMLPropString(node, "timeout"); if (tmp && def->os.bootmenu == VIR_TRISTATE_BOOL_YES) { - if (virStrToLong_uip(tmp, NULL, 0, &def->os.bm_timeout) < 0 || - def->os.bm_timeout > 65535) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("invalid value for boot menu timeout, " - "must be in range [0,65535]")); + if (virStrToLong_uip(tmp, NULL, 0, &def->os.bm_timeout) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid value for boot menu timeout")); return -1; } def->os.bm_timeout_set = true; @@ -18892,11 +18894,9 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, if (tmp) { /* that was really just for the check if it is there */
- if (virStrToLong_i(tmp, NULL, 0, &def->os.bios.rt_delay) < 0 || - def->os.bios.rt_delay < -1 || def->os.bios.rt_delay > 65535) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("invalid value for rebootTimeout, " - "must be in range [-1,65535]")); + if (virStrToLong_i(tmp, NULL, 0, &def->os.bios.rt_delay) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid value for rebootTimeout")); return -1; } def->os.bios.rt_set = true; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c new file mode 100644 index 0000000000..e4d947c553 --- /dev/null +++ b/src/conf/domain_validate.c @@ -0,0 +1,51 @@ +/* + * domain_validate.c: domain general validation functions + * + * Copyright IBM Corp, 2020
Honestly, I have only vague idea how these Copyright lines work, but shouldn't they also include (at least subset) of the lines from the original file? I mean, my common sense tells me that if I have a file written by person X, and later the file is split into two the person X is still the original author. Extending that, if a company holds a copyright on a file then moving bits out to a different file should keep the copyright. But I admit that law has completely different model of "common sense". And also there is a disconnection between files and these Copyright lines. If a copyright holder Y changed a tiny bit that is not moved - should their Copyright line also appear in the new file? Michal