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 <ari...@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
