Re: [PATCH 2/6] conf: Introduce acpi-generic-initiator device

9 May 2025

On 4/2/25 10:25, Andrea Righi via Devel wrote:
...
Introduce apci-generic-initiator device to the domain XML.
Example definition:
<acpi-generic-initiator>
    <alias name="gi1" />
    <pci-dev>dev0</pci-dev>
    <numa-node>1</numa-node>
  </acpi-generic-initiator>
This enables partitioning of PCI resources into multiple isolated
instances, each requiring a dedicated NUMA node definition, that can be
represented by the acpi-generic-initiator object.
Signed-off-by: Andrea Righi <arighi@nvidia.com>
---
 src/ch/ch_domain.c                |   1 +
 src/conf/domain_conf.c            | 133 ++++++++++++++++++++++++++++++
 src/conf/domain_conf.h            |   1 +
 src/conf/domain_postparse.c       |   1 +
 src/conf/domain_validate.c        |  37 +++++++++
 src/conf/schemas/domaincommon.rng |   2 +-
 src/libxl/libxl_driver.c          |   6 ++
 src/lxc/lxc_driver.c              |   6 ++
 src/qemu/qemu_command.c           |   1 +
 src/qemu/qemu_domain.c            |   2 +
 src/qemu/qemu_domain_address.c    |   4 +
 src/qemu/qemu_driver.c            |   3 +
 src/qemu/qemu_hotplug.c           |   5 ++
 src/qemu/qemu_postparse.c         |   1 +
 src/qemu/qemu_validate.c          |   1 +
 src/test/test_driver.c            |   4 +
 16 files changed, 207 insertions(+), 1 deletion(-)
There's another switch() in hypervDomainAttachDeviceFlags() in
src/hyperv/hyperv_driver.c that wants this new enum addition covered too.
...

diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
index a08b18c5b9..f88ce32803 100644
--- a/src/ch/ch_domain.c
+++ b/src/ch/ch_domain.c
@@ -162,6 +162,7 @@ chValidateDomainDeviceDef(const virDomainDeviceDef *dev,
     case VIR_DOMAIN_DEVICE_CONTROLLER:
     case VIR_DOMAIN_DEVICE_CHR:
     case VIR_DOMAIN_DEVICE_HOSTDEV:
+    case VIR_DOMAIN_DEVICE_ACPI_INITIATOR:
         break;
case VIR_DOMAIN_DEVICE_LEASE:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 405f0207e9..2294ad42bf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -342,6 +342,7 @@ VIR_ENUM_IMPL(virDomainDevice,
               "audio",
               "crypto",
               "pstore",
+              "acpiinitiator",
 );
VIR_ENUM_IMPL(virDomainDiskDevice,
@@ -3727,6 +3728,9 @@ void virDomainDeviceDefFree(virDomainDeviceDef *def)
     case VIR_DOMAIN_DEVICE_PSTORE:
         virDomainPstoreDefFree(def->data.pstore);
         break;
+    case VIR_DOMAIN_DEVICE_ACPI_INITIATOR:
+        virDomainAcpiInitiatorDefFree(def->data.acpiinitiator);
+        break;
     case VIR_DOMAIN_DEVICE_LAST:
     case VIR_DOMAIN_DEVICE_NONE:
         break;
@@ -4705,6 +4709,8 @@ virDomainDeviceGetInfo(const virDomainDeviceDef *device)
         return &device->data.crypto->info;
     case VIR_DOMAIN_DEVICE_PSTORE:
         return &device->data.pstore->info;
+    case VIR_DOMAIN_DEVICE_ACPI_INITIATOR:
+        return device->data.acpiinitiator->info;
/* The following devices do not contain virDomainDeviceInfo */
     case VIR_DOMAIN_DEVICE_LEASE:
@@ -4813,6 +4819,9 @@ virDomainDeviceSetData(virDomainDeviceDef *device,
     case VIR_DOMAIN_DEVICE_PSTORE:
         device->data.pstore = devicedata;
         break;
+    case VIR_DOMAIN_DEVICE_ACPI_INITIATOR:
+        device->data.acpiinitiator = devicedata;
+        break;
     case VIR_DOMAIN_DEVICE_NONE:
     case VIR_DOMAIN_DEVICE_LAST:
         break;
@@ -5038,6 +5047,13 @@ virDomainDeviceInfoIterateFlags(virDomainDef *def,
             return rc;
     }
+    device.type = VIR_DOMAIN_DEVICE_ACPI_INITIATOR;
+    for (i = 0; i < def->nacpiinitiator; i++) {
+        device.data.acpiinitiator = def->acpiinitiator[i];
+        if ((rc = cb(def, &device, def->acpiinitiator[i]->info, opaque)) != 0)
+            return rc;
+    }
+
     /* If the flag below is set, make sure @cb can handle @info being NULL */
     if (iteratorFlags & DOMAIN_DEVICE_ITERATE_MISSING_INFO) {
         device.type = VIR_DOMAIN_DEVICE_GRAPHICS;
@@ -5098,6 +5114,7 @@ virDomainDeviceInfoIterateFlags(virDomainDef *def,
     case VIR_DOMAIN_DEVICE_AUDIO:
     case VIR_DOMAIN_DEVICE_CRYPTO:
     case VIR_DOMAIN_DEVICE_PSTORE:
+    case VIR_DOMAIN_DEVICE_ACPI_INITIATOR:
         break;
     }
 #endif
@@ -13685,6 +13702,65 @@ virDomainHostdevDefParseXML(virDomainXMLOption *xmlopt,
 }
+static virDomainAcpiInitiatorDef *
+virDomainAcpiInitiatorDefParseXML(virDomainXMLOption *xmlopt,
+                                  xmlNodePtr node,
+                                  xmlXPathContextPtr ctxt,
+                                  unsigned int flags)
+{
+    virDomainAcpiInitiatorDef *def;
This could be:

  g_autoptr(virDomainAcpiInitiatorDef) def = NULL;
...
+    g_autofree char *tmp = NULL;
+
+    VIR_XPATH_NODE_AUTORESTORE(ctxt)
+
+    ctxt->node = node;
+
+    def = virDomainAcpiInitiatorDefNew();
+
+    def->name = virXPathString("string(./alias/@name)", ctxt);
+    if (!def->name) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("missing initiator name"));
+        goto error;
and then all these 'goto error' could be plain 'return NULL'.
...
+    }
+
+    def->pciDev = virXPathString("string(./pci-dev)", ctxt);
+    if (!def->pciDev) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("missing initiator pci-dev"));
+        goto error;
+    }
+
+    tmp = virXPathString("string(./numa-node)", ctxt);
+    if (tmp) {
+        if (virStrToLong_i((const char *)tmp, NULL, 10, &def->numaNode) < 0) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("invalid value for numa-node: '%1$s'"), tmp);
+            goto error;
+        }
+    } else {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("missing initiator numa-node"));
+        goto error;
+    }
+
+    if (def->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+        if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, def->info,
+                                        flags) < 0) {
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("failed to parse device information"));
+            goto error;
+        }
+    }
+
+    return def;
+
+ error:
+    virDomainAcpiInitiatorDefFree(def);
+    return NULL;
+}
+
+
 static virDomainRedirdevDef *
 virDomainRedirdevDefParseXML(virDomainXMLOption *xmlopt,
                              xmlNodePtr node,
@@ -14686,6 +14762,12 @@ virDomainDeviceDefParse(const char *xmlStr,
             return NULL;
         }
         break;
+    case VIR_DOMAIN_DEVICE_ACPI_INITIATOR:
+        if (!(dev->data.acpiinitiator = virDomainAcpiInitiatorDefParseXML(xmlopt, node,
+                                                                          ctxt, flags))) {
+            return NULL;
+        }
+        break;
     case VIR_DOMAIN_DEVICE_NONE:
     case VIR_DOMAIN_DEVICE_LAST:
         break;
@@ -20095,6 +20177,23 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt,
     }
     VIR_FREE(nodes);
+    /* analysis of the acpi generic initiator */
+    if ((n = virXPathNodeSet("./devices/acpi-generic-initiator", ctxt, &nodes)) < 0)
+        return NULL;
+
Missing:

if (n)
...
+    def->acpiinitiator = g_new0(virDomainAcpiInitiatorDef *, n);
We probably don't need to allocate any memory if n==0. Not even a dummy
pointer.
...
+
+    for (i = 0; i < n; i++) {
+        virDomainAcpiInitiatorDef *acpiinitiator;
+
+        acpiinitiator = virDomainAcpiInitiatorDefParseXML(xmlopt, nodes[i], ctxt, flags);
+        if (!acpiinitiator)
+            return NULL;
+
+        def->acpiinitiator[def->nacpiinitiator++] = acpiinitiator;
+    }
+    VIR_FREE(nodes);
+
Michal

    

Re: [PATCH 2/6] conf: Introduce acpi-generic-initiator device

Michal Prívozník