[PATCH 00/12] lib: Use virXMLProp*() more

*** BLURB HERE *** Michal Prívozník (12): virxml: Extend virXMLPropU{Int,LongLong}() error message virNetworkPortDefParseXML: Fix a typo in an error message qemuValidateDomainDeviceDefFS: Use correct enum for fs->multidevs comparison qemu: Use virTristateBool instead of virTristateSwitch in a few places lib: Eliminate use of virTristateSwitchTypeFromString() lib: Almost eliminate use of virTristateBoolTypeFromString() conf: Convert virCPUDefParseXML() to virXMLProp*() conf: Convert virDomainDefParseBootXML() to virXMLProp*() conf: Convert virDomainFSDefParseXML() to virXMLProp*() conf: Convert virDomainNetDefParseXML() to virXMLProp*() conf: Convert virNetworkPortDefParseXML() to virXMLProp*() conf: Convert virDomainHostdevDefParseXMLSubsys() to virXMLProp*() src/bhyve/bhyve_command.c | 2 +- src/conf/cpu_conf.c | 76 +-- src/conf/cpu_conf.h | 2 +- src/conf/domain_conf.c | 637 +++++++----------- src/conf/domain_conf.h | 63 +- src/conf/network_conf.c | 116 +--- src/conf/network_conf.h | 12 +- src/conf/storage_conf.c | 17 +- src/conf/storage_source_conf.c | 14 +- src/conf/storage_source_conf.h | 2 +- src/conf/virnetworkportdef.c | 51 +- src/conf/virnetworkportdef.h | 6 +- src/cpu/cpu_x86.c | 58 +- src/libxl/libxl_conf.c | 2 + src/libxl/xen_xl.c | 2 + src/libxl/xen_xm.c | 2 + src/lxc/lxc_container.c | 3 + src/qemu/qemu_capabilities.c | 28 +- src/qemu/qemu_command.c | 12 +- src/qemu/qemu_domain.c | 16 +- src/qemu/qemu_domain_address.c | 2 +- src/qemu/qemu_hostdev.c | 4 +- src/qemu/qemu_hotplug.c | 3 +- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_validate.c | 8 +- src/security/virt-aa-helper.c | 2 +- src/util/virxml.c | 4 +- src/vz/vz_sdk.c | 4 +- .../qemuxml2argvdata/vhost_queues-invalid.err | 2 +- 29 files changed, 426 insertions(+), 726 deletions(-) -- 2.34.1

In case virXMLPropUInt() or virXMLPropULongLong() meets an attribute with a negative integer the following error message is printed: Invalid value ...: Expected integer value This message is not as good as it could be. Let users know it's a non-negative integer we are expecting. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virxml.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index 4b09374107..bb1ae3e305 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -679,7 +679,7 @@ virXMLPropUInt(xmlNodePtr node, if (ret < 0) { virReportError(VIR_ERR_XML_ERROR, - _("Invalid value for attribute '%s' in element '%s': '%s'. Expected integer value"), + _("Invalid value for attribute '%s' in element '%s': '%s'. Expected non-negative integer value"), name, node->name, tmp); return -1; } @@ -738,7 +738,7 @@ virXMLPropULongLong(xmlNodePtr node, if (ret < 0) { virReportError(VIR_ERR_XML_ERROR, - _("Invalid value for attribute '%s' in element '%s': '%s'. Expected integer value"), + _("Invalid value for attribute '%s' in element '%s': '%s'. Expected non-negative integer value"), name, node->name, tmp); return -1; } -- 2.34.1

There's a typo in error message that's printed when parsing of <plug type=''/> fails: "prt" is reported instead of "port". Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/virnetworkportdef.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c index 1ca1eddb5a..c4f5f96392 100644 --- a/src/conf/virnetworkportdef.c +++ b/src/conf/virnetworkportdef.c @@ -186,7 +186,7 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt) if (plugtype && (def->plugtype = virNetworkPortPlugTypeFromString(plugtype)) < 0) { virReportError(VIR_ERR_XML_ERROR, - _("Invalid network prt plug type '%s'"), plugtype); + _("Invalid network port plug type '%s'"), plugtype); } switch (def->plugtype) { -- 2.34.1

During validation of a virDomainFSDef QEMU capabilities are check for multidevs support if the FS definition has it enabled. However, the fs->multidevs is really type of virDomainFSMultidevs but is compared against virDomainFSModel enum. Fortunately, both values are the same so no user visible harm done here. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_validate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index ae0ee4e744..24e0866f8c 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -4234,7 +4234,7 @@ qemuValidateDomainDeviceDefFS(virDomainFSDef *fs, _("only supports mount filesystem type")); return -1; } - if (fs->multidevs != VIR_DOMAIN_FS_MODEL_DEFAULT && + if (fs->multidevs != VIR_DOMAIN_FS_MULTIDEVS_DEFAULT && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV_MULTIDEVS)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("multidevs is not supported with this QEMU binary")); -- 2.34.1

