[PATCH 7/7] conf: Store 'origstates' of PCI hostdevs in a bitmap

Peter Krempa pkrempa at redhat.com
Thu Feb 2 15:18:10 UTC 2023


Refactor the code to use a bitmap with an enum to avoid ugly XML parser.

Signed-off-by: Peter Krempa <pkrempa at 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



More information about the libvir-list mailing list