[libvirt PATCH 00/10] Refactor more XML parsing boilerplate code, part VIII
For background, see https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html Note that patch #1 depends on https://listman.redhat.com/archives/libvir-list/2021-April/msg01260.html from part VII. Tim Wiederhake (10): virDomainRedirFilterUSBDevDefParseXML: Use g_auto* virDomainPerfEventDefParseXML: Use virXMLProp* virDomainMemoryDefParseXML: Use virXMLProp* virDomainVcpuPinDefParseXML: Use virXMLProp* virDomainIOThreadPinDefParseXML: Use virXMLProp* virDomainSchedulerParseCommonAttrs: Use virXMLProp* virDomainDef: Change type of placement_mode to virDomainCpuPlacementMode virDomainVcpuParse: Use virXMLProp* virDomainCachetuneDefParseCache: Use virXMLProp* virDomainResctrlMonDefParse: Use virXMLProp* src/conf/domain_conf.c | 300 +++++++++-------------------------------- src/conf/domain_conf.h | 2 +- 2 files changed, 63 insertions(+), 239 deletions(-) -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0050b952f3..5822f3d85a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14689,7 +14689,7 @@ virDomainRedirFilterUSBVersionHelper(const char *version, static virDomainRedirFilterUSBDevDef * virDomainRedirFilterUSBDevDefParseXML(xmlNodePtr node) { - virDomainRedirFilterUSBDevDef *def; + g_autofree virDomainRedirFilterUSBDevDef *def = NULL; g_autofree char *version = NULL; virTristateBool allow; @@ -14697,42 +14697,38 @@ virDomainRedirFilterUSBDevDefParseXML(xmlNodePtr node) def->usbClass = -1; if (virXMLPropInt(node, "class", 0, VIR_XML_PROP_NONE, &def->usbClass) < 0) - goto error; + return NULL; if (def->usbClass != -1 && def->usbClass &~ 0xFF) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid USB Class code 0x%x"), def->usbClass); - goto error; + return NULL; } def->vendor = -1; if (virXMLPropInt(node, "vendor", 0, VIR_XML_PROP_NONE, &def->vendor) < 0) - goto error; + return NULL; def->product = -1; if (virXMLPropInt(node, "product", 0, VIR_XML_PROP_NONE, &def->product) < 0) - goto error; + return NULL; version = virXMLPropString(node, "version"); if (version) { if (STREQ(version, "-1")) def->version = -1; else if ((virDomainRedirFilterUSBVersionHelper(version, def)) < 0) - goto error; + return NULL; } else { def->version = -1; } if (virXMLPropTristateBool(node, "allow", VIR_XML_PROP_REQUIRED, &allow) < 0) - goto error; + return NULL; def->allow = allow == VIR_TRISTATE_BOOL_YES; - return def; - - error: - VIR_FREE(def); - return NULL; + return g_steal_pointer(&def); } static virDomainRedirFilterDef * -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 33 +++++++++------------------------ 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5822f3d85a..8930dc33ce 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14813,39 +14813,24 @@ static int virDomainPerfEventDefParseXML(virDomainPerfDef *perf, xmlNodePtr node) { - int event; - g_autofree char *name = NULL; - g_autofree char *enabled = NULL; - - if (!(name = virXMLPropString(node, "name"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", _("missing perf event name")); - return -1; - } + virPerfEventType name; + virTristateBool enabled; - if ((event = virPerfEventTypeFromString(name)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("'unsupported perf event '%s'"), name); + if (virXMLPropEnum(node, "name", virPerfEventTypeFromString, + VIR_XML_PROP_REQUIRED, &name) < 0) return -1; - } - if (perf->events[event] != VIR_TRISTATE_BOOL_ABSENT) { - virReportError(VIR_ERR_XML_ERROR, - _("perf event '%s' was already specified"), name); + if (virXMLPropTristateBool(node, "enabled", VIR_XML_PROP_REQUIRED, &enabled) < 0) return -1; - } - if (!(enabled = virXMLPropString(node, "enabled"))) { + if (perf->events[name] != VIR_TRISTATE_BOOL_ABSENT) { virReportError(VIR_ERR_XML_ERROR, - _("missing state of perf event '%s'"), name); + _("perf event '%s' was already specified"), + virPerfEventTypeToString(name)); return -1; } - if ((perf->events[event] = virTristateBoolTypeFromString(enabled)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid state '%s' of perf event '%s'"), - enabled, name); - return -1; - } + perf->events[name] = enabled; return 0; } -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 41 ++++++++--------------------------------- 1 file changed, 8 insertions(+), 33 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8930dc33ce..b8ac399f5c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15040,48 +15040,24 @@ virDomainMemoryDefParseXML(virDomainXMLOption *xmlopt, VIR_XPATH_NODE_AUTORESTORE(ctxt) xmlNodePtr node; virDomainMemoryDef *def; - int val; g_autofree char *tmp = NULL; def = g_new0(virDomainMemoryDef, 1); ctxt->node = memdevNode; - if (!(tmp = virXMLPropString(memdevNode, "model"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing memory model")); + if (virXMLPropEnum(memdevNode, "model", virDomainMemoryModelTypeFromString, + VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO, + &def->model) < 0) goto error; - } - if ((val = virDomainMemoryModelTypeFromString(tmp)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid memory model '%s'"), tmp); + if (virXMLPropEnum(memdevNode, "access", virDomainMemoryAccessTypeFromString, + VIR_XML_PROP_NONZERO, &def->access) < 0) goto error; - } - VIR_FREE(tmp); - def->model = val; - - if ((tmp = virXMLPropString(memdevNode, "access"))) { - if ((val = virDomainMemoryAccessTypeFromString(tmp)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid access mode '%s'"), tmp); - goto error; - } - def->access = val; - } - VIR_FREE(tmp); - - if ((tmp = virXMLPropString(memdevNode, "discard"))) { - if ((val = virTristateBoolTypeFromString(tmp)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid discard value '%s'"), tmp); - goto error; - } - - def->discard = val; - } - VIR_FREE(tmp); + if (virXMLPropTristateBool(memdevNode, "discard", VIR_XML_PROP_NONE, + &def->discard) < 0) + goto error; /* Extract NVDIMM UUID. */ if (def->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && @@ -15094,7 +15070,6 @@ virDomainMemoryDefParseXML(virDomainXMLOption *xmlopt, goto error; } } - VIR_FREE(tmp); /* source */ if ((node = virXPathNode("./source", ctxt)) && -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b8ac399f5c..e95be17989 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17273,17 +17273,8 @@ virDomainVcpuPinDefParseXML(virDomainDef *def, unsigned int vcpuid; g_autofree char *tmp = NULL; - if (!(tmp = virXMLPropString(node, "vcpu"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", _("missing vcpu id in vcpupin")); + if (virXMLPropUInt(node, "vcpu", 10, VIR_XML_PROP_REQUIRED, &vcpuid) < 0) return -1; - } - - if (virStrToLong_uip(tmp, NULL, 10, &vcpuid) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid setting for vcpu '%s'"), tmp); - return -1; - } - VIR_FREE(tmp); if (!(vcpu = virDomainDefGetVcpu(def, vcpuid))) { VIR_WARN("Ignoring vcpupin for missing vcpus"); -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e95be17989..d7cef00246 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17319,24 +17319,10 @@ virDomainIOThreadPinDefParseXML(xmlNodePtr node, g_autofree char *tmp = NULL; g_autoptr(virBitmap) cpumask = NULL; - if (!(tmp = virXMLPropString(node, "iothread"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing iothread id in iothreadpin")); - return -1; - } - - if (virStrToLong_uip(tmp, NULL, 10, &iothreadid) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid setting for iothread '%s'"), tmp); - return -1; - } - VIR_FREE(tmp); - - if (iothreadid == 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("zero is an invalid iothread id value")); + if (virXMLPropUInt(node, "iothread", 10, + VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO, + &iothreadid) < 0) return -1; - } if (!(iothrid = virDomainIOThreadIDFind(def, iothreadid))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 34 ++++++---------------------------- 1 file changed, 6 insertions(+), 28 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d7cef00246..8f4fd0e3bc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18054,37 +18054,15 @@ virDomainSchedulerParseCommonAttrs(xmlNodePtr node, virProcessSchedPolicy *policy, int *priority) { - int pol = 0; - g_autofree char *tmp = NULL; - - if (!(tmp = virXMLPropString(node, "scheduler"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing scheduler attribute")); - return -1; - } - - if ((pol = virProcessSchedPolicyTypeFromString(tmp)) <= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid scheduler attribute: '%s'"), tmp); + if (virXMLPropEnum(node, "scheduler", virProcessSchedPolicyTypeFromString, + VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO, + policy) < 0) return -1; - } - *policy = pol; - VIR_FREE(tmp); - - if (pol == VIR_PROC_POLICY_FIFO || - pol == VIR_PROC_POLICY_RR) { - if (!(tmp = virXMLPropString(node, "priority"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Missing scheduler priority")); - return -1; - } - - if (virStrToLong_i(tmp, NULL, 10, priority) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Invalid value for element priority")); + if (*policy == VIR_PROC_POLICY_FIFO || *policy == VIR_PROC_POLICY_RR) { + if (virXMLPropInt(node, "priority", 10, VIR_XML_PROP_REQUIRED, + priority) < 0) return -1; - } } return 0; -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 4 +++- src/conf/domain_conf.h | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8f4fd0e3bc..20cf987176 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18241,13 +18241,15 @@ virDomainVcpuParse(virDomainDef *def, tmp = virXMLPropString(vcpuNode, "placement"); if (tmp) { - if ((def->placement_mode = + int placement_mode; + if ((placement_mode = virDomainCpuPlacementModeTypeFromString(tmp)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported CPU placement mode '%s'"), tmp); return -1; } + def->placement_mode = placement_mode; VIR_FREE(tmp); } else { def->placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8133d19fca..1022f5b114 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2716,7 +2716,7 @@ struct _virDomainDef { size_t maxvcpus; /* set if the vcpu definition was specified individually */ bool individualvcpus; - int placement_mode; + virDomainCpuPlacementMode placement_mode; virBitmap *cpumask; size_t niothreadids; -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 84 ++++++++++-------------------------------- 1 file changed, 20 insertions(+), 64 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 20cf987176..52d12bfb43 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18228,32 +18228,16 @@ virDomainVcpuParse(virDomainDef *def, } VIR_FREE(tmp); - if ((tmp = virXMLPropString(vcpuNode, "current"))) { - if (virStrToLong_ui(tmp, NULL, 10, &vcpus) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("current vcpus count must be an integer")); - return -1; - } - VIR_FREE(tmp); - } else { - vcpus = maxvcpus; - } + vcpus = maxvcpus; - tmp = virXMLPropString(vcpuNode, "placement"); - if (tmp) { - int placement_mode; - if ((placement_mode = - virDomainCpuPlacementModeTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unsupported CPU placement mode '%s'"), - tmp); - return -1; - } - def->placement_mode = placement_mode; - VIR_FREE(tmp); - } else { - def->placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC; - } + if (virXMLPropUInt(vcpuNode, "current", 10, VIR_XML_PROP_NONE, &vcpus) < 0) + return -1; + + def->placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC; + if (virXMLPropEnum(vcpuNode, "placement", + virDomainCpuPlacementModeTypeFromString, + VIR_XML_PROP_NONE, &def->placement_mode) < 0) + return -1; if (def->placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) { tmp = virXMLPropString(vcpuNode, "cpuset"); @@ -18284,18 +18268,11 @@ virDomainVcpuParse(virDomainDef *def, for (i = 0; i < n; i++) { virDomainVcpuDef *vcpu; - int state; + virTristateBool state; unsigned int id; - unsigned int order; - if (!(tmp = virXMLPropString(nodes[i], "id")) || - virStrToLong_uip(tmp, NULL, 10, &id) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing or invalid vcpu id")); + if (virXMLPropUInt(nodes[i], "id", 10, VIR_XML_PROP_REQUIRED, &id) < 0) return -1; - } - - VIR_FREE(tmp); if (id >= def->maxvcpus) { virReportError(VIR_ERR_XML_ERROR, @@ -18306,41 +18283,20 @@ virDomainVcpuParse(virDomainDef *def, vcpu = virDomainDefGetVcpu(def, id); - if (!(tmp = virXMLPropString(nodes[i], "enabled"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing vcpu enabled state")); - return -1; - } - - if ((state = virTristateBoolTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid vcpu 'enabled' value '%s'"), tmp); + if (virXMLPropTristateBool(nodes[i], "enabled", + VIR_XML_PROP_REQUIRED, &state) < 0) return -1; - } - VIR_FREE(tmp); vcpu->online = state == VIR_TRISTATE_BOOL_YES; - if ((tmp = virXMLPropString(nodes[i], "hotpluggable"))) { - int hotpluggable; - if ((hotpluggable = virTristateBoolTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid vcpu 'hotpluggable' value '%s'"), tmp); - return -1; - } - vcpu->hotpluggable = hotpluggable; - VIR_FREE(tmp); - } + if (virXMLPropTristateBool(nodes[i], "hotpluggable", + VIR_XML_PROP_NONE, + &vcpu->hotpluggable) < 0) + return -1; - if ((tmp = virXMLPropString(nodes[i], "order"))) { - if (virStrToLong_uip(tmp, NULL, 10, &order) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("invalid vcpu order")); - return -1; - } - vcpu->order = order; - VIR_FREE(tmp); - } + if (virXMLPropUInt(nodes[i], "order", 10, VIR_XML_PROP_NONE, + &vcpu->order) < 0) + return -1; } } else { if (virDomainDefSetVcpus(def, vcpus) < 0) -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 43 +++++------------------------------------- 1 file changed, 5 insertions(+), 38 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 52d12bfb43..483df91880 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18625,53 +18625,20 @@ virDomainCachetuneDefParseCache(xmlXPathContextPtr ctxt, VIR_XPATH_NODE_AUTORESTORE(ctxt) unsigned int level; unsigned int cache; - int type; + virCacheType type; unsigned long long size; - g_autofree char *tmp = NULL; ctxt->node = node; - tmp = virXMLPropString(node, "id"); - if (!tmp) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Missing cachetune attribute 'id'")); + if (virXMLPropUInt(node, "id", 10, VIR_XML_PROP_REQUIRED, &cache) < 0) return -1; - } - if (virStrToLong_uip(tmp, NULL, 10, &cache) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid cachetune attribute 'id' value '%s'"), - tmp); - return -1; - } - VIR_FREE(tmp); - tmp = virXMLPropString(node, "level"); - if (!tmp) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Missing cachetune attribute 'level'")); + if (virXMLPropUInt(node, "level", 10, VIR_XML_PROP_REQUIRED, &level) < 0) return -1; - } - if (virStrToLong_uip(tmp, NULL, 10, &level) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid cachetune attribute 'level' value '%s'"), - tmp); - return -1; - } - VIR_FREE(tmp); - tmp = virXMLPropString(node, "type"); - if (!tmp) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Missing cachetune attribute 'type'")); - return -1; - } - type = virCacheTypeFromString(tmp); - if (type < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid cachetune attribute 'type' value '%s'"), - tmp); + if (virXMLPropEnum(node, "type", virCacheTypeFromString, + VIR_XML_PROP_REQUIRED, &type) < 0) return -1; - } if (virParseScaledValue("./@size", "./@unit", ctxt, &size, 1024, -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 483df91880..b3ef2db3fa 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18764,19 +18764,9 @@ virDomainResctrlMonDefParse(virDomainDef *def, } if (tag == VIR_RESCTRL_MONITOR_TYPE_CACHE) { - tmp = virXMLPropString(nodes[i], "level"); - if (!tmp) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Missing monitor attribute 'level'")); + if (virXMLPropUInt(nodes[i], "level", 10, VIR_XML_PROP_REQUIRED, + &level) < 0) goto cleanup; - } - - if (virStrToLong_uip(tmp, NULL, 10, &level) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid monitor attribute 'level' value '%s'"), - tmp); - goto cleanup; - } if (level != VIR_DOMAIN_RESCTRL_MONITOR_CACHELEVEL) { virReportError(VIR_ERR_XML_ERROR, @@ -18784,8 +18774,6 @@ virDomainResctrlMonDefParse(virDomainDef *def, level); goto cleanup; } - - VIR_FREE(tmp); } if (virDomainResctrlParseVcpus(def, nodes[i], &domresmon->vcpus) < 0) -- 2.26.3
On 5/4/21 4:02 PM, Tim Wiederhake wrote:
For background, see https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html
Note that patch #1 depends on https://listman.redhat.com/archives/libvir-list/2021-April/msg01260.html from part VII.
Tim Wiederhake (10): virDomainRedirFilterUSBDevDefParseXML: Use g_auto* virDomainPerfEventDefParseXML: Use virXMLProp* virDomainMemoryDefParseXML: Use virXMLProp* virDomainVcpuPinDefParseXML: Use virXMLProp* virDomainIOThreadPinDefParseXML: Use virXMLProp* virDomainSchedulerParseCommonAttrs: Use virXMLProp* virDomainDef: Change type of placement_mode to virDomainCpuPlacementMode virDomainVcpuParse: Use virXMLProp* virDomainCachetuneDefParseCache: Use virXMLProp* virDomainResctrlMonDefParse: Use virXMLProp*
src/conf/domain_conf.c | 300 +++++++++-------------------------------- src/conf/domain_conf.h | 2 +- 2 files changed, 63 insertions(+), 239 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and pushed. Michal
participants (2)
-
Michal Prívozník -
Tim Wiederhake