[PATCH REBASE 00/17] Use virXMLPropEnum() more (part I)

This is rebased version of: https://listman.redhat.com/archives/libvir-list/2022-April/229941.html Michal Prívozník (17): Drop needless typecast to virStorageType enum virStorageSourceGetActualType: Change type of retval virDomainBackupDefParse: Switch to virXMLPropEnumDefault() virDomainDeviceAddressParseXML: Switch to virXMLPropEnumDefault() virDomainStorageNetworkParseHost: Switch to virXMLPropEnumDefault() virDomainHostdevSubsysSCSIDefParseXML: Switch to virXMLPropEnumDefault() virDomainHostdevSubsysSCSIVHostDefParseXML: Switch to virXMLPropEnumDefault() virDomainDiskSourceNVMeParse: Switch to virXMLPropEnumDefault() virDomainDiskDefMirrorParse: Switch to virXMLPropEnumDefault() virDomainDiskSourcePoolDefParse: Switch to virXMLPropEnumDefault() virDomainDiskDefParseSourceXML: Switch to virXMLPropEnumDefault() virDomainChrDefParseXML: Switch to virXMLPropEnumDefault() virDomainTPMDefParseXML: Switch to virXMLPropEnumDefault() virDomainPanicDefParseXML: Switch to virXMLPropEnumDefault() virDomainInputDefParseXML: Switch to virXMLPropEnumDefault() virDomainHubDefParseXML: Switch to virXMLPropEnumDefault() virDomainTimerDefParseXML: Switch to virXMLPropEnumDefault() src/ch/ch_monitor.c | 2 +- src/conf/backup_conf.c | 16 +- src/conf/backup_conf.h | 2 +- src/conf/device_conf.c | 12 +- src/conf/device_conf.h | 4 +- src/conf/domain_audit.c | 2 +- src/conf/domain_conf.c | 494 ++++++------------ src/conf/domain_conf.h | 54 +- src/conf/domain_validate.c | 6 +- src/conf/storage_source_conf.c | 2 +- src/conf/storage_source_conf.h | 8 +- src/conf/virchrdev.c | 29 + src/libxl/libxl_conf.c | 32 +- src/libxl/libxl_domain.c | 2 +- src/libxl/xen_common.c | 29 +- src/libxl/xen_xl.c | 7 +- src/libxl/xen_xm.c | 3 + src/locking/domain_lock.c | 2 +- src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/qemu/qemu_backup.c | 2 +- src/qemu/qemu_block.c | 14 +- src/qemu/qemu_cgroup.c | 12 + src/qemu/qemu_command.c | 49 +- src/qemu/qemu_domain.c | 44 +- src/qemu/qemu_domain_address.c | 4 +- src/qemu/qemu_driver.c | 4 +- src/qemu/qemu_hotplug.c | 4 +- src/qemu/qemu_migration.c | 6 +- src/qemu/qemu_monitor.c | 54 +- src/qemu/qemu_monitor_json.c | 4 +- src/qemu/qemu_process.c | 10 +- src/qemu/qemu_snapshot.c | 16 +- src/qemu/qemu_validate.c | 19 +- src/security/security_apparmor.c | 6 +- src/security/security_dac.c | 8 +- src/security/security_selinux.c | 28 +- .../storage_file_backend_gluster.c | 2 +- src/storage_file/storage_source.c | 4 +- .../storage_source_backingstore.c | 16 +- src/vmx/vmx.c | 26 + tests/qemuxml2argvtest.c | 2 +- tests/testutilsqemu.c | 2 +- 43 files changed, 557 insertions(+), 489 deletions(-) -- 2.35.1