Both @accel2d and @accel3d are parsed as virTristateBool, but in a few places (qemuDeviceVideoGetModel() and qemuValidateDomainDeviceDefVideo()) they are compared to virTristateSwitch enum either directly or via a variable of that type. Clear this confusion by using the correct enum. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 8 ++++---- src/qemu/qemu_validate.c | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 60b4f96e06..99ac44d7f1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -695,7 +695,7 @@ qemuDeviceVideoGetModel(virQEMUCaps *qemuCaps, { const char *model = NULL; bool primaryVga = false; - virTristateSwitch accel3d = VIR_TRISTATE_SWITCH_ABSENT; + virTristateBool accel3d = VIR_TRISTATE_BOOL_ABSENT; *virtio = false; *virtioBusSuffix = false; @@ -735,7 +735,7 @@ qemuDeviceVideoGetModel(virQEMUCaps *qemuCaps, break; case VIR_DOMAIN_VIDEO_TYPE_VIRTIO: if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_VGA_GL) && - accel3d == VIR_TRISTATE_SWITCH_ON) + accel3d == VIR_TRISTATE_BOOL_YES) model = "virtio-vga-gl"; else model = "virtio-vga"; @@ -765,7 +765,7 @@ qemuDeviceVideoGetModel(virQEMUCaps *qemuCaps, break; case VIR_DOMAIN_VIDEO_TYPE_VIRTIO: if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_GL_PCI) && - accel3d == VIR_TRISTATE_SWITCH_ON) + accel3d == VIR_TRISTATE_BOOL_YES) model = "virtio-gpu-gl"; else model = "virtio-gpu"; @@ -4816,7 +4816,7 @@ qemuBuildDeviceVideoCmd(virCommand *cmd, virQEMUCaps *qemuCaps) { const char *model = NULL; - virTristateSwitch virgl = VIR_TRISTATE_SWITCH_ABSENT; + virTristateBool virgl = VIR_TRISTATE_BOOL_ABSENT; bool virtio = false; bool virtioBusSuffix = false; g_autoptr(virJSONValue) props = NULL; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 24e0866f8c..7eaad3614e 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2489,7 +2489,7 @@ qemuValidateDomainDeviceDefVideo(const virDomainVideoDef *video, } } - if (video->accel && video->accel->accel2d == VIR_TRISTATE_SWITCH_ON) { + if (video->accel && video->accel->accel2d == VIR_TRISTATE_BOOL_YES) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("qemu does not support the accel2d setting")); return -1; @@ -2553,7 +2553,7 @@ qemuValidateDomainDeviceDefVideo(const virDomainVideoDef *video, return -1; } } else if (video->accel) { - if (video->accel->accel3d == VIR_TRISTATE_SWITCH_ON && + if (video->accel->accel3d == VIR_TRISTATE_BOOL_YES && (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO || !(virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_VIRGL) || virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_GL_PCI) || -- 2.34.1

There are couple of places (all of them in XML parsing) where virTristateSwitchTypeFromString() is called. Well, the same result can be achieved by virXMLPropTristateSwitch() and on fewer lines. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/cpu_conf.c | 32 +++++------- src/conf/domain_conf.c | 111 ++++++++++++++--------------------------- src/conf/domain_conf.h | 6 +-- src/cpu/cpu_x86.c | 35 ++++++------- 4 files changed, 66 insertions(+), 118 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index fbceac1657..4d61bfd01b 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -323,12 +323,12 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, xmlNodePtr topology = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) int n; + int rv; size_t i; g_autofree char *cpuMode = NULL; g_autofree char *fallback = NULL; g_autofree char *vendor_id = NULL; g_autofree char *tscScaling = NULL; - g_autofree char *migratable = NULL; g_autofree virHostCPUTscInfo *tsc = NULL; *cpu = NULL; @@ -394,25 +394,17 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, def->mode = VIR_CPU_MODE_CUSTOM; } - if ((migratable = virXMLPropString(ctxt->node, "migratable"))) { - int val; - - if (def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH && - def->mode != VIR_CPU_MODE_MAXIMUM) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Attribute migratable is only allowed for " - "'host-passthrough' / 'maximum' CPU mode")); - return -1; - } - - if ((val = virTristateSwitchTypeFromString(migratable)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Invalid value in migratable attribute: '%s'"), - migratable); - return -1; - } - - def->migratable = val; + if ((rv = virXMLPropTristateSwitch(ctxt->node, "migratable", + VIR_XML_PROP_NONE, + &def->migratable)) < 0) { + return -1; + } else if (rv > 0 && + def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH && + def->mode != VIR_CPU_MODE_MAXIMUM) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Attribute migratable is only allowed for " + "'host-passthrough' / 'maximum' CPU mode")); + return -1; } if (def->type == VIR_CPU_TYPE_GUEST) { diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 393f9d9478..83212f9c10 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7310,8 +7310,6 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, g_autofree char *rawio = NULL; g_autofree char *backendStr = NULL; g_autofree char *model = NULL; - g_autofree char *display = NULL; - g_autofree char *ramfb = NULL; /* @managed can be read from the xml document - it is always an * attribute of the toplevel element, no matter what type of @@ -7324,8 +7322,6 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, sgio = virXMLPropString(node, "sgio"); rawio = virXMLPropString(node, "rawio"); model = virXMLPropString(node, "model"); - display = virXMLPropString(node, "display"); - ramfb = virXMLPropString(node, "ramfb"); /* @type is passed in from the caller rather than read from the * xml document, because it is specified in different places for @@ -7426,23 +7422,15 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, return -1; } - if (display && - (mdevsrc->display = virTristateSwitchTypeFromString(display)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown value '%s' for <hostdev> attribute " - "'display'"), - display); + if (virXMLPropTristateSwitch(node, "display", + VIR_XML_PROP_NONE, + &mdevsrc->display) < 0) return -1; - } - if (ramfb && - (mdevsrc->ramfb = virTristateSwitchTypeFromString(ramfb)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown value '%s' for <hostdev> attribute " - "'ramfb'"), - ramfb); + if (virXMLPropTristateSwitch(node, "ramfb", + VIR_XML_PROP_NONE, + &mdevsrc->ramfb) < 0) return -1; - } } switch (def->source.subsys.type) { @@ -9924,11 +9912,10 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt, if (def->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) { g_autofree char *queue_size = virXPathString("string(./driver/@queue)", ctxt); g_autofree char *binary = virXPathString("string(./binary/@path)", ctxt); - g_autofree char *xattr = virXPathString("string(./binary/@xattr)", ctxt); g_autofree char *cache = virXPathString("string(./binary/cache/@mode)", ctxt); g_autofree char *sandbox = virXPathString("string(./binary/sandbox/@mode)", ctxt); - g_autofree char *posix_lock = virXPathString("string(./binary/lock/@posix)", ctxt); - g_autofree char *flock = virXPathString("string(./binary/lock/@flock)", ctxt); + xmlNodePtr binary_node = virXPathNode("./binary", ctxt); + xmlNodePtr binary_lock_node = virXPathNode("./binary/lock", ctxt); int val; if (queue_size && virStrToLong_ull(queue_size, NULL, 10, &def->queue_size) < 0) { @@ -9941,14 +9928,20 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt, if (binary) def->binary = virFileSanitizePath(binary); - if (xattr) { - if ((val = virTristateSwitchTypeFromString(xattr)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown xattr value '%s'"), xattr); - goto error; - } - def->xattr = val; - } + if (virXMLPropTristateSwitch(binary_node, "xattr", + VIR_XML_PROP_NONE, + &def->xattr) < 0) + goto error; + + if (virXMLPropTristateSwitch(binary_lock_node, "posix", + VIR_XML_PROP_NONE, + &def->posix_lock) < 0) + goto error; + + if (virXMLPropTristateSwitch(binary_lock_node, "flock", + VIR_XML_PROP_NONE, + &def->flock) < 0) + goto error; if (cache) { if ((val = virDomainFSCacheModeTypeFromString(cache)) <= 0) { @@ -9970,23 +9963,6 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt, def->sandbox = val; } - if (posix_lock) { - if ((val = virTristateSwitchTypeFromString(posix_lock)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown posix lock value '%s'"), posix_lock); - goto error; - } - def->posix_lock = val; - } - - if (flock) { - if ((val = virTristateSwitchTypeFromString(flock)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown flock value '%s'"), flock); - goto error; - } - def->flock = val; - } } if (source == NULL && def->type != VIR_DOMAIN_FS_TYPE_RAM @@ -10309,8 +10285,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *model = NULL; g_autofree char *backend = NULL; g_autofree char *txmode = NULL; - g_autofree char *ioeventfd = NULL; - g_autofree char *event_idx = NULL; g_autofree char *queues = NULL; g_autofree char *rx_queue_size = NULL; g_autofree char *tx_queue_size = NULL; @@ -10462,8 +10436,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, backend = virXMLPropString(driver_node, "name"); txmode = virXMLPropString(driver_node, "txmode"); - ioeventfd = virXMLPropString(driver_node, "ioeventfd"); - event_idx = virXMLPropString(driver_node, "event_idx"); queues = virXMLPropString(driver_node, "queues"); rx_queue_size = virXMLPropString(driver_node, "rx_queue_size"); tx_queue_size = virXMLPropString(driver_node, "tx_queue_size"); @@ -10828,24 +10800,17 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, } def->driver.virtio.txmode = val; } - if (ioeventfd) { - if ((val = virTristateSwitchTypeFromString(ioeventfd)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown interface ioeventfd mode '%s'"), - ioeventfd); - goto error; - } - def->driver.virtio.ioeventfd = val; - } - if (event_idx) { - if ((val = virTristateSwitchTypeFromString(event_idx)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown interface event_idx mode '%s'"), - event_idx); - goto error; - } - def->driver.virtio.event_idx = val; - } + + if (virXMLPropTristateSwitch(driver_node, "ioeventfd", + VIR_XML_PROP_NONE, + &def->driver.virtio.ioeventfd) < 0) + goto error; + + if (virXMLPropTristateSwitch(driver_node, "event_idx", + VIR_XML_PROP_NONE, + &def->driver.virtio.event_idx) < 0) + goto error; + if (queues) { unsigned int q; if (virStrToLong_uip(queues, NULL, 10, &q) < 0) { @@ -19112,13 +19077,11 @@ virDomainDefParseMemory(virDomainDef *def, } /* and info about it */ - if ((tmp = virXPathString("string(./memory[1]/@dumpCore)", ctxt)) && - (def->mem.dump_core = virTristateSwitchTypeFromString(tmp)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Invalid memory core dump attribute value '%s'"), tmp); + if ((node = virXPathNode("./memory[1]", ctxt)) && + virXMLPropTristateSwitch(node, "dumpCore", + VIR_XML_PROP_NONE, + &def->mem.dump_core) < 0) return -1; - } - VIR_FREE(tmp); tmp = virXPathString("string(./memoryBacking/source/@type)", ctxt); if (tmp) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e2f35fe20b..dcc8eec5a7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -272,9 +272,9 @@ struct _virDomainHostdevSubsysSCSI { struct _virDomainHostdevSubsysMediatedDev { int model; /* enum virMediatedDeviceModelType */ - int display; /* virTristateSwitch */ + virTristateSwitch display; char uuidstr[VIR_UUID_STRING_BUFLEN]; /* mediated device's uuid string */ - int ramfb; /* virTristateSwitch */ + virTristateSwitch ramfb; }; typedef enum { @@ -2674,7 +2674,7 @@ struct _virDomainMemtune { bool nosharepages; bool locked; - int dump_core; /* enum virTristateSwitch */ + virTristateSwitch dump_core; unsigned long long hard_limit; /* in kibibytes, limit at off_t bytes */ unsigned long long soft_limit; /* in kibibytes, limit at off_t bytes */ unsigned long long min_guarantee; /* in kibibytes, limit at off_t bytes */ diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 5cb9caef8a..f9a076dddf 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1444,36 +1444,29 @@ static int x86ModelParseDecode(virCPUx86Model *model, xmlXPathContextPtr ctxt) { - g_autofree char *host = NULL; - g_autofree char *guest = NULL; - int val; + xmlNodePtr decode_node = NULL; + virTristateSwitch decodeHost; + virTristateSwitch decodeGuest; - if ((host = virXPathString("string(./decode/@host)", ctxt))) - val = virTristateSwitchTypeFromString(host); - else - val = VIR_TRISTATE_SWITCH_ABSENT; - - if (val <= 0) { + if (!(decode_node = virXPathNode("./decode", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("invalid or missing decode/host attribute in CPU model %s"), + _("missing decode element in CPU model %s"), model->name); return -1; } - model->decodeHost = val == VIR_TRISTATE_SWITCH_ON; - if ((guest = virXPathString("string(./decode/@guest)", ctxt))) - val = virTristateSwitchTypeFromString(guest); - else - val = VIR_TRISTATE_SWITCH_ABSENT; + if (virXMLPropTristateSwitch(decode_node, "host", + VIR_XML_PROP_REQUIRED, + &decodeHost) < 0) + return -1; - if (val <= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("invalid or missing decode/guest attribute in CPU model %s"), - model->name); + if (virXMLPropTristateSwitch(decode_node, "guest", + VIR_XML_PROP_REQUIRED, + &decodeGuest) < 0) return -1; - } - model->decodeGuest = val == VIR_TRISTATE_SWITCH_ON; + virTristateSwitchToBool(decodeHost, &model->decodeHost); + virTristateSwitchToBool(decodeGuest, &model->decodeGuest); return 0; } -- 2.34.1

There are couple of places where virTristateBoolTypeFromString() is called. Well, the same result can be achieved by virXMLPropTristateBool() and on fewer lines. Note there are couple of places left untouched because those don't care about error reporting and thus are shorter they way they are now. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/cpu_conf.c | 20 ++-- src/conf/domain_conf.c | 170 ++++++++++++--------------------- src/conf/domain_conf.h | 13 ++- src/conf/network_conf.c | 116 ++++++---------------- src/conf/network_conf.h | 12 +-- src/conf/storage_conf.c | 17 ++-- src/conf/storage_source_conf.c | 14 +-- src/conf/storage_source_conf.h | 2 +- src/conf/virnetworkportdef.c | 31 +++--- src/conf/virnetworkportdef.h | 4 +- src/cpu/cpu_x86.c | 23 ++--- src/qemu/qemu_capabilities.c | 28 ++---- src/qemu/qemu_domain.c | 16 +--- 13 files changed, 157 insertions(+), 309 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 4d61bfd01b..8e61a16919 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -328,7 +328,6 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, g_autofree char *cpuMode = NULL; g_autofree char *fallback = NULL; g_autofree char *vendor_id = NULL; - g_autofree char *tscScaling = NULL; g_autofree virHostCPUTscInfo *tsc = NULL; *cpu = NULL; @@ -427,6 +426,8 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, if (def->type == VIR_CPU_TYPE_HOST) { g_autofree char *arch = virXPathString("string(./arch[1])", ctxt); + xmlNodePtr counter_node = NULL; + if (!arch) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Missing CPU architecture")); @@ -446,7 +447,7 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, return -1; } - if (virXPathBoolean("boolean(./counter[@name='tsc'])", ctxt) > 0) { + if ((counter_node = virXPathNode("./counter[@name='tsc'])", ctxt))) { tsc = g_new0(virHostCPUTscInfo, 1); if (virXPathULongLong("string(./counter[@name='tsc']/@frequency)", @@ -456,17 +457,10 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, return -1; } - tscScaling = virXPathString("string(./counter[@name='tsc']/@scaling)", - ctxt); - if (tscScaling) { - int scaling = virTristateBoolTypeFromString(tscScaling); - if (scaling < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Invalid TSC scaling attribute")); - return -1; - } - tsc->scaling = scaling; - } + if (virXMLPropTristateBool(counter_node, "scaling", + VIR_XML_PROP_NONE, + &tsc->scaling) < 0) + return -1; def->tsc = g_steal_pointer(&tsc); } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 83212f9c10..281879b915 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6912,23 +6912,16 @@ virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node, virDomainHostdevDef *def, unsigned int flags) { - g_autofree char *filtering = NULL; xmlNodePtr address = NULL; xmlNodePtr origstates = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) ctxt->node = node; - if ((filtering = virXMLPropString(node, "writeFiltering"))) { - int val; - if ((val = virTristateBoolTypeFromString(filtering)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown pci writeFiltering setting '%s'"), - filtering); - return -1; - } - def->writeFiltering = val; - } + if (virXMLPropTristateBool(node, "writeFiltering", + VIR_XML_PROP_NONE, + &def->writeFiltering) < 0) + return -1; if ((address = virXPathNode("./address", ctxt)) && virPCIDeviceAddressParseXML(address, &def->source.subsys.u.pci.addr) < 0) @@ -7305,22 +7298,22 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, virDomainHostdevSubsysSCSI *scsisrc = &def->source.subsys.u.scsi; virDomainHostdevSubsysSCSIVHost *scsihostsrc = &def->source.subsys.u.scsi_host; virDomainHostdevSubsysMediatedDev *mdevsrc = &def->source.subsys.u.mdev; - g_autofree char *managed = NULL; + virTristateBool managed; g_autofree char *sgio = NULL; - g_autofree char *rawio = NULL; g_autofree char *backendStr = NULL; g_autofree char *model = NULL; + int rv; /* @managed can be read from the xml document - it is always an * attribute of the toplevel element, no matter what type of * element that might be (pure hostdev, or higher level device * (e.g. <interface>) with type='hostdev') */ - if ((managed = virXMLPropString(node, "managed")) != NULL) - ignore_value(virStringParseYesNo(managed, &def->managed)); + ignore_value(virXMLPropTristateBool(node, "managed", + VIR_XML_PROP_NONE, &managed)); + virTristateBoolToBool(managed, &def->managed); sgio = virXMLPropString(node, "sgio"); - rawio = virXMLPropString(node, "rawio"); model = virXMLPropString(node, "model"); /* @type is passed in from the caller rather than read from the @@ -7373,19 +7366,15 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, } } - if (rawio) { - if (def->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("rawio is only supported for scsi host device")); - return -1; - } - - if ((scsisrc->rawio = virTristateBoolTypeFromString(rawio)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown hostdev rawio setting '%s'"), - rawio); - return -1; - } + if ((rv = virXMLPropTristateBool(node, "rawio", + VIR_XML_PROP_NONE, + &scsisrc->rawio)) < 0) { + return -1; + } else if (rv > 0 && + def->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("rawio is only supported for scsi host device")); + return -1; } if (def->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV && @@ -10259,6 +10248,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr vlan_node = NULL; xmlNodePtr bandwidth_node = NULL; xmlNodePtr tmpNode; + xmlNodePtr mac_node = NULL; g_autoptr(GHashTable) filterparams = NULL; virDomainActualNetDef *actual = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) @@ -10266,7 +10256,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, int rv, val; g_autofree char *macaddr = NULL; g_autofree char *macaddr_type = NULL; - g_autofree char *macaddr_check = NULL; g_autofree char *network = NULL; g_autofree char *portgroup = NULL; g_autofree char *portid = NULL; @@ -10467,7 +10456,9 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, if ((vhost = virXPathString("string(./backend/@vhost)", ctxt))) vhost_path = virFileSanitizePath(vhost); - if ((macaddr = virXPathString("string(./mac/@address)", ctxt))) { + mac_node = virXPathNode("./mac", ctxt); + + if ((macaddr = virXMLPropString(mac_node, "address"))) { if (virMacAddrParse((const char *)macaddr, &def->mac) < 0) { virReportError(VIR_ERR_XML_ERROR, _("unable to parse mac address '%s'"), @@ -10498,17 +10489,10 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, def->mac_type = tmp; } - if ((macaddr_check = virXPathString("string(./mac/@check)", ctxt))) { - int tmpCheck; - - if ((tmpCheck = virTristateBoolTypeFromString(macaddr_check)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid mac address check value: '%s'"), - macaddr_check); - goto error; - } - def->mac_check = tmpCheck; - } + if (virXMLPropTristateBool(mac_node, "check", + VIR_XML_PROP_NONE, + &def->mac_check) < 0) + goto error; if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, &def->info, flags | VIR_DOMAIN_DEF_PARSE_ALLOW_BOOT @@ -14165,40 +14149,21 @@ static virDomainVideoAccelDef * virDomainVideoAccelDefParseXML(xmlNodePtr node) { g_autofree virDomainVideoAccelDef *def = NULL; - int val; - g_autofree char *accel2d = NULL; - g_autofree char *accel3d = NULL; g_autofree char *rendernode = NULL; - accel3d = virXMLPropString(node, "accel3d"); - accel2d = virXMLPropString(node, "accel2d"); rendernode = virXMLPropString(node, "rendernode"); - if (!accel3d && !accel2d && !rendernode) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("missing values for acceleration")); - return NULL; - } - def = g_new0(virDomainVideoAccelDef, 1); - if (accel3d) { - if ((val = virTristateBoolTypeFromString(accel3d)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown accel3d value '%s'"), accel3d); - return NULL; - } - def->accel3d = val; - } + if (virXMLPropTristateBool(node, "accel3d", + VIR_XML_PROP_NONE, + &def->accel3d) < 0) + return NULL; - if (accel2d) { - if ((val = virTristateBoolTypeFromString(accel2d)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown accel2d value '%s'"), accel2d); - return NULL; - } - def->accel2d = val; - } + if (virXMLPropTristateBool(node, "accel2d", + VIR_XML_PROP_NONE, + &def->accel2d) < 0) + return NULL; if (rendernode) def->rendernode = virFileSanitizePath(rendernode); @@ -14633,20 +14598,13 @@ virDomainEventActionParseXML(xmlXPathContextPtr ctxt, static int virDomainPMStateParseXML(xmlXPathContextPtr ctxt, const char *xpath, - int *val) + virTristateBool *val) { - g_autofree char *tmp = virXPathString(xpath, ctxt); + xmlNodePtr node = virXPathNode(xpath, ctxt); - if (tmp) { - *val = virTristateBoolTypeFromString(tmp); - if (*val < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown PM state value %s"), tmp); - return -1; - } - } - - return 0; + return virXMLPropTristateBool(node, "enabled", + VIR_XML_PROP_NONE, + val); } @@ -16972,18 +16930,14 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, } if ((node = virXPathNode("./os/bootmenu[1]", ctxt))) { - tmp = virXMLPropString(node, "enable"); - if (tmp) { - def->os.bootmenu = virTristateBoolTypeFromString(tmp); - if (def->os.bootmenu <= 0) { - /* In order not to break misconfigured machines, this - * should not emit an error, but rather set the bootmenu - * to disabled */ - VIR_WARN("disabling bootmenu due to unknown option '%s'", - tmp); - def->os.bootmenu = VIR_TRISTATE_BOOL_NO; - } - VIR_FREE(tmp); + if (virXMLPropTristateBool(node, "enable", + VIR_XML_PROP_NONE, + &def->os.bootmenu) < 0) { + /* In order not to break misconfigured machines, this + * should not emit an error, but rather set the bootmenu + * to disabled */ + VIR_WARN("disabling bootmenu due to unknown option"); + def->os.bootmenu = VIR_TRISTATE_BOOL_NO; } tmp = virXMLPropString(node, "timeout"); @@ -18388,26 +18342,21 @@ virDomainDefParseBootFirmwareOptions(virDomainDef *def, features = g_new0(int, VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_LAST); for (i = 0; i < n; i++) { - g_autofree char *name = virXMLPropString(nodes[i], "name"); - g_autofree char *enabled = virXMLPropString(nodes[i], "enabled"); - int feature = virDomainOsDefFirmwareFeatureTypeFromString(name); - int val = virTristateBoolTypeFromString(enabled); + unsigned int feature; + virTristateBool enabled; - if (feature < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid firmware feature name '%s'"), - name); + if (virXMLPropEnum(nodes[i], "name", + virDomainOsDefFirmwareFeatureTypeFromString, + VIR_XML_PROP_REQUIRED, + &feature) < 0) return -1; - } - if (val < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid firmware feature enabled value '%s'"), - enabled); + if (virXMLPropTristateBool(nodes[i], "enabled", + VIR_XML_PROP_REQUIRED, + &enabled) < 0) return -1; - } - features[feature] = val; + features[feature] = enabled; } def->os.firmwareFeatures = g_steal_pointer(&features); @@ -19523,12 +19472,12 @@ virDomainDefLifecycleParse(virDomainDef *def, return -1; if (virDomainPMStateParseXML(ctxt, - "string(./pm/suspend-to-mem/@enabled)", + "./pm/suspend-to-mem", &def->pm.s3) < 0) return -1; if (virDomainPMStateParseXML(ctxt, - "string(./pm/suspend-to-disk/@enabled)", + "./pm/suspend-to-disk", &def->pm.s4) < 0) return -1; @@ -30565,6 +30514,7 @@ virDomainNetDefActualFromNetworkPort(virDomainNetDef *iface, break; case VIR_TRISTATE_BOOL_ABSENT: case VIR_TRISTATE_BOOL_NO: + case VIR_TRISTATE_BOOL_LAST: actual->data.hostdev.def.managed = false; break; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index dcc8eec5a7..c2c263dfab 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -263,7 +263,7 @@ struct _virDomainHostdevSubsysSCSIiSCSI { struct _virDomainHostdevSubsysSCSI { int protocol; /* enum virDomainHostdevSCSIProtocolType */ int sgio; /* enum virDomainDeviceSGIO */ - int rawio; /* enum virTristateBool */ + virTristateBool rawio; union { virDomainHostdevSubsysSCSIHost host; virDomainHostdevSubsysSCSIiSCSI iscsi; @@ -1706,8 +1706,8 @@ typedef enum { VIR_ENUM_DECL(virDomainVideoVGAConf); struct _virDomainVideoAccelDef { - int accel2d; /* enum virTristateBool */ - int accel3d; /* enum virTristateBool */ + virTristateBool accel2d; + virTristateBool accel3d; char *rendernode; }; @@ -2325,7 +2325,7 @@ struct _virDomainOSDef { char *machine; size_t nBootDevs; int bootDevs[VIR_DOMAIN_BOOT_LAST]; - int bootmenu; /* enum virTristateBool */ + virTristateBool bootmenu; unsigned int bm_timeout; bool bm_timeout_set; char *init; @@ -2688,9 +2688,8 @@ struct _virDomainMemtune { }; struct _virDomainPowerManagement { - /* These options are of type enum virTristateBool */ - int s3; - int s4; + virTristateBool s3; + virTristateBool s4; }; struct _virDomainPerfDef { diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 681273a547..10d3330fdf 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -928,37 +928,21 @@ virNetworkDNSDefParseXML(const char *networkName, g_autofree xmlNodePtr *srvNodes = NULL; g_autofree xmlNodePtr *txtNodes = NULL; g_autofree xmlNodePtr *fwdNodes = NULL; - g_autofree char *forwardPlainNames = NULL; - g_autofree char *enable = NULL; int nhosts, nsrvs, ntxts, nfwds; size_t i; VIR_XPATH_NODE_AUTORESTORE(ctxt) ctxt->node = node; - enable = virXPathString("string(./@enable)", ctxt); - if (enable) { - def->enable = virTristateBoolTypeFromString(enable); - if (def->enable <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid dns enable setting '%s' " - "in network '%s'"), - enable, networkName); - return -1; - } - } + if (virXMLPropTristateBool(node, "enable", + VIR_XML_PROP_NONE, + &def->enable) < 0) + return -1; - forwardPlainNames = virXPathString("string(./@forwardPlainNames)", ctxt); - if (forwardPlainNames) { - def->forwardPlainNames = virTristateBoolTypeFromString(forwardPlainNames); - if (def->forwardPlainNames <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid dns forwardPlainNames setting '%s' " - "in network '%s'"), - forwardPlainNames, networkName); - return -1; - } - } + if (virXMLPropTristateBool(node, "forwardPlainNames", + VIR_XML_PROP_NONE, + &def->forwardPlainNames) < 0) + return -1; nfwds = virXPathNodeSet("./forwarder", ctxt, &fwdNodes); if (nfwds < 0) { @@ -1076,7 +1060,6 @@ virNetworkIPDefParseXML(const char *networkName, xmlNodePtr dhcp; g_autofree char *address = NULL; g_autofree char *netmask = NULL; - g_autofree char *localPtr = NULL; unsigned long prefix = 0; int prefixRc; int ret = -1; @@ -1121,16 +1104,10 @@ virNetworkIPDefParseXML(const char *networkName, else def->prefix = prefix; - localPtr = virXPathString("string(./@localPtr)", ctxt); - if (localPtr) { - def->localPTR = virTristateBoolTypeFromString(localPtr); - if (def->localPTR <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid localPtr value '%s' in network '%s'"), - localPtr, networkName); - goto cleanup; - } - } + if (virXMLPropTristateBool(node, "localPtr", + VIR_XML_PROP_NONE, + &def->localPTR) < 0) + goto cleanup; /* validate address, etc. for each family */ if ((def->family == NULL) || (STREQ(def->family, "ipv4"))) { @@ -1217,19 +1194,11 @@ int virNetworkPortOptionsParseXML(xmlXPathContextPtr ctxt, virTristateBool *isolatedPort) { - g_autofree char *str = NULL; - int tmp = VIR_TRISTATE_BOOL_ABSENT; + xmlNodePtr port_node = virXPathNode("./port", ctxt); - if ((str = virXPathString("string(./port/@isolated)", ctxt))) { - if ((tmp = virTristateBoolTypeFromString(str)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown port isolated value '%s'"), str); - return -1; - } - } - - *isolatedPort = tmp; - return 0; + return virXMLPropTristateBool(port_node, "isolated", + VIR_XML_PROP_NONE, + isolatedPort); } @@ -1248,7 +1217,6 @@ virNetworkPortGroupParseXML(virPortGroupDef *def, xmlNodePtr vlanNode; xmlNodePtr bandwidth_node; g_autofree char *isDefault = NULL; - g_autofree char *trustGuestRxFilters = NULL; int ret = -1; @@ -1265,17 +1233,10 @@ virNetworkPortGroupParseXML(virPortGroupDef *def, isDefault = virXPathString("string(./@default)", ctxt); def->isDefault = isDefault && STRCASEEQ(isDefault, "yes"); - trustGuestRxFilters - = virXPathString("string(./@trustGuestRxFilters)", ctxt); - if (trustGuestRxFilters) { - if ((def->trustGuestRxFilters - = virTristateBoolTypeFromString(trustGuestRxFilters)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid trustGuestRxFilters setting '%s' " - "in portgroup"), trustGuestRxFilters); - goto cleanup; - } - } + if (virXMLPropTristateBool(node, "trustGuestRxFilters", + VIR_XML_PROP_NONE, + &def->trustGuestRxFilters) < 0) + goto cleanup; virtPortNode = virXPathNode("./virtualport", ctxt); if (virtPortNode && @@ -1662,7 +1623,6 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt, { g_autoptr(virNetworkDef) def = NULL; g_autofree char *uuid = NULL; - g_autofree char *localOnly = NULL; g_autofree char *stp = NULL; g_autofree char *stpDelay = NULL; g_autofree char *macTableManager = NULL; @@ -1676,11 +1636,11 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt, xmlNodePtr virtPortNode = NULL; xmlNodePtr forwardNode = NULL; g_autofree char *ipv6nogwStr = NULL; - g_autofree char *trustGuestRxFilters = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) xmlNodePtr bandwidthNode = NULL; xmlNodePtr vlanNode; xmlNodePtr metadataNode = NULL; + xmlNodePtr domain_node = NULL; def = g_new0(virNetworkDef, 1); @@ -1724,32 +1684,18 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt, } } - trustGuestRxFilters - = virXPathString("string(./@trustGuestRxFilters)", ctxt); - if (trustGuestRxFilters) { - if ((def->trustGuestRxFilters - = virTristateBoolTypeFromString(trustGuestRxFilters)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid trustGuestRxFilters setting '%s' " - "in network '%s'"), - trustGuestRxFilters, def->name); - return NULL; - } - } + if (virXMLPropTristateBool(ctxt->node, "trustGuestRxFilters", + VIR_XML_PROP_NONE, + &def->trustGuestRxFilters) < 0) + return NULL; /* Parse network domain information */ - def->domain = virXPathString("string(./domain[1]/@name)", ctxt); - localOnly = virXPathString("string(./domain[1]/@localOnly)", ctxt); - if (localOnly) { - def->domainLocalOnly = virTristateBoolTypeFromString(localOnly); - if (def->domainLocalOnly <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid domain localOnly setting '%s' " - "in network '%s'"), - localOnly, def->name); - return NULL; - } - } + domain_node = virXPathNode("./domain[1]", ctxt); + def->domain = virXMLPropString(domain_node, "name"); + if (virXMLPropTristateBool(domain_node, "localOnly", + VIR_XML_PROP_NONE, + &def->domainLocalOnly) < 0) + return NULL; if ((bandwidthNode = virXPathNode("./bandwidth", ctxt)) && virNetDevBandwidthParse(&def->bandwidth, NULL, bandwidthNode, false) < 0) diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index cf24c8e268..b374bbba48 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -158,8 +158,8 @@ struct _virNetworkDNSForwarder { typedef struct _virNetworkDNSDef virNetworkDNSDef; struct _virNetworkDNSDef { - int enable; /* enum virTristateBool */ - int forwardPlainNames; /* enum virTristateBool */ + virTristateBool enable; + virTristateBool forwardPlainNames; size_t ntxts; virNetworkDNSTxtDef *txts; size_t nhosts; @@ -184,7 +184,7 @@ struct _virNetworkIPDef { unsigned int prefix; /* ipv6 - only prefix allowed */ virSocketAddr netmask; /* ipv4 - either netmask or prefix specified */ - int localPTR; /* virTristateBool */ + virTristateBool localPTR; size_t nranges; /* Zero or more dhcp ranges */ virNetworkDHCPRangeDef *ranges; @@ -243,7 +243,7 @@ struct _virPortGroupDef { virNetDevVPortProfile *virtPortProfile; virNetDevBandwidth *bandwidth; virNetDevVlan vlan; - int trustGuestRxFilters; /* enum virTristateBool */ + virTristateBool trustGuestRxFilters; }; typedef struct _virNetworkDef virNetworkDef; @@ -257,7 +257,7 @@ struct _virNetworkDef { char *bridgeZone; /* name of firewalld zone for bridge */ int macTableManager; /* enum virNetworkBridgeMACTableManager */ char *domain; - int domainLocalOnly; /* enum virTristateBool: yes disables dns forwarding */ + virTristateBool domainLocalOnly; /* yes disables dns forwarding */ unsigned long delay; /* Bridge forward delay (ms) */ bool stp; /* Spanning tree protocol */ unsigned int mtu; /* MTU for bridge, 0 means "default" i.e. unset in config */ @@ -284,7 +284,7 @@ struct _virNetworkDef { virPortGroupDef *portGroups; virNetDevBandwidth *bandwidth; virNetDevVlan vlan; - int trustGuestRxFilters; /* enum virTristateBool */ + virTristateBool trustGuestRxFilters; virTristateBool isolatedPort; /* Application-specific custom metadata */ diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 6690d26ffd..85260431e7 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -804,22 +804,21 @@ static int virStoragePoolDefParseFeatures(virStoragePoolDef *def, xmlXPathContextPtr ctxt) { - g_autofree char *cow = virXPathString("string(./features/cow/@state)", ctxt); + xmlNodePtr node = virXPathNode("./features/cow", ctxt); + virTristateBool val; + int rv; - if (cow) { - int val; + if ((rv = virXMLPropTristateBool(node, "state", + VIR_XML_PROP_NONE, + &val)) < 0) { + return -1; + } else if (rv > 0) { if (def->type != VIR_STORAGE_POOL_FS && def->type != VIR_STORAGE_POOL_DIR) { virReportError(VIR_ERR_NO_SUPPORT, "%s", _("cow feature may only be used for 'fs' and 'dir' pools")); return -1; } - if ((val = virTristateBoolTypeFromString(cow)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid storage pool cow feature state '%s'"), - cow); - return -1; - } def->features.cow = val; } diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c index d42f715f26..851a2da877 100644 --- a/src/conf/storage_source_conf.c +++ b/src/conf/storage_source_conf.c @@ -322,24 +322,16 @@ virStoragePRDefParseXML(xmlXPathContextPtr ctxt) { virStoragePRDef *prd; virStoragePRDef *ret = NULL; - g_autofree char *managed = NULL; g_autofree char *type = NULL; g_autofree char *path = NULL; g_autofree char *mode = NULL; prd = g_new0(virStoragePRDef, 1); - if (!(managed = virXPathString("string(./@managed)", ctxt))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing @managed attribute for <reservations/>")); + if (virXMLPropTristateBool(ctxt->node, "managed", + VIR_XML_PROP_REQUIRED, + &prd->managed) < 0) goto cleanup; - } - - if ((prd->managed = virTristateBoolTypeFromString(managed)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid value for 'managed': %s"), managed); - goto cleanup; - } type = virXPathString("string(./source[1]/@type)", ctxt); path = virXPathString("string(./source[1]/@path)", ctxt); diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h index c4a026881c..c720d093be 100644 --- a/src/conf/storage_source_conf.h +++ b/src/conf/storage_source_conf.h @@ -226,7 +226,7 @@ struct _virStorageAuthDef { typedef struct _virStoragePRDef virStoragePRDef; struct _virStoragePRDef { - int managed; /* enum virTristateBool */ + virTristateBool managed; char *path; /* manager object alias */ diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c index c4f5f96392..1d75436085 100644 --- a/src/conf/virnetworkportdef.c +++ b/src/conf/virnetworkportdef.c @@ -87,12 +87,12 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt) xmlNodePtr vlanNode; xmlNodePtr bandwidthNode; xmlNodePtr addressNode; - g_autofree char *trustGuestRxFilters = NULL; + xmlNodePtr rxfiltersNode = NULL; + xmlNodePtr plugNode = NULL; g_autofree char *mac = NULL; g_autofree char *macmgr = NULL; g_autofree char *mode = NULL; g_autofree char *plugtype = NULL; - g_autofree char *managed = NULL; g_autofree char *driver = NULL; def = g_new0(virNetworkPortDef, 1); @@ -169,18 +169,13 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt) if (virNetworkPortOptionsParseXML(ctxt, &def->isolatedPort) < 0) return NULL; - trustGuestRxFilters - = virXPathString("string(./rxfilters/@trustGuest)", ctxt); - if (trustGuestRxFilters) { - if ((def->trustGuestRxFilters - = virTristateBoolTypeFromString(trustGuestRxFilters)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid guest rx filters trust setting '%s' "), - trustGuestRxFilters); - return NULL; - } - } + rxfiltersNode = virXPathNode("./rxfilters", ctxt); + if (virXMLPropTristateBool(rxfiltersNode, "trustGuest", + VIR_XML_PROP_NONE, + &def->trustGuestRxFilters) < 0) + return NULL; + plugNode = virXPathNode("./plug", ctxt); plugtype = virXPathString("string(./plug/@type)", ctxt); if (plugtype && @@ -228,14 +223,10 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt) break; case VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI: - managed = virXPathString("string(./plug/@managed)", ctxt); - if (managed && - (def->plug.hostdevpci.managed = - virTristateBoolTypeFromString(managed)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid managed setting '%s' in network port"), mode); + if (virXMLPropTristateBool(plugNode, "managed", + VIR_XML_PROP_NONE, + &def->plug.hostdevpci.managed) < 0) return NULL; - } driver = virXPathString("string(./plug/driver/@name)", ctxt); if (driver && (def->plug.hostdevpci.driver = diff --git a/src/conf/virnetworkportdef.h b/src/conf/virnetworkportdef.h index 5c7cd2953e..c6474c4ad8 100644 --- a/src/conf/virnetworkportdef.h +++ b/src/conf/virnetworkportdef.h @@ -55,7 +55,7 @@ struct _virNetworkPortDef { virNetDevBandwidth *bandwidth; unsigned int class_id; /* class ID for bandwidth 'floor' */ virNetDevVlan vlan; - int trustGuestRxFilters; /* enum virTristateBool */ + virTristateBool trustGuestRxFilters; virTristateBool isolatedPort; int plugtype; /* virNetworkPortPlugType */ @@ -71,7 +71,7 @@ struct _virNetworkPortDef { struct { virPCIDeviceAddress addr; /* PCI Address of device */ int driver; /* virNetworkForwardDriverNameType */ - int managed; + virTristateBool managed; } hostdevpci; } plug; }; diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index f9a076dddf..9826ec6190 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1603,8 +1603,8 @@ x86ModelParseFeatures(virCPUx86Model *model, for (i = 0; i < n; i++) { g_autofree char *ftname = NULL; - g_autofree char *removed = NULL; virCPUx86Feature *feature; + virTristateBool rem; if (!(ftname = virXMLPropString(nodes[i], "name"))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1620,21 +1620,14 @@ x86ModelParseFeatures(virCPUx86Model *model, return -1; } - if ((removed = virXMLPropString(nodes[i], "removed"))) { - int rem; + if (virXMLPropTristateBool(nodes[i], "removed", + VIR_XML_PROP_NONE, + &rem) < 0) + return -1; - if ((rem = virTristateBoolTypeFromString(removed)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid 'removed' attribute for feature %s " - "in model %s"), - ftname, model->name); - return -1; - } - - if (rem == VIR_TRISTATE_BOOL_YES) { - model->removedFeatures[nremoved++] = g_strdup(ftname); - continue; - } + if (rem == VIR_TRISTATE_BOOL_YES) { + model->removedFeatures[nremoved++] = g_strdup(ftname); + continue; } if (x86DataAdd(&model->data, &feature->data)) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ea35948b01..461a818776 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3758,7 +3758,6 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps, xmlXPathContextPtr ctxt, const char *typeStr) { - g_autofree char *migratability = NULL; xmlNodePtr hostCPUNode; g_autofree xmlNodePtr *nodes = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) @@ -3766,6 +3765,7 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps, g_autofree char *xpath = g_strdup_printf("./hostCPU[@type='%s']", typeStr); size_t i; int n; + virTristateBool migratability; int val; if (!(hostCPUNode = virXPathNode(xpath, ctxt))) { @@ -3781,13 +3781,12 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps, return -1; } - if (!(migratability = virXMLPropString(hostCPUNode, "migratability")) || - (val = virTristateBoolTypeFromString(migratability)) <= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("invalid migratability value for host CPU model")); + if (virXMLPropTristateBool(hostCPUNode, "migratability", + VIR_XML_PROP_REQUIRED, + &migratability) < 0) return -1; - } - hostCPU->migratability = val == VIR_TRISTATE_BOOL_YES; + + virTristateBoolToBool(migratability, &hostCPU->migratability); ctxt->node = hostCPUNode; @@ -3798,7 +3797,6 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps, for (i = 0; i < n; i++) { qemuMonitorCPUProperty *prop = hostCPU->props + i; g_autofree char *type = NULL; - g_autofree char *migratable = NULL; ctxt->node = nodes[i]; @@ -3850,17 +3848,11 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps, break; } - if ((migratable = virXMLPropString(ctxt->node, "migratable"))) { - if ((val = virTristateBoolTypeFromString(migratable)) <= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown migratable value for '%s' host " - "CPU model property"), - prop->name); - return -1; - } + if (virXMLPropTristateBool(ctxt->node, "migratable", + VIR_XML_PROP_NONE, + &prop->migratable) < 0) + return -1; - prop->migratable = val; - } } } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a8401bac30..aa8f6b8d05 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2829,19 +2829,11 @@ int qemuDomainObjPrivateXMLParseAllowReboot(xmlXPathContextPtr ctxt, virTristateBool *allowReboot) { - int val; - g_autofree char *valStr = NULL; + xmlNodePtr node = virXPathNode("./allowReboot", ctxt); - if ((valStr = virXPathString("string(./allowReboot/@value)", ctxt))) { - if ((val = virTristateBoolTypeFromString(valStr)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("invalid allowReboot value '%s'"), valStr); - return -1; - } - *allowReboot = val; - } - - return 0; + return virXMLPropTristateBool(node, "value", + VIR_XML_PROP_NONE, + allowReboot); } -- 2.34.1

On a Friday in 2022, Michal Privoznik wrote:
There are couple of places where virTristateBoolTypeFromString() is called. Well, the same result can be achieved by virXMLPropTristateBool() and on fewer lines.
Note there are couple of places left untouched because those don't care about error reporting and thus are shorter they way they are now.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/cpu_conf.c | 20 ++-- src/conf/domain_conf.c | 170 ++++++++++++--------------------- src/conf/domain_conf.h | 13 ++- src/conf/network_conf.c | 116 ++++++---------------- src/conf/network_conf.h | 12 +-- src/conf/storage_conf.c | 17 ++-- src/conf/storage_source_conf.c | 14 +-- src/conf/storage_source_conf.h | 2 +- src/conf/virnetworkportdef.c | 31 +++--- src/conf/virnetworkportdef.h | 4 +- src/cpu/cpu_x86.c | 23 ++--- src/qemu/qemu_capabilities.c | 28 ++---- src/qemu/qemu_domain.c | 16 +--- 13 files changed, 157 insertions(+), 309 deletions(-)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 4d61bfd01b..8e61a16919 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -16972,18 +16930,14 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, }
if ((node = virXPathNode("./os/bootmenu[1]", ctxt))) { - tmp = virXMLPropString(node, "enable"); - if (tmp) { - def->os.bootmenu = virTristateBoolTypeFromString(tmp); - if (def->os.bootmenu <= 0) { - /* In order not to break misconfigured machines, this - * should not emit an error, but rather set the bootmenu - * to disabled */ - VIR_WARN("disabling bootmenu due to unknown option '%s'", - tmp); - def->os.bootmenu = VIR_TRISTATE_BOOL_NO; - } - VIR_FREE(tmp); + if (virXMLPropTristateBool(node, "enable", + VIR_XML_PROP_NONE, + &def->os.bootmenu) < 0) { + /* In order not to break misconfigured machines, this + * should not emit an error, but rather set the bootmenu + * to disabled */
The comment is now incorrect - virXMLPropTristateBool would log an error. Introduced by commit 4f24ca01e881ec8df69861a3386b697ff528d3e3 CommitDate: 2010-07-27 16:38:32 -0400 qemu: Allow setting boot menu on/off we accepted a value of "yes" meaning "yes", and all other values meant "no". This was later refactored by commit 8c952908681ca66ba4be75362de7e593d6c44f94 CommitDate: 2012-09-20 10:59:35 +0200 qemu: Cleanup boot parameter building to use an enum with "yes" and "no", but no error checking. I think we can safely return an error here after all the years. Jano
+ VIR_WARN("disabling bootmenu due to unknown option"); + def->os.bootmenu = VIR_TRISTATE_BOOL_NO; }
tmp = virXMLPropString(node, "timeout");

After previous cleanups, the virCPUDefParseXML() function uses a mixture of virXMLProp*() and the old virXMLPropString() + virXXXTypeFromString() patterns. Rework it so that virXMLProp*() is used. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/cpu_conf.c | 24 ++++++++---------------- src/conf/cpu_conf.h | 2 +- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 8e61a16919..da83e99371 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -407,17 +407,11 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, } if (def->type == VIR_CPU_TYPE_GUEST) { - g_autofree char *match = virXMLPropString(ctxt->node, "match"); - - if (match) { - def->match = virCPUMatchTypeFromString(match); - if (def->match < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Invalid match attribute for CPU " - "specification")); - return -1; - } - } + if (virXMLPropEnum(ctxt->node, "match", + virCPUMatchTypeFromString, + VIR_XML_PROP_NONE, + &def->match) < 0) + return -1; if (virXMLPropEnum(ctxt->node, "check", virCPUCheckTypeFromString, VIR_XML_PROP_NONE, &def->check) < 0) @@ -450,12 +444,10 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, if ((counter_node = virXPathNode("./counter[@name='tsc'])", ctxt))) { tsc = g_new0(virHostCPUTscInfo, 1); - if (virXPathULongLong("string(./counter[@name='tsc']/@frequency)", - ctxt, &tsc->frequency) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Invalid TSC frequency")); + if (virXMLPropULongLong(counter_node, "frequency", 10, + VIR_XML_PROP_REQUIRED, + &tsc->frequency) < 0) return -1; - } if (virXMLPropTristateBool(counter_node, "scaling", VIR_XML_PROP_NONE, diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index b0a81895be..2cda4ee1f4 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -122,7 +122,7 @@ struct _virCPUDef { int refs; int type; /* enum virCPUType */ int mode; /* enum virCPUMode */ - int match; /* enum virCPUMatch */ + virCPUMatch match; virCPUCheck check; virArch arch; char *model; -- 2.34.1

After previous cleanups, the virDomainDefParseBootXML() function uses a mixture of virXMLProp*() and the old virXMLPropString() + virXXXTypeFromString() patterns. Rework it so that virXMLProp*() is used. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 61 +++++++++++++++++------------------------ src/conf/domain_conf.h | 4 +-- src/libxl/libxl_conf.c | 2 ++ src/libxl/xen_xl.c | 2 ++ src/libxl/xen_xm.c | 2 ++ src/qemu/qemu_process.c | 2 +- src/vz/vz_sdk.c | 4 ++- 7 files changed, 37 insertions(+), 40 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 281879b915..30f0a13e2a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16905,7 +16905,6 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, xmlNodePtr node; size_t i; int n; - g_autofree char *tmp = NULL; g_autofree xmlNodePtr *nodes = NULL; /* analysis of the boot devices */ @@ -16913,20 +16912,13 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, return -1; for (i = 0; i < n && i < VIR_DOMAIN_BOOT_LAST; i++) { - int val; - g_autofree char *dev = virXMLPropString(nodes[i], "dev"); - if (!dev) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing boot device")); + if (virXMLPropEnum(nodes[i], "dev", + virDomainBootTypeFromString, + VIR_XML_PROP_REQUIRED, + &def->os.bootDevs[def->os.nBootDevs]) < 0) return -1; - } - if ((val = virDomainBootTypeFromString(dev)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown boot device '%s'"), - dev); - return -1; - } - def->os.bootDevs[def->os.nBootDevs++] = val; + + def->os.nBootDevs++; } if ((node = virXPathNode("./os/bootmenu[1]", ctxt))) { @@ -16940,36 +16932,33 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, def->os.bootmenu = VIR_TRISTATE_BOOL_NO; } - tmp = virXMLPropString(node, "timeout"); - if (tmp && def->os.bootmenu == VIR_TRISTATE_BOOL_YES) { - if (virStrToLong_uip(tmp, NULL, 0, &def->os.bm_timeout) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("invalid value for boot menu timeout")); + if (def->os.bootmenu == VIR_TRISTATE_BOOL_YES) { + int rv; + + if ((rv = virXMLPropUInt(node, "timeout", 10, + VIR_XML_PROP_NONE, + &def->os.bm_timeout)) < 0) { return -1; + } else if (rv > 0) { + def->os.bm_timeout_set = true; } - def->os.bm_timeout_set = true; } - VIR_FREE(tmp); } if ((node = virXPathNode("./os/bios[1]", ctxt))) { - tmp = virXMLPropString(node, "useserial"); - if (tmp) { - bool state = false; - ignore_value(virStringParseYesNo(tmp, &state)); - def->os.bios.useserial = virTristateBoolFromBool(state); - VIR_FREE(tmp); + int rv; + + if (virXMLPropTristateBool(node, "useserial", + VIR_XML_PROP_NONE, + &def->os.bios.useserial) < 0) { + def->os.bios.useserial = VIR_TRISTATE_BOOL_NO; } - tmp = virXMLPropString(node, "rebootTimeout"); - 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) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("invalid value for rebootTimeout")); - return -1; - } + if ((rv = virXMLPropInt(node, "rebootTimeout", 10, + VIR_XML_PROP_NONE, + &def->os.bios.rt_delay, 0)) < 0) { + return -1; + } else if (rv > 0) { def->os.bios.rt_set = true; } } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c2c263dfab..584e15b6c7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2195,7 +2195,7 @@ typedef enum { VIR_ENUM_DECL(virDomainLockFailure); struct _virDomainBIOSDef { - int useserial; /* enum virTristateBool */ + virTristateBool useserial; /* reboot-timeout parameters */ bool rt_set; int rt_delay; @@ -2324,7 +2324,7 @@ struct _virDomainOSDef { virArch arch; char *machine; size_t nBootDevs; - int bootDevs[VIR_DOMAIN_BOOT_LAST]; + virDomainBootOrder bootDevs[VIR_DOMAIN_BOOT_LAST]; virTristateBool bootmenu; unsigned int bm_timeout; bool bm_timeout_set; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 1ac6253ad7..561171126c 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -609,6 +609,8 @@ libxlMakeDomBuildInfo(virDomainDef *def, case VIR_DOMAIN_BOOT_NET: bootorder[i] = 'n'; break; + case VIR_DOMAIN_BOOT_LAST: + break; } } if (def->os.nBootDevs == 0) { diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c index 799fe05cbd..87194ccbd1 100644 --- a/src/libxl/xen_xl.c +++ b/src/libxl/xen_xl.c @@ -1153,6 +1153,8 @@ xenFormatXLOS(virConf *conf, virDomainDef *def) default: boot[i] = 'c'; break; + case VIR_DOMAIN_BOOT_LAST: + break; } } diff --git a/src/libxl/xen_xm.c b/src/libxl/xen_xm.c index 2e636d874e..a962da9cad 100644 --- a/src/libxl/xen_xm.c +++ b/src/libxl/xen_xm.c @@ -457,6 +457,8 @@ xenFormatXMOS(virConf *conf, virDomainDef *def) default: boot[i] = 'c'; break; + case VIR_DOMAIN_BOOT_LAST: + break; } } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7ff4dc1835..336f0bab2e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6183,7 +6183,7 @@ qemuProcessPrepareDeviceBootorder(virDomainDef *def) return; for (i = 0; i < def->os.nBootDevs; i++) { - switch ((virDomainBootOrder) def->os.bootDevs[i]) { + switch (def->os.bootDevs[i]) { case VIR_DOMAIN_BOOT_CDROM: bootCD = i + 1; break; diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 94c6cd5c7a..ccfd3e9d55 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3799,7 +3799,7 @@ prlsdkSetBootOrderVm(PRL_HANDLE sdkdom, virDomainDef *def) for (i = 0; i < def->os.nBootDevs; ++i) { virType = def->os.bootDevs[i]; - switch ((int)virType) { + switch (virType) { case VIR_DOMAIN_BOOT_CDROM: sdkType = PDE_OPTICAL_DISK; break; @@ -3809,6 +3809,8 @@ prlsdkSetBootOrderVm(PRL_HANDLE sdkdom, virDomainDef *def) case VIR_DOMAIN_BOOT_NET: sdkType = PDE_GENERIC_NETWORK_ADAPTER; break; + case VIR_DOMAIN_BOOT_FLOPPY: + case VIR_DOMAIN_BOOT_LAST: default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported boot device type: '%s'"), -- 2.34.1

After previous cleanups, the virDomainFSDefParseXML() function uses a mixture of virXMLProp*() and the old virXMLPropString() + virXXXTypeFromString() patterns. Rework it so that virXMLProp*() is used. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/bhyve/bhyve_command.c | 2 +- src/conf/domain_conf.c | 127 +++++++++++++-------------------- src/conf/domain_conf.h | 10 +-- src/lxc/lxc_container.c | 3 + src/qemu/qemu_domain_address.c | 2 +- 5 files changed, 60 insertions(+), 84 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 3368d20a04..af8ec30fe7 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -577,7 +577,7 @@ bhyveBuildFSArgStr(const virDomainDef *def G_GNUC_UNUSED, { g_auto(virBuffer) params = VIR_BUFFER_INITIALIZER; - switch ((virDomainFSType) fs->type) { + switch (fs->type) { case VIR_DOMAIN_FS_TYPE_MOUNT: break; case VIR_DOMAIN_FS_TYPE_BLOCK: diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 30f0a13e2a..f4b05b1c21 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9759,86 +9759,66 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt, virDomainFSDef *def; xmlNodePtr driver_node = NULL; xmlNodePtr source_node = NULL; - g_autofree char *type = NULL; g_autofree char *source = NULL; g_autofree char *target = NULL; g_autofree char *format = NULL; - g_autofree char *accessmode = NULL; g_autofree char *usage = NULL; g_autofree char *units = NULL; - g_autofree char *model = NULL; - g_autofree char *multidevs = NULL; - g_autofree char *fmode = NULL; - g_autofree char *dmode = NULL; g_autofree char *sock = NULL; + int rv; ctxt->node = node; if (!(def = virDomainFSDefNew(xmlopt))) return NULL; - type = virXMLPropString(node, "type"); - if (type) { - if ((def->type = virDomainFSTypeFromString(type)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown filesystem type '%s'"), type); - goto error; - } - } else { - def->type = VIR_DOMAIN_FS_TYPE_MOUNT; - } + if (virXMLPropEnum(node, "type", + virDomainFSTypeFromString, + VIR_XML_PROP_NONE, + &def->type) < 0) + goto error; - accessmode = virXMLPropString(node, "accessmode"); - if (accessmode) { - if ((def->accessmode = virDomainFSAccessModeTypeFromString(accessmode)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown accessmode '%s'"), accessmode); - goto error; - } - } else { - def->accessmode = VIR_DOMAIN_FS_ACCESSMODE_DEFAULT; - } + if (virXMLPropEnum(node, "accessmode", + virDomainFSAccessModeTypeFromString, + VIR_XML_PROP_NONE, + &def->accessmode) < 0) + goto error; - fmode = virXMLPropString(node, "fmode"); - if (fmode) { - if ((virStrToLong_uip(fmode, NULL, 8, &def->fmode) < 0) || - (def->fmode > 0777)) { + if ((rv = virXMLPropUInt(node, "fmode", 8, + VIR_XML_PROP_NONE, + &def->fmode)) < 0) { + goto error; + } else if (rv > 0) { + if (def->fmode > 0777) { virReportError(VIR_ERR_XML_ERROR, - _("invalid fmode: '%s'"), fmode); + _("invalid fmode: '0%o'"), def->fmode); goto error; } } - dmode = virXMLPropString(node, "dmode"); - if (dmode) { - if ((virStrToLong_uip(dmode, NULL, 8, &def->dmode) < 0) || - (def->dmode > 0777)) { + if ((rv = virXMLPropUInt(node, "dmode", 8, + VIR_XML_PROP_NONE, + &def->dmode)) < 0) { + goto error; + } else if (rv > 0) { + if (def->dmode > 0777) { virReportError(VIR_ERR_XML_ERROR, - _("invalid dmode: '%s'"), dmode); + _("invalid dmode: '0%o'"), def->dmode); goto error; } } - model = virXMLPropString(node, "model"); - if (model) { - if ((def->model = virDomainFSModelTypeFromString(model)) < 0 || - def->model == VIR_DOMAIN_FS_MODEL_DEFAULT) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown model '%s'"), model); - goto error; - } - } + if (virXMLPropEnum(node, "model", + virDomainFSModelTypeFromString, + VIR_XML_PROP_NONZERO, + &def->model) < 0) + goto error; - multidevs = virXMLPropString(node, "multidevs"); - if (multidevs) { - if ((def->multidevs = virDomainFSMultidevsTypeFromString(multidevs)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown multidevs '%s'"), multidevs); - goto error; - } - } else { - def->multidevs = VIR_DOMAIN_FS_MULTIDEVS_DEFAULT; - } + if (virXMLPropEnum(node, "multidevs", + virDomainFSMultidevsTypeFromString, + VIR_XML_PROP_NONE, + &def->multidevs) < 0) + goto error; if (virParseScaledValue("./space_hard_limit[1]", NULL, ctxt, &def->space_hard_limit, @@ -9901,11 +9881,10 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt, if (def->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) { g_autofree char *queue_size = virXPathString("string(./driver/@queue)", ctxt); g_autofree char *binary = virXPathString("string(./binary/@path)", ctxt); - g_autofree char *cache = virXPathString("string(./binary/cache/@mode)", ctxt); - g_autofree char *sandbox = virXPathString("string(./binary/sandbox/@mode)", ctxt); xmlNodePtr binary_node = virXPathNode("./binary", ctxt); xmlNodePtr binary_lock_node = virXPathNode("./binary/lock", ctxt); - int val; + xmlNodePtr binary_cache_node = virXPathNode("./binary/cache", ctxt); + xmlNodePtr binary_sandbox_node = virXPathNode("./binary/sandbox", ctxt); if (queue_size && virStrToLong_ull(queue_size, NULL, 10, &def->queue_size) < 0) { virReportError(VIR_ERR_XML_ERROR, @@ -9932,26 +9911,17 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt, &def->flock) < 0) goto error; - if (cache) { - if ((val = virDomainFSCacheModeTypeFromString(cache)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("cannot parse cache mode '%s' for virtiofs"), - cache); - goto error; - } - def->cache = val; - } - - if (sandbox) { - if ((val = virDomainFSSandboxModeTypeFromString(sandbox)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("cannot parse sandbox mode '%s' for virtiofs"), - sandbox); - goto error; - } - def->sandbox = val; - } + if (virXMLPropEnum(binary_cache_node, "mode", + virDomainFSCacheModeTypeFromString, + VIR_XML_PROP_NONZERO, + &def->cache) < 0) + goto error; + if (virXMLPropEnum(binary_sandbox_node, "mode", + virDomainFSSandboxModeTypeFromString, + VIR_XML_PROP_NONZERO, + &def->sandbox) < 0) + goto error; } if (source == NULL && def->type != VIR_DOMAIN_FS_TYPE_RAM @@ -24188,6 +24158,9 @@ virDomainFSDefFormat(virBuffer *buf, virBufferEscapeString(buf, " volume='%s'", def->src->srcpool->volume); virBufferAddLit(buf, "/>\n"); break; + + case VIR_DOMAIN_FS_TYPE_LAST: + break; } virBufferEscapeString(buf, "<target dir='%s'/>\n", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 584e15b6c7..bbb2f463e2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -787,7 +787,7 @@ struct _virDomainControllerDef { /* Types of disk backends */ typedef enum { - VIR_DOMAIN_FS_TYPE_MOUNT, /* Mounts (binds) a host dir on a guest dir */ + VIR_DOMAIN_FS_TYPE_MOUNT = 0, /* Mounts (binds) a host dir on a guest dir */ VIR_DOMAIN_FS_TYPE_BLOCK, /* Mounts a host block dev on a guest dir */ VIR_DOMAIN_FS_TYPE_FILE, /* Loopback mounts a host file on a guest dir */ VIR_DOMAIN_FS_TYPE_TEMPLATE, /* Expands a OS template to a guest dir */ @@ -867,15 +867,15 @@ typedef enum { } virDomainFSSandboxMode; struct _virDomainFSDef { - int type; + virDomainFSType type; virDomainFSDriverType fsdriver; - int accessmode; /* enum virDomainFSAccessMode */ + virDomainFSAccessMode accessmode; int format; /* virStorageFileFormat */ virDomainFSWrpolicy wrpolicy; - int model; /* virDomainFSModel */ + virDomainFSModel model; unsigned int fmode; unsigned int dmode; - int multidevs; /* virDomainFSMultidevs */ + virDomainFSMultidevs multidevs; unsigned long long usage; /* in bytes */ virStorageSource *src; char *sock; diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 3f38c55fc6..d5410e9fa6 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1447,6 +1447,9 @@ static int lxcContainerMountFS(virDomainFSDef *fs, _("Unexpected filesystem type %s"), virDomainFSTypeToString(fs->type)); return -1; + case VIR_DOMAIN_FS_TYPE_TEMPLATE: + case VIR_DOMAIN_FS_TYPE_VOLUME: + case VIR_DOMAIN_FS_TYPE_LAST: default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Cannot mount filesystem type %s"), diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 18fc34d049..3e6eed6ec9 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -705,7 +705,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev, case VIR_DOMAIN_FS_DRIVER_TYPE_PATH: case VIR_DOMAIN_FS_DRIVER_TYPE_HANDLE: /* these drivers are handled by virtio-9p-pci */ - switch ((virDomainFSModel) dev->data.fs->model) { + switch (dev->data.fs->model) { case VIR_DOMAIN_FS_MODEL_VIRTIO_TRANSITIONAL: /* Transitional devices only work in conventional PCI slots */ return pciFlags; -- 2.34.1

After previous cleanups, the virDomainNetDefParseXML() function uses a mixture of virXMLProp*() and the old virXMLPropString() + virXXXTypeFromString() patterns. Rework it so that virXMLProp*() is used. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 112 ++++++------------ src/conf/domain_conf.h | 4 +- .../qemuxml2argvdata/vhost_queues-invalid.err | 2 +- 3 files changed, 38 insertions(+), 80 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f4b05b1c21..a25beaee3e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10225,7 +10225,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, virDomainChrSourceReconnectDef reconnect = {0}; int rv, val; g_autofree char *macaddr = NULL; - g_autofree char *macaddr_type = NULL; g_autofree char *network = NULL; g_autofree char *portgroup = NULL; g_autofree char *portid = NULL; @@ -10242,11 +10241,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *localaddr = NULL; g_autofree char *localport = NULL; g_autofree char *model = NULL; - g_autofree char *backend = NULL; - g_autofree char *txmode = NULL; - g_autofree char *queues = NULL; - g_autofree char *rx_queue_size = NULL; - g_autofree char *tx_queue_size = NULL; g_autofree char *filter = NULL; g_autofree char *internal = NULL; g_autofree char *mode = NULL; @@ -10393,12 +10387,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, (virDomainVirtioOptionsParseXML(driver_node, &def->virtio) < 0)) goto error; - backend = virXMLPropString(driver_node, "name"); - txmode = virXMLPropString(driver_node, "txmode"); - queues = virXMLPropString(driver_node, "queues"); - rx_queue_size = virXMLPropString(driver_node, "rx_queue_size"); - tx_queue_size = virXMLPropString(driver_node, "tx_queue_size"); - if ((filterref_node = virXPathNode("./filterref", ctxt))) { filter = virXMLPropString(filterref_node, "filter"); filterparams = virNWFilterParseParamAttributes(filterref_node); @@ -10446,18 +10434,11 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, def->mac_generated = true; } - if ((macaddr_type = virXPathString("string(./mac/@type)", ctxt))) { - int tmp; - - if ((tmp = virDomainNetMacTypeTypeFromString(macaddr_type)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid mac address type value: '%s'. Valid " - "values are \"generated\" and \"static\"."), - macaddr_type); - goto error; - } - def->mac_type = tmp; - } + if (virXMLPropEnum(mac_node, "type", + virDomainNetMacTypeTypeFromString, + VIR_XML_PROP_NONZERO, + &def->mac_type) < 0) + goto error; if (virXMLPropTristateBool(mac_node, "check", VIR_XML_PROP_NONE, @@ -10732,28 +10713,18 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, if (def->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && virDomainNetIsVirtioModel(def)) { - if (backend != NULL) { - if ((val = virDomainNetBackendTypeFromString(backend)) < 0 || - val == VIR_DOMAIN_NET_BACKEND_TYPE_DEFAULT) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown interface <driver name='%s'> " - "has been specified"), - backend); - goto error; - } - def->driver.virtio.name = val; - } - if (txmode != NULL) { - if ((val = virDomainNetVirtioTxModeTypeFromString(txmode)) < 0 || - val == VIR_DOMAIN_NET_VIRTIO_TX_MODE_DEFAULT) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown interface <driver txmode='%s'> " - "has been specified"), - txmode); - goto error; - } - def->driver.virtio.txmode = val; - } + + if (virXMLPropEnum(driver_node, "name", + virDomainNetBackendTypeFromString, + VIR_XML_PROP_NONZERO, + &def->driver.virtio.name) < 0) + goto error; + + if (virXMLPropEnum(driver_node, "txmode", + virDomainNetVirtioTxModeTypeFromString, + VIR_XML_PROP_NONZERO, + &def->driver.virtio.txmode) < 0) + goto error; if (virXMLPropTristateSwitch(driver_node, "ioeventfd", VIR_XML_PROP_NONE, @@ -10765,37 +10736,24 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, &def->driver.virtio.event_idx) < 0) goto error; - if (queues) { - unsigned int q; - if (virStrToLong_uip(queues, NULL, 10, &q) < 0) { - virReportError(VIR_ERR_XML_DETAIL, - _("'queues' attribute must be positive number: %s"), - queues); - goto error; - } - if (q > 1) - def->driver.virtio.queues = q; - } - if (rx_queue_size) { - unsigned int q; - if (virStrToLong_uip(rx_queue_size, NULL, 10, &q) < 0) { - virReportError(VIR_ERR_XML_DETAIL, - _("'rx_queue_size' attribute must be positive number: %s"), - rx_queue_size); - goto error; - } - def->driver.virtio.rx_queue_size = q; - } - if (tx_queue_size) { - unsigned int q; - if (virStrToLong_uip(tx_queue_size, NULL, 10, &q) < 0) { - virReportError(VIR_ERR_XML_DETAIL, - _("'tx_queue_size' attribute must be positive number: %s"), - tx_queue_size); - goto error; - } - def->driver.virtio.tx_queue_size = q; - } + if (virXMLPropUInt(driver_node, "queues", 10, + VIR_XML_PROP_NONE, + &def->driver.virtio.queues) < 0) + goto error; + + /* There's always at least one TX/RX queue. */ + if (def->driver.virtio.queues == 1) + def->driver.virtio.queues = 0; + + if (virXMLPropUInt(driver_node, "rx_queue_size", 10, + VIR_XML_PROP_NONE, + &def->driver.virtio.rx_queue_size) < 0) + goto error; + + if (virXMLPropUInt(driver_node, "tx_queue_size", 10, + VIR_XML_PROP_NONE, + &def->driver.virtio.tx_queue_size) < 0) + goto error; if ((tmpNode = virXPathNode("./driver/host", ctxt))) { if (virXMLPropTristateSwitch(tmpNode, "csum", VIR_XML_PROP_NONE, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index bbb2f463e2..8a4d7b6f99 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -945,7 +945,7 @@ typedef enum { /* the backend driver used for virtio interfaces */ typedef enum { - VIR_DOMAIN_NET_BACKEND_TYPE_DEFAULT, /* prefer kernel, fall back to user */ + VIR_DOMAIN_NET_BACKEND_TYPE_DEFAULT = 0, /* prefer kernel, fall back to user */ VIR_DOMAIN_NET_BACKEND_TYPE_QEMU, /* userland */ VIR_DOMAIN_NET_BACKEND_TYPE_VHOST, /* kernel */ @@ -954,7 +954,7 @@ typedef enum { /* the TX algorithm used for virtio interfaces */ typedef enum { - VIR_DOMAIN_NET_VIRTIO_TX_MODE_DEFAULT, /* default for this version of qemu */ + VIR_DOMAIN_NET_VIRTIO_TX_MODE_DEFAULT = 0, /* default for this version of qemu */ VIR_DOMAIN_NET_VIRTIO_TX_MODE_IOTHREAD, VIR_DOMAIN_NET_VIRTIO_TX_MODE_TIMER, diff --git a/tests/qemuxml2argvdata/vhost_queues-invalid.err b/tests/qemuxml2argvdata/vhost_queues-invalid.err index e89358f0a3..10c2c81339 100644 --- a/tests/qemuxml2argvdata/vhost_queues-invalid.err +++ b/tests/qemuxml2argvdata/vhost_queues-invalid.err @@ -1 +1 @@ -'queues' attribute must be positive number: -5 +XML error: Invalid value for attribute 'queues' in element 'driver': '-5'. Expected non-negative integer value -- 2.34.1

After previous cleanups, the virNetworkPortDefParseXML() function uses a mixture of virXMLProp*() and the old virXMLPropString() + virXXXTypeFromString() patterns. Rework it so that virXMLProp*() is used. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/virnetworkportdef.c | 20 +++++++++----------- src/conf/virnetworkportdef.h | 2 +- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c index 1d75436085..fcd9e55a55 100644 --- a/src/conf/virnetworkportdef.c +++ b/src/conf/virnetworkportdef.c @@ -92,7 +92,6 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt) g_autofree char *mac = NULL; g_autofree char *macmgr = NULL; g_autofree char *mode = NULL; - g_autofree char *plugtype = NULL; g_autofree char *driver = NULL; def = g_new0(virNetworkPortDef, 1); @@ -176,13 +175,12 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt) return NULL; plugNode = virXPathNode("./plug", ctxt); - plugtype = virXPathString("string(./plug/@type)", ctxt); - if (plugtype && - (def->plugtype = virNetworkPortPlugTypeFromString(plugtype)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid network port plug type '%s'"), plugtype); - } + if (virXMLPropEnum(plugNode, "type", + virNetworkPortPlugTypeFromString, + VIR_XML_PROP_NONE, + &def->plugtype) < 0) + return NULL; switch (def->plugtype) { case VIR_NETWORK_PORT_PLUG_TYPE_NONE: @@ -190,12 +188,12 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt) case VIR_NETWORK_PORT_PLUG_TYPE_NETWORK: case VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE: - if (!(def->plug.bridge.brname = virXPathString("string(./plug/@bridge)", ctxt))) { + if (!(def->plug.bridge.brname = virXMLPropString(plugNode, "bridge"))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Missing network port bridge name")); return NULL; } - macmgr = virXPathString("string(./plug/@macTableManager)", ctxt); + macmgr = virXMLPropString(plugNode, "macTableManager"); if (macmgr && (def->plug.bridge.macTableManager = virNetworkBridgeMACTableManagerTypeFromString(macmgr)) <= 0) { @@ -207,12 +205,12 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt) break; case VIR_NETWORK_PORT_PLUG_TYPE_DIRECT: - if (!(def->plug.direct.linkdev = virXPathString("string(./plug/@dev)", ctxt))) { + if (!(def->plug.direct.linkdev = virXMLPropString(plugNode, "dev"))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Missing network port link device name")); return NULL; } - mode = virXPathString("string(./plug/@mode)", ctxt); + mode = virXMLPropString(plugNode, "mode"); if (mode && (def->plug.direct.mode = virNetDevMacVLanModeTypeFromString(mode)) < 0) { diff --git a/src/conf/virnetworkportdef.h b/src/conf/virnetworkportdef.h index c6474c4ad8..ee13f8a084 100644 --- a/src/conf/virnetworkportdef.h +++ b/src/conf/virnetworkportdef.h @@ -58,7 +58,7 @@ struct _virNetworkPortDef { virTristateBool trustGuestRxFilters; virTristateBool isolatedPort; - int plugtype; /* virNetworkPortPlugType */ + virNetworkPortPlugType plugtype; union { struct { char *brname; -- 2.34.1

After previous cleanups, the virDomainHostdevDefParseXMLSubsys() function uses a mixture of virXMLProp*() and the old virXMLPropString() + virXXXTypeFromString() patterns. Rework it so that virXMLProp*() is used. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 60 ++++++++++++----------------------- src/conf/domain_conf.h | 26 +++++++-------- src/qemu/qemu_command.c | 4 +-- src/qemu/qemu_hostdev.c | 4 +-- src/qemu/qemu_hotplug.c | 3 +- src/qemu/qemu_validate.c | 2 +- src/security/virt-aa-helper.c | 2 +- 7 files changed, 42 insertions(+), 59 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a25beaee3e..f97bd7bf22 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7293,14 +7293,12 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, virDomainXMLOption *xmlopt) { xmlNodePtr sourcenode; - int backend; + xmlNodePtr driver_node = NULL; virDomainHostdevSubsysPCI *pcisrc = &def->source.subsys.u.pci; virDomainHostdevSubsysSCSI *scsisrc = &def->source.subsys.u.scsi; virDomainHostdevSubsysSCSIVHost *scsihostsrc = &def->source.subsys.u.scsi_host; virDomainHostdevSubsysMediatedDev *mdevsrc = &def->source.subsys.u.mdev; virTristateBool managed; - g_autofree char *sgio = NULL; - g_autofree char *backendStr = NULL; g_autofree char *model = NULL; int rv; @@ -7313,7 +7311,6 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, VIR_XML_PROP_NONE, &managed)); virTristateBoolToBool(managed, &def->managed); - sgio = virXMLPropString(node, "sgio"); model = virXMLPropString(node, "model"); /* @type is passed in from the caller rather than read from the @@ -7352,18 +7349,17 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, return -1; } - if (sgio) { + if ((rv = virXMLPropEnum(node, "sgio", + virDomainDeviceSGIOTypeFromString, + VIR_XML_PROP_NONZERO, + &scsisrc->sgio)) < 0) { + return -1; + } else if (rv > 0) { if (def->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { virReportError(VIR_ERR_XML_ERROR, "%s", _("sgio is only supported for scsi host device")); return -1; } - - if ((scsisrc->sgio = virDomainDeviceSGIOTypeFromString(sgio)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown sgio mode '%s'"), sgio); - return -1; - } } if ((rv = virXMLPropTristateBool(node, "rawio", @@ -7389,27 +7385,17 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, } if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST) { - if (model && - ((scsihostsrc->model = virDomainHostdevSubsysSCSIVHostModelTypeFromString(model)) < 0)) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown hostdev model '%s'"), - model); + if (virXMLPropEnum(node, "model", + virDomainHostdevSubsysSCSIVHostModelTypeFromString, + VIR_XML_PROP_NONE, + &scsihostsrc->model) < 0) return -1; - } } else if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) { - if (!model) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Missing 'model' attribute in mediated device's " - "<hostdev> element")); + if (virXMLPropEnum(node, "model", + virMediatedDeviceModelTypeFromString, + VIR_XML_PROP_REQUIRED, + &mdevsrc->model) < 0) return -1; - } - - if ((mdevsrc->model = virMediatedDeviceModelTypeFromString(model)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown hostdev model '%s'"), - model); - return -1; - } if (virXMLPropTristateSwitch(node, "display", VIR_XML_PROP_NONE, @@ -7427,16 +7413,12 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, if (virDomainHostdevSubsysPCIDefParseXML(sourcenode, ctxt, def, flags) < 0) return -1; - backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT; - if ((backendStr = virXPathString("string(./driver/@name)", ctxt)) && - (((backend = virDomainHostdevSubsysPCIBackendTypeFromString(backendStr)) < 0) || - backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown PCI device <driver name='%s'/> " - "has been specified"), backendStr); + driver_node = virXPathNode("./driver", ctxt); + if (virXMLPropEnum(driver_node, "name", + virDomainHostdevSubsysPCIBackendTypeFromString, + VIR_XML_PROP_NONZERO, + &pcisrc->backend) < 0) return -1; - } - pcisrc->backend = backend; break; @@ -30564,7 +30546,7 @@ virDomainNetDefActualToNetworkPort(virDomainDef *dom, } port->plug.hostdevpci.managed = virTristateBoolFromBool(actual->data.hostdev.def.managed); port->plug.hostdevpci.addr = actual->data.hostdev.def.source.subsys.u.pci.addr; - switch ((virDomainHostdevSubsysPCIBackendType)actual->data.hostdev.def.source.subsys.u.pci.backend) { + switch (actual->data.hostdev.def.source.subsys.u.pci.backend) { case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: port->plug.hostdevpci.driver = VIR_NETWORK_FORWARD_DRIVER_NAME_DEFAULT; break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8a4d7b6f99..3e63d2513b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -214,7 +214,7 @@ typedef enum { /* the backend driver used for PCI hostdev devices */ typedef enum { - VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, /* detect automatically, prefer VFIO */ + VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT = 0, /* detect automatically, prefer VFIO */ VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM, /* force legacy kvm style */ VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO, /* force vfio */ VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN, /* force legacy xen style, use pciback */ @@ -245,7 +245,7 @@ struct _virDomainHostdevSubsysUSB { struct _virDomainHostdevSubsysPCI { virPCIDeviceAddress addr; /* host address */ - int backend; /* enum virDomainHostdevSubsysPCIBackendType */ + virDomainHostdevSubsysPCIBackendType backend; }; struct _virDomainHostdevSubsysSCSIHost { @@ -260,9 +260,17 @@ struct _virDomainHostdevSubsysSCSIiSCSI { virStorageSource *src; }; +typedef enum { + VIR_DOMAIN_DEVICE_SGIO_DEFAULT = 0, + VIR_DOMAIN_DEVICE_SGIO_FILTERED, + VIR_DOMAIN_DEVICE_SGIO_UNFILTERED, + + VIR_DOMAIN_DEVICE_SGIO_LAST +} virDomainDeviceSGIO; + struct _virDomainHostdevSubsysSCSI { int protocol; /* enum virDomainHostdevSCSIProtocolType */ - int sgio; /* enum virDomainDeviceSGIO */ + virDomainDeviceSGIO sgio; virTristateBool rawio; union { virDomainHostdevSubsysSCSIHost host; @@ -271,7 +279,7 @@ struct _virDomainHostdevSubsysSCSI { }; struct _virDomainHostdevSubsysMediatedDev { - int model; /* enum virMediatedDeviceModelType */ + virMediatedDeviceModelType model; virTristateSwitch display; char uuidstr[VIR_UUID_STRING_BUFLEN]; /* mediated device's uuid string */ virTristateSwitch ramfb; @@ -300,7 +308,7 @@ VIR_ENUM_DECL(virDomainHostdevSubsysSCSIVHostModel); struct _virDomainHostdevSubsysSCSIVHost { int protocol; /* enum virDomainHostdevSubsysSCSIHostProtocolType */ char *wwpn; - int model; /* enum virDomainHostdevSubsysSCSIVHostModelType */ + virDomainHostdevSubsysSCSIVHostModelType model; }; struct _virDomainHostdevSubsys { @@ -449,14 +457,6 @@ typedef enum { VIR_DOMAIN_DISK_IO_LAST } virDomainDiskIo; -typedef enum { - VIR_DOMAIN_DEVICE_SGIO_DEFAULT = 0, - VIR_DOMAIN_DEVICE_SGIO_FILTERED, - VIR_DOMAIN_DEVICE_SGIO_UNFILTERED, - - VIR_DOMAIN_DEVICE_SGIO_LAST -} virDomainDeviceSGIO; - typedef enum { VIR_DOMAIN_DISK_DISCARD_DEFAULT = 0, VIR_DOMAIN_DISK_DISCARD_UNMAP, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 99ac44d7f1..f5d7cd2c1b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4967,7 +4967,7 @@ qemuBuildPCIHostdevDevProps(const virDomainDef *def, const char *failover_pair_id = NULL; /* caller has to assign proper passthrough backend type */ - switch ((virDomainHostdevSubsysPCIBackendType) pcisrc->backend) { + switch (pcisrc->backend) { case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO: break; @@ -5480,7 +5480,7 @@ qemuBuildHostdevCommandLine(virCommand *cmd, /* MDEV */ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: - switch ((virMediatedDeviceModelType) mdevsrc->model) { + switch (mdevsrc->model) { case VIR_MDEV_MODEL_TYPE_VFIO_PCI: case VIR_MDEV_MODEL_TYPE_VFIO_CCW: case VIR_MDEV_MODEL_TYPE_VFIO_AP: diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index dfe657c51e..8af22bdd58 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -169,14 +169,14 @@ qemuHostdevPreparePCIDevicesCheckSupport(virDomainHostdevDef **hostdevs, /* assign defaults for hostdev passthrough */ for (i = 0; i < nhostdevs; i++) { virDomainHostdevDef *hostdev = hostdevs[i]; - int *backend = &hostdev->source.subsys.u.pci.backend; + virDomainHostdevSubsysPCIBackendType *backend = &hostdev->source.subsys.u.pci.backend; if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) continue; if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) continue; - switch ((virDomainHostdevSubsysPCIBackendType)*backend) { + switch (*backend) { case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: if (supportsPassthroughVFIO && virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f3ec24635d..22acbd0852 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1622,7 +1622,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriver *driver, /* this could have been changed by qemuHostdevPreparePCIDevices */ backend = hostdev->source.subsys.u.pci.backend; - switch ((virDomainHostdevSubsysPCIBackendType)backend) { + switch (backend) { case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO: if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -2814,6 +2814,7 @@ qemuDomainAttachMediatedDevice(virQEMUDriver *driver, if (qemuDomainEnsureVirtioAddress(&releaseaddr, vm, &dev) < 0) return -1; } break; + case VIR_MDEV_MODEL_TYPE_VFIO_AP: case VIR_MDEV_MODEL_TYPE_LAST: break; } diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 7eaad3614e..0a879f0115 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2357,7 +2357,7 @@ qemuValidateDomainMdevDef(const virDomainHostdevDef *hostdev, const virDomainHostdevSubsysMediatedDev *mdevsrc; mdevsrc = &hostdev->source.subsys.u.mdev; - switch ((virMediatedDeviceModelType) mdevsrc->model) { + switch (mdevsrc->model) { case VIR_MDEV_MODEL_TYPE_VFIO_PCI: return qemuValidateDomainMdevDefVFIOPCI(hostdev, def, qemuCaps); case VIR_MDEV_MODEL_TYPE_VFIO_AP: diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 28717b7e38..1f1cce8b3d 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1070,7 +1070,7 @@ get_files(vahControl * ctl) case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: { virDomainHostdevSubsysMediatedDev *mdevsrc = &dev->source.subsys.u.mdev; - switch ((virMediatedDeviceModelType) mdevsrc->model) { + switch (mdevsrc->model) { case VIR_MDEV_MODEL_TYPE_VFIO_PCI: case VIR_MDEV_MODEL_TYPE_VFIO_AP: case VIR_MDEV_MODEL_TYPE_VFIO_CCW: -- 2.34.1

On a Friday in 2022, Michal Privoznik wrote:
*** BLURB HERE ***
Michal Prívozník (12): virxml: Extend virXMLPropU{Int,LongLong}() error message virNetworkPortDefParseXML: Fix a typo in an error message qemuValidateDomainDeviceDefFS: Use correct enum for fs->multidevs comparison qemu: Use virTristateBool instead of virTristateSwitch in a few places lib: Eliminate use of virTristateSwitchTypeFromString() lib: Almost eliminate use of virTristateBoolTypeFromString() conf: Convert virCPUDefParseXML() to virXMLProp*() conf: Convert virDomainDefParseBootXML() to virXMLProp*() conf: Convert virDomainFSDefParseXML() to virXMLProp*() conf: Convert virDomainNetDefParseXML() to virXMLProp*() conf: Convert virNetworkPortDefParseXML() to virXMLProp*() conf: Convert virDomainHostdevDefParseXMLSubsys() to virXMLProp*()
src/bhyve/bhyve_command.c | 2 +- src/conf/cpu_conf.c | 76 +-- src/vz/vz_sdk.c | 4 +- [..] .../qemuxml2argvdata/vhost_queues-invalid.err | 2 +- 29 files changed, 426 insertions(+), 726 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko
-
Michal Privoznik