[libvirt PATCH 00/10] Refactor more XML parsing boilerplate code, part V
For background, see https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html Tim Wiederhake (10): virXMLPropEnum: Fix return value virDomainControllerDef: Change type of ioeventfd to virTristateSwitch virDomainPCIControllerOpts: Change type of modelName to virDomainControllerPCIModelName virDomainControllerDefParseXML: Use virXMLProp* virDomainActualNetDef: Change type of type to virDomainNetType virDomainActualNetDefParseXML: Use virXMLProp* virDomainNetDefParseXML: Use virXMLProp* virDomainGraphicsListenDefParseXML: Use virXMLProp* virDomainGraphicsDef: Change type of sharePolicy to virDomainGraphicsVNCSharePolicy virDomainGraphicsDefParseXMLVNC: Use virXMLProp* src/conf/domain_conf.c | 643 +++++++++++---------------------- src/conf/domain_conf.h | 8 +- src/qemu/qemu_domain_address.c | 2 +- src/util/virxml.c | 2 +- 4 files changed, 227 insertions(+), 428 deletions(-) -- 2.26.3
Function incorrectly returns 0 when property was successfully read. Fixes: ab5d2776c925ec45eb54ec5432f5645cebb80c85 Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virxml.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index b79050db35..01c0114072 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -856,7 +856,7 @@ virXMLPropEnum(xmlNodePtr node, } *result = ret; - return 0; + return 1; } /** -- 2.26.3
On a Friday in 2021, Tim Wiederhake wrote:
Function incorrectly returns 0 when property was successfully read.
Fixes: ab5d2776c925ec45eb54ec5432f5645cebb80c85 Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virxml.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 13 ++++++++----- src/conf/domain_conf.h | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 24c0943d62..4ed7c0ef83 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9619,11 +9619,14 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, return NULL; } - if (ioeventfd && - (def->ioeventfd = virTristateSwitchTypeFromString(ioeventfd)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Malformed 'ioeventfd' value %s"), ioeventfd); - return NULL; + if (ioeventfd) { + int value; + if ((value = virTristateSwitchTypeFromString(ioeventfd)) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Malformed 'ioeventfd' value %s"), ioeventfd); + return NULL; + } + def->ioeventfd = value; } if (iothread) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a7cad31896..d4344a7158 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -763,7 +763,7 @@ struct _virDomainControllerDef { unsigned int queues; unsigned int cmd_per_lun; unsigned int max_sectors; - int ioeventfd; /* enum virTristateSwitch */ + virTristateSwitch ioeventfd; unsigned int iothread; /* unused = 0, > 0 specific thread # */ union { virDomainVirtioSerialOpts vioserial; -- 2.26.3
On a Friday in 2021, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 13 ++++++++----- src/conf/domain_conf.h | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 16 +++++++++------- src/conf/domain_conf.h | 2 +- src/qemu/qemu_domain_address.c | 2 +- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4ed7c0ef83..33e79b20e6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9722,13 +9722,15 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, /* Other controller models don't require extra checks */ break; } - if (modelName && - (def->opts.pciopts.modelName - = virDomainControllerPCIModelNameTypeFromString(modelName)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown PCI controller model name '%s'"), - modelName); - return NULL; + if (modelName) { + int value; + if ((value = virDomainControllerPCIModelNameTypeFromString(modelName)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown PCI controller model name '%s'"), + modelName); + return NULL; + } + def->opts.pciopts.modelName = value; } if (chassisNr) { if (virStrToLong_i(chassisNr, NULL, 0, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d4344a7158..336b76aa5c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -724,7 +724,7 @@ struct _virDomainPCIControllerOpts { * <model name='ioh3420''/> * ... */ - int modelName; /* the exact name of the device in hypervisor */ + virDomainControllerPCIModelName modelName; /* the following items are attributes of the "target" subelement * of controller type='pci'. They are bits of configuration that diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 1ee75b8f2e..a73f30ddcb 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2456,7 +2456,7 @@ qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDef *cont, virDomainDef *def, virQEMUCaps *qemuCaps) { - int *modelName = &cont->opts.pciopts.modelName; + virDomainControllerPCIModelName *modelName = &cont->opts.pciopts.modelName; /* make sure it's not already set */ if (*modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) -- 2.26.3
On a Friday in 2021, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 16 +++++++++------- src/conf/domain_conf.h | 2 +- src/qemu/qemu_domain_address.c | 2 +- 3 files changed, 11 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 281 +++++++++++++++-------------------------- 1 file changed, 103 insertions(+), 178 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 33e79b20e6..152b4b8813 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9491,7 +9491,7 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, unsigned int flags) { g_autoptr(virDomainControllerDef) def = NULL; - int type = 0; + virDomainControllerType type = 0; xmlNodePtr cur = NULL; bool processedModel = false; bool processedTarget = false; @@ -9499,39 +9499,19 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, int ports = -1; VIR_XPATH_NODE_AUTORESTORE(ctxt) int rc; - g_autofree char *typeStr = NULL; g_autofree char *idx = NULL; g_autofree char *model = NULL; - g_autofree char *queues = NULL; - g_autofree char *cmd_per_lun = NULL; - g_autofree char *max_sectors = NULL; - g_autofree char *modelName = NULL; - g_autofree char *chassisNr = NULL; - g_autofree char *chassis = NULL; - g_autofree char *port = NULL; - g_autofree char *busNr = NULL; - g_autofree char *targetIndex = NULL; - g_autofree char *hotplug = NULL; - g_autofree char *ioeventfd = NULL; - g_autofree char *portsStr = NULL; - g_autofree char *iothread = NULL; ctxt->node = node; - typeStr = virXMLPropString(node, "type"); - if (typeStr) { - if ((type = virDomainControllerTypeFromString(typeStr)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown controller type '%s'"), typeStr); - return NULL; - } - } + if (virXMLPropEnum(node, "type", virDomainControllerTypeFromString, + VIR_XML_PROP_NONE, &type) < 0) + return NULL; if (!(def = virDomainControllerDefNew(type))) return NULL; - model = virXMLPropString(node, "model"); - if (model) { + if ((model = virXMLPropString(node, "model"))) { if ((def->model = virDomainControllerModelTypeFromString(def, model)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unknown model type '%s'"), model); @@ -9542,8 +9522,7 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, idx = virXMLPropString(node, "index"); if (idx) { unsigned int idxVal; - if (virStrToLong_ui(idx, NULL, 10, &idxVal) < 0 || - idxVal > INT_MAX) { + if (virStrToLong_ui(idx, NULL, 10, &idxVal) < 0 || idxVal > INT_MAX) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot parse controller index %s"), idx); return NULL; @@ -9555,11 +9534,26 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE) { if (virXMLNodeNameEqual(cur, "driver")) { - queues = virXMLPropString(cur, "queues"); - cmd_per_lun = virXMLPropString(cur, "cmd_per_lun"); - max_sectors = virXMLPropString(cur, "max_sectors"); - ioeventfd = virXMLPropString(cur, "ioeventfd"); - iothread = virXMLPropString(cur, "iothread"); + if (virXMLPropUInt(cur, "queues", 10, VIR_XML_PROP_NONE, + &def->queues) < 0) + return NULL; + + if (virXMLPropUInt(cur, "cmd_per_lun", 10, VIR_XML_PROP_NONE, + &def->cmd_per_lun) < 0) + return NULL; + + if (virXMLPropUInt(cur, "max_sectors", 10, VIR_XML_PROP_NONE, + &def->max_sectors) < 0) + return NULL; + + if (virXMLPropTristateSwitch(cur, "ioeventfd", + VIR_XML_PROP_NONE, + &def->ioeventfd) < 0) + return NULL; + + if (virXMLPropUInt(cur, "iothread", 10, VIR_XML_PROP_NONE, + &def->iothread) < 0) + return NULL; if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0) return NULL; @@ -9570,7 +9564,15 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, "controller definition not allowed")); return NULL; } - modelName = virXMLPropString(cur, "name"); + + if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { + if (virXMLPropEnum(cur, "name", + virDomainControllerPCIModelNameTypeFromString, + VIR_XML_PROP_NONE, + &def->opts.pciopts.modelName) < 0) + return NULL; + } + processedModel = true; } else if (virXMLNodeNameEqual(cur, "target")) { if (processedTarget) { @@ -9579,12 +9581,39 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, "controller definition not allowed")); return NULL; } - chassisNr = virXMLPropString(cur, "chassisNr"); - chassis = virXMLPropString(cur, "chassis"); - port = virXMLPropString(cur, "port"); - busNr = virXMLPropString(cur, "busNr"); - hotplug = virXMLPropString(cur, "hotplug"); - targetIndex = virXMLPropString(cur, "index"); + if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { + if (virXMLPropInt(cur, "chassisNr", 0, VIR_XML_PROP_NONE, + &def->opts.pciopts.chassisNr) < 0) + return NULL; + + if (virXMLPropInt(cur, "chassis", 0, VIR_XML_PROP_NONE, + &def->opts.pciopts.chassis) < 0) + return NULL; + + if (virXMLPropInt(cur, "port", 0, VIR_XML_PROP_NONE, + &def->opts.pciopts.port) < 0) + return NULL; + + if (virXMLPropInt(cur, "busNr", 0, VIR_XML_PROP_NONE, + &def->opts.pciopts.busNr) < 0) + return NULL; + + if (virXMLPropTristateSwitch(cur, "hotplug", + VIR_XML_PROP_NONE, + &def->opts.pciopts.hotplug) < 0) + return NULL; + + if ((rc = virXMLPropInt(cur, "index", 0, VIR_XML_PROP_NONE, + &def->opts.pciopts.targetIndex)) < 0) + return NULL; + + if (rc && def->opts.pciopts.targetIndex == -1) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid target index '%i' in PCI controller"), + def->opts.pciopts.targetIndex); + } + } + processedTarget = true; } } @@ -9601,42 +9630,6 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, return NULL; } - if (queues && virStrToLong_ui(queues, NULL, 10, &def->queues) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Malformed 'queues' value '%s'"), queues); - return NULL; - } - - if (cmd_per_lun && virStrToLong_ui(cmd_per_lun, NULL, 10, &def->cmd_per_lun) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Malformed 'cmd_per_lun' value '%s'"), cmd_per_lun); - return NULL; - } - - if (max_sectors && virStrToLong_ui(max_sectors, NULL, 10, &def->max_sectors) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Malformed 'max_sectors' value %s"), max_sectors); - return NULL; - } - - if (ioeventfd) { - int value; - if ((value = virTristateSwitchTypeFromString(ioeventfd)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Malformed 'ioeventfd' value %s"), ioeventfd); - return NULL; - } - def->ioeventfd = value; - } - - if (iothread) { - if (virStrToLong_uip(iothread, NULL, 10, &def->iothread) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid 'iothread' value '%s'"), iothread); - return NULL; - } - } - if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && def->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) { VIR_DEBUG("Ignoring device address for none model usb controller"); @@ -9645,30 +9638,28 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, return NULL; } - portsStr = virXMLPropString(node, "ports"); - if (portsStr) { - int r = virStrToLong_i(portsStr, NULL, 10, &ports); - if (r != 0 || ports < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid ports: %s"), portsStr); - return NULL; - } + if ((rc = virXMLPropInt(node, "ports", 10, VIR_XML_PROP_NONE, &ports)) < 0) + return NULL; + if (rc && ports < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid ports: %i"), ports); + return NULL; } switch (def->type) { case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: { - g_autofree char *vectors = virXMLPropString(node, "vectors"); + if ((rc = virXMLPropInt(node, "vectors", 10, VIR_XML_PROP_NONE, + &def->opts.vioserial.vectors)) < 0) + return NULL; - def->opts.vioserial.ports = ports; - if (vectors) { - int r = virStrToLong_i(vectors, NULL, 10, - &def->opts.vioserial.vectors); - if (r != 0 || def->opts.vioserial.vectors < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid vectors: %s"), vectors); - return NULL; - } + if (rc && def->opts.vioserial.vectors < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid vectors: %i"), + def->opts.vioserial.vectors); + return NULL; } + + def->opts.vioserial.ports = ports; break; } case VIR_DOMAIN_CONTROLLER_TYPE_USB: { @@ -9722,98 +9713,32 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, /* Other controller models don't require extra checks */ break; } - if (modelName) { - int value; - if ((value = virDomainControllerPCIModelNameTypeFromString(modelName)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown PCI controller model name '%s'"), - modelName); - return NULL; - } - def->opts.pciopts.modelName = value; - } - if (chassisNr) { - if (virStrToLong_i(chassisNr, NULL, 0, - &def->opts.pciopts.chassisNr) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid chassisNr '%s' in PCI controller"), - chassisNr); - return NULL; - } - } - if (chassis) { - if (virStrToLong_i(chassis, NULL, 0, - &def->opts.pciopts.chassis) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid chassis '%s' in PCI controller"), - chassis); - return NULL; - } - } - if (port) { - if (virStrToLong_i(port, NULL, 0, - &def->opts.pciopts.port) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid port '%s' in PCI controller"), - port); - return NULL; - } - } - if (busNr) { - if (virStrToLong_i(busNr, NULL, 0, - &def->opts.pciopts.busNr) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid busNr '%s' in PCI controller"), - busNr); - return NULL; - } - } - if (targetIndex) { - if (virStrToLong_i(targetIndex, NULL, 0, - &def->opts.pciopts.targetIndex) < 0 || - def->opts.pciopts.targetIndex == -1) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid target index '%s' in PCI controller"), - targetIndex); - return NULL; - } - } + if (numaNode >= 0) def->opts.pciopts.numaNode = numaNode; - if (hotplug) { - int val = virTristateSwitchTypeFromString(hotplug); - - if (val <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("PCI controller unrecognized hotplug setting '%s'"), - hotplug); - return NULL; - } - def->opts.pciopts.hotplug = val; - } break; case VIR_DOMAIN_CONTROLLER_TYPE_XENBUS: { - g_autofree char *gntframes = virXMLPropString(node, "maxGrantFrames"); - g_autofree char *eventchannels = virXMLPropString(node, "maxEventChannels"); + if ((rc = virXMLPropInt(node, "maxGrantFrames", 10, VIR_XML_PROP_NONE, + &def->opts.xenbusopts.maxGrantFrames)) < 0) + return NULL; - if (gntframes) { - int r = virStrToLong_i(gntframes, NULL, 10, - &def->opts.xenbusopts.maxGrantFrames); - if (r != 0 || def->opts.xenbusopts.maxGrantFrames < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid maxGrantFrames: %s"), gntframes); - return NULL; - } + if (rc && def->opts.xenbusopts.maxGrantFrames < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid maxGrantFrames: %i"), + def->opts.xenbusopts.maxGrantFrames); + return NULL; } - if (eventchannels) { - int r = virStrToLong_i(eventchannels, NULL, 10, - &def->opts.xenbusopts.maxEventChannels); - if (r != 0 || def->opts.xenbusopts.maxEventChannels < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid maxEventChannels: %s"), eventchannels); - return NULL; - } + + if ((rc = virXMLPropInt(node, "maxEventChannels", 10, VIR_XML_PROP_NONE, + &def->opts.xenbusopts.maxEventChannels)) < 0) + return NULL; + + if (rc && def->opts.xenbusopts.maxEventChannels < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid maxEventChannels: %i"), + def->opts.xenbusopts.maxEventChannels); + return NULL; } break; } -- 2.26.3
On a Friday in 2021, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 281 +++++++++++++++-------------------------- 1 file changed, 103 insertions(+), 178 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 33e79b20e6..152b4b8813 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9499,39 +9499,19 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt,
[...]
if (!(def = virDomainControllerDefNew(type))) return NULL;
- model = virXMLPropString(node, "model"); - if (model) { + if ((model = virXMLPropString(node, "model"))) { if ((def->model = virDomainControllerModelTypeFromString(def, model)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unknown model type '%s'"), model); @@ -9542,8 +9522,7 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, idx = virXMLPropString(node, "index"); if (idx) { unsigned int idxVal; - if (virStrToLong_ui(idx, NULL, 10, &idxVal) < 0 || - idxVal > INT_MAX) { + if (virStrToLong_ui(idx, NULL, 10, &idxVal) < 0 || idxVal > INT_MAX) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot parse controller index %s"), idx); return NULL;
The two hunks above only reformat code. Please don't mix functional and cosmetic changes in one commit.
@@ -9579,12 +9581,39 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, "controller definition not allowed")); return NULL; } - chassisNr = virXMLPropString(cur, "chassisNr"); - chassis = virXMLPropString(cur, "chassis"); - port = virXMLPropString(cur, "port"); - busNr = virXMLPropString(cur, "busNr"); - hotplug = virXMLPropString(cur, "hotplug"); - targetIndex = virXMLPropString(cur, "index"); + if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { + if (virXMLPropInt(cur, "chassisNr", 0, VIR_XML_PROP_NONE, + &def->opts.pciopts.chassisNr) < 0) + return NULL; + + if (virXMLPropInt(cur, "chassis", 0, VIR_XML_PROP_NONE, + &def->opts.pciopts.chassis) < 0) + return NULL; + + if (virXMLPropInt(cur, "port", 0, VIR_XML_PROP_NONE, + &def->opts.pciopts.port) < 0) + return NULL; + + if (virXMLPropInt(cur, "busNr", 0, VIR_XML_PROP_NONE, + &def->opts.pciopts.busNr) < 0) + return NULL; + + if (virXMLPropTristateSwitch(cur, "hotplug", + VIR_XML_PROP_NONE, + &def->opts.pciopts.hotplug) < 0) + return NULL; + + if ((rc = virXMLPropInt(cur, "index", 0, VIR_XML_PROP_NONE, + &def->opts.pciopts.targetIndex)) < 0) + return NULL; + + if (rc && def->opts.pciopts.targetIndex == -1) {
if (rc == 1 && ...) per our coding style: https://libvirt.org/coding-style.html#conditional-expressions
+ virReportError(VIR_ERR_XML_ERROR, + _("Invalid target index '%i' in PCI controller"), + def->opts.pciopts.targetIndex); + } + } + processedTarget = true; } } @@ -9645,30 +9638,28 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, return NULL; }
- portsStr = virXMLPropString(node, "ports"); - if (portsStr) { - int r = virStrToLong_i(portsStr, NULL, 10, &ports); - if (r != 0 || ports < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid ports: %s"), portsStr); - return NULL; - } + if ((rc = virXMLPropInt(node, "ports", 10, VIR_XML_PROP_NONE, &ports)) < 0) + return NULL; + if (rc && ports < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid ports: %i"), ports); + return NULL; }
Same here.
switch (def->type) { case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: { - g_autofree char *vectors = virXMLPropString(node, "vectors"); + if ((rc = virXMLPropInt(node, "vectors", 10, VIR_XML_PROP_NONE, + &def->opts.vioserial.vectors)) < 0) + return NULL;
- def->opts.vioserial.ports = ports; - if (vectors) { - int r = virStrToLong_i(vectors, NULL, 10, - &def->opts.vioserial.vectors); - if (r != 0 || def->opts.vioserial.vectors < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid vectors: %s"), vectors); - return NULL; - } + if (rc && def->opts.vioserial.vectors < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid vectors: %i"), + def->opts.vioserial.vectors); + return NULL;
And here.
} + + def->opts.vioserial.ports = ports; break; } case VIR_DOMAIN_CONTROLLER_TYPE_USB: {
[...]
case VIR_DOMAIN_CONTROLLER_TYPE_XENBUS: { - g_autofree char *gntframes = virXMLPropString(node, "maxGrantFrames"); - g_autofree char *eventchannels = virXMLPropString(node, "maxEventChannels"); + if ((rc = virXMLPropInt(node, "maxGrantFrames", 10, VIR_XML_PROP_NONE, + &def->opts.xenbusopts.maxGrantFrames)) < 0)
Indentation is off.
+ return NULL;
- if (gntframes) { - int r = virStrToLong_i(gntframes, NULL, 10, - &def->opts.xenbusopts.maxGrantFrames); - if (r != 0 || def->opts.xenbusopts.maxGrantFrames < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid maxGrantFrames: %s"), gntframes); - return NULL; - } + if (rc && def->opts.xenbusopts.maxGrantFrames < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid maxGrantFrames: %i"), + def->opts.xenbusopts.maxGrantFrames); + return NULL;
Here too.
} - if (eventchannels) { - int r = virStrToLong_i(eventchannels, NULL, 10, - &def->opts.xenbusopts.maxEventChannels); - if (r != 0 || def->opts.xenbusopts.maxEventChannels < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid maxEventChannels: %s"), eventchannels); - return NULL; - } + + if ((rc = virXMLPropInt(node, "maxEventChannels", 10, VIR_XML_PROP_NONE, + &def->opts.xenbusopts.maxEventChannels)) < 0) + return NULL; + + if (rc && def->opts.xenbusopts.maxEventChannels < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid maxEventChannels: %i"), + def->opts.xenbusopts.maxEventChannels);
And here.
+ return NULL; } break; }
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 30 ++++++++++++++++++++++++++---- src/conf/domain_conf.h | 2 +- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 152b4b8813..554efe0aea 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2468,7 +2468,16 @@ virDomainActualNetDefFree(virDomainActualNetDef *def) case VIR_DOMAIN_NET_TYPE_HOSTDEV: virDomainHostdevDefClear(&def->data.hostdev.def); break; - default: + case VIR_DOMAIN_NET_TYPE_USER: + case VIR_DOMAIN_NET_TYPE_ETHERNET: + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + case VIR_DOMAIN_NET_TYPE_SERVER: + case VIR_DOMAIN_NET_TYPE_CLIENT: + case VIR_DOMAIN_NET_TYPE_MCAST: + case VIR_DOMAIN_NET_TYPE_INTERNAL: + case VIR_DOMAIN_NET_TYPE_UDP: + case VIR_DOMAIN_NET_TYPE_VDPA: + case VIR_DOMAIN_NET_TYPE_LAST: break; } @@ -10062,6 +10071,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node, g_autofree char *addrtype = NULL; g_autofree char *trustGuestRxFilters = NULL; g_autofree char *macTableManager = NULL; + int type_value; actual = g_new0(virDomainActualNetDef, 1); @@ -10073,11 +10083,12 @@ virDomainActualNetDefParseXML(xmlNodePtr node, _("missing type attribute in interface's <actual> element")); goto error; } - if ((actual->type = virDomainNetTypeFromString(type)) < 0) { + if ((type_value = virDomainNetTypeFromString(type)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown type '%s' in interface's <actual> element"), type); goto error; } + actual->type = type_value; if (actual->type != VIR_DOMAIN_NET_TYPE_BRIDGE && actual->type != VIR_DOMAIN_NET_TYPE_DIRECT && actual->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && @@ -30114,9 +30125,20 @@ virDomainNetGetActualVirtPortProfile(const virDomainNetDef *iface) case VIR_DOMAIN_NET_TYPE_BRIDGE: case VIR_DOMAIN_NET_TYPE_HOSTDEV: return iface->data.network.actual->virtPortProfile; - default: - return NULL; + case VIR_DOMAIN_NET_TYPE_USER: + case VIR_DOMAIN_NET_TYPE_ETHERNET: + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + case VIR_DOMAIN_NET_TYPE_SERVER: + case VIR_DOMAIN_NET_TYPE_CLIENT: + case VIR_DOMAIN_NET_TYPE_MCAST: + case VIR_DOMAIN_NET_TYPE_NETWORK: + case VIR_DOMAIN_NET_TYPE_INTERNAL: + case VIR_DOMAIN_NET_TYPE_UDP: + case VIR_DOMAIN_NET_TYPE_VDPA: + case VIR_DOMAIN_NET_TYPE_LAST: + break; } + return NULL; case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_ETHERNET: case VIR_DOMAIN_NET_TYPE_VHOSTUSER: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 336b76aa5c..cb21ee7872 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -990,7 +990,7 @@ typedef enum { * different versions of libvirt may read the same data file. */ struct _virDomainActualNetDef { - int type; /* enum virDomainNetType */ + virDomainNetType type; union { struct { char *brname; -- 2.26.3
On a Friday in 2021, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 30 ++++++++++++++++++++++++++---- src/conf/domain_conf.h | 2 +- 2 files changed, 27 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 57 ++++++++++++++---------------------------- 1 file changed, 19 insertions(+), 38 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 554efe0aea..3051a5d56d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10066,49 +10066,30 @@ virDomainActualNetDefParseXML(xmlNodePtr node, xmlNodePtr bandwidth_node = NULL; xmlNodePtr vlanNode; xmlNodePtr virtPortNode; - g_autofree char *type = NULL; - g_autofree char *mode = NULL; g_autofree char *addrtype = NULL; - g_autofree char *trustGuestRxFilters = NULL; g_autofree char *macTableManager = NULL; - int type_value; actual = g_new0(virDomainActualNetDef, 1); ctxt->node = node; - type = virXMLPropString(node, "type"); - if (!type) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing type attribute in interface's <actual> element")); + if (virXMLPropEnum(node, "type", virDomainNetTypeFromString, + VIR_XML_PROP_REQUIRED, &actual->type) < 0) goto error; - } - if ((type_value = virDomainNetTypeFromString(type)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown type '%s' in interface's <actual> element"), type); - goto error; - } - actual->type = type_value; + if (actual->type != VIR_DOMAIN_NET_TYPE_BRIDGE && actual->type != VIR_DOMAIN_NET_TYPE_DIRECT && actual->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && actual->type != VIR_DOMAIN_NET_TYPE_NETWORK) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported type '%s' in interface's <actual> element"), - type); + virDomainNetTypeToString(actual->type)); goto error; } - if ((trustGuestRxFilters = virXMLPropString(node, "trustGuestRxFilters"))) { - int value; - if ((value = virTristateBoolTypeFromString(trustGuestRxFilters)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown trustGuestRxFilters value '%s'"), - trustGuestRxFilters); - goto error; - } - actual->trustGuestRxFilters = value; - } + if (virXMLPropTristateBool(node, "trustGuestRxFilters", VIR_XML_PROP_NONE, + &actual->trustGuestRxFilters) < 0) + goto error; virtPortNode = virXPathNode("./virtualport", ctxt); if (virtPortNode) { @@ -10127,7 +10108,8 @@ virDomainActualNetDefParseXML(xmlNodePtr node, } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("<virtualport> element unsupported for type='%s'" - " in interface's <actual> element"), type); + " in interface's <actual> element"), + virDomainNetTypeToString(actual->type)); goto error; } } @@ -10136,19 +10118,18 @@ virDomainActualNetDefParseXML(xmlNodePtr node, xmlNodePtr sourceNode = virXPathNode("./source[1]", ctxt); if (sourceNode) { + int rc; + virNetDevMacVLanMode mode; + actual->data.direct.linkdev = virXMLPropString(sourceNode, "dev"); - mode = virXMLPropString(sourceNode, "mode"); - if (mode) { - int m; - if ((m = virNetDevMacVLanModeTypeFromString(mode)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown mode '%s' in interface <actual> element"), - mode); - goto error; - } - actual->data.direct.mode = m; - } + if ((rc = virXMLPropEnum(sourceNode, "mode", + virNetDevMacVLanModeTypeFromString, + VIR_XML_PROP_NONE, &mode)) < 0) + goto error; + + if (rc) + actual->data.direct.mode = mode; } } else if (actual->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) { virDomainHostdevDef *hostdev = &actual->data.hostdev.def; -- 2.26.3
On a Friday in 2021, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 57 ++++++++++++++---------------------------- 1 file changed, 19 insertions(+), 38 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 554efe0aea..3051a5d56d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10066,49 +10066,30 @@ virDomainActualNetDefParseXML(xmlNodePtr node, xmlNodePtr bandwidth_node = NULL; xmlNodePtr vlanNode; xmlNodePtr virtPortNode; - g_autofree char *type = NULL; - g_autofree char *mode = NULL; g_autofree char *addrtype = NULL; - g_autofree char *trustGuestRxFilters = NULL; g_autofree char *macTableManager = NULL; - int type_value;
actual = g_new0(virDomainActualNetDef, 1);
ctxt->node = node;
- type = virXMLPropString(node, "type"); - if (!type) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing type attribute in interface's <actual> element")); + if (virXMLPropEnum(node, "type", virDomainNetTypeFromString, + VIR_XML_PROP_REQUIRED, &actual->type) < 0) goto error; - } - if ((type_value = virDomainNetTypeFromString(type)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown type '%s' in interface's <actual> element"), type); - goto error; - } - actual->type = type_value; + if (actual->type != VIR_DOMAIN_NET_TYPE_BRIDGE && actual->type != VIR_DOMAIN_NET_TYPE_DIRECT && actual->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && actual->type != VIR_DOMAIN_NET_TYPE_NETWORK) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported type '%s' in interface's <actual> element"), - type); + virDomainNetTypeToString(actual->type)); goto error; }
- if ((trustGuestRxFilters = virXMLPropString(node, "trustGuestRxFilters"))) { - int value; - if ((value = virTristateBoolTypeFromString(trustGuestRxFilters)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown trustGuestRxFilters value '%s'"), - trustGuestRxFilters); - goto error; - } - actual->trustGuestRxFilters = value; - } + if (virXMLPropTristateBool(node, "trustGuestRxFilters", VIR_XML_PROP_NONE, + &actual->trustGuestRxFilters) < 0) + goto error;
virtPortNode = virXPathNode("./virtualport", ctxt); if (virtPortNode) { @@ -10127,7 +10108,8 @@ virDomainActualNetDefParseXML(xmlNodePtr node, } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("<virtualport> element unsupported for type='%s'" - " in interface's <actual> element"), type); + " in interface's <actual> element"), + virDomainNetTypeToString(actual->type)); goto error; } } @@ -10136,19 +10118,18 @@ virDomainActualNetDefParseXML(xmlNodePtr node, xmlNodePtr sourceNode = virXPathNode("./source[1]", ctxt);
if (sourceNode) { + int rc; + virNetDevMacVLanMode mode; + actual->data.direct.linkdev = virXMLPropString(sourceNode, "dev");
- mode = virXMLPropString(sourceNode, "mode"); - if (mode) { - int m; - if ((m = virNetDevMacVLanModeTypeFromString(mode)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown mode '%s' in interface <actual> element"), - mode); - goto error; - } - actual->data.direct.mode = m; - } + if ((rc = virXMLPropEnum(sourceNode, "mode", + virNetDevMacVLanModeTypeFromString, + VIR_XML_PROP_NONE, &mode)) < 0)
Indentation is off.
+ goto error; + + if (rc) + actual->data.direct.mode = mode;
You can just pass actual->data.direct.mode as an argument to the virXMLPropEnum call, eliminating the need for this if and both of the local variables, unless I'm missing something.
} } else if (actual->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) { virDomainHostdevDef *hostdev = &actual->data.hostdev.def;
With that addressed: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On Fri, 2021-04-23 at 15:35 +0200, Ján Tomko wrote:
On a Friday in 2021, Tim Wiederhake wrote:
(...)
+ goto error; + + if (rc) + actual->data.direct.mode = mode;
You can just pass actual->data.direct.mode as an argument to the virXMLPropEnum call, eliminating the need for this if and both of the local variables, unless I'm missing something.
acutal->data.direct.mode is an int, so we have to do the extra step.
} } else if (actual->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) { virDomainHostdevDef *hostdev = &actual->data.hostdev.def;
With that addressed: Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 199 ++++++++++++----------------------------- 1 file changed, 57 insertions(+), 142 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3051a5d56d..2103aae2a3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10329,7 +10329,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *macaddr = NULL; g_autofree char *macaddr_type = NULL; g_autofree char *macaddr_check = NULL; - g_autofree char *type = NULL; g_autofree char *network = NULL; g_autofree char *portgroup = NULL; g_autofree char *portid = NULL; @@ -10353,7 +10352,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *queues = NULL; g_autofree char *rx_queue_size = NULL; g_autofree char *tx_queue_size = NULL; - g_autofree char *str = NULL; g_autofree char *filter = NULL; g_autofree char *internal = NULL; g_autofree char *mode = NULL; @@ -10363,7 +10361,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *vhostuser_mode = NULL; g_autofree char *vhostuser_path = NULL; g_autofree char *vhostuser_type = NULL; - g_autofree char *trustGuestRxFilters = NULL; g_autofree char *vhost_path = NULL; const char *prefix = xmlopt ? xmlopt->config.netPrefix : NULL; @@ -10372,27 +10369,16 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, ctxt->node = node; - type = virXMLPropString(node, "type"); - if (type != NULL) { - if ((int)(def->type = virDomainNetTypeFromString(type)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown interface type '%s'"), type); - goto error; - } - } else { + if ((rv = virXMLPropEnum(node, "type", virDomainNetTypeFromString, + VIR_XML_PROP_NONE, &def->type)) < 0) + goto error; + + if (rv == 0) def->type = VIR_DOMAIN_NET_TYPE_USER; - } - if ((trustGuestRxFilters = virXMLPropString(node, "trustGuestRxFilters"))) { - int value; - if ((value = virTristateBoolTypeFromString(trustGuestRxFilters)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown trustGuestRxFilters value '%s'"), - trustGuestRxFilters); - goto error; - } - def->trustGuestRxFilters = value; - } + if (virXMLPropTristateBool(node, "trustGuestRxFilters", VIR_XML_PROP_NONE, + &def->trustGuestRxFilters) < 0) + goto error; cur = node->children; while (cur != NULL) { @@ -10484,7 +10470,8 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("<virtualport> element unsupported for" - " <interface type='%s'>"), type); + " <interface type='%s'>"), + virDomainNetTypeToString(def->type)); goto error; } } else if (!address && @@ -10962,128 +10949,56 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, } if ((tmpNode = virXPathNode("./driver/host", ctxt))) { - if ((str = virXMLPropString(tmpNode, "csum"))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown host csum mode '%s'"), - str); - goto error; - } - def->driver.virtio.host.csum = val; - } - VIR_FREE(str); - if ((str = virXMLPropString(tmpNode, "gso"))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown host gso mode '%s'"), - str); - goto error; - } - def->driver.virtio.host.gso = val; - } - VIR_FREE(str); - if ((str = virXMLPropString(tmpNode, "tso4"))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown host tso4 mode '%s'"), - str); - goto error; - } - def->driver.virtio.host.tso4 = val; - } - VIR_FREE(str); - if ((str = virXMLPropString(tmpNode, "tso6"))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown host tso6 mode '%s'"), - str); - goto error; - } - def->driver.virtio.host.tso6 = val; - } - VIR_FREE(str); - if ((str = virXMLPropString(tmpNode, "ecn"))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown host ecn mode '%s'"), - str); - goto error; - } - def->driver.virtio.host.ecn = val; - } - VIR_FREE(str); - if ((str = virXMLPropString(tmpNode, "ufo"))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown host ufo mode '%s'"), - str); - goto error; - } - def->driver.virtio.host.ufo = val; - } - VIR_FREE(str); - if ((str = virXMLPropString(tmpNode, "mrg_rxbuf"))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown host mrg_rxbuf mode '%s'"), - str); - goto error; - } - def->driver.virtio.host.mrg_rxbuf = val; - } - VIR_FREE(str); + if (virXMLPropTristateSwitch(tmpNode, "csum", VIR_XML_PROP_NONE, + &def->driver.virtio.host.csum) < 0) + goto error; + + if (virXMLPropTristateSwitch(tmpNode, "gso", VIR_XML_PROP_NONE, + &def->driver.virtio.host.gso) < 0) + goto error; + + if (virXMLPropTristateSwitch(tmpNode, "tso4", VIR_XML_PROP_NONE, + &def->driver.virtio.host.tso4) < 0) + goto error; + + if (virXMLPropTristateSwitch(tmpNode, "tso6", VIR_XML_PROP_NONE, + &def->driver.virtio.host.tso6) < 0) + goto error; + + if (virXMLPropTristateSwitch(tmpNode, "ecn", VIR_XML_PROP_NONE, + &def->driver.virtio.host.ecn) < 0) + goto error; + + if (virXMLPropTristateSwitch(tmpNode, "ufo", VIR_XML_PROP_NONE, + &def->driver.virtio.host.ufo) < 0) + goto error; + + if (virXMLPropTristateSwitch(tmpNode, "mrg_rxbuf", + VIR_XML_PROP_NONE, + &def->driver.virtio.host.mrg_rxbuf) < 0) + goto error; } if ((tmpNode = virXPathNode("./driver/guest", ctxt))) { - if ((str = virXMLPropString(tmpNode, "csum"))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown guest csum mode '%s'"), - str); - goto error; - } - def->driver.virtio.guest.csum = val; - } - VIR_FREE(str); - if ((str = virXMLPropString(tmpNode, "tso4"))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown guest tso4 mode '%s'"), - str); - goto error; - } - def->driver.virtio.guest.tso4 = val; - } - VIR_FREE(str); - if ((str = virXMLPropString(tmpNode, "tso6"))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown guest tso6 mode '%s'"), - str); - goto error; - } - def->driver.virtio.guest.tso6 = val; - } - VIR_FREE(str); - if ((str = virXMLPropString(tmpNode, "ecn"))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown guest ecn mode '%s'"), - str); - goto error; - } - def->driver.virtio.guest.ecn = val; - } - VIR_FREE(str); - if ((str = virXMLPropString(tmpNode, "ufo"))) { - if ((val = virTristateSwitchTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown guest ufo mode '%s'"), - str); - goto error; - } - def->driver.virtio.guest.ufo = val; - } + if (virXMLPropTristateSwitch(tmpNode, "csum", VIR_XML_PROP_NONE, + &def->driver.virtio.guest.csum) < 0) + goto error; + + if (virXMLPropTristateSwitch(tmpNode, "tso4", VIR_XML_PROP_NONE, + &def->driver.virtio.guest.tso4) < 0) + goto error; + + if (virXMLPropTristateSwitch(tmpNode, "tso6", VIR_XML_PROP_NONE, + &def->driver.virtio.guest.tso6) < 0) + goto error; + + if (virXMLPropTristateSwitch(tmpNode, "ecn", VIR_XML_PROP_NONE, + &def->driver.virtio.guest.ecn) < 0) + goto error; + + if (virXMLPropTristateSwitch(tmpNode, "ufo", VIR_XML_PROP_NONE, + &def->driver.virtio.guest.ufo) < 0) + goto error; } def->backend.vhost = g_steal_pointer(&vhost_path); } -- 2.26.3
On a Friday in 2021, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 199 ++++++++++++----------------------------- 1 file changed, 57 insertions(+), 142 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2103aae2a3..1acd692720 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12439,12 +12439,9 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDef *def, { int ret = -1; const char *graphicsType = virDomainGraphicsTypeToString(graphics->type); - int tmp, typeVal; - g_autofree char *type = virXMLPropString(node, "type"); g_autofree char *address = virXMLPropString(node, "address"); g_autofree char *network = virXMLPropString(node, "network"); g_autofree char *socketPath = virXMLPropString(node, "socket"); - g_autofree char *fromConfig = virXMLPropString(node, "fromConfig"); g_autofree char *autoGenerated = virXMLPropString(node, "autoGenerated"); g_autofree char *addressCompat = NULL; g_autofree char *socketCompat = NULL; @@ -12454,18 +12451,9 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDef *def, socketCompat = virXMLPropString(parent, "socket"); } - if (!type) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("graphics listen type must be specified")); - goto error; - } - - if ((typeVal = virDomainGraphicsListenTypeFromString(type)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown graphics listen type '%s'"), type); + if (virXMLPropEnum(node, "type", virDomainGraphicsListenTypeFromString, + VIR_XML_PROP_REQUIRED, &def->type) < 0) goto error; - } - def->type = typeVal; switch (def->type) { case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: @@ -12545,14 +12533,10 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDef *def, def->socket = g_steal_pointer(&socketPath); } - if (fromConfig && - flags & VIR_DOMAIN_DEF_PARSE_STATUS) { - if (virStrToLong_i(fromConfig, NULL, 10, &tmp) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid fromConfig value: %s"), - fromConfig); - goto error; - } + if (flags & VIR_DOMAIN_DEF_PARSE_STATUS) { + int tmp; + if (virXMLPropInt(node, "fromConfig", 10, VIR_XML_PROP_NONE, &tmp) < 0) + return -1; def->fromConfig = tmp != 0; } -- 2.26.3
On a Friday in 2021, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cb21ee7872..4838687edf 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1834,7 +1834,7 @@ struct _virDomainGraphicsDef { bool autoport; char *keymap; virDomainGraphicsAuthDef auth; - int sharePolicy; + virDomainGraphicsVNCSharePolicy sharePolicy; virTristateBool powerControl; unsigned int audioId; } vnc; -- 2.26.3
On a Friday in 2021, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 59 ++++++++++-------------------------------- 1 file changed, 13 insertions(+), 46 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1acd692720..b752ccd70b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12645,11 +12645,8 @@ virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDef *def, unsigned int flags) { g_autofree char *port = virXMLPropString(node, "port"); - g_autofree char *websocket = virXMLPropString(node, "websocket"); g_autofree char *websocketGenerated = virXMLPropString(node, "websocketGenerated"); - g_autofree char *sharePolicy = virXMLPropString(node, "sharePolicy"); g_autofree char *autoport = virXMLPropString(node, "autoport"); - g_autofree char *powerControl = virXMLPropString(node, "powerControl"); xmlNodePtr audioNode; VIR_XPATH_NODE_AUTORESTORE(ctxt) @@ -12680,62 +12677,32 @@ virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDef *def, def->data.vnc.port = 0; } - if (websocket) { - if (virStrToLong_i(websocket, - NULL, 10, - &def->data.vnc.websocket) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse vnc WebSocket port %s"), websocket); - return -1; - } - } + if (virXMLPropInt(node, "websocket", 10, VIR_XML_PROP_NONE, + &def->data.vnc.websocket) < 0) + return -1; if (websocketGenerated) ignore_value(virStringParseYesNo(websocketGenerated, &def->data.vnc.websocketGenerated)); - if (sharePolicy) { - int policy = - virDomainGraphicsVNCSharePolicyTypeFromString(sharePolicy); - - if (policy < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown vnc display sharing policy '%s'"), - sharePolicy); - return -1; - } else { - def->data.vnc.sharePolicy = policy; - } - } + if (virXMLPropEnum(node, "sharePolicy", + virDomainGraphicsVNCSharePolicyTypeFromString, + VIR_XML_PROP_NONE, &def->data.vnc.sharePolicy) < 0) + return -1; - if (powerControl) { - int powerControlVal = virTristateBoolTypeFromString(powerControl); - if (powerControlVal < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse vnc power control '%s'"), powerControl); - return -1; - } - def->data.vnc.powerControl = powerControlVal; - } + if ((virXMLPropTristateBool(node, "powerControl", VIR_XML_PROP_NONE, + &def->data.vnc.powerControl)) < 0) + return -1; def->data.vnc.keymap = virXMLPropString(node, "keymap"); ctxt->node = node; audioNode = virXPathNode("./audio", ctxt); if (audioNode) { - g_autofree char *tmp = NULL; - tmp = virXMLPropString(audioNode, "id"); - if (!tmp) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing audio 'id' attribute")); - return -1; - } - if (virStrToLong_ui(tmp, NULL, 10, &def->data.vnc.audioId) < 0 || - def->data.vnc.audioId == 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid audio 'id' value '%s'"), tmp); + if (virXMLPropUInt(audioNode, "id", 10, + VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO, + &def->data.vnc.audioId) < 0) return -1; - } } if (virDomainGraphicsAuthDefParseXML(node, &def->data.vnc.auth, -- 2.26.3
On a Friday in 2021, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 59 ++++++++++-------------------------------- 1 file changed, 13 insertions(+), 46 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On a Friday in 2021, Tim Wiederhake wrote:
For background, see https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html
Tim Wiederhake (10): virXMLPropEnum: Fix return value virDomainControllerDef: Change type of ioeventfd to virTristateSwitch virDomainPCIControllerOpts: Change type of modelName to virDomainControllerPCIModelName virDomainControllerDefParseXML: Use virXMLProp* virDomainActualNetDef: Change type of type to virDomainNetType virDomainActualNetDefParseXML: Use virXMLProp* virDomainNetDefParseXML: Use virXMLProp* virDomainGraphicsListenDefParseXML: Use virXMLProp* virDomainGraphicsDef: Change type of sharePolicy to virDomainGraphicsVNCSharePolicy virDomainGraphicsDefParseXMLVNC: Use virXMLProp*
src/conf/domain_conf.c | 643 +++++++++++---------------------- src/conf/domain_conf.h | 8 +- src/qemu/qemu_domain_address.c | 2 +- src/util/virxml.c | 2 +- 4 files changed, 227 insertions(+), 428 deletions(-)
I have pushed all but 4/10 and 6/10. Jano
participants (2)
-
Ján Tomko -
Tim Wiederhake