[PATCH 0/7] Remove last instances of looped XML element lookup from domain_conf.c
Had this around for a while but it needed some finishing touches. This ousts the last loop-based XML parser iterating over XML nodes and replaces it by something more modern, fixing a few bugs i the process and cleaning stuff up. There are a few of such parsers still around e.g in the network XML parsing part. Peter Krempa (7): virPCIDeviceAddressParseXML: Use virXMLNodeGetSubelement to find 'zpci' virNetDevVPortProfileParse: Use virXMLNodeGetSubelement to find '<parameters>' virNetworkDHCPRangeDefParseXML: Use virXMLNodeGetSubelement to find 'lease' virNetworkDHCPHostDefParseXML: Use virXMLNodeGetSubelement to find 'lease' virBitmapIsBitSet: Allow NULL bitmap qemustatusxml2xmltest: Add test data for testing '<origstates>' of PCI hostdev conf: Store 'origstates' of PCI hostdevs in a bitmap src/conf/device_conf.c | 16 +--- src/conf/domain_conf.c | 97 +++++++++++------------ src/conf/domain_conf.h | 31 +++----- src/conf/netdev_vport_profile_conf.c | 20 ++--- src/conf/network_conf.c | 28 ++----- src/conf/virconftypes.h | 2 - src/hypervisor/virhostdev.c | 25 +++--- src/util/virbitmap.c | 4 +- src/util/virbitmap.h | 2 +- tests/qemustatusxml2xmldata/modern-in.xml | 12 +++ 10 files changed, 107 insertions(+), 130 deletions(-) -- 2.39.1
Use the helper designed to find the subelement. A slight semantic difference after this patch is that the first <zpci> element will be considered instead of the last, but only one is expected in a valid XML. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/device_conf.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 762efe5f0b..a116e39c75 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -199,8 +199,7 @@ int virPCIDeviceAddressParseXML(xmlNodePtr node, virPCIDeviceAddress *addr) { - xmlNodePtr cur; - xmlNodePtr zpci = NULL; + xmlNodePtr zpci; memset(addr, 0, sizeof(*addr)); @@ -227,18 +226,11 @@ virPCIDeviceAddressParseXML(xmlNodePtr node, if (!virPCIDeviceAddressIsEmpty(addr) && !virPCIDeviceAddressIsValid(addr, true)) return -1; - cur = node->children; - while (cur) { - if (cur->type == XML_ELEMENT_NODE && - virXMLNodeNameEqual(cur, "zpci")) { - zpci = cur; - } - cur = cur->next; + if ((zpci = virXMLNodeGetSubelement(node, "zpci"))) { + if (virZPCIDeviceAddressParseXML(zpci, addr) < 0) + return -1; } - if (zpci && virZPCIDeviceAddressParseXML(zpci, addr) < 0) - return -1; - return 0; } -- 2.39.1
On Thu, Feb 02, 2023 at 04:18:04PM +0100, Peter Krempa wrote:
Use the helper designed to find the subelement. A slight semantic difference after this patch is that the first <zpci> element will be considered instead of the last, but only one is expected in a valid XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/device_conf.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 762efe5f0b..a116e39c75 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -199,8 +199,7 @@ int virPCIDeviceAddressParseXML(xmlNodePtr node, virPCIDeviceAddress *addr) { - xmlNodePtr cur; - xmlNodePtr zpci = NULL; + xmlNodePtr zpci;
You can initialise the variable right away here instead of ...
memset(addr, 0, sizeof(*addr));
@@ -227,18 +226,11 @@ virPCIDeviceAddressParseXML(xmlNodePtr node, if (!virPCIDeviceAddressIsEmpty(addr) && !virPCIDeviceAddressIsValid(addr, true)) return -1;
- cur = node->children; - while (cur) { - if (cur->type == XML_ELEMENT_NODE && - virXMLNodeNameEqual(cur, "zpci")) { - zpci = cur; - } - cur = cur->next; + if ((zpci = virXMLNodeGetSubelement(node, "zpci"))) {
... making this look like lisp ;) in first 4 patches. But I get it's a subjective preference.
+ if (virZPCIDeviceAddressParseXML(zpci, addr) < 0) + return -1; }
- if (zpci && virZPCIDeviceAddressParseXML(zpci, addr) < 0) - return -1; - return 0; }
-- 2.39.1
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/netdev_vport_profile_conf.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/conf/netdev_vport_profile_conf.c b/src/conf/netdev_vport_profile_conf.c index cdde7efcd9..26f379248f 100644 --- a/src/conf/netdev_vport_profile_conf.c +++ b/src/conf/netdev_vport_profile_conf.c @@ -36,7 +36,7 @@ virNetDevVPortProfileParse(xmlNodePtr node, unsigned int flags) g_autofree char *virtPortProfileID = NULL; g_autofree char *virtPortInterfaceID = NULL; g_autofree virNetDevVPortProfile *virtPort = NULL; - xmlNodePtr cur = node->children; + xmlNodePtr parameters; virtPort = g_new0(virNetDevVPortProfile, 1); @@ -54,17 +54,13 @@ virNetDevVPortProfileParse(xmlNodePtr node, unsigned int flags) return NULL; } - while (cur != NULL) { - if (virXMLNodeNameEqual(cur, "parameters")) { - virtPortManagerID = virXMLPropString(cur, "managerid"); - virtPortTypeID = virXMLPropString(cur, "typeid"); - virtPortTypeIDVersion = virXMLPropString(cur, "typeidversion"); - virtPortInstanceID = virXMLPropString(cur, "instanceid"); - virtPortProfileID = virXMLPropString(cur, "profileid"); - virtPortInterfaceID = virXMLPropString(cur, "interfaceid"); - break; - } - cur = cur->next; + if ((parameters = virXMLNodeGetSubelement(node, "parameters"))) { + virtPortManagerID = virXMLPropString(parameters, "managerid"); + virtPortTypeID = virXMLPropString(parameters, "typeid"); + virtPortTypeIDVersion = virXMLPropString(parameters, "typeidversion"); + virtPortInstanceID = virXMLPropString(parameters, "instanceid"); + virtPortProfileID = virXMLPropString(parameters, "profileid"); + virtPortInterfaceID = virXMLPropString(parameters, "interfaceid"); } if (virtPortManagerID) { -- 2.39.1
This also prevents a potential memleak when multiple elements would be present. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/network_conf.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index b98ae6aa3f..e33925f857 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -453,7 +453,7 @@ virNetworkDHCPRangeDefParseXML(const char *networkName, virNetworkDHCPRangeDef *range) { virSocketAddrRange *addr = &range->addr; - xmlNodePtr cur = node->children; + xmlNodePtr lease; g_autofree char *start = NULL; g_autofree char *end = NULL; @@ -480,15 +480,9 @@ virNetworkDHCPRangeDefParseXML(const char *networkName, virNetworkIPDefPrefix(ipdef)) < 0) return -1; - while (cur != NULL) { - if (cur->type == XML_ELEMENT_NODE && - virXMLNodeNameEqual(cur, "lease")) { - - if (virNetworkDHCPLeaseTimeDefParseXML(&range->lease, cur) < 0) - return -1; - } - cur = cur->next; - } + if ((lease = virXMLNodeGetSubelement(node, "lease")) && + virNetworkDHCPLeaseTimeDefParseXML(&range->lease, lease) < 0) + return -1; return 0; } -- 2.39.1
This also prevents a potential memleak when multiple elements would be present. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/network_conf.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index e33925f857..5d1f0e5203 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -501,7 +501,7 @@ virNetworkDHCPHostDefParseXML(const char *networkName, g_autofree char *id = NULL; virMacAddr addr; virSocketAddr inaddr; - xmlNodePtr cur = node->children; + xmlNodePtr lease; mac = virXMLPropString(node, "mac"); if (mac != NULL) { @@ -593,15 +593,9 @@ virNetworkDHCPHostDefParseXML(const char *networkName, } } - while (cur != NULL) { - if (cur->type == XML_ELEMENT_NODE && - virXMLNodeNameEqual(cur, "lease")) { - - if (virNetworkDHCPLeaseTimeDefParseXML(&host->lease, cur) < 0) - return -1; - } - cur = cur->next; - } + if ((lease = virXMLNodeGetSubelement(node, "lease")) && + virNetworkDHCPLeaseTimeDefParseXML(&host->lease, lease) < 0) + return -1; host->mac = g_steal_pointer(&mac); host->id = g_steal_pointer(&id); -- 2.39.1
The virBitmapIsBitSet API is a permissive one which returns false when the bit is not set or is out of range. We can do the same if the bitmap is NULL to aid certain situations when this can happen, but we don't want to add extra checks. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 4 ++-- src/util/virbitmap.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 5b9204cbd7..bdcbd0aece 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -216,7 +216,7 @@ virBitmapIsSet(virBitmap *bitmap, size_t b) /** * virBitmapIsBitSet: - * @bitmap: Pointer to bitmap + * @bitmap: Pointer to bitmap (May be NULL) * @b: bit position to get * * Get setting of bit position @b in @bitmap. @@ -228,7 +228,7 @@ bool virBitmapIsBitSet(virBitmap *bitmap, size_t b) { - if (bitmap->nbits <= b) + if (!bitmap || bitmap->nbits <= b) return false; return virBitmapIsSet(bitmap, b); diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index e2314904b0..dbd88c9bb5 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -61,7 +61,7 @@ void virBitmapClearBitExpand(virBitmap *bitmap, size_t b) * Get bit @b in @bitmap. Returns false if b is out of range. */ bool virBitmapIsBitSet(virBitmap *bitmap, size_t b) - ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; + G_GNUC_WARN_UNUSED_RESULT; /* * Get setting of bit position @b in @bitmap and store in @result */ -- 2.39.1
The <origstates> XML element captures private data of a PCI device needed to restore it after a VM is started. Unfortunately at the point when it was added we didn't yet have the existing private data infrastructure. Since the element is parsed only in cases similar to the status XML we need to test it there. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemustatusxml2xmldata/modern-in.xml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/qemustatusxml2xmldata/modern-in.xml b/tests/qemustatusxml2xmldata/modern-in.xml index f5beab722b..cdab1d7178 100644 --- a/tests/qemustatusxml2xmldata/modern-in.xml +++ b/tests/qemustatusxml2xmldata/modern-in.xml @@ -483,6 +483,18 @@ <alias name='hostdev0'/> <address type='drive' controller='0' bus='0' target='2' unit='4'/> </hostdev> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x2'/> + <origstates> + <unbind/> + <removeslot/> + <reprobe/> + </origstates> + </source> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> + </hostdev> <redirdev bus='usb' type='spicevmc'> <alias name='redir0'/> <address type='usb' bus='0' port='2'/> -- 2.39.1
Refactor the code to use a bitmap with an enum to avoid ugly XML parser. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 97 ++++++++++++++++++------------------- src/conf/domain_conf.h | 31 ++++-------- src/conf/virconftypes.h | 2 - src/hypervisor/virhostdev.c | 25 ++++++---- 4 files changed, 72 insertions(+), 83 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6e33a4472f..b21efeacf9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1061,6 +1061,14 @@ VIR_ENUM_IMPL(virDomainHostdevSubsysSCSIProtocol, "iscsi", ); + +VIR_ENUM_IMPL(virDomainHostdevPCIOrigstate, + VIR_DOMAIN_HOSTDEV_PCI_ORIGSTATE_LAST, + "unbind", + "removeslot", + "reprobe", +); + VIR_ENUM_IMPL(virDomainHostdevSubsysUSBGuestReset, VIR_DOMAIN_HOSTDEV_USB_GUEST_RESET_LAST, "default", @@ -3365,8 +3373,10 @@ void virDomainHostdevDefClear(virDomainHostdevDef *def) case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST: VIR_FREE(def->source.subsys.u.scsi_host.wwpn); break; - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: + virBitmapFree(def->source.subsys.u.pci.origstates); + break; + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST: break; @@ -5731,41 +5741,6 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, return 0; } -/* The internal XML for host PCI device's original states: - * - * <origstates> - * <unbind/> - * <removeslot/> - * <reprobe/> - * </origstates> - */ -static int -virDomainHostdevSubsysPCIOrigStatesDefParseXML(xmlNodePtr node, - virDomainHostdevOrigStates *def) -{ - xmlNodePtr cur; - cur = node->children; - - while (cur != NULL) { - if (cur->type == XML_ELEMENT_NODE) { - if (virXMLNodeNameEqual(cur, "unbind")) { - def->states.pci.unbind_from_stub = true; - } else if (virXMLNodeNameEqual(cur, "removeslot")) { - def->states.pci.remove_slot = true; - } else if (virXMLNodeNameEqual(cur, "reprobe")) { - def->states.pci.reprobe = true; - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported element '%s' of 'origstates'"), - cur->name); - return -1; - } - } - cur = cur->next; - } - - return 0; -} static int virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node, @@ -5774,7 +5749,6 @@ virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node, unsigned int flags) { xmlNodePtr address = NULL; - xmlNodePtr origstates = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) ctxt->node = node; @@ -5788,10 +5762,35 @@ virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node, virPCIDeviceAddressParseXML(address, &def->source.subsys.u.pci.addr) < 0) return -1; - if ((flags & VIR_DOMAIN_DEF_PARSE_PCI_ORIG_STATES) && - (origstates = virXPathNode("./origstates", ctxt)) && - virDomainHostdevSubsysPCIOrigStatesDefParseXML(origstates, &def->origstates) < 0) - return -1; + if ((flags & VIR_DOMAIN_DEF_PARSE_PCI_ORIG_STATES)) { + virDomainHostdevSubsysPCI *pcisrc = &def->source.subsys.u.pci; + g_autofree xmlNodePtr *nodes = NULL; + ssize_t nnodes; + size_t i; + + if ((nnodes = virXPathNodeSet("./origstates/*", ctxt, &nodes)) < 0) + return -1; + + if (nnodes > 0) { + if (!pcisrc->origstates) + pcisrc->origstates = virBitmapNew(VIR_DOMAIN_HOSTDEV_PCI_ORIGSTATE_LAST); + else + virBitmapClearAll(pcisrc->origstates); + + for (i = 0; i < nnodes; i++) { + int state; + + if ((state = virDomainHostdevPCIOrigstateTypeFromString((const char *) nodes[i]->name)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported element '%s' of 'origstates'"), + (const char *) nodes[i]->name); + return -1; + } + + virBitmapSetBitExpand(pcisrc->origstates, state); + } + } + } return 0; } @@ -23154,7 +23153,6 @@ virDomainHostdevDefFormatSubsysPCI(virBuffer *buf, { g_auto(virBuffer) sourceAttrBuf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) sourceChildBuf = VIR_BUFFER_INIT_CHILD(buf); - g_auto(virBuffer) origstatesChildBuf = VIR_BUFFER_INIT_CHILD(&sourceChildBuf); virDomainHostdevSubsysPCI *pcisrc = &def->source.subsys.u.pci; if (def->writeFiltering != VIR_TRISTATE_BOOL_ABSENT) @@ -23176,15 +23174,14 @@ virDomainHostdevDefFormatSubsysPCI(virBuffer *buf, virPCIDeviceAddressFormat(&sourceChildBuf, pcisrc->addr, includeTypeInAddr); - if ((flags & VIR_DOMAIN_DEF_FORMAT_PCI_ORIG_STATES)) { - if (def->origstates.states.pci.unbind_from_stub) - virBufferAddLit(&origstatesChildBuf, "<unbind/>\n"); - - if (def->origstates.states.pci.remove_slot) - virBufferAddLit(&origstatesChildBuf, "<removeslot/>\n"); + if (pcisrc->origstates && + (flags & VIR_DOMAIN_DEF_FORMAT_PCI_ORIG_STATES)) { + g_auto(virBuffer) origstatesChildBuf = VIR_BUFFER_INIT_CHILD(&sourceChildBuf); + ssize_t n = -1; - if (def->origstates.states.pci.reprobe) - virBufferAddLit(&origstatesChildBuf, "<reprobe/>\n"); + while ((n = virBitmapNextSetBit(pcisrc->origstates, n)) >= 0) + virBufferAsprintf(&origstatesChildBuf, "<%s/>\n", + virDomainHostdevPCIOrigstateTypeToString(n)); virXMLFormatElement(&sourceChildBuf, "origstates", NULL, &origstatesChildBuf); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1a80399c9c..0bec072478 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -167,28 +167,14 @@ typedef enum { } virDomainHyperVMode; VIR_ENUM_DECL(virDomainHyperVMode); -struct _virDomainHostdevOrigStates { - union { - struct { - /* Does the device need to unbind from stub when - * reattaching to host? - */ - bool unbind_from_stub; - - /* Does it need to use remove_slot when reattaching - * the device to host? - */ - bool remove_slot; +typedef enum { + VIR_DOMAIN_HOSTDEV_PCI_ORIGSTATE_UNBIND = 0, /* device needs to unbind from stub when reattaching */ + VIR_DOMAIN_HOSTDEV_PCI_ORIGSTATE_REMOVESLOT, /* use remove_slot when reattaching */ + VIR_DOMAIN_HOSTDEV_PCI_ORIGSTATE_REPROBE, /* reprobe driver for device when reattaching */ - /* Does it need to reprobe driver for the device when - * reattaching to host? - */ - bool reprobe; - } pci; - - /* Perhaps 'usb' in future */ - } states; -}; + VIR_DOMAIN_HOSTDEV_PCI_ORIGSTATE_LAST +} virDomainHostdevPCIOrigstate; +VIR_ENUM_DECL(virDomainHostdevPCIOrigstate); struct _virDomainLeaseDef { char *lockspace; @@ -262,6 +248,8 @@ struct _virDomainHostdevSubsysUSB { struct _virDomainHostdevSubsysPCI { virPCIDeviceAddress addr; /* host address */ virDomainHostdevSubsysPCIBackendType backend; + + virBitmap *origstates; }; struct _virDomainHostdevSubsysSCSIHost { @@ -394,7 +382,6 @@ struct _virDomainHostdevDef { virDomainHostdevSubsys subsys; virDomainHostdevCaps caps; } source; - virDomainHostdevOrigStates origstates; virDomainNetTeamingInfo *teaming; virDomainDeviceInfo *info; /* Guest address */ }; diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index d03d1d132e..e07f967814 100644 --- a/src/conf/virconftypes.h +++ b/src/conf/virconftypes.h @@ -116,8 +116,6 @@ typedef struct _virDomainHostdevCaps virDomainHostdevCaps; typedef struct _virDomainHostdevDef virDomainHostdevDef; -typedef struct _virDomainHostdevOrigStates virDomainHostdevOrigStates; - typedef struct _virDomainHostdevSubsys virDomainHostdevSubsys; typedef struct _virDomainHostdevSubsysMediatedDev virDomainHostdevSubsysMediatedDev; diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c index c0ce867596..bc5a20d691 100644 --- a/src/hypervisor/virhostdev.c +++ b/src/hypervisor/virhostdev.c @@ -873,12 +873,18 @@ virHostdevPreparePCIDevicesImpl(virHostdevManager *mgr, if (actual) { VIR_DEBUG("Saving network configuration of PCI device %s", virPCIDeviceGetName(actual)); - hostdev->origstates.states.pci.unbind_from_stub = - virPCIDeviceGetUnbindFromStub(actual); - hostdev->origstates.states.pci.remove_slot = - virPCIDeviceGetRemoveSlot(actual); - hostdev->origstates.states.pci.reprobe = - virPCIDeviceGetReprobe(actual); + + if (!pcisrc->origstates) + pcisrc->origstates = virBitmapNew(VIR_DOMAIN_HOSTDEV_PCI_ORIGSTATE_LAST); + else + virBitmapClearAll(pcisrc->origstates); + + if (virPCIDeviceGetUnbindFromStub(actual)) + virBitmapSetBitExpand(pcisrc->origstates, VIR_DOMAIN_HOSTDEV_PCI_ORIGSTATE_UNBIND); + if (virPCIDeviceGetRemoveSlot(actual)) + virBitmapSetBitExpand(pcisrc->origstates, VIR_DOMAIN_HOSTDEV_PCI_ORIGSTATE_REMOVESLOT); + if (virPCIDeviceGetReprobe(actual)) + virBitmapSetBitExpand(pcisrc->origstates, VIR_DOMAIN_HOSTDEV_PCI_ORIGSTATE_REPROBE); } } @@ -1132,6 +1138,7 @@ virHostdevUpdateActivePCIDevices(virHostdevManager *mgr, for (i = 0; i < nhostdevs; i++) { const virDomainHostdevDef *hostdev = hostdevs[i]; g_autoptr(virPCIDevice) actual = NULL; + virBitmap *orig = hostdev->source.subsys.u.pci.origstates; if (virHostdevGetPCIHostDevice(hostdev, &actual) < 0) goto cleanup; @@ -1143,9 +1150,9 @@ virHostdevUpdateActivePCIDevices(virHostdevManager *mgr, goto cleanup; /* Setup the original states for the PCI device */ - virPCIDeviceSetUnbindFromStub(actual, hostdev->origstates.states.pci.unbind_from_stub); - virPCIDeviceSetRemoveSlot(actual, hostdev->origstates.states.pci.remove_slot); - virPCIDeviceSetReprobe(actual, hostdev->origstates.states.pci.reprobe); + virPCIDeviceSetUnbindFromStub(actual, virBitmapIsBitSet(orig, VIR_DOMAIN_HOSTDEV_PCI_ORIGSTATE_UNBIND)); + virPCIDeviceSetRemoveSlot(actual, virBitmapIsBitSet(orig, VIR_DOMAIN_HOSTDEV_PCI_ORIGSTATE_REMOVESLOT)); + virPCIDeviceSetReprobe(actual, virBitmapIsBitSet(orig, VIR_DOMAIN_HOSTDEV_PCI_ORIGSTATE_REPROBE)); if (virPCIDeviceListAdd(mgr->activePCIHostdevs, actual) < 0) goto cleanup; -- 2.39.1
On a Thursday in 2023, Peter Krempa wrote:
Refactor the code to use a bitmap with an enum to avoid ugly XML parser.
Looking at the new code, I think I'd end this sentence early: Refactor the code to use a bitmap with an enum. Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 97 ++++++++++++++++++------------------- src/conf/domain_conf.h | 31 ++++-------- src/conf/virconftypes.h | 2 - src/hypervisor/virhostdev.c | 25 ++++++---- 4 files changed, 72 insertions(+), 83 deletions(-)
On a Thursday in 2023, Peter Krempa wrote:
Had this around for a while but it needed some finishing touches.
This ousts the last loop-based XML parser iterating over XML nodes and replaces it by something more modern, fixing a few bugs i the process
s/ i / in /
and cleaning stuff up.
There are a few of such parsers still around e.g in the network XML parsing part.
Peter Krempa (7): virPCIDeviceAddressParseXML: Use virXMLNodeGetSubelement to find 'zpci' virNetDevVPortProfileParse: Use virXMLNodeGetSubelement to find '<parameters>' virNetworkDHCPRangeDefParseXML: Use virXMLNodeGetSubelement to find 'lease' virNetworkDHCPHostDefParseXML: Use virXMLNodeGetSubelement to find 'lease' virBitmapIsBitSet: Allow NULL bitmap qemustatusxml2xmltest: Add test data for testing '<origstates>' of PCI hostdev conf: Store 'origstates' of PCI hostdevs in a bitmap
src/conf/device_conf.c | 16 +--- src/conf/domain_conf.c | 97 +++++++++++------------ src/conf/domain_conf.h | 31 +++----- src/conf/netdev_vport_profile_conf.c | 20 ++--- src/conf/network_conf.c | 28 ++----- src/conf/virconftypes.h | 2 - src/hypervisor/virhostdev.c | 25 +++--- src/util/virbitmap.c | 4 +- src/util/virbitmap.h | 2 +- tests/qemustatusxml2xmldata/modern-in.xml | 12 +++ 10 files changed, 107 insertions(+), 130 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On Thu, Feb 02, 2023 at 04:18:03PM +0100, Peter Krempa wrote:
Had this around for a while but it needed some finishing touches.
This ousts the last loop-based XML parser iterating over XML nodes and replaces it by something more modern, fixing a few bugs i the process and cleaning stuff up.
There are a few of such parsers still around e.g in the network XML parsing part.
Peter Krempa (7): virPCIDeviceAddressParseXML: Use virXMLNodeGetSubelement to find 'zpci' virNetDevVPortProfileParse: Use virXMLNodeGetSubelement to find '<parameters>' virNetworkDHCPRangeDefParseXML: Use virXMLNodeGetSubelement to find 'lease' virNetworkDHCPHostDefParseXML: Use virXMLNodeGetSubelement to find 'lease' virBitmapIsBitSet: Allow NULL bitmap qemustatusxml2xmltest: Add test data for testing '<origstates>' of PCI hostdev conf: Store 'origstates' of PCI hostdevs in a bitmap
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
src/conf/device_conf.c | 16 +--- src/conf/domain_conf.c | 97 +++++++++++------------ src/conf/domain_conf.h | 31 +++----- src/conf/netdev_vport_profile_conf.c | 20 ++--- src/conf/network_conf.c | 28 ++----- src/conf/virconftypes.h | 2 - src/hypervisor/virhostdev.c | 25 +++--- src/util/virbitmap.c | 4 +- src/util/virbitmap.h | 2 +- tests/qemustatusxml2xmldata/modern-in.xml | 12 +++ 10 files changed, 107 insertions(+), 130 deletions(-)
-- 2.39.1
participants (3)
-
Ján Tomko -
Martin Kletzander -
Peter Krempa