[libvirt PATCH 00/14] Use virTristateXXX for more struct members
This is a preparation step for some refactoring of the XML parser, see https://listman.redhat.com/archives/libvir-list/2021-March/msg01066.html Many libvirt structs have members that are of type `int` but actually are virTristateBool or virTristateSwitch. Fix this to increase type safety. Note that the comments on `virStorageAdapterFCHost::managed` and `virStoragePoolSourceDevice::part_separator` (patches #12 and #13) were actually wrong. Cheers, Tim Tim Wiederhake (14): conf: Use virTristateXXX in virStorageSource conf: Use virTristateXXX in virStorageSourceNVMeDef conf: Use virTristateXXX in virDomainDeviceInfo conf: Use virTristateXXX in virDomainDiskDef conf: Use virTristateXXX in virDomainActualNetDef conf: Use virTristateXXX in virDomainNetDef conf: Use virTristateXXX in virDomainChrSourceDef conf: Use virTristateXXX in virDomainGraphicsDef conf: Use virTristateXXX in virDomainMemballoonDef conf: Use virTristateXXX in virDomainLoaderDef conf: Use virTristateXXX in virDomainDef conf: Use virTristateXXX in virStorageAdapterFCHost conf: Use virTristateXXX in virStoragePoolSourceDevice conf: Use virTristateXXX in virPCIDeviceAddress src/conf/device_conf.c | 16 +-- src/conf/device_conf.h | 4 +- src/conf/domain_conf.c | 201 +++++++++++++++++++------------- src/conf/domain_conf.h | 28 ++--- src/conf/storage_adapter_conf.c | 4 +- src/conf/storage_adapter_conf.h | 2 +- src/conf/storage_conf.c | 7 +- src/conf/storage_conf.h | 2 +- src/conf/storage_source_conf.h | 4 +- src/qemu/qemu_command.c | 3 +- src/qemu/qemu_hotplug.c | 2 +- src/util/virpci.h | 2 +- 12 files changed, 162 insertions(+), 113 deletions(-) -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 12 ++++++++---- src/conf/storage_source_conf.h | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1e72171586..b8f6c3d606 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8340,11 +8340,15 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, return -1; } - if ((haveTLS = virXMLPropString(node, "tls")) && - (src->haveTLS = virTristateBoolTypeFromString(haveTLS)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown disk source 'tls' setting '%s'"), haveTLS); + if ((haveTLS = virXMLPropString(node, "tls"))) { + int value; + + if ((value = virTristateBoolTypeFromString(haveTLS)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown disk source 'tls' setting '%s'"), haveTLS); return -1; + } + src->haveTLS = value; } if ((flags & VIR_DOMAIN_DEF_PARSE_STATUS) && diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h index f42bb1c67d..e6702a1ffc 100644 --- a/src/conf/storage_source_conf.h +++ b/src/conf/storage_source_conf.h @@ -356,7 +356,7 @@ struct _virStorageSource { char *nodestorage; /* name of the storage object */ /* An optional setting to enable usage of TLS for the storage source */ - int haveTLS; /* enum virTristateBool */ + virTristateBool haveTLS; /* Indication whether the haveTLS value was altered due to qemu.conf * setting when haveTLS is missing from the domain config file */ -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 4 +++- src/conf/storage_source_conf.h | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b8f6c3d606..dfdca1891a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8480,12 +8480,14 @@ virDomainDiskSourceNVMeParse(xmlNodePtr node, } if ((managed = virXMLPropString(node, "managed"))) { - if ((nvme->managed = virTristateBoolTypeFromString(managed)) <= 0) { + int value; + if ((value = virTristateBoolTypeFromString(managed)) <= 0) { virReportError(VIR_ERR_XML_ERROR, _("malformed managed value '%s'"), managed); return -1; } + nvme->managed = value; } if (!(address = virXPathNode("./address", ctxt))) { diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h index e6702a1ffc..1783dc195e 100644 --- a/src/conf/storage_source_conf.h +++ b/src/conf/storage_source_conf.h @@ -251,7 +251,7 @@ typedef struct _virStorageSourceNVMeDef virStorageSourceNVMeDef; typedef virStorageSourceNVMeDef *virStorageSourceNVMeDefPtr; struct _virStorageSourceNVMeDef { unsigned long long namespc; - int managed; /* enum virTristateBool */ + virTristateBool managed; virPCIDeviceAddress pciAddr; /* Don't forget to update virStorageSourceNVMeDefCopy */ -- 2.26.2
Note that the wrong "VIR_TRISTATE_*_ABSENT" was used in qemuDomainChangeNet. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/device_conf.h | 4 ++-- src/conf/domain_conf.c | 26 ++++++++++++++++---------- src/qemu/qemu_command.c | 3 ++- src/qemu/qemu_hotplug.c | 2 +- 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h index a51bdf10ee..289af1153a 100644 --- a/src/conf/device_conf.h +++ b/src/conf/device_conf.h @@ -153,8 +153,8 @@ struct _virDomainDeviceInfo { } master; /* rombar and romfile are only used for pci hostdev and network * devices. */ - int romenabled; /* enum virTristateBool */ - int rombar; /* enum virTristateSwitch */ + virTristateBool romenabled; + virTristateSwitch rombar; char *romfile; /* bootIndex is only used for disk, network interface, hostdev * and redirdev devices */ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index dfdca1891a..67954bb42b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6687,17 +6687,23 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt, if ((flags & VIR_DOMAIN_DEF_PARSE_ALLOW_ROM) && (rom = virXPathNode("./rom", ctxt))) { - if ((romenabled = virXPathString("string(./rom/@enabled)", ctxt)) && - ((info->romenabled = virTristateBoolTypeFromString(romenabled)) <= 0)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown rom enabled value '%s'"), romenabled); - goto cleanup; + if ((romenabled = virXPathString("string(./rom/@enabled)", ctxt))) { + int value; + if ((value = virTristateBoolTypeFromString(romenabled)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown rom enabled value '%s'"), romenabled); + goto cleanup; + } + info->romenabled = value; } - if ((rombar = virXPathString("string(./rom/@bar)", ctxt)) && - ((info->rombar = virTristateSwitchTypeFromString(rombar)) <= 0)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown rom bar value '%s'"), rombar); - goto cleanup; + if ((rombar = virXPathString("string(./rom/@bar)", ctxt))) { + int value; + if ((value = virTristateSwitchTypeFromString(rombar)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown rom bar value '%s'"), rombar); + goto cleanup; + } + info->rombar = value; } info->romfile = virXMLPropString(rom, "file"); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7cb07306f3..92ab7eb34a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -685,7 +685,8 @@ qemuBuildRomStr(virBufferPtr buf, case VIR_TRISTATE_SWITCH_ON: virBufferAddLit(buf, ",rombar=1"); break; - default: + case VIR_TRISTATE_SWITCH_ABSENT: + case VIR_TRISTATE_SWITCH_LAST: break; } if (info->romfile) { diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 58d2abb862..142a4b18e1 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3653,7 +3653,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, /* device alias is checked already in virDomainDefCompatibleDevice */ - if (newdev->info.rombar == VIR_TRISTATE_BOOL_ABSENT) + if (newdev->info.rombar == VIR_TRISTATE_SWITCH_ABSENT) newdev->info.rombar = olddev->info.rombar; if (olddev->info.rombar != newdev->info.rombar) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 39 ++++++++++++++++++++++++--------------- src/conf/domain_conf.h | 6 +++--- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 67954bb42b..a99553cb0b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9198,27 +9198,36 @@ virDomainDiskDefDriverParseXML(virDomainDiskDefPtr def, } VIR_FREE(tmp); - if ((tmp = virXMLPropString(cur, "ioeventfd")) && - (def->ioeventfd = virTristateSwitchTypeFromString(tmp)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown disk ioeventfd mode '%s'"), tmp); - return -1; + if ((tmp = virXMLPropString(cur, "ioeventfd"))) { + int value; + if ((value = virTristateSwitchTypeFromString(tmp)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown disk ioeventfd mode '%s'"), tmp); + return -1; + } + def->ioeventfd = value; } VIR_FREE(tmp); - if ((tmp = virXMLPropString(cur, "event_idx")) && - (def->event_idx = virTristateSwitchTypeFromString(tmp)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown disk event_idx mode '%s'"), tmp); - return -1; + if ((tmp = virXMLPropString(cur, "event_idx"))) { + int value; + if ((value = virTristateSwitchTypeFromString(tmp)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown disk event_idx mode '%s'"), tmp); + return -1; + } + def->event_idx = value; } VIR_FREE(tmp); - if ((tmp = virXMLPropString(cur, "copy_on_read")) && - (def->copy_on_read = virTristateSwitchTypeFromString(tmp)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown disk copy_on_read mode '%s'"), tmp); - return -1; + if ((tmp = virXMLPropString(cur, "copy_on_read"))) { + int value; + if ((value = virTristateSwitchTypeFromString(tmp)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown disk copy_on_read mode '%s'"), tmp); + return -1; + } + def->copy_on_read = value; } VIR_FREE(tmp); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3da9ba01bf..1bbf859ca6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -569,9 +569,9 @@ struct _virDomainDiskDef { int error_policy; /* enum virDomainDiskErrorPolicy */ int rerror_policy; /* enum virDomainDiskErrorPolicy */ int iomode; /* enum virDomainDiskIo */ - int ioeventfd; /* enum virTristateSwitch */ - int event_idx; /* enum virTristateSwitch */ - int copy_on_read; /* enum virTristateSwitch */ + virTristateSwitch ioeventfd; + virTristateSwitch event_idx; + virTristateSwitch copy_on_read; int snapshot; /* virDomainSnapshotLocation, snapshot_conf.h */ int startupPolicy; /* enum virDomainStartupPolicy */ bool transient; -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 17 +++++++++-------- src/conf/domain_conf.h | 2 +- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a99553cb0b..d0d03b16bb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10562,14 +10562,15 @@ virDomainActualNetDefParseXML(xmlNodePtr node, goto error; } - trustGuestRxFilters = virXMLPropString(node, "trustGuestRxFilters"); - if (trustGuestRxFilters && - ((actual->trustGuestRxFilters - = virTristateBoolTypeFromString(trustGuestRxFilters)) <= 0)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown trustGuestRxFilters value '%s'"), - trustGuestRxFilters); - goto error; + if ((trustGuestRxFilters = virXMLPropString(node, "trustGuestRxFilters"))) { + int value; + if ((value = virTristateBoolTypeFromString(trustGuestRxFilters)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown trustGuestRxFilters value '%s'"), + trustGuestRxFilters); + goto error; + } + actual->trustGuestRxFilters = value; } virtPortNode = virXPathNode("./virtualport", ctxt); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1bbf859ca6..21c34b33c2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -995,7 +995,7 @@ struct _virDomainActualNetDef { virNetDevVPortProfilePtr virtPortProfile; virNetDevBandwidthPtr bandwidth; virNetDevVlan vlan; - int trustGuestRxFilters; /* enum virTristateBool */ + virTristateBool trustGuestRxFilters; virTristateBool isolatedPort; unsigned int class_id; /* class ID for bandwidth 'floor' */ }; -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 17 +++++++++-------- src/conf/domain_conf.h | 2 +- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d0d03b16bb..e529eb7bc1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10865,14 +10865,15 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, def->type = VIR_DOMAIN_NET_TYPE_USER; } - trustGuestRxFilters = virXMLPropString(node, "trustGuestRxFilters"); - if (trustGuestRxFilters && - ((def->trustGuestRxFilters - = virTristateBoolTypeFromString(trustGuestRxFilters)) <= 0)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown trustGuestRxFilters value '%s'"), - trustGuestRxFilters); - goto error; + if ((trustGuestRxFilters = virXMLPropString(node, "trustGuestRxFilters"))) { + int value; + if ((value = virTristateBoolTypeFromString(trustGuestRxFilters)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown trustGuestRxFilters value '%s'"), + trustGuestRxFilters); + goto error; + } + def->trustGuestRxFilters = value; } cur = node->children; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 21c34b33c2..0c3d74095e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1103,7 +1103,7 @@ struct _virDomainNetDef { GHashTable *filterparams; virNetDevBandwidthPtr bandwidth; virNetDevVlan vlan; - int trustGuestRxFilters; /* enum virTristateBool */ + virTristateBool trustGuestRxFilters; virTristateBool isolatedPort; int linkstate; unsigned int mtu; -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 34 +++++++++++++++++++++------------- src/conf/domain_conf.h | 6 +++--- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e529eb7bc1..2ae3080a8c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11901,12 +11901,14 @@ virDomainChrSourceDefParseTCP(virDomainChrSourceDefPtr def, def->data.tcp.service = virXMLPropString(source, "service"); if ((tmp = virXMLPropString(source, "tls"))) { - if ((def->data.tcp.haveTLS = virTristateBoolTypeFromString(tmp)) <= 0) { + int value; + if ((value = virTristateBoolTypeFromString(tmp)) <= 0) { virReportError(VIR_ERR_XML_ERROR, _("unknown chardev 'tls' setting '%s'"), tmp); return -1; } + def->data.tcp.haveTLS = value; VIR_FREE(tmp); } @@ -11985,12 +11987,15 @@ virDomainChrSourceDefParseFile(virDomainChrSourceDefPtr def, def->data.file.path = virXMLPropString(source, "path"); - if ((append = virXMLPropString(source, "append")) && - (def->data.file.append = virTristateSwitchTypeFromString(append)) <= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid append attribute value '%s'"), - append); - return -1; + if ((append = virXMLPropString(source, "append"))) { + int value; + if ((value = virTristateSwitchTypeFromString(append)) <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid append attribute value '%s'"), + append); + return -1; + } + def->data.file.append = value; } return 0; @@ -12026,12 +12031,15 @@ virDomainChrSourceDefParseLog(virDomainChrSourceDefPtr def, def->logfile = virXMLPropString(log, "file"); - if ((append = virXMLPropString(log, "append")) && - (def->logappend = virTristateSwitchTypeFromString(append)) <= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid append attribute value '%s'"), - append); - return -1; + if ((append = virXMLPropString(log, "append"))) { + int value; + if ((value = virTristateSwitchTypeFromString(append)) <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid append attribute value '%s'"), + append); + return -1; + } + def->logappend = value; } return 0; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0c3d74095e..ad9b8f632b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1233,7 +1233,7 @@ struct _virDomainChrSourceDef { /* no <source> for null, vc, stdio */ struct { char *path; - int append; /* enum virTristateSwitch */ + virTristateSwitch append; } file; /* pty, file, pipe, or device */ struct { char *master; @@ -1245,7 +1245,7 @@ struct _virDomainChrSourceDef { bool listen; int protocol; bool tlscreds; - int haveTLS; /* enum virTristateBool */ + virTristateBool haveTLS; bool tlsFromConfig; virDomainChrSourceReconnectDef reconnect; } tcp; @@ -1266,7 +1266,7 @@ struct _virDomainChrSourceDef { } spiceport; } data; char *logfile; - int logappend; + virTristateSwitch logappend; size_t nseclabels; virSecurityDeviceLabelDefPtr *seclabels; -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ad9b8f632b..445f48d8ed 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1863,7 +1863,7 @@ struct _virDomainGraphicsDef { int image; int jpeg; int zlib; - int playback; + virTristateSwitch playback; int streaming; virTristateBool copypaste; virTristateBool filetransfer; -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 26 ++++++++++++++++---------- src/conf/domain_conf.h | 4 ++-- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2ae3080a8c..313d7fd291 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14557,18 +14557,24 @@ virDomainMemballoonDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } - if ((deflate = virXMLPropString(node, "autodeflate")) && - (def->autodeflate = virTristateSwitchTypeFromString(deflate)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("invalid autodeflate attribute value '%s'"), deflate); - goto error; + if ((deflate = virXMLPropString(node, "autodeflate"))) { + int value; + if ((value = virTristateSwitchTypeFromString(deflate)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid autodeflate attribute value '%s'"), deflate); + goto error; + } + def->autodeflate = value; } - if ((freepage_reporting = virXMLPropString(node, "freePageReporting")) && - (def->free_page_reporting = virTristateSwitchTypeFromString(freepage_reporting)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("invalid freePageReporting attribute value '%s'"), freepage_reporting); - goto error; + if ((freepage_reporting = virXMLPropString(node, "freePageReporting"))) { + int value; + if ((value = virTristateSwitchTypeFromString(freepage_reporting)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid freePageReporting attribute value '%s'"), freepage_reporting); + goto error; + } + def->free_page_reporting = value; } ctxt->node = node; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 445f48d8ed..cdf3e2ed8e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1922,8 +1922,8 @@ struct _virDomainMemballoonDef { int model; virDomainDeviceInfo info; int period; /* seconds between collections */ - int autodeflate; /* enum virTristateSwitch */ - int free_page_reporting; /* enum virTristateSwitch */ + virTristateSwitch autodeflate; + virTristateSwitch free_page_reporting; virDomainVirtioOptionsPtr virtio; }; -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 26 ++++++++++++++++---------- src/conf/domain_conf.h | 4 ++-- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 313d7fd291..24d124158e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19209,18 +19209,24 @@ virDomainLoaderDefParseXML(xmlNodePtr node, VIR_FREE(loader->path); } - if (readonly_str && - (loader->readonly = virTristateBoolTypeFromString(readonly_str)) <= 0) { - virReportError(VIR_ERR_XML_DETAIL, - _("unknown readonly value: %s"), readonly_str); - return -1; + if (readonly_str) { + int value; + if ((value = virTristateBoolTypeFromString(readonly_str)) <= 0) { + virReportError(VIR_ERR_XML_DETAIL, + _("unknown readonly value: %s"), readonly_str); + return -1; + } + loader->readonly = value; } - if (secure_str && - (loader->secure = virTristateBoolTypeFromString(secure_str)) <= 0) { - virReportError(VIR_ERR_XML_DETAIL, - _("unknown secure value: %s"), secure_str); - return -1; + if (secure_str) { + int value; + if ((value = virTristateBoolTypeFromString(secure_str)) <= 0) { + virReportError(VIR_ERR_XML_DETAIL, + _("unknown secure value: %s"), secure_str); + return -1; + } + loader->secure = value; } if (type_str) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cdf3e2ed8e..b2e14ba2e1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2153,9 +2153,9 @@ VIR_ENUM_DECL(virDomainLoader); struct _virDomainLoaderDef { char *path; - int readonly; /* enum virTristateBool */ + virTristateBool readonly; virDomainLoader type; - int secure; /* enum virTristateBool */ + virTristateBool secure; char *nvram; /* path to non-volatile RAM */ char *templt; /* user override of path to master nvram */ }; -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b2e14ba2e1..e53f5d1b47 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2756,7 +2756,7 @@ struct _virDomainDef { virDomainHPTResizing hpt_resizing; unsigned long long hpt_maxpagesize; /* Stored in KiB */ char *hyperv_vendor_id; - int apic_eoi; + virTristateSwitch apic_eoi; bool tseg_specified; unsigned long long tseg_size; -- 2.26.2
Note that the comment for virStorageAdapterFCHost::managed was wrong. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/storage_adapter_conf.c | 4 +++- src/conf/storage_adapter_conf.h | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/conf/storage_adapter_conf.c b/src/conf/storage_adapter_conf.c index 77ecb8d5f2..69062b4b58 100644 --- a/src/conf/storage_adapter_conf.c +++ b/src/conf/storage_adapter_conf.c @@ -68,13 +68,15 @@ virStorageAdapterParseXMLFCHost(xmlNodePtr node, fchost->parent = virXMLPropString(node, "parent"); if ((managed = virXMLPropString(node, "managed"))) { - if ((fchost->managed = virTristateBoolTypeFromString(managed)) < 0) { + int value; + if ((value = virTristateBoolTypeFromString(managed)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown fc_host managed setting '%s'"), managed); VIR_FREE(managed); return -1; } + fchost->managed = value; } fchost->parent_wwnn = virXMLPropString(node, "parent_wwnn"); diff --git a/src/conf/storage_adapter_conf.h b/src/conf/storage_adapter_conf.h index 4c7da7c8d9..3f64cda9af 100644 --- a/src/conf/storage_adapter_conf.h +++ b/src/conf/storage_adapter_conf.h @@ -51,7 +51,7 @@ struct _virStorageAdapterFCHost { char *parent_fabric_wwn; char *wwnn; char *wwpn; - int managed; /* enum virTristateSwitch */ + virTristateBool managed; }; typedef struct _virStorageAdapter virStorageAdapter; -- 2.26.2
Note that the comment for virStoragePoolSourceDevice::part_separator was wrong. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/storage_conf.c | 7 ++++--- src/conf/storage_conf.h | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 310a062ee7..e92de7e6a3 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -617,14 +617,15 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, partsep = virXMLPropString(nodeset[i], "part_separator"); if (partsep) { - dev.part_separator = virTristateBoolTypeFromString(partsep); - if (dev.part_separator <= 0) { + int value = virTristateBoolTypeFromString(partsep); + if (value <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("invalid part_separator setting '%s'"), partsep); virStoragePoolSourceDeviceClear(&dev); goto cleanup; } + dev.part_separator = value; } if (VIR_APPEND_ELEMENT(source->devices, source->ndevice, dev) < 0) { @@ -1097,7 +1098,7 @@ virStoragePoolSourceFormat(virBufferPtr buf, virBufferEscapeString(buf, "<device path='%s'", src->devices[i].path); if (src->devices[i].part_separator != - VIR_TRISTATE_SWITCH_ABSENT) { + VIR_TRISTATE_BOOL_ABSENT) { virBufferAsprintf(buf, " part_separator='%s'", virTristateBoolTypeToString(src->devices[i].part_separator)); } diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 647eb847bf..8d417af7bb 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -168,7 +168,7 @@ struct _virStoragePoolSourceDevice { virStoragePoolSourceDeviceExtentPtr freeExtents; char *path; int format; /* Pool specific source format */ - int part_separator; /* enum virTristateSwitch */ + virTristateBool part_separator; /* When the source device is a physical disk, * the geometry data is needed -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/device_conf.c | 16 +++++++++------- src/util/virpci.h | 2 +- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 714ac50762..0dd60985e9 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -246,13 +246,15 @@ virPCIDeviceAddressParseXML(xmlNodePtr node, return -1; } - if (multi && - ((addr->multi = virTristateSwitchTypeFromString(multi)) <= 0)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown value '%s' for <address> 'multifunction' attribute"), - multi); - return -1; - + if (multi) { + int value; + if ((value = virTristateSwitchTypeFromString(multi)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown value '%s' for <address> 'multifunction' attribute"), + multi); + return -1; + } + addr->multi = value; } if (!virPCIDeviceAddressIsEmpty(addr) && !virPCIDeviceAddressIsValid(addr, true)) return -1; diff --git a/src/util/virpci.h b/src/util/virpci.h index 9b37a12883..2c86642ea7 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -65,7 +65,7 @@ struct _virPCIDeviceAddress { unsigned int bus; unsigned int slot; unsigned int function; - int multi; /* virTristateSwitch */ + virTristateSwitch multi; int extFlags; /* enum virPCIDeviceAddressExtensionFlags */ virZPCIDeviceAddress zpci; /* Don't forget to update virPCIDeviceAddressCopy if needed. */ -- 2.26.2
polite ping On Wed, 2021-04-07 at 13:48 +0200, Tim Wiederhake wrote:
This is a preparation step for some refactoring of the XML parser, see https://listman.redhat.com/archives/libvir-list/2021-March/msg01066.html
Many libvirt structs have members that are of type `int` but actually are virTristateBool or virTristateSwitch. Fix this to increase type safety.
Note that the comments on `virStorageAdapterFCHost::managed` and `virStoragePoolSourceDevice::part_separator` (patches #12 and #13) were actually wrong.
Cheers, Tim
Tim Wiederhake (14): conf: Use virTristateXXX in virStorageSource conf: Use virTristateXXX in virStorageSourceNVMeDef conf: Use virTristateXXX in virDomainDeviceInfo conf: Use virTristateXXX in virDomainDiskDef conf: Use virTristateXXX in virDomainActualNetDef conf: Use virTristateXXX in virDomainNetDef conf: Use virTristateXXX in virDomainChrSourceDef conf: Use virTristateXXX in virDomainGraphicsDef conf: Use virTristateXXX in virDomainMemballoonDef conf: Use virTristateXXX in virDomainLoaderDef conf: Use virTristateXXX in virDomainDef conf: Use virTristateXXX in virStorageAdapterFCHost conf: Use virTristateXXX in virStoragePoolSourceDevice conf: Use virTristateXXX in virPCIDeviceAddress
src/conf/device_conf.c | 16 +-- src/conf/device_conf.h | 4 +- src/conf/domain_conf.c | 201 +++++++++++++++++++----------- -- src/conf/domain_conf.h | 28 ++--- src/conf/storage_adapter_conf.c | 4 +- src/conf/storage_adapter_conf.h | 2 +- src/conf/storage_conf.c | 7 +- src/conf/storage_conf.h | 2 +- src/conf/storage_source_conf.h | 4 +- src/qemu/qemu_command.c | 3 +- src/qemu/qemu_hotplug.c | 2 +- src/util/virpci.h | 2 +- 12 files changed, 162 insertions(+), 113 deletions(-)
-- 2.26.2
On Wed, Apr 07, 2021 at 13:48:27 +0200, Tim Wiederhake wrote:
This is a preparation step for some refactoring of the XML parser, see https://listman.redhat.com/archives/libvir-list/2021-March/msg01066.html
Many libvirt structs have members that are of type `int` but actually are virTristateBool or virTristateSwitch. Fix this to increase type safety.
Note that the comments on `virStorageAdapterFCHost::managed` and `virStoragePoolSourceDevice::part_separator` (patches #12 and #13) were actually wrong.
This is stuff for the individual commit messages rather than the cover letter. I won't be modifying those for this instance. Series: Reviewed-by: Peter Krempa <pkrempa@redhat.com> and will be pushed shortly.
participants (2)
-
Peter Krempa -
Tim Wiederhake