There are three places (two in domain_conf.c and one in qemu_migration.c) where a virStorageSource->type is typecasted to virStorageType (for the purpose of catching missing enum member in a switch() statement at compile time). This is needless, because as of v8.2.0-rc1~120 the struct member is of proper type. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 4 ++-- src/qemu/qemu_migration.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0b30ebc96b..6ff994dd12 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8659,7 +8659,7 @@ virDomainStorageSourceParse(xmlNodePtr node, ctxt->node = node; - switch ((virStorageType)src->type) { + switch (src->type) { case VIR_STORAGE_TYPE_FILE: src->path = virXMLPropString(node, "file"); break; @@ -23350,7 +23350,7 @@ virDomainDiskSourceFormat(virBuffer *buf, g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); - switch ((virStorageType)src->type) { + switch (src->type) { case VIR_STORAGE_TYPE_FILE: virBufferEscapeString(&attrBuf, " file='%s'", src->path); break; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 38596fa4de..176c25cd11 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -182,7 +182,7 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn, VIR_DEBUG("Precreate disk type=%s", virStorageTypeToString(disk->src->type)); - switch ((virStorageType)disk->src->type) { + switch (disk->src->type) { case VIR_STORAGE_TYPE_FILE: if (!virDomainDiskGetSource(disk)) { VIR_DEBUG("Dropping sourceless disk '%s'", -- 2.35.1

On 5/23/22 3:08 PM, Michal Privoznik wrote:
There are three places (two in domain_conf.c and one in qemu_migration.c) where a virStorageSource->type is typecasted to virStorageType (for the purpose of catching missing enum member in a switch() statement at compile time). This is needless, because as of v8.2.0-rc1~120 the struct member is of proper type.
Signed-off-by: Michal Privoznik<mprivozn@redhat.com> ---
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> -- 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

The virStorageSourceGetActualType() function returns either virStorageSource->type (which is of type virStorageType), or virStorageSourcePoolDef->type, which really stores a value of the same enum. Thus, the latter struct can be changed so that the virStorageSourceGetActualType() function can return correct type instead of generic int. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/storage_source_conf.c | 2 +- src/conf/storage_source_conf.h | 4 ++-- src/libxl/libxl_conf.c | 2 +- src/libxl/libxl_domain.c | 2 +- src/libxl/xen_xl.c | 4 ++-- src/locking/domain_lock.c | 2 +- src/qemu/qemu_block.c | 12 ++++++------ src/qemu/qemu_command.c | 8 ++++---- src/qemu/qemu_domain.c | 4 ++-- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_migration.c | 4 ++-- src/qemu/qemu_snapshot.c | 16 ++++++++-------- src/storage_file/storage_source.c | 4 ++-- 13 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c index 1a7284ec12..2b4cf5e241 100644 --- a/src/conf/storage_source_conf.c +++ b/src/conf/storage_source_conf.c @@ -1004,7 +1004,7 @@ virStorageSourcePoolDefFree(virStorageSourcePoolDef *def) * and virDomainDiskTranslateSourcePool was called on @def the actual type * of the storage volume is returned rather than VIR_STORAGE_TYPE_VOLUME. */ -int +virStorageType virStorageSourceGetActualType(const virStorageSource *def) { if (def->type == VIR_STORAGE_TYPE_VOLUME && diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h index 035ea7016d..f2440cec6a 100644 --- a/src/conf/storage_source_conf.h +++ b/src/conf/storage_source_conf.h @@ -199,7 +199,7 @@ struct _virStorageSourcePoolDef { char *volume; /* volume name */ int voltype; /* virStorageVolType, internal only */ int pooltype; /* virStoragePoolType from storage_conf.h, internal only */ - int actualtype; /* virStorageType, internal only */ + virStorageType actualtype; /* internal only */ int mode; /* virStorageSourcePoolMode, currently makes sense only for iscsi pool */ }; @@ -469,7 +469,7 @@ virStorageSourcePoolDefFree(virStorageSourcePoolDef *def); void virStorageSourceClear(virStorageSource *def); -int +virStorageType virStorageSourceGetActualType(const virStorageSource *def); bool diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 726ecdc945..401e47344e 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1067,7 +1067,7 @@ libxlMakeDisk(virDomainDiskDef *l_disk, libxl_device_disk *x_disk) { const char *driver = virDomainDiskGetDriver(l_disk); int format = virDomainDiskGetFormat(l_disk); - int actual_type = virStorageSourceGetActualType(l_disk->src); + virStorageType actual_type = virStorageSourceGetActualType(l_disk->src); if (actual_type == VIR_STORAGE_TYPE_NETWORK) { if (STRNEQ_NULLABLE(driver, "qemu")) { diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index af938192a9..49ac09d8a4 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -330,7 +330,7 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDef *dev, if (dev->type == VIR_DOMAIN_DEVICE_DISK) { virDomainDiskDef *disk = dev->data.disk; - int actual_type = virStorageSourceGetActualType(disk->src); + virStorageType actual_type = virStorageSourceGetActualType(disk->src); int format = virDomainDiskGetFormat(disk); /* for network-based disks, set 'qemu' as the default driver */ diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c index 6b7f638783..eb3b0b3718 100644 --- a/src/libxl/xen_xl.c +++ b/src/libxl/xen_xl.c @@ -1489,14 +1489,14 @@ xenFormatXLDiskSrcNet(virStorageSource *src) static int xenFormatXLDiskSrc(virStorageSource *src, char **srcstr) { - int actualType = virStorageSourceGetActualType(src); + virStorageType actualType = virStorageSourceGetActualType(src); *srcstr = NULL; if (virStorageSourceIsEmpty(src)) return 0; - switch ((virStorageType)actualType) { + switch (actualType) { case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_FILE: case VIR_STORAGE_TYPE_DIR: diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c index 9934e91aa9..8966980532 100644 --- a/src/locking/domain_lock.c +++ b/src/locking/domain_lock.c @@ -72,7 +72,7 @@ static int virDomainLockManagerAddImage(virLockManager *lock, virStorageSource *src) { unsigned int diskFlags = 0; - int type = virStorageSourceGetActualType(src); + virStorageType type = virStorageSourceGetActualType(src); if (!src->path) return 0; diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 60e03d418e..877c66d62b 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1128,7 +1128,7 @@ virJSONValue * qemuBlockStorageSourceGetBackendProps(virStorageSource *src, unsigned int flags) { - int actualType = virStorageSourceGetActualType(src); + virStorageType actualType = virStorageSourceGetActualType(src); g_autoptr(virJSONValue) fileprops = NULL; const char *driver = NULL; virTristateBool aro = VIR_TRISTATE_BOOL_ABSENT; @@ -1145,7 +1145,7 @@ qemuBlockStorageSourceGetBackendProps(virStorageSource *src, ro = VIR_TRISTATE_BOOL_NO; } - switch ((virStorageType)actualType) { + switch (actualType) { case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_FILE: if (virStorageSourceIsBlockLocal(src)) { @@ -2200,7 +2200,7 @@ char * qemuBlockGetBackingStoreString(virStorageSource *src, bool pretty) { - int actualType = virStorageSourceGetActualType(src); + virStorageType actualType = virStorageSourceGetActualType(src); g_autoptr(virJSONValue) backingProps = NULL; g_autoptr(virJSONValue) sliceProps = NULL; virJSONValue *props = NULL; @@ -2599,12 +2599,12 @@ int qemuBlockStorageSourceCreateGetStorageProps(virStorageSource *src, virJSONValue **props) { - int actualType = virStorageSourceGetActualType(src); + virStorageType actualType = virStorageSourceGetActualType(src); g_autoptr(virJSONValue) location = NULL; const char *driver = NULL; const char *filename = NULL; - switch ((virStorageType) actualType) { + switch (actualType) { case VIR_STORAGE_TYPE_FILE: driver = "file"; filename = src->path; @@ -2745,7 +2745,7 @@ qemuBlockStorageSourceCreateStorage(virDomainObj *vm, virStorageSource *chain, virDomainAsyncJob asyncJob) { - int actualType = virStorageSourceGetActualType(src); + virStorageType actualType = virStorageSourceGetActualType(src); g_autoptr(virJSONValue) createstorageprops = NULL; int ret; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 34dffb0615..33069ea131 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1736,7 +1736,7 @@ qemuGetDriveSourceString(virStorageSource *src, qemuDomainSecretInfo *secinfo, char **source) { - int actualType = virStorageSourceGetActualType(src); + virStorageType actualType = virStorageSourceGetActualType(src); *source = NULL; @@ -1744,7 +1744,7 @@ qemuGetDriveSourceString(virStorageSource *src, if (virStorageSourceIsEmpty(src)) return 1; - switch ((virStorageType)actualType) { + switch (actualType) { case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_FILE: case VIR_STORAGE_TYPE_DIR: @@ -1803,7 +1803,7 @@ qemuDiskBusIsSD(int bus) static bool qemuDiskSourceNeedsProps(virStorageSource *src) { - int actualType = virStorageSourceGetActualType(src); + virStorageType actualType = virStorageSourceGetActualType(src); if (actualType == VIR_STORAGE_TYPE_NETWORK && src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER && @@ -1879,7 +1879,7 @@ static int qemuBuildDriveSourceStr(virDomainDiskDef *disk, virBuffer *buf) { - int actualType = virStorageSourceGetActualType(disk->src); + virStorageType actualType = virStorageSourceGetActualType(disk->src); qemuDomainStorageSourcePrivate *srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src); qemuDomainSecretInfo *secinfo = NULL; qemuDomainSecretInfo *encinfo = NULL; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 31e60c7359..948ab76304 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4809,7 +4809,7 @@ qemuDomainValidateStorageSource(virStorageSource *src, virQEMUCaps *qemuCaps, bool maskBlockdev) { - int actualType = virStorageSourceGetActualType(src); + virStorageType actualType = virStorageSourceGetActualType(src); bool blockdev = virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV); if (maskBlockdev) @@ -10932,7 +10932,7 @@ qemuDomainPrepareDiskSource(virDomainDiskDef *disk, /* set default format for storage pool based disks */ if (disk->src->type == VIR_STORAGE_TYPE_VOLUME && disk->src->format <= VIR_STORAGE_FILE_NONE) { - int actualType = virStorageSourceGetActualType(disk->src); + virStorageType actualType = virStorageSourceGetActualType(disk->src); if (actualType == VIR_STORAGE_TYPE_DIR) disk->src->format = VIR_STORAGE_FILE_FAT; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6631edb9d6..1d50aa5271 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14790,7 +14790,7 @@ qemuDomainBlockCopyValidateMirror(virStorageSource *mirror, const char *dst, bool *reuse) { - int desttype = virStorageSourceGetActualType(mirror); + virStorageType desttype = virStorageSourceGetActualType(mirror); struct stat st; if (!virStorageSourceIsLocalStorage(mirror)) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 176c25cd11..97ce9ac27e 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1504,7 +1504,7 @@ qemuMigrationSrcIsSafe(virDomainDef *def, for (i = 0; i < def->ndisks; i++) { virDomainDiskDef *disk = def->disks[i]; const char *src = virDomainDiskGetSource(disk); - int actualType = virStorageSourceGetActualType(disk->src); + virStorageType actualType = virStorageSourceGetActualType(disk->src); bool unsafe = false; /* Disks without any source (i.e. floppies and CD-ROMs) @@ -1519,7 +1519,7 @@ qemuMigrationSrcIsSafe(virDomainDef *def, continue; /* However, disks on local FS (e.g. ext4) are not safe. */ - switch ((virStorageType) actualType) { + switch (actualType) { case VIR_STORAGE_TYPE_FILE: if ((rc = virFileIsSharedFS(src)) < 0) { return false; diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index b62fab7bb3..c6beeda1d0 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -380,10 +380,10 @@ static int qemuSnapshotPrepareDiskExternalInactive(virDomainSnapshotDiskDef *snapdisk, virDomainDiskDef *domdisk) { - int domDiskType = virStorageSourceGetActualType(domdisk->src); - int snapDiskType = virStorageSourceGetActualType(snapdisk->src); + virStorageType domDiskType = virStorageSourceGetActualType(domdisk->src); + virStorageType snapDiskType = virStorageSourceGetActualType(snapdisk->src); - switch ((virStorageType)domDiskType) { + switch (domDiskType) { case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_FILE: break; @@ -425,7 +425,7 @@ qemuSnapshotPrepareDiskExternalInactive(virDomainSnapshotDiskDef *snapdisk, return -1; } - switch ((virStorageType)snapDiskType) { + switch (snapDiskType) { case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_FILE: break; @@ -456,7 +456,7 @@ qemuSnapshotPrepareDiskExternalActive(virDomainObj *vm, virDomainDiskDef *domdisk, bool blockdev) { - int actualType = virStorageSourceGetActualType(snapdisk->src); + virStorageType actualType = virStorageSourceGetActualType(snapdisk->src); if (snapdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_MANUAL) return 0; @@ -471,7 +471,7 @@ qemuSnapshotPrepareDiskExternalActive(virDomainObj *vm, if (!qemuDomainDiskBlockJobIsSupported(vm, domdisk)) return -1; - switch ((virStorageType)actualType) { + switch (actualType) { case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_FILE: break; @@ -620,7 +620,7 @@ static int qemuSnapshotPrepareDiskInternal(virDomainDiskDef *disk, bool active) { - int actualType; + virStorageType actualType; /* active disks are handled by qemu itself so no need to worry about those */ if (active) @@ -631,7 +631,7 @@ qemuSnapshotPrepareDiskInternal(virDomainDiskDef *disk, actualType = virStorageSourceGetActualType(disk->src); - switch ((virStorageType)actualType) { + switch (actualType) { case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_FILE: return 0; diff --git a/src/storage_file/storage_source.c b/src/storage_file/storage_source.c index 698b9eb79d..dd2bb2641d 100644 --- a/src/storage_file/storage_source.c +++ b/src/storage_file/storage_source.c @@ -807,7 +807,7 @@ static int virStorageSourceGetBackendForSupportCheck(const virStorageSource *src, virStorageFileBackend **backend) { - int actualType; + virStorageType actualType; if (!src) { @@ -944,7 +944,7 @@ int virStorageSourceInitAs(virStorageSource *src, uid_t uid, gid_t gid) { - int actualType = virStorageSourceGetActualType(src); + virStorageType actualType = virStorageSourceGetActualType(src); virStorageDriverData *drv = g_new0(virStorageDriverData, 1); src->drv = drv; -- 2.35.1

On 5/23/22 3:08 PM, Michal Privoznik wrote:
The virStorageSourceGetActualType() function returns either virStorageSource->type (which is of type virStorageType), or virStorageSourcePoolDef->type, which really stores a value of the same enum. Thus, the latter struct can be changed so that the virStorageSourceGetActualType() function can return correct type instead of generic int.
Signed-off-by: Michal Privoznik<mprivozn@redhat.com> --- src/conf/storage_source_conf.c | 2 +- src/conf/storage_source_conf.h | 4 ++-- src/libxl/libxl_conf.c | 2 +- src/libxl/libxl_domain.c | 2 +- src/libxl/xen_xl.c | 4 ++-- src/locking/domain_lock.c | 2 +- src/qemu/qemu_block.c | 12 ++++++------ src/qemu/qemu_command.c | 8 ++++---- src/qemu/qemu_domain.c | 4 ++-- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_migration.c | 4 ++-- src/qemu/qemu_snapshot.c | 16 ++++++++-------- src/storage_file/storage_source.c | 4 ++-- 13 files changed, 33 insertions(+), 33 deletions(-)
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> -- 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

The virDomainBackupDefParse() 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/backup_conf.c | 16 ++++++---------- src/conf/backup_conf.h | 2 +- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c index 2a7fa95e0c..11d533a905 100644 --- a/src/conf/backup_conf.c +++ b/src/conf/backup_conf.c @@ -201,22 +201,18 @@ virDomainBackupDefParse(xmlXPathContextPtr ctxt, g_autoptr(virDomainBackupDef) def = NULL; g_autofree xmlNodePtr *nodes = NULL; xmlNodePtr node = NULL; - g_autofree char *mode = NULL; bool push; size_t i; int n; def = g_new0(virDomainBackupDef, 1); - def->type = VIR_DOMAIN_BACKUP_TYPE_PUSH; - - if ((mode = virXMLPropString(ctxt->node, "mode"))) { - if ((def->type = virDomainBackupTypeFromString(mode)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown backup mode '%s'"), mode); - return NULL; - } - } + if (virXMLPropEnumDefault(ctxt->node, "mode", + virDomainBackupTypeFromString, + VIR_XML_PROP_NONZERO, + &def->type, + VIR_DOMAIN_BACKUP_TYPE_PUSH) < 0) + return NULL; push = def->type == VIR_DOMAIN_BACKUP_TYPE_PUSH; diff --git a/src/conf/backup_conf.h b/src/conf/backup_conf.h index dc66b75892..413488e123 100644 --- a/src/conf/backup_conf.h +++ b/src/conf/backup_conf.h @@ -76,7 +76,7 @@ struct _virDomainBackupDiskDef { typedef struct _virDomainBackupDef virDomainBackupDef; struct _virDomainBackupDef { /* Public XML. */ - int type; /* virDomainBackupType */ + virDomainBackupType type; char *incremental; virStorageNetHostDef *server; /* only when type == PULL */ virTristateBool tls; /* use TLS for NBD */ -- 2.35.1

On 5/23/22 3:08 PM, Michal Privoznik wrote:
The virDomainBackupDefParse() 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/backup_conf.c | 16 ++++++---------- src/conf/backup_conf.h | 2 +- 2 files changed, 7 insertions(+), 11 deletions(-)
Besides what I commented in patch 4 Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> -- 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

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.1

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? -- 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

The virDomainStorageNetworkParseHost() 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/domain_conf.c | 19 +++++++------------ src/conf/storage_source_conf.h | 2 +- src/qemu/qemu_backup.c | 2 +- src/qemu/qemu_block.c | 2 +- src/qemu/qemu_command.c | 2 ++ src/qemu/qemu_monitor_json.c | 2 +- .../storage_file_backend_gluster.c | 2 +- .../storage_source_backingstore.c | 16 ++++++++++------ 8 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a9c424e71d..17ac74abcd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7006,22 +7006,17 @@ virDomainStorageNetworkParseHost(xmlNodePtr hostnode, virStorageNetHostDef *host) { int ret = -1; - g_autofree char *transport = NULL; g_autofree char *port = NULL; memset(host, 0, sizeof(*host)); - host->transport = VIR_STORAGE_NET_HOST_TRANS_TCP; /* transport can be tcp (default), unix or rdma. */ - if ((transport = virXMLPropString(hostnode, "transport"))) { - host->transport = virStorageNetHostTransportTypeFromString(transport); - if (host->transport < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown protocol transport type '%s'"), - transport); - goto cleanup; - } - } + if (virXMLPropEnumDefault(hostnode, "transport", + virStorageNetHostTransportTypeFromString, + VIR_XML_PROP_NONE, + &host->transport, + VIR_STORAGE_NET_HOST_TRANS_TCP) < 0) + goto cleanup; host->socket = virXMLPropString(hostnode, "socket"); @@ -7037,7 +7032,7 @@ virDomainStorageNetworkParseHost(xmlNodePtr hostnode, virReportError(VIR_ERR_XML_ERROR, _("transport '%s' does not support " "socket attribute"), - transport); + virStorageNetHostTransportTypeToString(host->transport)); goto cleanup; } diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h index f2440cec6a..3afad96bdb 100644 --- a/src/conf/storage_source_conf.h +++ b/src/conf/storage_source_conf.h @@ -150,7 +150,7 @@ typedef struct _virStorageNetHostDef virStorageNetHostDef; struct _virStorageNetHostDef { char *name; unsigned int port; - int transport; /* virStorageNetHostTransport */ + virStorageNetHostTransport transport; char *socket; /* path to unix socket */ }; diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index 427c090dd8..ef87f99177 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -73,7 +73,7 @@ qemuBackupPrepare(virDomainBackupDef *def) def->server->name = g_strdup("localhost"); } - switch ((virStorageNetHostTransport) def->server->transport) { + switch (def->server->transport) { case VIR_STORAGE_NET_HOST_TRANS_TCP: /* TODO: Update qemu.conf to provide a port range, * probably starting at 10809, for obtaining automatic diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 877c66d62b..ba5e5ef625 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -452,7 +452,7 @@ qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDef *host, const char *field; g_autofree char *port = NULL; - switch ((virStorageNetHostTransport) host->transport) { + switch (host->transport) { case VIR_STORAGE_NET_HOST_TRANS_TCP: if (legacy) transport = "tcp"; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7eb7def747..dac8aabad4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1619,6 +1619,8 @@ qemuBuildNetworkDriveStr(virStorageSource *src, virBufferAsprintf(&buf, "unix:%s", src->hosts->socket); break; + case VIR_STORAGE_NET_HOST_TRANS_RDMA: + case VIR_STORAGE_NET_HOST_TRANS_LAST: default: virReportError(VIR_ERR_INTERNAL_ERROR, _("nbd does not support transport '%s'"), diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index dc05dfd047..afe45c415b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6499,7 +6499,7 @@ qemuMonitorJSONNBDServerStart(qemuMonitor *mon, g_autoptr(virJSONValue) addr = NULL; g_autofree char *port_str = NULL; - switch ((virStorageNetHostTransport)server->transport) { + switch (server->transport) { case VIR_STORAGE_NET_HOST_TRANS_TCP: port_str = g_strdup_printf("%u", server->port); addr = qemuMonitorJSONBuildInetSocketAddress(server->name, port_str); diff --git a/src/storage_file/storage_file_backend_gluster.c b/src/storage_file/storage_file_backend_gluster.c index 2aee61bf33..cd16a6c40c 100644 --- a/src/storage_file/storage_file_backend_gluster.c +++ b/src/storage_file/storage_file_backend_gluster.c @@ -65,7 +65,7 @@ virStorageFileBackendGlusterInitServer(virStorageFileBackendGlusterPriv *priv, const char *hoststr = NULL; int port = 0; - switch ((virStorageNetHostTransport) host->transport) { + switch (host->transport) { case VIR_STORAGE_NET_HOST_TRANS_RDMA: case VIR_STORAGE_NET_HOST_TRANS_TCP: hoststr = host->name; diff --git a/src/storage_file/storage_source_backingstore.c b/src/storage_file/storage_source_backingstore.c index e48ae725ab..71c5eca13c 100644 --- a/src/storage_file/storage_source_backingstore.c +++ b/src/storage_file/storage_source_backingstore.c @@ -65,12 +65,16 @@ virStorageSourceParseBackingURI(virStorageSource *src, return -1; } - if (scheme[1] && - (src->hosts->transport = virStorageNetHostTransportTypeFromString(scheme[1])) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("invalid protocol transport type '%s'"), - scheme[1]); - return -1; + if (scheme[1]) { + int transport; + if ((transport = virStorageNetHostTransportTypeFromString(scheme[1])) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid protocol transport type '%s'"), + scheme[1]); + return -1; + } + + src->hosts->transport = transport; } if (uri->query) { -- 2.35.1

On 5/23/22 3:08 PM, Michal Privoznik wrote:
The virDomainStorageNetworkParseHost() 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/domain_conf.c | 19 +++++++------------ src/conf/storage_source_conf.h | 2 +- src/qemu/qemu_backup.c | 2 +- src/qemu/qemu_block.c | 2 +- src/qemu/qemu_command.c | 2 ++ src/qemu/qemu_monitor_json.c | 2 +- .../storage_file_backend_gluster.c | 2 +- .../storage_source_backingstore.c | 16 ++++++++++------ 8 files changed, 24 insertions(+), 23 deletions(-)
Besides what I commented in patch 4 Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> -- 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

The virDomainHostdevSubsysSCSIDefParseXML() 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/domain_conf.c | 19 ++++++------------- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_domain.c | 4 ++-- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 2 +- tests/qemuxml2argvtest.c | 2 +- 7 files changed, 14 insertions(+), 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 17ac74abcd..ded2c4aacf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7233,20 +7233,13 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr sourcenode, unsigned int flags, virDomainXMLOption *xmlopt) { - g_autofree char *protocol = NULL; + if (virXMLPropEnum(sourcenode, "protocol", + virDomainHostdevSubsysSCSIProtocolTypeFromString, + VIR_XML_PROP_NONE, + &scsisrc->protocol) < 0) + return -1; - if ((protocol = virXMLPropString(sourcenode, "protocol"))) { - scsisrc->protocol = - virDomainHostdevSubsysSCSIProtocolTypeFromString(protocol); - if (scsisrc->protocol < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown SCSI subsystem protocol '%s'"), - protocol); - return -1; - } - } - - switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) { + switch (scsisrc->protocol) { case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE: return virDomainHostdevSubsysSCSIHostDefParseXML(sourcenode, ctxt, scsisrc, flags, xmlopt); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f325bfb51e..667845ac10 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -270,7 +270,7 @@ typedef enum { } virDomainDeviceSGIO; struct _virDomainHostdevSubsysSCSI { - int protocol; /* enum virDomainHostdevSCSIProtocolType */ + virDomainHostdevSCSIProtocolType protocol; virDomainDeviceSGIO sgio; virTristateBool rawio; union { diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dac8aabad4..5ea88bf239 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5284,7 +5284,7 @@ qemuBuildHostdevSCSIDetachPrepare(virDomainHostdevDef *hostdev, virStorageSource *src; qemuDomainStorageSourcePrivate *srcpriv; - switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) { + switch (scsisrc->protocol) { case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE: src = scsisrc->u.host.src; break; @@ -5325,7 +5325,7 @@ qemuBuildHostdevSCSIAttachPrepare(virDomainHostdevDef *hostdev, virStorageSource *src = NULL; if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV_HOSTDEV_SCSI)) { - switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) { + switch (scsisrc->protocol) { case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE: src = scsisrc->u.host.src; break; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 948ab76304..30ef5b7550 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5731,7 +5731,7 @@ qemuDomainDeviceHostdevDefPostParseRestoreBackendAlias(virDomainHostdevDef *host !virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV_HOSTDEV_SCSI)) return 0; - switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) { + switch (scsisrc->protocol) { case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE: if (!scsisrc->u.host.src) scsisrc->u.host.src = virStorageSourceNew(); @@ -10961,7 +10961,7 @@ qemuDomainPrepareHostdev(virDomainHostdevDef *hostdev, virDomainHostdevSubsysSCSI *scsisrc = &hostdev->source.subsys.u.scsi; virStorageSource *src = NULL; - switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) { + switch (scsisrc->protocol) { case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE: virObjectUnref(scsisrc->u.host.src); scsisrc->u.host.src = virStorageSourceNew(); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1d50aa5271..f0be25a12d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6284,7 +6284,7 @@ qemuConnectDomainXMLToNativePrepareHostHostdev(virDomainHostdevDef *hostdev) if (virHostdevIsSCSIDevice(hostdev)) { virDomainHostdevSubsysSCSI *scsisrc = &hostdev->source.subsys.u.scsi; - switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) { + switch (scsisrc->protocol) { case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE: { virDomainHostdevSubsysSCSIHost *scsihostsrc = &scsisrc->u.host; virStorageSource *src = scsisrc->u.host.src; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 033d3d5bc6..929986745e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6368,7 +6368,7 @@ qemuProcessPrepareHostHostdev(virDomainHostdevDef *hostdev) if (virHostdevIsSCSIDevice(hostdev)) { virDomainHostdevSubsysSCSI *scsisrc = &hostdev->source.subsys.u.scsi; - switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) { + switch (scsisrc->protocol) { case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE: { virDomainHostdevSubsysSCSIHost *scsihostsrc = &scsisrc->u.host; virStorageSource *src = scsisrc->u.host.src; diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index aadfaddc17..4a5da784fa 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -425,7 +425,7 @@ testCompareXMLToArgvCreateArgs(virQEMUDriver *drv, if (virHostdevIsSCSIDevice(hostdev)) { virDomainHostdevSubsysSCSI *scsisrc = &hostdev->source.subsys.u.scsi; - switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) { + switch (scsisrc->protocol) { case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE: scsisrc->u.host.src->path = g_strdup("/dev/sg0"); break; -- 2.35.1

On 5/23/22 3:08 PM, Michal Privoznik wrote:
The virDomainHostdevSubsysSCSIDefParseXML() 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/domain_conf.c | 19 ++++++------------- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_domain.c | 4 ++-- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 2 +- tests/qemuxml2argvtest.c | 2 +- 7 files changed, 14 insertions(+), 21 deletions(-)
Besides what I commented in patch 4 Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> -- 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

The virDomainHostdevSubsysSCSIVHostDefParseXML() 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/domain_conf.c | 19 +++++-------------- src/conf/domain_conf.h | 4 ++-- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ded2c4aacf..561af84eed 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7262,24 +7262,15 @@ virDomainHostdevSubsysSCSIVHostDefParseXML(xmlNodePtr sourcenode, virDomainHostdevDef *def) { virDomainHostdevSubsysSCSIVHost *hostsrc = &def->source.subsys.u.scsi_host; - g_autofree char *protocol = NULL; g_autofree char *wwpn = NULL; - if (!(protocol = virXMLPropString(sourcenode, "protocol"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Missing scsi_host subsystem protocol")); + if (virXMLPropEnum(sourcenode, "protocol", + virDomainHostdevSubsysSCSIHostProtocolTypeFromString, + VIR_XML_PROP_NONZERO | VIR_XML_PROP_REQUIRED, + &hostsrc->protocol) < 0) return -1; - } - if ((hostsrc->protocol = - virDomainHostdevSubsysSCSIHostProtocolTypeFromString(protocol)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown scsi_host subsystem protocol '%s'"), - protocol); - return -1; - } - - switch ((virDomainHostdevSubsysSCSIHostProtocolType) hostsrc->protocol) { + switch (hostsrc->protocol) { case VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_HOST_PROTOCOL_TYPE_VHOST: if (!(wwpn = virXMLPropString(sourcenode, "wwpn"))) { virReportError(VIR_ERR_XML_ERROR, "%s", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 667845ac10..2f00cc7f79 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -287,7 +287,7 @@ struct _virDomainHostdevSubsysMediatedDev { }; typedef enum { - VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_HOST_PROTOCOL_TYPE_NONE, + VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_HOST_PROTOCOL_TYPE_NONE = 0, VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_HOST_PROTOCOL_TYPE_VHOST, VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_HOST_PROTOCOL_TYPE_LAST, @@ -307,7 +307,7 @@ typedef enum { VIR_ENUM_DECL(virDomainHostdevSubsysSCSIVHostModel); struct _virDomainHostdevSubsysSCSIVHost { - int protocol; /* enum virDomainHostdevSubsysSCSIHostProtocolType */ + virDomainHostdevSubsysSCSIHostProtocolType protocol; char *wwpn; virDomainHostdevSubsysSCSIVHostModelType model; }; -- 2.35.1

On 5/23/22 3:08 PM, Michal Privoznik wrote:
The virDomainHostdevSubsysSCSIVHostDefParseXML() 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/domain_conf.c | 19 +++++-------------- src/conf/domain_conf.h | 4 ++-- 2 files changed, 7 insertions(+), 16 deletions(-)
Besides what I commented in patch 4 Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> -- 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

The virDomainDiskSourceNVMeParse() 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/domain_conf.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 561af84eed..bf9db38340 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8405,7 +8405,6 @@ virDomainDiskSourceNVMeParse(xmlNodePtr node, { g_autoptr(virStorageSourceNVMeDef) nvme = NULL; g_autofree char *type = NULL; - g_autofree char *namespc = NULL; xmlNodePtr address; nvme = g_new0(virStorageSourceNVMeDef, 1); @@ -8423,18 +8422,10 @@ virDomainDiskSourceNVMeParse(xmlNodePtr node, return -1; } - if (!(namespc = virXMLPropString(node, "namespace"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing 'namespace' attribute to disk source")); + if (virXMLPropULongLong(node, "namespace", 10, + VIR_XML_PROP_REQUIRED, + &nvme->namespc) < 0) return -1; - } - - if (virStrToLong_ull(namespc, NULL, 10, &nvme->namespc) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("malformed namespace '%s'"), - namespc); - return -1; - } if (virXMLPropTristateBool(node, "managed", VIR_XML_PROP_NONE, &nvme->managed) < 0) -- 2.35.1

On 5/23/22 3:08 PM, Michal Privoznik wrote:
The virDomainDiskSourceNVMeParse() 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/domain_conf.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-)
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> -- 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 6:02 PM, Boris Fiuczynski wrote:
On 5/23/22 3:08 PM, Michal Privoznik wrote:
The virDomainDiskSourceNVMeParse() function uses old style of parsing XML (virXMLPropString + str2enum conversion). Use virXMLPropEnumDefault() which encapsulates those steps.
ups, above should be mentioning str2uul and virXMLPropULongLong
Signed-off-by: Michal Privoznik<mprivozn@redhat.com> --- src/conf/domain_conf.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-)
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
-- 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

The virDomainDiskDefMirrorParse() 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/domain_conf.c | 26 ++++++++++---------------- src/conf/domain_conf.h | 4 ++-- src/qemu/qemu_process.c | 6 ++++-- 3 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bf9db38340..4e1835fd34 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8804,21 +8804,16 @@ virDomainDiskDefMirrorParse(virDomainDiskDef *def, VIR_XPATH_NODE_AUTORESTORE(ctxt) g_autofree char *mirrorFormat = NULL; g_autofree char *mirrorType = NULL; - g_autofree char *ready = NULL; - g_autofree char *blockJob = NULL; g_autofree char *index = NULL; ctxt->node = cur; - if ((blockJob = virXMLPropString(cur, "job"))) { - if ((def->mirrorJob = virDomainBlockJobTypeFromString(blockJob)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown mirror job type '%s'"), blockJob); - return -1; - } - } else { - def->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY; - } + if (virXMLPropEnumDefault(cur, "job", + virDomainBlockJobTypeFromString, + VIR_XML_PROP_NONZERO, + &def->mirrorJob, + VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) < 0) + return -1; if ((mirrorType = virXMLPropString(cur, "type"))) { mirrorFormat = virXPathString("string(./format/@type)", ctxt); @@ -8860,12 +8855,11 @@ virDomainDiskDefMirrorParse(virDomainDiskDef *def, } } - if ((ready = virXMLPropString(cur, "ready")) && - (def->mirrorState = virDomainDiskMirrorStateTypeFromString(ready)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown mirror ready state %s"), ready); + if (virXMLPropEnum(cur, "ready", + virDomainDiskMirrorStateTypeFromString, + VIR_XML_PROP_NONE, + &def->mirrorState) < 0) return -1; - } if (virParseScaledValue("./format/metadata_cache/max_size", NULL, ctxt, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2f00cc7f79..30aa0ed8d3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -564,8 +564,8 @@ struct _virDomainDiskDef { unsigned int rotation_rate; virStorageSource *mirror; - int mirrorState; /* enum virDomainDiskMirrorState */ - int mirrorJob; /* virDomainBlockJobType */ + virDomainDiskMirrorState mirrorState; + virDomainBlockJobType mirrorJob; struct { unsigned int cylinders; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 929986745e..85bf452a59 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8660,8 +8660,10 @@ qemuProcessRefreshLegacyBlockjob(void *payload, } if (jobtype == QEMU_BLOCKJOB_TYPE_COMMIT && - disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT) - jobtype = disk->mirrorJob; + disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT) { + /* This typecast is safe because of how the enum is declared. */ + jobtype = (qemuBlockJobType) disk->mirrorJob; + } if (!(job = qemuBlockJobDiskNew(vm, disk, jobtype, jobname))) return -1; -- 2.35.1

On 5/23/22 3:08 PM, Michal Privoznik wrote:
The virDomainDiskDefMirrorParse() function uses old style of parsing XML (virXMLPropString + str2enum conversion). Use virXMLPropEnumDefault() which encapsulates those steps.
... and virXMLPropEnum()
Signed-off-by: Michal Privoznik<mprivozn@redhat.com> --- src/conf/domain_conf.c | 26 ++++++++++---------------- src/conf/domain_conf.h | 4 ++-- src/qemu/qemu_process.c | 6 ++++-- 3 files changed, 16 insertions(+), 20 deletions(-)
Besides above and what I commented in patch 4 Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> -- 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

The virDomainDiskSourcePoolDefParse() 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/domain_conf.c | 12 ++++-------- src/conf/storage_source_conf.h | 2 +- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4e1835fd34..c42ac9c0c9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8193,7 +8193,6 @@ virDomainDiskSourcePoolDefParse(xmlNodePtr node, { virStorageSourcePoolDef *source; int ret = -1; - g_autofree char *mode = NULL; *srcpool = NULL; @@ -8201,7 +8200,6 @@ virDomainDiskSourcePoolDefParse(xmlNodePtr node, source->pool = virXMLPropString(node, "pool"); source->volume = virXMLPropString(node, "volume"); - mode = virXMLPropString(node, "mode"); /* CD-ROM and Floppy allows no source */ if (!source->pool && !source->volume) { @@ -8216,13 +8214,11 @@ virDomainDiskSourcePoolDefParse(xmlNodePtr node, goto cleanup; } - if (mode && - (source->mode = virStorageSourcePoolModeTypeFromString(mode)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown source mode '%s' for volume type disk"), - mode); + if (virXMLPropEnum(node, "mode", + virStorageSourcePoolModeTypeFromString, + VIR_XML_PROP_NONZERO, + &source->mode) < 0) goto cleanup; - } *srcpool = g_steal_pointer(&source); ret = 0; diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h index 3afad96bdb..b99a952702 100644 --- a/src/conf/storage_source_conf.h +++ b/src/conf/storage_source_conf.h @@ -200,7 +200,7 @@ struct _virStorageSourcePoolDef { int voltype; /* virStorageVolType, internal only */ int pooltype; /* virStoragePoolType from storage_conf.h, internal only */ virStorageType actualtype; /* internal only */ - int mode; /* virStorageSourcePoolMode, currently makes sense only for iscsi pool */ + virStorageSourcePoolMode mode; /* currently makes sense only for iscsi pool */ }; -- 2.35.1

On 5/23/22 3:08 PM, Michal Privoznik wrote:
The virDomainDiskSourcePoolDefParse() function uses old style of parsing XML (virXMLPropString + str2enum conversion). Use virXMLPropEnumDefault() which encapsulates those steps.
virXMLPropEnum (above and in subject)
Signed-off-by: Michal Privoznik<mprivozn@redhat.com> --- src/conf/domain_conf.c | 12 ++++-------- src/conf/storage_source_conf.h | 2 +- 2 files changed, 5 insertions(+), 9 deletions(-)
Besides above and what I commented in patch 4 Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> -- 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

The virDomainDiskDefParseSourceXML() 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/domain_conf.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c42ac9c0c9..44b507b74d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9039,15 +9039,11 @@ virDomainDiskDefParseSourceXML(virDomainXMLOption *xmlopt, if (virDomainStorageSourceParse(tmp, ctxt, src, flags, xmlopt) < 0) return NULL; - if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) { - g_autofree char *sourceindex = NULL; - - if ((sourceindex = virXMLPropString(tmp, "index")) && - virStrToLong_uip(sourceindex, NULL, 10, &src->id) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid disk index '%s'"), sourceindex); - return NULL; - } + if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && + virXMLPropUInt(tmp, "index", 10, + VIR_XML_PROP_NONE, + &src->id) < 0) { + return NULL; } } else { /* Reset src->type in case when 'source' was not present */ -- 2.35.1

On 5/23/22 3:08 PM, Michal Privoznik wrote:
The virDomainDiskDefParseSourceXML() function uses old style of parsing XML (virXMLPropString + str2enum conversion). Use virXMLPropEnumDefault() which encapsulates those steps.
strtoul and virXMLPropUInt (also subject)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
Besides above Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c42ac9c0c9..44b507b74d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9039,15 +9039,11 @@ virDomainDiskDefParseSourceXML(virDomainXMLOption *xmlopt, if (virDomainStorageSourceParse(tmp, ctxt, src, flags, xmlopt) < 0) return NULL;
- if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) { - g_autofree char *sourceindex = NULL; - - if ((sourceindex = virXMLPropString(tmp, "index")) && - virStrToLong_uip(sourceindex, NULL, 10, &src->id) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid disk index '%s'"), sourceindex); - return NULL; - } + if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && + virXMLPropUInt(tmp, "index", 10, + VIR_XML_PROP_NONE, + &src->id) < 0) { + return NULL; } } else { /* Reset src->type in case when 'source' was not present */
-- 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

The virDomainChrDefParseXML() 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/ch/ch_monitor.c | 2 +- src/conf/domain_conf.c | 82 +++++++++++--------------------- src/conf/domain_conf.h | 2 +- src/conf/domain_validate.c | 2 +- src/conf/virchrdev.c | 29 +++++++++++ src/libxl/libxl_conf.c | 20 ++++++++ src/libxl/xen_common.c | 23 ++++++++- src/qemu/qemu_command.c | 6 +-- src/qemu/qemu_domain.c | 34 ++++++++++++- src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_validate.c | 2 +- src/security/security_apparmor.c | 4 +- src/security/security_dac.c | 4 +- src/security/security_selinux.c | 24 ++++++++++ src/vmx/vmx.c | 26 ++++++++++ tests/testutilsqemu.c | 2 +- 17 files changed, 197 insertions(+), 69 deletions(-) diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 2c6b83a1b5..d6fac642da 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -281,7 +281,7 @@ virCHMonitorBuildNetJson(virJSONValue *nets, } break; case VIR_DOMAIN_NET_TYPE_VHOSTUSER: - if ((virDomainChrType)netdef->data.vhostuser->type != VIR_DOMAIN_CHR_TYPE_UNIX) { + if (netdef->data.vhostuser->type != VIR_DOMAIN_CHR_TYPE_UNIX) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("vhost_user type support UNIX socket in this CH")); return -1; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 44b507b74d..b5ce80eb76 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2694,7 +2694,7 @@ virDomainChrSourceDefGetPath(virDomainChrSourceDef *chr) if (!chr) return NULL; - switch ((virDomainChrType) chr->type) { + switch (chr->type) { case VIR_DOMAIN_CHR_TYPE_PTY: case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: @@ -2761,6 +2761,13 @@ virDomainChrSourceDefClear(virDomainChrSourceDef *def) case VIR_DOMAIN_CHR_TYPE_DBUS: VIR_FREE(def->data.dbus.channel); break; + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT: + case VIR_DOMAIN_CHR_TYPE_LAST: + break; } VIR_FREE(def->logfile); @@ -2778,7 +2785,7 @@ virDomainChrSourceDefCopy(virDomainChrSourceDef *dest, dest->logfile = g_strdup(src->logfile); dest->logappend = src->logappend; - switch ((virDomainChrType)src->type) { + switch (src->type) { case VIR_DOMAIN_CHR_TYPE_FILE: case VIR_DOMAIN_CHR_TYPE_PTY: case VIR_DOMAIN_CHR_TYPE_DEV: @@ -2875,7 +2882,7 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src, if (tgt->type != src->type) return false; - switch ((virDomainChrType)src->type) { + switch (src->type) { case VIR_DOMAIN_CHR_TYPE_FILE: return src->data.file.append == tgt->data.file.append && STREQ_NULLABLE(src->data.file.path, tgt->data.file.path); @@ -11270,7 +11277,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDef *def, goto error; } - switch ((virDomainChrType) def->type) { + switch (def->type) { case VIR_DOMAIN_CHR_TYPE_FILE: if (virDomainChrSourceDefParseFile(def, sources[0]) < 0) goto error; @@ -11477,7 +11484,6 @@ virDomainChrDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr target; const char *nodeName; virDomainChrDef *def; - g_autofree char *type = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) ctxt->node = node; @@ -11485,15 +11491,12 @@ virDomainChrDefParseXML(virDomainXMLOption *xmlopt, if (!(def = virDomainChrDefNew(xmlopt))) return NULL; - type = virXMLPropString(node, "type"); - if (type == NULL) { - def->source->type = VIR_DOMAIN_CHR_TYPE_PTY; - } else if ((def->source->type = virDomainChrTypeFromString(type)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown type presented to host for character device: %s"), - type); + if (virXMLPropEnumDefault(node, "type", + virDomainChrTypeFromString, + VIR_XML_PROP_NONE, + &def->source->type, + VIR_DOMAIN_CHR_TYPE_PTY) < 0) goto error; - } nodeName = (const char *) node->name; if ((def->deviceType = virDomainChrDeviceTypeFromString(nodeName)) < 0) { @@ -11551,7 +11554,6 @@ virDomainSmartcardDefParseXML(virDomainXMLOption *xmlopt, unsigned int flags) { g_autoptr(virDomainSmartcardDef) def = NULL; - g_autofree char *type = NULL; g_autofree xmlNodePtr *certificates = NULL; int n = 0; VIR_XPATH_NODE_AUTORESTORE(ctxt) @@ -11597,23 +11599,14 @@ virDomainSmartcardDefParseXML(virDomainXMLOption *xmlopt, break; case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: - type = virXMLPropString(node, "type"); - if (type == NULL) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("passthrough mode requires a character " - "device type attribute")); - return NULL; - } - if (!(def->data.passthru = virDomainChrSourceDefNew(xmlopt))) return NULL; - if ((def->data.passthru->type = virDomainChrTypeFromString(type)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown type presented to host for " - "character device: %s"), type); + if (virXMLPropEnum(node, "type", + virDomainChrTypeFromString, + VIR_XML_PROP_REQUIRED, + &def->data.passthru->type) < 0) return NULL; - } if (virDomainChrSourceDefParseXML(def->data.passthru, node, flags, NULL, ctxt) < 0) @@ -13349,7 +13342,6 @@ virDomainRNGDefParseXML(virDomainXMLOption *xmlopt, g_autofree xmlNodePtr *backends = NULL; g_autofree char *model = NULL; g_autofree char *backend = NULL; - g_autofree char *type = NULL; def = g_new0(virDomainRNGDef, 1); @@ -13405,22 +13397,14 @@ virDomainRNGDefParseXML(virDomainXMLOption *xmlopt, break; case VIR_DOMAIN_RNG_BACKEND_EGD: - if (!(type = virXMLPropString(backends[0], "type"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing EGD backend type")); - goto error; - } - if (!(def->source.chardev = virDomainChrSourceDefNew(xmlopt))) goto error; - def->source.chardev->type = virDomainChrTypeFromString(type); - if (def->source.chardev->type < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown backend type '%s' for egd"), - type); + if (virXMLPropEnum(backends[0], "type", + virDomainChrTypeFromString, + VIR_XML_PROP_REQUIRED, + &def->source.chardev->type) < 0) goto error; - } if (virDomainChrSourceDefParseXML(def->source.chardev, backends[0], flags, @@ -14320,7 +14304,6 @@ virDomainRedirdevDefParseXML(virDomainXMLOption *xmlopt, { virDomainRedirdevDef *def; g_autofree char *bus = NULL; - g_autofree char *type = NULL; def = g_new0(virDomainRedirdevDef, 1); @@ -14338,18 +14321,11 @@ virDomainRedirdevDefParseXML(virDomainXMLOption *xmlopt, def->bus = VIR_DOMAIN_REDIRDEV_BUS_USB; } - type = virXMLPropString(node, "type"); - if (type) { - if ((def->source->type = virDomainChrTypeFromString(type)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown redirdev character device type '%s'"), type); - goto error; - } - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing type in redirdev")); + if (virXMLPropEnum(node, "type", + virDomainChrTypeFromString, + VIR_XML_PROP_REQUIRED, + &def->source->type) < 0) goto error; - } /* boot gets parsed in virDomainDeviceInfoParseXML * source gets parsed in virDomainChrSourceDefParseXML */ @@ -25070,7 +25046,7 @@ virDomainChrSourceDefFormat(virBuffer *buf, g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); - switch ((virDomainChrType)def->type) { + switch (def->type) { case VIR_DOMAIN_CHR_TYPE_NULL: case VIR_DOMAIN_CHR_TYPE_VC: case VIR_DOMAIN_CHR_TYPE_STDIO: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 30aa0ed8d3..a81fb09678 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1281,7 +1281,7 @@ typedef enum { /* The host side information for a character device. */ struct _virDomainChrSourceDef { virObject parent; - int type; /* virDomainChrType */ + virDomainChrType type; virObject *privateData; union { /* no <source> for null, vc, stdio */ diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 28234f910d..4fce7059dc 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -932,7 +932,7 @@ virDomainChrSourceDefValidate(const virDomainChrSourceDef *src_def, const virDomainChrDef *chr_def, const virDomainDef *def) { - switch ((virDomainChrType) src_def->type) { + switch (src_def->type) { case VIR_DOMAIN_CHR_TYPE_NULL: case VIR_DOMAIN_CHR_TYPE_PTY: case VIR_DOMAIN_CHR_TYPE_VC: diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c index 8610f0ac5c..730e4e23b4 100644 --- a/src/conf/virchrdev.c +++ b/src/conf/virchrdev.c @@ -339,6 +339,21 @@ int virChrdevOpen(virChrdevs *devs, case VIR_DOMAIN_CHR_TYPE_UNIX: path = source->data.nix.path; break; + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_DEV: + case VIR_DOMAIN_CHR_TYPE_FILE: + case VIR_DOMAIN_CHR_TYPE_PIPE: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_UDP: + case VIR_DOMAIN_CHR_TYPE_TCP: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + case VIR_DOMAIN_CHR_TYPE_NMDM: + case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT: + case VIR_DOMAIN_CHR_TYPE_DBUS: + case VIR_DOMAIN_CHR_TYPE_LAST: + default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported device type '%s'"), @@ -404,6 +419,20 @@ int virChrdevOpen(virChrdevs *devs, if (virFDStreamConnectUNIX(st, path, false) < 0) goto error; break; + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_DEV: + case VIR_DOMAIN_CHR_TYPE_FILE: + case VIR_DOMAIN_CHR_TYPE_PIPE: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_UDP: + case VIR_DOMAIN_CHR_TYPE_TCP: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + case VIR_DOMAIN_CHR_TYPE_NMDM: + case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT: + case VIR_DOMAIN_CHR_TYPE_DBUS: + case VIR_DOMAIN_CHR_TYPE_LAST: default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported device type '%s'"), diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 401e47344e..0cfd49ebcf 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -273,6 +273,12 @@ libxlMakeChrdevStr(virDomainChrDef *def, char **buf) srcdef->data.nix.listen ? ",server,nowait" : ""); break; + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + case VIR_DOMAIN_CHR_TYPE_NMDM: + case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT: + case VIR_DOMAIN_CHR_TYPE_DBUS: + case VIR_DOMAIN_CHR_TYPE_LAST: default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported chardev '%s'"), type); @@ -1972,6 +1978,20 @@ libxlMakeChannel(virDomainChrDef *l_channel, x_channel->connection = LIBXL_CHANNEL_CONNECTION_SOCKET; x_channel->u.socket.path = g_strdup(l_channel->source->data.nix.path); break; + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_DEV: + case VIR_DOMAIN_CHR_TYPE_FILE: + case VIR_DOMAIN_CHR_TYPE_PIPE: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_UDP: + case VIR_DOMAIN_CHR_TYPE_TCP: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + case VIR_DOMAIN_CHR_TYPE_NMDM: + case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT: + case VIR_DOMAIN_CHR_TYPE_DBUS: + case VIR_DOMAIN_CHR_TYPE_LAST: default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("channel source type not supported")); diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index b97ba0a199..6487cb63df 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -824,11 +824,14 @@ xenParseSxprChar(const char *value, def->source->type = VIR_DOMAIN_CHR_TYPE_TCP; def->source->data.tcp.protocol = VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET; } else { - if ((def->source->type = virDomainChrTypeFromString(prefix)) < 0) { + int type = virDomainChrTypeFromString(prefix); + + if (type < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown chr device type '%s'"), prefix); goto error; } + def->source->type = type; } } @@ -920,6 +923,18 @@ xenParseSxprChar(const char *value, def->source->data.nix.listen = true; } break; + + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_DEV: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + case VIR_DOMAIN_CHR_TYPE_NMDM: + case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT: + case VIR_DOMAIN_CHR_TYPE_DBUS: + case VIR_DOMAIN_CHR_TYPE_LAST: + break; } return def; @@ -1525,6 +1540,12 @@ xenFormatSxprChr(virDomainChrDef *def, virBufferAddLit(buf, ",server,nowait"); break; + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + case VIR_DOMAIN_CHR_TYPE_NMDM: + case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT: + case VIR_DOMAIN_CHR_TYPE_DBUS: + case VIR_DOMAIN_CHR_TYPE_LAST: default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported chr device type '%s'"), type); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5ea88bf239..365b7d8292 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1304,7 +1304,7 @@ qemuBuildChardevStr(const virDomainChrSourceDef *dev, const char *path; virTristateSwitch append; - switch ((virDomainChrType) dev->type) { + switch (dev->type) { case VIR_DOMAIN_CHR_TYPE_NULL: virBufferAsprintf(&buf, "null,id=%s", charAlias); break; @@ -1484,7 +1484,7 @@ qemuBuildChardevCommand(virCommand *cmd, qemuDomainChrSourcePrivate *chrSourcePriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev); g_autofree char *charstr = NULL; - switch ((virDomainChrType) dev->type) { + switch (dev->type) { case VIR_DOMAIN_CHR_TYPE_TCP: if (dev->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES) { g_autofree char *objalias = NULL; @@ -8670,7 +8670,7 @@ qemuInterfaceVhostuserConnect(virCommand *cmd, { g_autofree char *charAlias = qemuAliasChardevFromDevAlias(net->info.alias); - switch ((virDomainChrType)net->data.vhostuser->type) { + switch (net->data.vhostuser->type) { case VIR_DOMAIN_CHR_TYPE_UNIX: if (qemuBuildChardevCommand(cmd, net->data.vhostuser, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 30ef5b7550..3432c83153 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2310,10 +2310,28 @@ qemuDomainObjPrivateXMLFormat(virBuffer *buf, case VIR_DOMAIN_CHR_TYPE_UNIX: monitorpath = priv->monConfig->data.nix.path; break; - default: case VIR_DOMAIN_CHR_TYPE_PTY: monitorpath = priv->monConfig->data.file.path; break; + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_DEV: + case VIR_DOMAIN_CHR_TYPE_FILE: + case VIR_DOMAIN_CHR_TYPE_PIPE: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_UDP: + case VIR_DOMAIN_CHR_TYPE_TCP: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + case VIR_DOMAIN_CHR_TYPE_NMDM: + case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT: + case VIR_DOMAIN_CHR_TYPE_DBUS: + case VIR_DOMAIN_CHR_TYPE_LAST: + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported monitor type '%s'"), + virDomainChrTypeToString(priv->monConfig->type)); + return -1; } virBufferEscapeString(buf, "<monitor path='%s'", monitorpath); @@ -2963,6 +2981,20 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, case VIR_DOMAIN_CHR_TYPE_UNIX: priv->monConfig->data.nix.path = monitorpath; break; + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_DEV: + case VIR_DOMAIN_CHR_TYPE_FILE: + case VIR_DOMAIN_CHR_TYPE_PIPE: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_UDP: + case VIR_DOMAIN_CHR_TYPE_TCP: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + case VIR_DOMAIN_CHR_TYPE_NMDM: + case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT: + case VIR_DOMAIN_CHR_TYPE_DBUS: + case VIR_DOMAIN_CHR_TYPE_LAST: default: VIR_FREE(monitorpath); virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index afe45c415b..f6294e4ed5 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6651,7 +6651,7 @@ qemuMonitorJSONAttachCharDevGetProps(const char *chrID, g_autoptr(virJSONValue) backendData = virJSONValueNewObject(); const char *backendType = NULL; - switch ((virDomainChrType)chr->type) { + switch (chr->type) { case VIR_DOMAIN_CHR_TYPE_NULL: case VIR_DOMAIN_CHR_TYPE_VC: case VIR_DOMAIN_CHR_TYPE_PTY: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 85bf452a59..f25cc0acf9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6878,7 +6878,7 @@ qemuProcessPrepareHostBackendChardevOne(virDomainDeviceDef *dev, devalias = data->fdprefix; } - switch ((virDomainChrType) chardev->type) { + switch (chardev->type) { case VIR_DOMAIN_CHR_TYPE_NULL: case VIR_DOMAIN_CHR_TYPE_VC: case VIR_DOMAIN_CHR_TYPE_PTY: diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index e8830c4cd3..4d6355741e 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1968,7 +1968,7 @@ qemuValidateDomainChrSourceDef(const virDomainChrSourceDef *def, const virDomainDef *vmdef, virQEMUCaps *qemuCaps) { - switch ((virDomainChrType)def->type) { + switch (def->type) { case VIR_DOMAIN_CHR_TYPE_TCP: if (qemuValidateDomainChrSourceReconnectDef(&def->data.tcp.reconnect) < 0) return -1; diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 008384dee8..c05a2fbaac 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -991,7 +991,7 @@ AppArmorSetChardevLabel(virSecurityManager *mgr, if (!secdef) return 0; - switch ((virDomainChrType)dev_source->type) { + switch (dev_source->type) { case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: case VIR_DOMAIN_CHR_TYPE_UNIX: @@ -1068,7 +1068,7 @@ AppArmorSetNetdevLabel(virSecurityManager *mgr, return 0; dev_source = net->data.vhostuser; - switch ((virDomainChrType)dev_source->type) { + switch (dev_source->type) { case VIR_DOMAIN_CHR_TYPE_UNIX: ret = reload_profile(mgr, def, dev_source->data.file.path, true); break; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 69c462de8b..211f5cf9a2 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1506,7 +1506,7 @@ virSecurityDACSetChardevLabelHelper(virSecurityManager *mgr, return -1; } - switch ((virDomainChrType)dev_source->type) { + switch (dev_source->type) { case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: if (virSecurityDACSetOwnership(mgr, NULL, @@ -1598,7 +1598,7 @@ virSecurityDACRestoreChardevLabelHelper(virSecurityManager *mgr, chardevStdioLogd) return 0; - switch ((virDomainChrType)dev_source->type) { + switch (dev_source->type) { case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: if (virSecurityDACRestoreFileLabelInternal(mgr, NULL, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 6f02baf2ce..9d9e308a38 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -2570,6 +2570,18 @@ virSecuritySELinuxSetChardevLabel(virSecurityManager *mgr, ret = 0; break; + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_PTY: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_UDP: + case VIR_DOMAIN_CHR_TYPE_TCP: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + case VIR_DOMAIN_CHR_TYPE_NMDM: + case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT: + case VIR_DOMAIN_CHR_TYPE_DBUS: + case VIR_DOMAIN_CHR_TYPE_LAST: default: ret = 0; break; @@ -2643,6 +2655,18 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManager *mgr, ret = 0; break; + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_PTY: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_UDP: + case VIR_DOMAIN_CHR_TYPE_TCP: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + case VIR_DOMAIN_CHR_TYPE_NMDM: + case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT: + case VIR_DOMAIN_CHR_TYPE_DBUS: + case VIR_DOMAIN_CHR_TYPE_LAST: default: ret = 0; break; diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index c391caa910..9c63d48c59 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -4117,6 +4117,18 @@ virVMXFormatSerial(virVMXContext *ctx, virDomainChrDef *def, def->source->data.tcp.listen ? "server" : "client"); break; + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_PTY: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_UDP: + case VIR_DOMAIN_CHR_TYPE_UNIX: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + case VIR_DOMAIN_CHR_TYPE_NMDM: + case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT: + case VIR_DOMAIN_CHR_TYPE_DBUS: + case VIR_DOMAIN_CHR_TYPE_LAST: default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported character device type '%s'"), @@ -4174,6 +4186,20 @@ virVMXFormatParallel(virVMXContext *ctx, virDomainChrDef *def, VIR_FREE(fileName); break; + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_PTY: + case VIR_DOMAIN_CHR_TYPE_PIPE: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_UDP: + case VIR_DOMAIN_CHR_TYPE_TCP: + case VIR_DOMAIN_CHR_TYPE_UNIX: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + case VIR_DOMAIN_CHR_TYPE_NMDM: + case VIR_DOMAIN_CHR_TYPE_LAST: + case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT: + case VIR_DOMAIN_CHR_TYPE_DBUS: default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported character device type '%s'"), diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 004c7cf1d6..7fd3c94f5e 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -1044,7 +1044,7 @@ testQemuPrepareHostBackendChardevOne(virDomainDeviceDef *dev, devalias = "monitor"; } - switch ((virDomainChrType) chardev->type) { + switch (chardev->type) { case VIR_DOMAIN_CHR_TYPE_NULL: case VIR_DOMAIN_CHR_TYPE_VC: case VIR_DOMAIN_CHR_TYPE_PTY: -- 2.35.1

On 5/23/22 3:08 PM, Michal Privoznik wrote:
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 30ef5b7550..3432c83153 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2310,10 +2310,28 @@ qemuDomainObjPrivateXMLFormat(virBuffer *buf, case VIR_DOMAIN_CHR_TYPE_UNIX: monitorpath = priv->monConfig->data.nix.path; break; - default: case VIR_DOMAIN_CHR_TYPE_PTY: monitorpath = priv->monConfig->data.file.path; break; + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_DEV: + case VIR_DOMAIN_CHR_TYPE_FILE: + case VIR_DOMAIN_CHR_TYPE_PIPE: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_UDP: + case VIR_DOMAIN_CHR_TYPE_TCP: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + case VIR_DOMAIN_CHR_TYPE_NMDM: + case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT: + case VIR_DOMAIN_CHR_TYPE_DBUS: + case VIR_DOMAIN_CHR_TYPE_LAST: + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported monitor type '%s'"), + virDomainChrTypeToString(priv->monConfig->type)); + return -1; }
virBufferEscapeString(buf, "<monitor path='%s'", monitorpath);
Shouldn't this be a standalone patch for the fix? -- 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/23/22 3:08 PM, Michal Privoznik wrote:
The virDomainChrDefParseXML() function uses old style of parsing XML (virXMLPropString + str2enum conversion). Use virXMLPropEnumDefault() which encapsulates those steps.
virXMLPropEnumDefault and virXMLPropEnum (above and in subject)
Signed-off-by: Michal Privoznik<mprivozn@redhat.com> --- src/ch/ch_monitor.c | 2 +- src/conf/domain_conf.c | 82 +++++++++++--------------------- src/conf/domain_conf.h | 2 +- src/conf/domain_validate.c | 2 +- src/conf/virchrdev.c | 29 +++++++++++ src/libxl/libxl_conf.c | 20 ++++++++ src/libxl/xen_common.c | 23 ++++++++- src/qemu/qemu_command.c | 6 +-- src/qemu/qemu_domain.c | 34 ++++++++++++- src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_validate.c | 2 +- src/security/security_apparmor.c | 4 +- src/security/security_dac.c | 4 +- src/security/security_selinux.c | 24 ++++++++++ src/vmx/vmx.c | 26 ++++++++++ tests/testutilsqemu.c | 2 +- 17 files changed, 197 insertions(+), 69 deletions(-)
Besides above, the comment in the separate response and what I commented in patch 4 Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> -- 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

The virDomainTPMDefParseXML() 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/domain_conf.c | 43 ++++++++++++---------------------------- src/conf/domain_conf.h | 6 +++--- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_validate.c | 1 + 5 files changed, 19 insertions(+), 35 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b5ce80eb76..4044515bdc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11677,9 +11677,6 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, int nnodes; size_t i; g_autofree char *path = NULL; - g_autofree char *model = NULL; - g_autofree char *backend = NULL; - g_autofree char *version = NULL; g_autofree char *secretuuid = NULL; g_autofree char *persistent_state = NULL; g_autofree xmlNodePtr *backends = NULL; @@ -11688,13 +11685,11 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, def = g_new0(virDomainTPMDef, 1); - model = virXMLPropString(node, "model"); - if (model != NULL && - (def->model = virDomainTPMModelTypeFromString(model)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown TPM frontend model '%s'"), model); + if (virXMLPropEnum(node, "model", + virDomainTPMModelTypeFromString, + VIR_XML_PROP_NONE, + &def->model) < 0) goto error; - } ctxt->node = node; @@ -11713,30 +11708,18 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, goto error; } - if (!(backend = virXMLPropString(backends[0], "type"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing TPM device backend type")); + if (virXMLPropEnum(backends[0], "type", + virDomainTPMBackendTypeFromString, + VIR_XML_PROP_REQUIRED, + &def->type) < 0) goto error; - } - if ((def->type = virDomainTPMBackendTypeFromString(backend)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown TPM backend type '%s'"), - backend); + if (virXMLPropEnumDefault(backends[0], "version", + virDomainTPMVersionTypeFromString, + VIR_XML_PROP_NONE, + &def->version, + VIR_DOMAIN_TPM_VERSION_DEFAULT) < 0) goto error; - } - - version = virXMLPropString(backends[0], "version"); - if (!version) { - def->version = VIR_DOMAIN_TPM_VERSION_DEFAULT; - } else { - if ((def->version = virDomainTPMVersionTypeFromString(version)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unsupported TPM version '%s'"), - version); - goto error; - } - } switch (def->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a81fb09678..13ffdbfe88 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1423,10 +1423,10 @@ typedef enum { #define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0" struct _virDomainTPMDef { - int type; /* virDomainTPMBackendType */ + virDomainTPMBackendType type; virDomainDeviceInfo info; - int model; /* virDomainTPMModel */ - int version; /* virDomainTPMVersion */ + virDomainTPMModel model; + virDomainTPMVersion version; union { struct { virDomainChrSourceDef *source; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 365b7d8292..6f716c63a0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9805,7 +9805,7 @@ qemuBuildTPMCommandLine(virCommand *cmd, g_autoptr(qemuFDPass) passtpm = NULL; g_autoptr(qemuFDPass) passcancel = NULL; - switch ((virDomainTPMBackendType) tpm->type) { + switch (tpm->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: { VIR_AUTOCLOSE fdtpm = -1; VIR_AUTOCLOSE fdcancel = -1; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3432c83153..d3da566bae 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11737,7 +11737,7 @@ qemuDomainDeviceBackendChardevForeachOne(virDomainDeviceDef *dev, return cb(dev, dev->data.rng->source.chardev, opaque); case VIR_DOMAIN_DEVICE_TPM: - switch ((virDomainTPMBackendType) dev->data.tpm->type) { + switch (dev->data.tpm->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: return cb(dev, dev->data.tpm->data.passthrough.source, opaque); diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 4d6355741e..8842e2f837 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -4728,6 +4728,7 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm, flag = QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY; break; + case VIR_DOMAIN_TPM_MODEL_DEFAULT: case VIR_DOMAIN_TPM_MODEL_LAST: default: virReportEnumRangeError(virDomainTPMModel, tpm->model); -- 2.35.1

On 5/23/22 3:08 PM, Michal Privoznik wrote:
The virDomainTPMDefParseXML() function uses old style of parsing XML (virXMLPropString + str2enum conversion). Use virXMLPropEnumDefault() which encapsulates those steps.
virXMLPropEnum and virXMLPropEnumDefault
Signed-off-by: Michal Privoznik<mprivozn@redhat.com> --- src/conf/domain_conf.c | 43 ++++++++++++---------------------------- src/conf/domain_conf.h | 6 +++--- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_validate.c | 1 + 5 files changed, 19 insertions(+), 35 deletions(-)
Besides above and what I commented in patch 4 Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> -- 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

The virDomainPanicDefParseXML() 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/domain_conf.c | 11 ++++------- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_validate.c | 2 +- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4044515bdc..85aae73873 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11788,7 +11788,6 @@ virDomainPanicDefParseXML(virDomainXMLOption *xmlopt, unsigned int flags) { virDomainPanicDef *panic; - g_autofree char *model = NULL; panic = g_new0(virDomainPanicDef, 1); @@ -11796,13 +11795,11 @@ virDomainPanicDefParseXML(virDomainXMLOption *xmlopt, &panic->info, flags) < 0) goto error; - model = virXMLPropString(node, "model"); - if (model != NULL && - (panic->model = virDomainPanicModelTypeFromString(model)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown panic model '%s'"), model); + if (virXMLPropEnum(node, "model", + virDomainPanicModelTypeFromString, + VIR_XML_PROP_NONE, + &panic->model) < 0) goto error; - } return panic; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 13ffdbfe88..3abc1dba36 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2619,7 +2619,7 @@ typedef enum { } virDomainPanicModel; struct _virDomainPanicDef { - int model; /* virDomainPanicModel */ + virDomainPanicModel model; virDomainDeviceInfo info; }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6f716c63a0..d3d69fed4f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9995,7 +9995,7 @@ qemuBuildPanicCommandLine(virCommand *cmd, size_t i; for (i = 0; i < def->npanics; i++) { - switch ((virDomainPanicModel) def->panics[i]->model) { + switch (def->panics[i]->model) { case VIR_DOMAIN_PANIC_MODEL_ISA: { g_autoptr(virJSONValue) props = NULL; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 8842e2f837..3a82a2adfa 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -927,7 +927,7 @@ qemuValidateDomainDefPanic(const virDomainDef *def, size_t i; for (i = 0; i < def->npanics; i++) { - switch ((virDomainPanicModel) def->panics[i]->model) { + switch (def->panics[i]->model) { case VIR_DOMAIN_PANIC_MODEL_S390: /* For s390 guests, the hardware provides the same * functionality as the pvpanic device. The address -- 2.35.1

On 5/23/22 3:08 PM, Michal Privoznik wrote:
The virDomainPanicDefParseXML() function uses old style of parsing XML (virXMLPropString + str2enum conversion). Use virXMLPropEnumDefault() which encapsulates those steps.
virXMLPropEnum (also subject)
Signed-off-by: Michal Privoznik<mprivozn@redhat.com> --- src/conf/domain_conf.c | 11 ++++------- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_validate.c | 2 +- 4 files changed, 7 insertions(+), 10 deletions(-)
Besides above and what I commented in patch 4 Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> -- 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

The virDomainInputDefParseXML() 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/domain_audit.c | 2 +- src/conf/domain_conf.c | 86 +++++++++++++------------------- src/conf/domain_conf.h | 6 +-- src/conf/domain_validate.c | 2 +- src/libxl/libxl_conf.c | 4 ++ src/libxl/xen_xl.c | 3 ++ src/libxl/xen_xm.c | 3 ++ src/qemu/qemu_cgroup.c | 12 +++++ src/qemu/qemu_command.c | 10 ++-- src/qemu/qemu_domain_address.c | 4 +- src/qemu/qemu_hotplug.c | 4 +- src/qemu/qemu_validate.c | 4 +- src/security/security_apparmor.c | 2 +- src/security/security_dac.c | 4 +- src/security/security_selinux.c | 4 +- 15 files changed, 80 insertions(+), 70 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 17a01c51ba..9ce14de80b 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -949,7 +949,7 @@ virDomainAuditInput(virDomainObj *vm, if (!(vmname = virAuditEncode("vm", vm->def->name))) return; - switch ((virDomainInputType) input->type) { + switch (input->type) { case VIR_DOMAIN_INPUT_TYPE_MOUSE: case VIR_DOMAIN_INPUT_TYPE_TABLET: case VIR_DOMAIN_INPUT_TYPE_KBD: diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 85aae73873..44ab79c1f0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1942,7 +1942,7 @@ void virDomainGraphicsDefFree(virDomainGraphicsDef *def) const char *virDomainInputDefGetPath(virDomainInputDef *input) { - switch ((virDomainInputType) input->type) { + switch (input->type) { case VIR_DOMAIN_INPUT_TYPE_MOUSE: case VIR_DOMAIN_INPUT_TYPE_TABLET: case VIR_DOMAIN_INPUT_TYPE_KBD: @@ -11818,70 +11818,54 @@ virDomainInputDefParseXML(virDomainXMLOption *xmlopt, { VIR_XPATH_NODE_AUTORESTORE(ctxt) virDomainInputDef *def; - g_autofree char *type = NULL; - g_autofree char *bus = NULL; - g_autofree char *model = NULL; + virDomainInputBus bus = VIR_DOMAIN_INPUT_BUS_PS2; xmlNodePtr source = NULL; def = g_new0(virDomainInputDef, 1); ctxt->node = node; - type = virXMLPropString(node, "type"); - bus = virXMLPropString(node, "bus"); - model = virXMLPropString(node, "model"); - - if (!type) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing input device type")); - goto error; - } - - if ((def->type = virDomainInputTypeFromString(type)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown input device type '%s'"), type); + if (virXMLPropEnum(node, "type", + virDomainInputTypeFromString, + VIR_XML_PROP_REQUIRED, + &def->type) < 0) goto error; - } - if (model && - ((def->model = virDomainInputModelTypeFromString(model)) < 0 || - def->model == VIR_DOMAIN_INPUT_MODEL_DEFAULT)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown input model '%s'"), model); + if (virXMLPropEnum(node, "model", + virDomainInputModelTypeFromString, + VIR_XML_PROP_NONZERO, + &def->model) < 0) goto error; - } - - if (bus) { - if ((def->bus = virDomainInputBusTypeFromString(bus)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown input bus type '%s'"), bus); - goto error; - } - } else { - if (dom->os.type == VIR_DOMAIN_OSTYPE_HVM) { - if ((def->type == VIR_DOMAIN_INPUT_TYPE_MOUSE || - def->type == VIR_DOMAIN_INPUT_TYPE_KBD) && - (ARCH_IS_X86(dom->os.arch) || dom->os.arch == VIR_ARCH_NONE)) { - def->bus = VIR_DOMAIN_INPUT_BUS_PS2; - } else if (ARCH_IS_S390(dom->os.arch) || - def->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) { - def->bus = VIR_DOMAIN_INPUT_BUS_VIRTIO; - } else if (def->type == VIR_DOMAIN_INPUT_TYPE_EVDEV) { - def->bus = VIR_DOMAIN_INPUT_BUS_NONE; - } else { - def->bus = VIR_DOMAIN_INPUT_BUS_USB; - } - } else if (dom->os.type == VIR_DOMAIN_OSTYPE_XEN || - dom->os.type == VIR_DOMAIN_OSTYPE_XENPVH) { - def->bus = VIR_DOMAIN_INPUT_BUS_XEN; + if (dom->os.type == VIR_DOMAIN_OSTYPE_HVM) { + if ((def->type == VIR_DOMAIN_INPUT_TYPE_MOUSE || + def->type == VIR_DOMAIN_INPUT_TYPE_KBD) && + (ARCH_IS_X86(dom->os.arch) || dom->os.arch == VIR_ARCH_NONE)) { + bus = VIR_DOMAIN_INPUT_BUS_PS2; + } else if (ARCH_IS_S390(dom->os.arch) || + def->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) { + bus = VIR_DOMAIN_INPUT_BUS_VIRTIO; + } else if (def->type == VIR_DOMAIN_INPUT_TYPE_EVDEV) { + bus = VIR_DOMAIN_INPUT_BUS_NONE; } else { - if ((dom->virtType == VIR_DOMAIN_VIRT_VZ || - dom->virtType == VIR_DOMAIN_VIRT_PARALLELS)) - def->bus = VIR_DOMAIN_INPUT_BUS_PARALLELS; + bus = VIR_DOMAIN_INPUT_BUS_USB; } + } else if (dom->os.type == VIR_DOMAIN_OSTYPE_XEN || + dom->os.type == VIR_DOMAIN_OSTYPE_XENPVH) { + bus = VIR_DOMAIN_INPUT_BUS_XEN; + } else { + if ((dom->virtType == VIR_DOMAIN_VIRT_VZ || + dom->virtType == VIR_DOMAIN_VIRT_PARALLELS)) + bus = VIR_DOMAIN_INPUT_BUS_PARALLELS; } + if (virXMLPropEnumDefault(node, "bus", + virDomainInputBusTypeFromString, + VIR_XML_PROP_NONE, + &def->bus, bus) < 0) + goto error; + + if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, &def->info, flags) < 0) goto error; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3abc1dba36..b3d51565e3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1493,9 +1493,9 @@ typedef enum { } virDomainInputSourceGrabToggle; struct _virDomainInputDef { - int type; - int bus; - int model; /* virDomainInputModel */ + virDomainInputType type; + virDomainInputBus bus; + virDomainInputModel model; struct { char *evdev; virDomainInputSourceGrab grab; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 4fce7059dc..5205834d12 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -2276,7 +2276,7 @@ virDomainInputDefValidate(const virDomainInputDef *input, } } - switch ((virDomainInputType) input->type) { + switch (input->type) { case VIR_DOMAIN_INPUT_TYPE_MOUSE: case VIR_DOMAIN_INPUT_TYPE_TABLET: case VIR_DOMAIN_INPUT_TYPE_KBD: diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 0cfd49ebcf..82af406e2d 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -725,6 +725,10 @@ libxlMakeDomBuildInfo(virDomainDef *def, VIR_FREE(*usbdevice); *usbdevice = g_strdup("tablet"); break; + case VIR_DOMAIN_INPUT_TYPE_KBD: + case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: + case VIR_DOMAIN_INPUT_TYPE_EVDEV: + case VIR_DOMAIN_INPUT_TYPE_LAST: default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Unknown input device type")); diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c index eb3b0b3718..76237dfa25 100644 --- a/src/libxl/xen_xl.c +++ b/src/libxl/xen_xl.c @@ -1757,6 +1757,9 @@ xenFormatXLInputDevs(virConf *conf, virDomainDef *def) case VIR_DOMAIN_INPUT_TYPE_KBD: devtype = "keyboard"; break; + case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: + case VIR_DOMAIN_INPUT_TYPE_EVDEV: + case VIR_DOMAIN_INPUT_TYPE_LAST: default: continue; } diff --git a/src/libxl/xen_xm.c b/src/libxl/xen_xm.c index a962da9cad..31d2d2e6a1 100644 --- a/src/libxl/xen_xm.c +++ b/src/libxl/xen_xm.c @@ -521,6 +521,9 @@ xenFormatXMInputDevs(virConf *conf, virDomainDef *def) case VIR_DOMAIN_INPUT_TYPE_KBD: devtype = "keyboard"; break; + case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: + case VIR_DOMAIN_INPUT_TYPE_EVDEV: + case VIR_DOMAIN_INPUT_TYPE_LAST: default: continue; } diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index aa0c927578..7c24e187c9 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -384,6 +384,12 @@ qemuSetupInputCgroup(virDomainObj *vm, return qemuCgroupAllowDevicePath(vm, dev->source.evdev, VIR_CGROUP_DEVICE_RW, false); break; + + case VIR_DOMAIN_INPUT_TYPE_MOUSE: + case VIR_DOMAIN_INPUT_TYPE_TABLET: + case VIR_DOMAIN_INPUT_TYPE_KBD: + case VIR_DOMAIN_INPUT_TYPE_LAST: + break; } return ret; @@ -405,6 +411,12 @@ qemuTeardownInputCgroup(virDomainObj *vm, return qemuCgroupDenyDevicePath(vm, dev->source.evdev, VIR_CGROUP_DEVICE_RWM, false); break; + + case VIR_DOMAIN_INPUT_TYPE_MOUSE: + case VIR_DOMAIN_INPUT_TYPE_TABLET: + case VIR_DOMAIN_INPUT_TYPE_KBD: + case VIR_DOMAIN_INPUT_TYPE_LAST: + break; } return 0; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d3d69fed4f..b863fd1c32 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -873,7 +873,7 @@ qemuBuildVirtioDevGetConfigDev(const virDomainDeviceDef *device, case VIR_DOMAIN_DEVICE_INPUT: *virtioOptions = device->data.input->virtio; - switch ((virDomainInputType) device->data.input->type) { + switch (device->data.input->type) { case VIR_DOMAIN_INPUT_TYPE_MOUSE: *baseName = "virtio-mouse"; break; @@ -4510,7 +4510,7 @@ qemuBuildInputVirtioDevProps(const virDomainDef *def, g_autoptr(virJSONValue) props = NULL; const char *evdev = NULL; - switch ((virDomainInputType)dev->type) { + switch (dev->type) { case VIR_DOMAIN_INPUT_TYPE_MOUSE: case VIR_DOMAIN_INPUT_TYPE_TABLET: case VIR_DOMAIN_INPUT_TYPE_KBD: @@ -4559,6 +4559,10 @@ qemuBuildInputUSBDevProps(const virDomainDef *def, case VIR_DOMAIN_INPUT_TYPE_KBD: driver = "usb-kbd"; break; + case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: + case VIR_DOMAIN_INPUT_TYPE_EVDEV: + case VIR_DOMAIN_INPUT_TYPE_LAST: + break; } if (virJSONValueObjectAdd(&props, @@ -4623,7 +4627,7 @@ qemuBuildInputCommandLine(virCommand *cmd, } else { g_autoptr(virJSONValue) props = NULL; - switch ((virDomainInputBus) input->bus) { + switch (input->bus) { case VIR_DOMAIN_INPUT_BUS_USB: if (!(props = qemuBuildInputUSBDevProps(def, input))) return -1; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 753733d1b9..3b1139d975 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -960,9 +960,9 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev, return pciFlags; case VIR_DOMAIN_DEVICE_INPUT: - switch ((virDomainInputBus) dev->data.input->bus) { + switch (dev->data.input->bus) { case VIR_DOMAIN_INPUT_BUS_VIRTIO: - switch ((virDomainInputModel) dev->data.input->model) { + switch (dev->data.input->model) { case VIR_DOMAIN_INPUT_MODEL_VIRTIO_TRANSITIONAL: /* Transitional devices only work in conventional PCI slots */ return pciFlags; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 24df66cc9f..48dfac3f07 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3139,7 +3139,7 @@ qemuDomainAttachInputDevice(virQEMUDriver *driver, qemuAssignDeviceInputAlias(vm->def, input, -1); - switch ((virDomainInputBus) input->bus) { + switch (input->bus) { case VIR_DOMAIN_INPUT_BUS_USB: if (virDomainUSBAddressEnsure(priv->usbaddrs, &input->info) < 0) return -1; @@ -5882,7 +5882,7 @@ qemuDomainDetachPrepInput(virDomainObj *vm, } *detach = input = vm->def->inputs[idx]; - switch ((virDomainInputBus) input->bus) { + switch (input->bus) { case VIR_DOMAIN_INPUT_BUS_PS2: case VIR_DOMAIN_INPUT_BUS_XEN: case VIR_DOMAIN_INPUT_BUS_PARALLELS: diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 3a82a2adfa..f54b4587c0 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -4786,7 +4786,7 @@ qemuValidateDomainDeviceDefInput(const virDomainInputDef *input, return 0; /* model=virtio-(non-)transitional is unsupported */ - switch ((virDomainInputModel)input->model) { + switch (input->model) { case VIR_DOMAIN_INPUT_MODEL_VIRTIO_TRANSITIONAL: case VIR_DOMAIN_INPUT_MODEL_VIRTIO_NON_TRANSITIONAL: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -4804,7 +4804,7 @@ qemuValidateDomainDeviceDefInput(const virDomainInputDef *input, return -1; } - switch ((virDomainInputType)input->type) { + switch (input->type) { case VIR_DOMAIN_INPUT_TYPE_MOUSE: baseName = "virtio-mouse"; cap = QEMU_CAPS_VIRTIO_MOUSE; diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index c05a2fbaac..7160ef99b8 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -712,7 +712,7 @@ AppArmorSetInputLabel(virSecurityManager *mgr, if (input == NULL) return 0; - switch ((virDomainInputType)input->type) { + switch (input->type) { case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: case VIR_DOMAIN_INPUT_TYPE_EVDEV: if (input->source.evdev == NULL) { diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 211f5cf9a2..a74337fcf7 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1792,7 +1792,7 @@ virSecurityDACSetInputLabel(virSecurityManager *mgr, if (seclabel && !seclabel->relabel) return 0; - switch ((virDomainInputType)input->type) { + switch (input->type) { case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: case VIR_DOMAIN_INPUT_TYPE_EVDEV: if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL) < 0) @@ -1821,7 +1821,7 @@ virSecurityDACRestoreInputLabel(virSecurityManager *mgr, { int ret = -1; - switch ((virDomainInputType)input->type) { + switch (input->type) { case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: case VIR_DOMAIN_INPUT_TYPE_EVDEV: ret = virSecurityDACRestoreFileLabel(mgr, input->source.evdev); diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 9d9e308a38..22747b667a 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1512,7 +1512,7 @@ virSecuritySELinuxSetInputLabel(virSecurityManager *mgr, if (seclabel == NULL) return 0; - switch ((virDomainInputType)input->type) { + switch (input->type) { case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: case VIR_DOMAIN_INPUT_TYPE_EVDEV: if (virSecuritySELinuxSetFilecon(mgr, input->source.evdev, @@ -1543,7 +1543,7 @@ virSecuritySELinuxRestoreInputLabel(virSecurityManager *mgr, if (seclabel == NULL) return 0; - switch ((virDomainInputType)input->type) { + switch (input->type) { case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH: case VIR_DOMAIN_INPUT_TYPE_EVDEV: rc = virSecuritySELinuxRestoreFileLabel(mgr, input->source.evdev, true); -- 2.35.1

On 5/23/22 3:08 PM, Michal Privoznik wrote:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 85aae73873..44ab79c1f0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1942,7 +1942,7 @@ void virDomainGraphicsDefFree(virDomainGraphicsDef *def)
const char *virDomainInputDefGetPath(virDomainInputDef *input) { - switch ((virDomainInputType) input->type) { + switch (input->type) { case VIR_DOMAIN_INPUT_TYPE_MOUSE: case VIR_DOMAIN_INPUT_TYPE_TABLET: case VIR_DOMAIN_INPUT_TYPE_KBD: @@ -11818,70 +11818,54 @@ virDomainInputDefParseXML(virDomainXMLOption *xmlopt, { VIR_XPATH_NODE_AUTORESTORE(ctxt) virDomainInputDef *def; - g_autofree char *type = NULL; - g_autofree char *bus = NULL; - g_autofree char *model = NULL; + virDomainInputBus bus = VIR_DOMAIN_INPUT_BUS_PS2; xmlNodePtr source = NULL;
def = g_new0(virDomainInputDef, 1);
ctxt->node = node;
- type = virXMLPropString(node, "type"); - bus = virXMLPropString(node, "bus"); - model = virXMLPropString(node, "model"); - - if (!type) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing input device type")); - goto error; - } - - if ((def->type = virDomainInputTypeFromString(type)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown input device type '%s'"), type); + if (virXMLPropEnum(node, "type", + virDomainInputTypeFromString, + VIR_XML_PROP_REQUIRED, + &def->type) < 0) goto error; - }
- if (model && - ((def->model = virDomainInputModelTypeFromString(model)) < 0 || - def->model == VIR_DOMAIN_INPUT_MODEL_DEFAULT)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown input model '%s'"), model); + if (virXMLPropEnum(node, "model", + virDomainInputModelTypeFromString, + VIR_XML_PROP_NONZERO, + &def->model) < 0) goto error; - } - - if (bus) { - if ((def->bus = virDomainInputBusTypeFromString(bus)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown input bus type '%s'"), bus); - goto error; - }
- } else { - if (dom->os.type == VIR_DOMAIN_OSTYPE_HVM) { - if ((def->type == VIR_DOMAIN_INPUT_TYPE_MOUSE || - def->type == VIR_DOMAIN_INPUT_TYPE_KBD) && - (ARCH_IS_X86(dom->os.arch) || dom->os.arch == VIR_ARCH_NONE)) { - def->bus = VIR_DOMAIN_INPUT_BUS_PS2; - } else if (ARCH_IS_S390(dom->os.arch) || - def->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) { - def->bus = VIR_DOMAIN_INPUT_BUS_VIRTIO; - } else if (def->type == VIR_DOMAIN_INPUT_TYPE_EVDEV) { - def->bus = VIR_DOMAIN_INPUT_BUS_NONE; - } else { - def->bus = VIR_DOMAIN_INPUT_BUS_USB; - } - } else if (dom->os.type == VIR_DOMAIN_OSTYPE_XEN || - dom->os.type == VIR_DOMAIN_OSTYPE_XENPVH) { - def->bus = VIR_DOMAIN_INPUT_BUS_XEN; + if (dom->os.type == VIR_DOMAIN_OSTYPE_HVM) { + if ((def->type == VIR_DOMAIN_INPUT_TYPE_MOUSE || + def->type == VIR_DOMAIN_INPUT_TYPE_KBD) && + (ARCH_IS_X86(dom->os.arch) || dom->os.arch == VIR_ARCH_NONE)) { + bus = VIR_DOMAIN_INPUT_BUS_PS2; + } else if (ARCH_IS_S390(dom->os.arch) || + def->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) { + bus = VIR_DOMAIN_INPUT_BUS_VIRTIO; + } else if (def->type == VIR_DOMAIN_INPUT_TYPE_EVDEV) { + bus = VIR_DOMAIN_INPUT_BUS_NONE; } else { - if ((dom->virtType == VIR_DOMAIN_VIRT_VZ || - dom->virtType == VIR_DOMAIN_VIRT_PARALLELS)) - def->bus = VIR_DOMAIN_INPUT_BUS_PARALLELS; + bus = VIR_DOMAIN_INPUT_BUS_USB; } + } else if (dom->os.type == VIR_DOMAIN_OSTYPE_XEN || + dom->os.type == VIR_DOMAIN_OSTYPE_XENPVH) { + bus = VIR_DOMAIN_INPUT_BUS_XEN; + } else { + if ((dom->virtType == VIR_DOMAIN_VIRT_VZ || + dom->virtType == VIR_DOMAIN_VIRT_PARALLELS)) + bus = VIR_DOMAIN_INPUT_BUS_PARALLELS; }
+ if (virXMLPropEnumDefault(node, "bus", + virDomainInputBusTypeFromString, + VIR_XML_PROP_NONE, + &def->bus, bus) < 0) + goto error; + +
One empty line should be sufficient.
if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, &def->info, flags) < 0) goto error;
-- 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/23/22 3:08 PM, Michal Privoznik wrote:
The virDomainInputDefParseXML() function uses old style of parsing XML (virXMLPropString + str2enum conversion). Use virXMLPropEnumDefault() which encapsulates those steps.
virXMLPropEnum and virXMLPropEnumDefault
Signed-off-by: Michal Privoznik<mprivozn@redhat.com> --- src/conf/domain_audit.c | 2 +- src/conf/domain_conf.c | 86 +++++++++++++------------------- src/conf/domain_conf.h | 6 +-- src/conf/domain_validate.c | 2 +- src/libxl/libxl_conf.c | 4 ++ src/libxl/xen_xl.c | 3 ++ src/libxl/xen_xm.c | 3 ++ src/qemu/qemu_cgroup.c | 12 +++++ src/qemu/qemu_command.c | 10 ++-- src/qemu/qemu_domain_address.c | 4 +- src/qemu/qemu_hotplug.c | 4 +- src/qemu/qemu_validate.c | 4 +- src/security/security_apparmor.c | 2 +- src/security/security_dac.c | 4 +- src/security/security_selinux.c | 4 +- 15 files changed, 80 insertions(+), 70 deletions(-)
Besides above and what I commented in patch 4 Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> -- 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

The virDomainHubDefParseXML() 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/domain_conf.c | 17 ++++------------- src/conf/domain_conf.h | 14 +++++++------- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 44ab79c1f0..52a34cd131 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11934,23 +11934,14 @@ virDomainHubDefParseXML(virDomainXMLOption *xmlopt, unsigned int flags) { virDomainHubDef *def; - g_autofree char *type = NULL; def = g_new0(virDomainHubDef, 1); - type = virXMLPropString(node, "type"); - - if (!type) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing hub device type")); + if (virXMLPropEnum(node, "type", + virDomainHubTypeFromString, + VIR_XML_PROP_REQUIRED, + &def->type) < 0) goto error; - } - - if ((def->type = virDomainHubTypeFromString(type)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown hub device type '%s'"), type); - goto error; - } if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, &def->info, flags) < 0) goto error; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b3d51565e3..cad19a3d5d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1381,8 +1381,14 @@ struct _virDomainSmartcardDef { virDomainDeviceInfo info; }; +typedef enum { + VIR_DOMAIN_HUB_TYPE_USB, + + VIR_DOMAIN_HUB_TYPE_LAST +} virDomainHubType; + struct _virDomainHubDef { - int type; + virDomainHubType type; virDomainDeviceInfo info; }; @@ -1881,12 +1887,6 @@ typedef enum { VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST } virDomainGraphicsListenType; -typedef enum { - VIR_DOMAIN_HUB_TYPE_USB, - - VIR_DOMAIN_HUB_TYPE_LAST -} virDomainHubType; - struct _virDomainGraphicsListenDef { virDomainGraphicsListenType type; char *address; -- 2.35.1

On 5/23/22 3:08 PM, Michal Privoznik wrote:
The virDomainHubDefParseXML() function uses old style of parsing XML (virXMLPropString + str2enum conversion). Use virXMLPropEnumDefault() which encapsulates those steps.
virXMLPropEnum
Signed-off-by: Michal Privoznik<mprivozn@redhat.com> --- src/conf/domain_conf.c | 17 ++++------------- src/conf/domain_conf.h | 14 +++++++------- 2 files changed, 11 insertions(+), 20 deletions(-)
Besides above and what I commented in patch 4 Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> -- 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

The virDomainTimerDefParseXML() 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/domain_conf.c | 105 +++++++++++++-------------------------- src/conf/domain_conf.h | 14 +++--- src/libxl/libxl_conf.c | 6 ++- src/libxl/xen_common.c | 6 ++- src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/qemu/qemu_command.c | 11 +++- src/qemu/qemu_validate.c | 8 ++- 8 files changed, 69 insertions(+), 85 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 52a34cd131..27fe6c9fbf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11962,98 +11962,61 @@ virDomainTimerDefParseXML(xmlNodePtr node, virDomainTimerDef *def; VIR_XPATH_NODE_AUTORESTORE(ctxt) xmlNodePtr catchup; - int ret; - g_autofree char *name = NULL; - g_autofree char *tickpolicy = NULL; - g_autofree char *track = NULL; - g_autofree char *mode = NULL; def = g_new0(virDomainTimerDef, 1); ctxt->node = node; - name = virXMLPropString(node, "name"); - if (name == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing timer name")); + if (virXMLPropEnum(node, "name", + virDomainTimerNameTypeFromString, + VIR_XML_PROP_REQUIRED, + &def->name) < 0) goto error; - } - if ((def->name = virDomainTimerNameTypeFromString(name)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown timer name '%s'"), name); - goto error; - } if (virXMLPropTristateBool(node, "present", VIR_XML_PROP_NONE, &def->present) < 0) goto error; - tickpolicy = virXMLPropString(node, "tickpolicy"); - if (tickpolicy != NULL) { - if ((def->tickpolicy = virDomainTimerTickpolicyTypeFromString(tickpolicy)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown timer tickpolicy '%s'"), tickpolicy); - goto error; - } - } + if (virXMLPropEnum(node, "tickpolicy", + virDomainTimerTickpolicyTypeFromString, + VIR_XML_PROP_NONZERO, + &def->tickpolicy) < 0) + goto error; - track = virXMLPropString(node, "track"); - if (track != NULL) { - if ((def->track = virDomainTimerTrackTypeFromString(track)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown timer track '%s'"), track); - goto error; - } - } + if (virXMLPropEnum(node, "track", + virDomainTimerTrackTypeFromString, + VIR_XML_PROP_NONZERO, + &def->track) < 0) + goto error; - ret = virXPathULongLong("string(./@frequency)", ctxt, &def->frequency); - if (ret == -1) { - def->frequency = 0; - } else if (ret < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("invalid timer frequency")); + if (virXMLPropULongLong(node, "frequency", 10, + VIR_XML_PROP_NONE, + &def->frequency) < 0) goto error; - } - mode = virXMLPropString(node, "mode"); - if (mode != NULL) { - if ((def->mode = virDomainTimerModeTypeFromString(mode)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown timer mode '%s'"), mode); - goto error; - } - } + if (virXMLPropEnum(node, "mode", + virDomainTimerModeTypeFromString, + VIR_XML_PROP_NONZERO, + &def->mode) < 0) + goto error; catchup = virXPathNode("./catchup", ctxt); if (catchup != NULL) { - ret = virXPathULong("string(./catchup/@threshold)", ctxt, - &def->catchup.threshold); - if (ret == -1) { - def->catchup.threshold = 0; - } else if (ret < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("invalid catchup threshold")); + if (virXMLPropUInt(catchup, "threshold", 10, + VIR_XML_PROP_NONE, + &def->catchup.threshold) < 0) goto error; - } - ret = virXPathULong("string(./catchup/@slew)", ctxt, &def->catchup.slew); - if (ret == -1) { - def->catchup.slew = 0; - } else if (ret < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("invalid catchup slew")); + if (virXMLPropUInt(catchup, "slew", 10, + VIR_XML_PROP_NONE, + &def->catchup.slew) < 0) goto error; - } - ret = virXPathULong("string(./catchup/@limit)", ctxt, &def->catchup.limit); - if (ret == -1) { - def->catchup.limit = 0; - } else if (ret < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("invalid catchup limit")); + if (virXMLPropUInt(catchup, "limit", 10, + VIR_XML_PROP_NONE, + &def->catchup.limit) < 0) goto error; - } } return def; @@ -26197,11 +26160,11 @@ virDomainTimerDefFormat(virBuffer *buf, } if (def->catchup.threshold > 0) - virBufferAsprintf(&catchupAttr, " threshold='%lu'", def->catchup.threshold); + virBufferAsprintf(&catchupAttr, " threshold='%u'", def->catchup.threshold); if (def->catchup.slew > 0) - virBufferAsprintf(&catchupAttr, " slew='%lu'", def->catchup.slew); + virBufferAsprintf(&catchupAttr, " slew='%u'", def->catchup.slew); if (def->catchup.limit > 0) - virBufferAsprintf(&catchupAttr, " limit='%lu'", def->catchup.limit); + virBufferAsprintf(&catchupAttr, " limit='%u'", def->catchup.limit); virXMLFormatElement(&timerChld, "catchup", &catchupAttr, NULL); virXMLFormatElement(buf, "timer", &timerAttr, &timerChld); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cad19a3d5d..a6acffb4a4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2442,24 +2442,24 @@ struct _virDomainThreadSchedParam { }; struct _virDomainTimerCatchupDef { - unsigned long threshold; - unsigned long slew; - unsigned long limit; + unsigned int threshold; + unsigned int slew; + unsigned int limit; }; struct _virDomainTimerDef { - int name; + virDomainTimerNameType name; virTristateBool present; - int tickpolicy; /* enum virDomainTimerTickpolicyType */ + virDomainTimerTickpolicyType tickpolicy; virDomainTimerCatchupDef catchup; /* track is only valid for name='platform|rtc' */ - int track; /* enum virDomainTimerTrackType */ + virDomainTimerTrackType track; /* frequency & mode are only valid for name='tsc' */ unsigned long long frequency; /* in Hz, unspecified = 0 */ - int mode; /* enum virDomainTimerModeType */ + virDomainTimerModeType mode; }; typedef enum { diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 82af406e2d..6aba458281 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -406,7 +406,7 @@ libxlMakeDomBuildInfo(virDomainDef *def, } for (i = 0; i < clock.ntimers; i++) { - switch ((virDomainTimerNameType) clock.timers[i]->name) { + switch (clock.timers[i]->name) { case VIR_DOMAIN_TIMER_NAME_TSC: switch (clock.timers[i]->mode) { case VIR_DOMAIN_TIMER_MODE_NATIVE: @@ -418,6 +418,10 @@ libxlMakeDomBuildInfo(virDomainDef *def, case VIR_DOMAIN_TIMER_MODE_EMULATE: b_info->tsc_mode = LIBXL_TSC_MODE_ALWAYS_EMULATE; break; + case VIR_DOMAIN_TIMER_MODE_NONE: + case VIR_DOMAIN_TIMER_MODE_AUTO: + case VIR_DOMAIN_TIMER_MODE_SMPSAFE: + case VIR_DOMAIN_TIMER_MODE_LAST: default: b_info->tsc_mode = LIBXL_TSC_MODE_DEFAULT; } diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index 6487cb63df..f8c63d5f92 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -2106,7 +2106,7 @@ xenFormatHypervisorFeatures(virConf *conf, virDomainDef *def) } for (i = 0; i < def->clock.ntimers; i++) { - switch ((virDomainTimerNameType)def->clock.timers[i]->name) { + switch (def->clock.timers[i]->name) { case VIR_DOMAIN_TIMER_NAME_TSC: switch (def->clock.timers[i]->mode) { case VIR_DOMAIN_TIMER_MODE_NATIVE: @@ -2121,6 +2121,10 @@ xenFormatHypervisorFeatures(virConf *conf, virDomainDef *def) if (xenConfigSetString(conf, "tsc_mode", "always_emulate") < 0) return -1; break; + case VIR_DOMAIN_TIMER_MODE_NONE: + case VIR_DOMAIN_TIMER_MODE_AUTO: + case VIR_DOMAIN_TIMER_MODE_SMPSAFE: + case VIR_DOMAIN_TIMER_MODE_LAST: default: if (xenConfigSetString(conf, "tsc_mode", "default") < 0) return -1; diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index d31fff5f98..420ec07650 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -335,7 +335,7 @@ static int virLXCCgroupSetupDeviceACL(virDomainDef *def, if (timer->present == VIR_TRISTATE_BOOL_NO) continue; - switch ((virDomainTimerNameType)timer->name) { + switch (timer->name) { case VIR_DOMAIN_TIMER_NAME_PLATFORM: case VIR_DOMAIN_TIMER_NAME_TSC: case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index d936f34793..42356eb1c9 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1506,7 +1506,7 @@ virLXCControllerSetupTimers(virLXCController *ctrl) if (timer->present == VIR_TRISTATE_BOOL_NO) continue; - switch ((virDomainTimerNameType)timer->name) { + switch (timer->name) { case VIR_DOMAIN_TIMER_NAME_PLATFORM: case VIR_DOMAIN_TIMER_NAME_TSC: case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b863fd1c32..6574c4c58b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6200,6 +6200,8 @@ qemuBuildClockArgStr(virDomainClockDef *def) case VIR_DOMAIN_TIMER_TRACK_REALTIME: virBufferAddLit(&buf, ",clock=rt"); break; + case VIR_DOMAIN_TIMER_TRACK_LAST: + break; } switch (def->timers[i]->tickpolicy) { @@ -6215,6 +6217,8 @@ qemuBuildClockArgStr(virDomainClockDef *def) case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: return NULL; + case VIR_DOMAIN_TIMER_TICKPOLICY_LAST: + break; } break; /* no need to check other timers - there is only one rtc */ } @@ -6246,7 +6250,7 @@ qemuBuildClockCommandLine(virCommand *cmd, } for (i = 0; i < def->clock.ntimers; i++) { - switch ((virDomainTimerNameType)def->clock.timers[i]->name) { + switch (def->clock.timers[i]->name) { case VIR_DOMAIN_TIMER_NAME_PLATFORM: /* qemuDomainDefValidateClockTimers will handle this * error condition */ @@ -6286,6 +6290,8 @@ qemuBuildClockCommandLine(virCommand *cmd, case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: /* no way to support this mode for pit in qemu */ return -1; + case VIR_DOMAIN_TIMER_TICKPOLICY_LAST: + return -1; } break; @@ -6640,7 +6646,7 @@ qemuBuildCpuCommandLine(virCommand *cmd, for (i = 0; i < def->clock.ntimers; i++) { virDomainTimerDef *timer = def->clock.timers[i]; - switch ((virDomainTimerNameType)timer->name) { + switch (timer->name) { case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: if (timer->present != VIR_TRISTATE_BOOL_ABSENT) { /* QEMU expects on/off -> virTristateSwitch. */ @@ -6667,6 +6673,7 @@ qemuBuildCpuCommandLine(virCommand *cmd, case VIR_DOMAIN_TIMER_TICKPOLICY_NONE: case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP: case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: + case VIR_DOMAIN_TIMER_TICKPOLICY_LAST: break; } break; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index f54b4587c0..a48b81c9c6 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -393,7 +393,7 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, for (i = 0; i < def->clock.ntimers; i++) { virDomainTimerDef *timer = def->clock.timers[i]; - switch ((virDomainTimerNameType)timer->name) { + switch (timer->name) { case VIR_DOMAIN_TIMER_NAME_PLATFORM: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported timer type (name) '%s'"), @@ -426,6 +426,7 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, case VIR_DOMAIN_TIMER_TRACK_REALTIME: break; case VIR_DOMAIN_TIMER_TRACK_BOOT: + case VIR_DOMAIN_TIMER_TRACK_LAST: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported rtc timer track '%s'"), virDomainTimerTrackTypeToString(timer->track)); @@ -443,6 +444,7 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, break; case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: + case VIR_DOMAIN_TIMER_TICKPOLICY_LAST: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported rtc timer tickpolicy '%s'"), virDomainTimerTickpolicyTypeToString( @@ -474,6 +476,8 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, virDomainTimerTickpolicyTypeToString( timer->tickpolicy)); return -1; + case VIR_DOMAIN_TIMER_TICKPOLICY_LAST: + break; } break; @@ -526,6 +530,8 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, virDomainTimerNameTypeToString(timer->name), virDomainTimerTickpolicyTypeToString(timer->tickpolicy)); return -1; + case VIR_DOMAIN_TIMER_TICKPOLICY_LAST: + break; } break; } -- 2.35.1

On 5/23/22 3:08 PM, Michal Privoznik wrote:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 52a34cd131..27fe6c9fbf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11962,98 +11962,61 @@ virDomainTimerDefParseXML(xmlNodePtr node, virDomainTimerDef *def; VIR_XPATH_NODE_AUTORESTORE(ctxt) xmlNodePtr catchup; - int ret; - g_autofree char *name = NULL; - g_autofree char *tickpolicy = NULL; - g_autofree char *track = NULL; - g_autofree char *mode = NULL;
def = g_new0(virDomainTimerDef, 1);
ctxt->node = node;
- name = virXMLPropString(node, "name"); - if (name == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing timer name")); + if (virXMLPropEnum(node, "name", + virDomainTimerNameTypeFromString, + VIR_XML_PROP_REQUIRED, + &def->name) < 0) goto error; - } - if ((def->name = virDomainTimerNameTypeFromString(name)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown timer name '%s'"), name); - goto error; - }
if (virXMLPropTristateBool(node, "present", VIR_XML_PROP_NONE, &def->present) < 0) goto error;
- tickpolicy = virXMLPropString(node, "tickpolicy"); - if (tickpolicy != NULL) { - if ((def->tickpolicy = virDomainTimerTickpolicyTypeFromString(tickpolicy)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown timer tickpolicy '%s'"), tickpolicy); - goto error; - } - } + if (virXMLPropEnum(node, "tickpolicy", + virDomainTimerTickpolicyTypeFromString, + VIR_XML_PROP_NONZERO, + &def->tickpolicy) < 0) + goto error;
- track = virXMLPropString(node, "track"); - if (track != NULL) { - if ((def->track = virDomainTimerTrackTypeFromString(track)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown timer track '%s'"), track); - goto error; - } - } + if (virXMLPropEnum(node, "track", + virDomainTimerTrackTypeFromString, + VIR_XML_PROP_NONZERO, + &def->track) < 0) + goto error;
- ret = virXPathULongLong("string(./@frequency)", ctxt, &def->frequency); - if (ret == -1) { - def->frequency = 0;
Is the above case covered in virXMLPropULongLong? A few other cases like following below.
- } else if (ret < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("invalid timer frequency")); + if (virXMLPropULongLong(node, "frequency", 10, + VIR_XML_PROP_NONE, + &def->frequency) < 0) goto error; - }
- mode = virXMLPropString(node, "mode"); - if (mode != NULL) { - if ((def->mode = virDomainTimerModeTypeFromString(mode)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown timer mode '%s'"), mode); - goto error; - } - } + if (virXMLPropEnum(node, "mode", + virDomainTimerModeTypeFromString, + VIR_XML_PROP_NONZERO, + &def->mode) < 0) + goto error;
catchup = virXPathNode("./catchup", ctxt); if (catchup != NULL) { - ret = virXPathULong("string(./catchup/@threshold)", ctxt, - &def->catchup.threshold); - if (ret == -1) { - def->catchup.threshold = 0; - } else if (ret < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("invalid catchup threshold")); + if (virXMLPropUInt(catchup, "threshold", 10, + VIR_XML_PROP_NONE, + &def->catchup.threshold) < 0) goto error; - }
- ret = virXPathULong("string(./catchup/@slew)", ctxt, &def->catchup.slew); - if (ret == -1) { - def->catchup.slew = 0; - } else if (ret < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("invalid catchup slew")); + if (virXMLPropUInt(catchup, "slew", 10, + VIR_XML_PROP_NONE, + &def->catchup.slew) < 0) goto error; - }
- ret = virXPathULong("string(./catchup/@limit)", ctxt, &def->catchup.limit); - if (ret == -1) { - def->catchup.limit = 0; - } else if (ret < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("invalid catchup limit")); + if (virXMLPropUInt(catchup, "limit", 10, + VIR_XML_PROP_NONE, + &def->catchup.limit) < 0) goto error; - } }
return def; @@ -26197,11 +26160,11 @@ virDomainTimerDefFormat(virBuffer *buf, }
if (def->catchup.threshold > 0) - virBufferAsprintf(&catchupAttr, " threshold='%lu'", def->catchup.threshold); + virBufferAsprintf(&catchupAttr, " threshold='%u'", def->catchup.threshold); if (def->catchup.slew > 0) - virBufferAsprintf(&catchupAttr, " slew='%lu'", def->catchup.slew); + virBufferAsprintf(&catchupAttr, " slew='%u'", def->catchup.slew); if (def->catchup.limit > 0) - virBufferAsprintf(&catchupAttr, " limit='%lu'", def->catchup.limit); + virBufferAsprintf(&catchupAttr, " limit='%u'", def->catchup.limit);
virXMLFormatElement(&timerChld, "catchup", &catchupAttr, NULL); virXMLFormatElement(buf, "timer", &timerAttr, &timerChld);
-- 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/23/22 3:08 PM, Michal Privoznik wrote:
The virDomainTimerDefParseXML() function uses old style of parsing XML (virXMLPropString + str2enum conversion). Use virXMLPropEnumDefault() which encapsulates those steps.
virXMLPropEnum, virXMLPropULongLong and virXMLPropUInt ...
Signed-off-by: Michal Privoznik<mprivozn@redhat.com> --- src/conf/domain_conf.c | 105 +++++++++++++-------------------------- src/conf/domain_conf.h | 14 +++--- src/libxl/libxl_conf.c | 6 ++- src/libxl/xen_common.c | 6 ++- src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/qemu/qemu_command.c | 11 +++- src/qemu/qemu_validate.c | 8 ++- 8 files changed, 69 insertions(+), 85 deletions(-)
-- 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
participants (3)
-
Boris Fiuczynski
-
Michal Privoznik
-
Michal Prívozník