[PATCH 0/2] conf: Convert 'tray_status' and 'startupPolicy' of virDomainDiskDef to enums

I was testing how virXMLPropEnum works when used on converted types, so I've converted these. Peter Krempa (2): conf: domain: Convert virDomainDiskDef's 'tray_status' to virDomainDiskTray conf: domain: Convert virDomainDiskDef's 'startupPolicy' to virDomainStartupPolicy src/conf/domain_conf.c | 31 +++++++------------------------ src/conf/domain_conf.h | 4 ++-- 2 files changed, 9 insertions(+), 26 deletions(-) -- 2.30.2

Use the appropriate type for the variable and refactor the XML parser to parse it correctly using virXMLPropEnum. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 13 +++---------- src/conf/domain_conf.h | 2 +- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 858ef5db9d..0128c9d480 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9344,7 +9344,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *bus = NULL; g_autofree char *serial = NULL; g_autofree char *startupPolicy = NULL; - g_autofree char *tray = NULL; g_autofree char *removable = NULL; g_autofree char *logical_block_size = NULL; g_autofree char *physical_block_size = NULL; @@ -9415,7 +9414,9 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, virXMLNodeNameEqual(cur, "target")) { target = virXMLPropString(cur, "dev"); bus = virXMLPropString(cur, "bus"); - tray = virXMLPropString(cur, "tray"); + if (virXMLPropEnum(cur, "tray", virDomainDiskTrayTypeFromString, + VIR_XML_PROP_OPTIONAL, &def->tray_status) < 0) + return NULL; removable = virXMLPropString(cur, "removable"); rotation_rate = virXMLPropString(cur, "rotation_rate"); @@ -9634,14 +9635,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, } } - if (tray) { - if ((def->tray_status = virDomainDiskTrayTypeFromString(tray)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown disk tray status '%s'"), tray); - return NULL; - } - } - if (removable) { if ((def->removable = virTristateSwitchTypeFromString(removable)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f610e78e3a..2f8ef74020 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -537,7 +537,7 @@ struct _virDomainDiskDef { int device; /* enum virDomainDiskDevice */ int bus; /* enum virDomainDiskBus */ char *dst; - int tray_status; /* enum virDomainDiskTray */ + virDomainDiskTray tray_status; int removable; /* enum virTristateSwitch */ unsigned int rotation_rate; -- 2.30.2

Use the appropriate type for the variable and refactor the XML parser to parse it correctly using virXMLPropEnum. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 18 ++++-------------- src/conf/domain_conf.h | 2 +- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0128c9d480..0bb58ccb5f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9343,7 +9343,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *target = NULL; g_autofree char *bus = NULL; g_autofree char *serial = NULL; - g_autofree char *startupPolicy = NULL; g_autofree char *removable = NULL; g_autofree char *logical_block_size = NULL; g_autofree char *physical_block_size = NULL; @@ -9401,7 +9400,10 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, source = true; - startupPolicy = virXMLPropString(cur, "startupPolicy"); + if (virXMLPropEnum(cur, "startupPolicy", + virDomainStartupPolicyTypeFromString, + VIR_XML_PROP_OPTIONAL, &def->startupPolicy) < 0) + return NULL; if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && (tmp = virXMLPropString(cur, "index")) && @@ -9655,18 +9657,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, return NULL; } - if (startupPolicy) { - int val; - - if ((val = virDomainStartupPolicyTypeFromString(startupPolicy)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown startupPolicy value '%s'"), - startupPolicy); - return NULL; - } - def->startupPolicy = val; - } - def->dst = g_steal_pointer(&target); if (authdef) { /* If we've already parsed <source> and found an <auth> child, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2f8ef74020..99ab2a96d9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -573,7 +573,7 @@ struct _virDomainDiskDef { virTristateSwitch event_idx; virTristateSwitch copy_on_read; int snapshot; /* virDomainSnapshotLocation, snapshot_conf.h */ - int startupPolicy; /* enum virDomainStartupPolicy */ + virDomainStartupPolicy startupPolicy; bool transient; virDomainDeviceInfo info; int rawio; /* enum virTristateBool */ -- 2.30.2

On Fri, 2021-04-16 at 13:50 +0200, Peter Krempa wrote:
I was testing how virXMLPropEnum works when used on converted types, so I've converted these.
Peter Krempa (2): conf: domain: Convert virDomainDiskDef's 'tray_status' to virDomainDiskTray conf: domain: Convert virDomainDiskDef's 'startupPolicy' to virDomainStartupPolicy
src/conf/domain_conf.c | 31 +++++++------------------------ src/conf/domain_conf.h | 4 ++-- 2 files changed, 9 insertions(+), 26 deletions(-)
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>

On Fri, Apr 16, 2021 at 14:22:32 +0200, Tim Wiederhake wrote:
On Fri, 2021-04-16 at 13:50 +0200, Peter Krempa wrote:
I was testing how virXMLPropEnum works when used on converted types, so I've converted these.
Peter Krempa (2): conf: domain: Convert virDomainDiskDef's 'tray_status' to virDomainDiskTray conf: domain: Convert virDomainDiskDef's 'startupPolicy' to virDomainStartupPolicy
Actually startupPolicy is checked as <= in the original code, so consider VIR_XML_PROP_NONZERO added in that case.
src/conf/domain_conf.c | 31 +++++++------------------------ src/conf/domain_conf.h | 4 ++-- 2 files changed, 9 insertions(+), 26 deletions(-)
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
participants (2)
-
Peter Krempa
-
Tim Wiederhake