[libvirt PATCH 00/10] Refactor more XML parsing boilerplate code, part II
For background, see https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html Tim Wiederhake (10): virDomainVirtioOptionsParseXML: Use virXMLProp* virDomainDeviceBootParseXML: Use virXMLProp* virDomainDeviceISAAddressParseXML: Use virXMLProp* virDomainDiskSourceNVMeParse: Use virXMLProp* virDomainChrSourceDefParseFile: Use virXMLProp* virDomainChrSourceDefParseLog: Use virXMLProp* virDomainAudioJackParse: Use virXMLProp* virDomainVideoResolutionDefParseXML: Use virXMLProp* virDomainVsockDefParseXML: Use virXMLProp* virDomainLoaderDefParseXML: Use virXMLProp* src/conf/domain_conf.c | 246 +++++++++-------------------------------- 1 file changed, 52 insertions(+), 194 deletions(-) -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 41 +++++++++-------------------------------- 1 file changed, 9 insertions(+), 32 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9a0d1f9285..5c13d9946c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1602,45 +1602,22 @@ static int virDomainVirtioOptionsParseXML(xmlNodePtr driver, virDomainVirtioOptions **virtio) { - int val; - virDomainVirtioOptions *res; - g_autofree char *str = NULL; - if (*virtio || !driver) return 0; *virtio = g_new0(virDomainVirtioOptions, 1); - res = *virtio; - - if ((str = virXMLPropString(driver, "iommu"))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("invalid iommu value")); - return -1; - } - res->iommu = val; - } - VIR_FREE(str); + if (virXMLPropTristateSwitch(driver, "iommu", VIR_XML_PROP_NONE, + &(*virtio)->iommu) < 0) + return -1; - if ((str = virXMLPropString(driver, "ats"))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("invalid ats value")); - return -1; - } - res->ats = val; - } - VIR_FREE(str); + if (virXMLPropTristateSwitch(driver, "ats", VIR_XML_PROP_NONE, + &(*virtio)->ats) < 0) + return -1; - if ((str = virXMLPropString(driver, "packed"))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("invalid packed value")); - return -1; - } - res->packed = val; - } + if (virXMLPropTristateSwitch(driver, "packed", VIR_XML_PROP_NONE, + &(*virtio)->packed) < 0) + return -1; return 0; } -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5c13d9946c..9113993bce 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6465,22 +6465,12 @@ static int virDomainDeviceBootParseXML(xmlNodePtr node, virDomainDeviceInfo *info) { - g_autofree char *order = NULL; g_autofree char *loadparm = NULL; - if (!(order = virXMLPropString(node, "order"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing boot order attribute")); - return -1; - } - - if (virStrToLong_uip(order, NULL, 10, &info->bootIndex) < 0 || - info->bootIndex == 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("incorrect boot order '%s', expecting positive integer"), - order); + if (virXMLPropUInt(node, "order", 10, + VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO, + &info->bootIndex) < 0) return -1; - } loadparm = virXMLPropString(node, "loadparm"); if (loadparm) { -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9113993bce..b28265cf4f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6494,27 +6494,14 @@ static int virDomainDeviceISAAddressParseXML(xmlNodePtr node, virDomainDeviceISAAddress *addr) { - g_autofree char *iobase = NULL; - g_autofree char *irq = NULL; - memset(addr, 0, sizeof(*addr)); - iobase = virXMLPropString(node, "iobase"); - irq = virXMLPropString(node, "irq"); - - if (iobase && - virStrToLong_uip(iobase, NULL, 16, &addr->iobase) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Cannot parse <address> 'iobase' attribute")); + if (virXMLPropUInt(node, "iobase", 16, VIR_XML_PROP_NONE, + &addr->iobase) < 0) return -1; - } - if (irq && - virStrToLong_uip(irq, NULL, 16, &addr->irq) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Cannot parse <address> 'irq' attribute")); + if (virXMLPropUInt(node, "irq", 16, VIR_XML_PROP_NONE, &addr->irq) < 0) return -1; - } return 0; } -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b28265cf4f..6ea347e05b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8453,7 +8453,6 @@ virDomainDiskSourceNVMeParse(xmlNodePtr node, g_autoptr(virStorageSourceNVMeDef) nvme = NULL; g_autofree char *type = NULL; g_autofree char *namespc = NULL; - g_autofree char *managed = NULL; xmlNodePtr address; nvme = g_new0(virStorageSourceNVMeDef, 1); @@ -8484,16 +8483,9 @@ virDomainDiskSourceNVMeParse(xmlNodePtr node, return -1; } - if ((managed = virXMLPropString(node, "managed"))) { - int value; - if ((value = virTristateBoolTypeFromString(managed)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("malformed managed value '%s'"), - managed); - return -1; - } - nvme->managed = value; - } + if (virXMLPropTristateBool(node, "managed", VIR_XML_PROP_NONE, + &nvme->managed) < 0) + return -1; if (!(address = virXPathNode("./address", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6ea347e05b..d10d697884 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11678,20 +11678,11 @@ static int virDomainChrSourceDefParseFile(virDomainChrSourceDef *def, xmlNodePtr source) { - g_autofree char *append = NULL; - def->data.file.path = virXMLPropString(source, "path"); - if ((append = virXMLPropString(source, "append"))) { - int value; - if ((value = virTristateSwitchTypeFromString(append)) <= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid append attribute value '%s'"), - append); - return -1; - } - def->data.file.append = value; - } + if (virXMLPropTristateSwitch(source, "append", VIR_XML_PROP_NONE, + &def->data.file.append) < 0) + return -1; return 0; } -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d10d697884..c97a062d1f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11713,20 +11713,11 @@ static int virDomainChrSourceDefParseLog(virDomainChrSourceDef *def, xmlNodePtr log) { - g_autofree char *append = NULL; - def->logfile = virXMLPropString(log, "file"); - if ((append = virXMLPropString(log, "append"))) { - int value; - if ((value = virTristateSwitchTypeFromString(append)) <= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid append attribute value '%s'"), - append); - return -1; - } - def->logappend = value; - } + if (virXMLPropTristateSwitch(log, "append", VIR_XML_PROP_NONE, + &def->logappend) < 0) + return -1; return 0; } -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c97a062d1f..ec7d48b40e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13806,19 +13806,13 @@ static int virDomainAudioJackParse(virDomainAudioIOJack *def, xmlNodePtr node) { - g_autofree char *exactName = virXMLPropString(node, "exactName"); - def->serverName = virXMLPropString(node, "serverName"); def->clientName = virXMLPropString(node, "clientName"); def->connectPorts = virXMLPropString(node, "connectPorts"); - if (exactName && - ((def->exactName = - virTristateBoolTypeFromString(exactName)) <= 0)) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown 'exactName' value '%s'"), exactName); + if (virXMLPropTristateBool(node, "exactName", VIR_XML_PROP_NONE, + &def->exactName) < 0) return -1; - } return 0; } -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ec7d48b40e..9e968c087d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14914,31 +14914,14 @@ static virDomainVideoResolutionDef * virDomainVideoResolutionDefParseXML(xmlNodePtr node) { g_autofree virDomainVideoResolutionDef *def = NULL; - g_autofree char *x = NULL; - g_autofree char *y = NULL; - - x = virXMLPropString(node, "x"); - y = virXMLPropString(node, "y"); - - if (!x || !y) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("missing values for resolution")); - return NULL; - } def = g_new0(virDomainVideoResolutionDef, 1); - if (virStrToLong_uip(x, NULL, 10, &def->x) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("cannot parse video x-resolution '%s'"), x); + if (virXMLPropUInt(node, "x", 10, VIR_XML_PROP_REQUIRED, &def->x) < 0) return NULL; - } - if (virStrToLong_uip(y, NULL, 10, &def->y) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("cannot parse video y-resolution '%s'"), y); + if (virXMLPropUInt(node, "y", 10, VIR_XML_PROP_REQUIRED, &def->y) < 0) return NULL; - } return g_steal_pointer(&def); } -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 48 ++++++++++++------------------------------ 1 file changed, 13 insertions(+), 35 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9e968c087d..1878d9ed9d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15855,8 +15855,6 @@ virDomainVsockDefParseXML(virDomainXMLOption *xmlopt, { VIR_XPATH_NODE_AUTORESTORE(ctxt) xmlNodePtr cid; - int val; - g_autofree char *tmp = NULL; g_autoptr(virDomainVsockDef) vsock = NULL; ctxt->node = node; @@ -15864,49 +15862,29 @@ virDomainVsockDefParseXML(virDomainXMLOption *xmlopt, if (!(vsock = virDomainVsockDefNew(xmlopt))) return NULL; - if ((tmp = virXMLPropString(node, "model"))) { - if ((val = virDomainVsockModelTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_XML_ERROR, _("unknown vsock model: %s"), tmp); - return NULL; - } - vsock->model = val; - } - - cid = virXPathNode("./cid", ctxt); + if (virXMLPropEnum(node, "model", virDomainVsockModelTypeFromString, + VIR_XML_PROP_NONE, &vsock->model) < 0) + return NULL; - VIR_FREE(tmp); - if (cid) { - if ((tmp = virXMLPropString(cid, "address"))) { - if (virStrToLong_uip(tmp, NULL, 10, &vsock->guest_cid) < 0 || - vsock->guest_cid == 0) { - virReportError(VIR_ERR_XML_DETAIL, - _("'cid' attribute must be a positive number: %s"), - tmp); - return NULL; - } - } + if ((cid = virXPathNode("./cid", ctxt))) { + if (virXMLPropUInt(cid, "address", 10, + VIR_XML_PROP_NONE | VIR_XML_PROP_NONZERO, + &vsock->guest_cid) < 0) + return NULL; - VIR_FREE(tmp); - if ((tmp = virXMLPropString(cid, "auto"))) { - val = virTristateBoolTypeFromString(tmp); - if (val <= 0) { - virReportError(VIR_ERR_XML_DETAIL, - _("'auto' attribute can be 'yes' or 'no': %s"), - tmp); - return NULL; - } - vsock->auto_cid = val; - } + if (virXMLPropTristateBool(cid, "auto", VIR_XML_PROP_NONE, + &vsock->auto_cid) < 0) + return NULL; } - if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, &vsock->info, flags) < 0) + if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, &vsock->info, + flags) < 0) return NULL; if (virDomainVirtioOptionsParseXML(virXPathNode("./driver", ctxt), &vsock->virtio) < 0) return NULL; - return g_steal_pointer(&vsock); } -- 2.26.3
On Wed, Apr 21, 2021 at 15:33:31 +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 48 ++++++++++++------------------------------ 1 file changed, 13 insertions(+), 35 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9e968c087d..1878d9ed9d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15855,8 +15855,6 @@ virDomainVsockDefParseXML(virDomainXMLOption *xmlopt, { VIR_XPATH_NODE_AUTORESTORE(ctxt) xmlNodePtr cid; - int val; - g_autofree char *tmp = NULL; g_autoptr(virDomainVsockDef) vsock = NULL;
ctxt->node = node; @@ -15864,49 +15862,29 @@ virDomainVsockDefParseXML(virDomainXMLOption *xmlopt, if (!(vsock = virDomainVsockDefNew(xmlopt))) return NULL;
- if ((tmp = virXMLPropString(node, "model"))) { - if ((val = virDomainVsockModelTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_XML_ERROR, _("unknown vsock model: %s"), tmp); - return NULL; - } - vsock->model = val; - } - - cid = virXPathNode("./cid", ctxt); + if (virXMLPropEnum(node, "model", virDomainVsockModelTypeFromString, + VIR_XML_PROP_NONE, &vsock->model) < 0) + return NULL;
- VIR_FREE(tmp); - if (cid) { - if ((tmp = virXMLPropString(cid, "address"))) { - if (virStrToLong_uip(tmp, NULL, 10, &vsock->guest_cid) < 0 || - vsock->guest_cid == 0) { - virReportError(VIR_ERR_XML_DETAIL, - _("'cid' attribute must be a positive number: %s"), - tmp); - return NULL; - } - } + if ((cid = virXPathNode("./cid", ctxt))) { + if (virXMLPropUInt(cid, "address", 10, + VIR_XML_PROP_NONE | VIR_XML_PROP_NONZERO,
'or'-ing something to VIR_XML_PROP_NONE doesn't make sense.
+ &vsock->guest_cid) < 0) + return NULL;
- VIR_FREE(tmp); - if ((tmp = virXMLPropString(cid, "auto"))) { - val = virTristateBoolTypeFromString(tmp); - if (val <= 0) { - virReportError(VIR_ERR_XML_DETAIL, - _("'auto' attribute can be 'yes' or 'no': %s"), - tmp); - return NULL; - } - vsock->auto_cid = val; - } + if (virXMLPropTristateBool(cid, "auto", VIR_XML_PROP_NONE, + &vsock->auto_cid) < 0) + return NULL; }
- if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, &vsock->info, flags) < 0) + if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, &vsock->info, + flags) < 0)
This is an unrelated change.
return NULL;
I'll fix both before pushing.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 47 ++++++++++-------------------------------- 1 file changed, 11 insertions(+), 36 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1878d9ed9d..4c88a124bc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18837,15 +18837,16 @@ virDomainLoaderDefParseXML(xmlNodePtr node, virDomainLoaderDef *loader, bool fwAutoSelect) { - g_autofree char *readonly_str = NULL; - g_autofree char *secure_str = NULL; - g_autofree char *type_str = NULL; + if (!fwAutoSelect) { + if (virXMLPropTristateBool(node, "readonly", VIR_XML_PROP_NONE, + &loader->readonly) < 0) + return -1; - secure_str = virXMLPropString(node, "secure"); + if (virXMLPropEnum(node, "type", virDomainLoaderTypeFromString, + VIR_XML_PROP_NONE | VIR_XML_PROP_NONZERO, + &loader->type) < 0) + return -1; - if (!fwAutoSelect) { - readonly_str = virXMLPropString(node, "readonly"); - type_str = virXMLPropString(node, "type"); if (!(loader->path = virXMLNodeContentString(node))) return -1; @@ -18853,35 +18854,9 @@ virDomainLoaderDefParseXML(xmlNodePtr node, VIR_FREE(loader->path); } - if (readonly_str) { - int value; - if ((value = virTristateBoolTypeFromString(readonly_str)) <= 0) { - virReportError(VIR_ERR_XML_DETAIL, - _("unknown readonly value: %s"), readonly_str); - return -1; - } - loader->readonly = value; - } - - if (secure_str) { - int value; - if ((value = virTristateBoolTypeFromString(secure_str)) <= 0) { - virReportError(VIR_ERR_XML_DETAIL, - _("unknown secure value: %s"), secure_str); - return -1; - } - loader->secure = value; - } - - if (type_str) { - int type; - if ((type = virDomainLoaderTypeFromString(type_str)) <= 0) { - virReportError(VIR_ERR_XML_DETAIL, - _("unknown type value: %s"), type_str); - return -1; - } - loader->type = type; - } + if (virXMLPropTristateBool(node, "secure", VIR_XML_PROP_NONE, + &loader->secure) < 0) + return -1; return 0; } -- 2.26.3
On Wed, Apr 21, 2021 at 15:33:22 +0200, Tim Wiederhake wrote:
For background, see https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html
Tim Wiederhake (10): virDomainVirtioOptionsParseXML: Use virXMLProp* virDomainDeviceBootParseXML: Use virXMLProp* virDomainDeviceISAAddressParseXML: Use virXMLProp* virDomainDiskSourceNVMeParse: Use virXMLProp* virDomainChrSourceDefParseFile: Use virXMLProp* virDomainChrSourceDefParseLog: Use virXMLProp* virDomainAudioJackParse: Use virXMLProp* virDomainVideoResolutionDefParseXML: Use virXMLProp* virDomainVsockDefParseXML: Use virXMLProp* virDomainLoaderDefParseXML: Use virXMLProp*
Reviewed-by: Peter Krempa <pkrempa@redhat.com> and will be pushed shortly.
participants (2)
-
Peter Krempa -
Tim Wiederhake