The virDomainDeviceAddressParseXML() function uses old style of parsing
XML (virXMLPropString + str2enum conversion). Use
virXMLPropEnumDefault() which encapsulates those steps.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/conf/device_conf.c | 12 ++++++++-
src/conf/device_conf.h | 4 +--
src/conf/domain_conf.c | 22 +++++-----------
src/conf/domain_validate.c | 2 +-
src/qemu/qemu_command.c | 4 +--
src/qemu/qemu_monitor.c | 54 +++++++++++++++++++++++---------------
src/qemu/qemu_validate.c | 2 +-
7 files changed, 57 insertions(+), 43 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index cb523d3a0d..f5270a54ea 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -100,7 +100,7 @@ virDomainDeviceInfoAddressIsEqual(const virDomainDeviceInfo *a,
if (a->type != b->type)
return false;
- switch ((virDomainDeviceAddressType) a->type) {
+ switch (a->type) {
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE:
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST:
/* address types below don't have any specific data */
@@ -457,6 +457,16 @@ virDomainDeviceAddressIsValid(virDomainDeviceInfo *info,
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
return true;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST:
+ return false;
}
return false;
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index b6b710d313..d8f4ca4f17 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -32,7 +32,7 @@
#include "virenum.h"
typedef enum {
- VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE,
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE = 0,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL,
@@ -128,7 +128,7 @@ struct _virDomainDeviceDimmAddress {
typedef struct _virDomainDeviceInfo virDomainDeviceInfo;
struct _virDomainDeviceInfo {
char *alias;
- int type; /* virDomainDeviceAddressType */
+ virDomainDeviceAddressType type;
union {
virPCIDeviceAddress pci;
virDomainDeviceDriveAddress drive;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6ff994dd12..a9c424e71d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6536,7 +6536,7 @@ virDomainDeviceInfoFormat(virBuffer *buf,
virBufferAsprintf(&attrBuf, " type='%s'",
virDomainDeviceAddressTypeToString(info->type));
- switch ((virDomainDeviceAddressType) info->type) {
+ switch (info->type) {
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
if (!virPCIDeviceAddressIsEmpty(&info->addr.pci)) {
virBufferAsprintf(&attrBuf, " domain='0x%04x' bus='0x%02x' "
@@ -6709,21 +6709,13 @@ static int
virDomainDeviceAddressParseXML(xmlNodePtr address,
virDomainDeviceInfo *info)
{
- g_autofree char *type = virXMLPropString(address, "type");
-
- if (type) {
- if ((info->type = virDomainDeviceAddressTypeFromString(type)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown address type '%s'"), type);
- return -1;
- }
- } else {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("No type specified for device address"));
+ if (virXMLPropEnum(address, "type",
+ virDomainDeviceAddressTypeFromString,
+ VIR_XML_PROP_NONZERO | VIR_XML_PROP_REQUIRED,
+ &info->type) < 0)
return -1;
- }
- switch ((virDomainDeviceAddressType) info->type) {
+ switch (info->type) {
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
if (virPCIDeviceAddressParseXML(address, &info->addr.pci) < 0)
return -1;
@@ -20658,7 +20650,7 @@ virDomainDeviceInfoCheckABIStability(virDomainDeviceInfo *src,
return false;
}
- switch ((virDomainDeviceAddressType) src->type) {
+ switch (src->type) {
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
if (src->addr.pci.domain != dst->addr.pci.domain ||
src->addr.pci.bus != dst->addr.pci.bus ||
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 627c366fe9..28234f910d 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -2459,7 +2459,7 @@ virDomainDeviceInfoValidate(const virDomainDeviceDef *dev)
if (!(info = virDomainDeviceGetInfo(dev)))
return 0;
- switch ((virDomainDeviceAddressType) info->type) {
+ switch (info->type) {
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE:
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 33069ea131..7eb7def747 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -543,7 +543,7 @@ qemuBuildDeviceAddressProps(virJSONValue *props,
const virDomainDef *domainDef,
const virDomainDeviceInfo *info)
{
- switch ((virDomainDeviceAddressType) info->type) {
+ switch (info->type) {
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: {
g_autofree char *pciaddr = NULL;
g_autofree char *bus = qemuBuildDeviceAddressPCIGetBus(domainDef, info);
@@ -982,7 +982,7 @@ qemuBuildVirtioDevGetConfig(const virDomainDeviceDef *device,
virBufferAdd(&buf, baseName, -1);
- switch ((virDomainDeviceAddressType) info->type) {
+ switch (info->type) {
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
implName = "pci";
break;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index d44c7f0c60..3e4dd0d504 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -946,7 +946,7 @@ qemuMonitorInitBalloonObjectPath(qemuMonitor *mon,
{
ssize_t i, nprops = 0;
char *path = NULL;
- const char *name;
+ const char *name = NULL;
qemuMonitorJSONListPath **bprops = NULL;
if (mon->balloonpath) {
@@ -961,31 +961,43 @@ qemuMonitorInitBalloonObjectPath(qemuMonitor *mon,
switch (balloon->info.type) {
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
switch (balloon->model) {
- case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO:
- name = "virtio-balloon-pci";
- break;
- case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO_TRANSITIONAL:
- name = "virtio-balloon-pci-transitional";
- break;
- case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO_NON_TRANSITIONAL:
- name = "virtio-balloon-pci-non-transitional";
- break;
- case VIR_DOMAIN_MEMBALLOON_MODEL_XEN:
- case VIR_DOMAIN_MEMBALLOON_MODEL_NONE:
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("invalid model for virtio-balloon-pci"));
- return;
- case VIR_DOMAIN_MEMBALLOON_MODEL_LAST:
- default:
- virReportEnumRangeError(virDomainMemballoonModel,
- balloon->model);
- return;
+ case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO:
+ name = "virtio-balloon-pci";
+ break;
+ case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO_TRANSITIONAL:
+ name = "virtio-balloon-pci-transitional";
+ break;
+ case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO_NON_TRANSITIONAL:
+ name = "virtio-balloon-pci-non-transitional";
+ break;
+ case VIR_DOMAIN_MEMBALLOON_MODEL_XEN:
+ case VIR_DOMAIN_MEMBALLOON_MODEL_NONE:
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("invalid model for virtio-balloon-pci"));
+ return;
+ case VIR_DOMAIN_MEMBALLOON_MODEL_LAST:
+ default:
+ virReportEnumRangeError(virDomainMemballoonModel,
+ balloon->model);
+ return;
}
break;
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW:
name = "virtio-balloon-ccw";
break;
- default:
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST:
return;
}
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 9b6245e6d7..e8830c4cd3 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1399,7 +1399,7 @@ qemuValidateDomainDeviceDefAddress(const virDomainDeviceDef *dev,
const virDomainDef *def,
virQEMUCaps *qemuCaps)
{
- switch ((virDomainDeviceAddressType) info->type) {
+ switch (info->type) {
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
if (qemuValidateDomainDeviceDefZPCIAddress(info, qemuCaps) < 0)
return -1;
--
2.35.1On 5/23/22 3:08 PM, Michal Privoznik wrote:
> @@ -6709,21 +6709,13 @@ static int
> virDomainDeviceAddressParseXML(xmlNodePtr address,
> virDomainDeviceInfo *info)
> {
> - g_autofree char *type = virXMLPropString(address, "type");
> -
> - if (type) {
> - if ((info->type = virDomainDeviceAddressTypeFromString(type)) <= 0) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - _("unknown address type '%s'"), type);
> - return -1;
> - }
> - } else {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("No type specified for device address"));
> + if (virXMLPropEnum(address, "type",
> + virDomainDeviceAddressTypeFromString,
> + VIR_XML_PROP_NONZERO | VIR_XML_PROP_REQUIRED,
> + &info->type) < 0)
> return -1;
> - }
>
For my taste the resulting generic error messages look a bit different
and besides the different message texts the error numbers change:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown address type '%s'"), type);
turns into
virReportError(VIR_ERR_XML_ERROR,
_("Invalid value for attribute '%s' in element '%s':
'%s'."),
name, node->name, NULLSTR(tmp));
and
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("No type specified for device address"));
turns into
virReportError(VIR_ERR_XML_ERROR,
_("Missing required attribute '%s' in element '%s'"),
name, node->name);
Don't changes like this have an impact on the user?
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
On 5/24/22 13:50, Boris Fiuczynski wrote:
> On 5/23/22 3:08 PM, Michal Privoznik wrote:
>> @@ -6709,21 +6709,13 @@ static int
>> virDomainDeviceAddressParseXML(xmlNodePtr address,
>> virDomainDeviceInfo *info)
>> {
>> - g_autofree char *type = virXMLPropString(address, "type");
>> -
>> - if (type) {
>> - if ((info->type = virDomainDeviceAddressTypeFromString(type))
>> <= 0) {
>> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> - _("unknown address type '%s'"), type);
>> - return -1;
>> - }
>> - } else {
>> - virReportError(VIR_ERR_INTERNAL_ERROR,
>> - "%s", _("No type specified for device address"));
>> + if (virXMLPropEnum(address, "type",
>> + virDomainDeviceAddressTypeFromString,
>> + VIR_XML_PROP_NONZERO | VIR_XML_PROP_REQUIRED,
>> + &info->type) < 0)
>> return -1;
>> - }
>>
>
> For my taste the resulting generic error messages look a bit different
> and besides the different message texts the error numbers change:
>
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("unknown address type '%s'"), type);
> turns into
> virReportError(VIR_ERR_XML_ERROR,
> _("Invalid value for attribute '%s' in element '%s':
> '%s'."),
> name, node->name, NULLSTR(tmp));
>
> and
>
> virReportError(VIR_ERR_INTERNAL_ERROR,
> "%s", _("No type specified for device address"));
> turns into
> virReportError(VIR_ERR_XML_ERROR,
> _("Missing required attribute '%s' in element '%s'"),
> name, node->name);
>
> Don't changes like this have an impact on the user?
>
They do. We don't really guarantee error codes stability though. But
okay, let me postpone pushing these until there's a broader agreement.
Michal
© 2016 - 2026 Red Hat, Inc.