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

On Wed, Feb 12, 2025 at 07:26:09 +0100, Andrea Righi via Devel wrote: > Allow to define new acpi-generic-initiator objects to link a PCI device > with multiple NUMA nodes. > > Link: https://mail.gnu.org/archive/html/qemu-arm/2024-03/msg00358.html > Signed-off-by: Andrea Righi <arighi(a)nvidia.com&gt; > --- Note this is not a full review and I don't plan to review any further aspects of this series. > src/ch/ch_domain.c | 1 + > src/conf/domain_conf.c | 153 ++++++++++++++++++++++++++++++ > src/conf/domain_conf.h | 14 +++ > src/conf/domain_postparse.c | 1 + > src/conf/domain_validate.c | 40 ++++++++ > src/conf/schemas/domaincommon.rng | 19 ++++ We usually tend to separate generic parser infrastructure ... > src/conf/virconftypes.h | 2 + > src/libxl/libxl_driver.c | 6 ++ > src/lxc/lxc_driver.c | 6 ++ > src/qemu/qemu_command.c | 18 ++++ > 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 + ... from any driver implementation into a separate patch.

> src/test/test_driver.c | 4 + > 17 files changed, 280 insertions(+) So that's easier to review. You are also adding new XML but are completely missing test files which show how it's used and excercise the parser and formatter as well as the qemu driver implementation. The simplest way is to use qemuxmlconftest to add these.

You're also referencing 'acpi-generic-initiator' object which is not supported by all versions so you'll need to add a capability flag for it. See qemu_capabilities.c/h

> > +static int > +qemuBuildAcpiInitiatorCommandLine(virCommand *cmd, > + const virDomainAcpiInitiatorDef *acpiinitiator) > +{ > + g_autofree char *obj = NULL; > + > + obj = g_strdup_printf("acpi-generic-initiator,id=%s,pci-dev=%s,node=%d", > + acpiinitiator->name, acpiinitiator->pciDev, acpiinitiator->numaNode); > + virCommandAddArgList(cmd, "-object", obj, NULL); All other code which uses defines -object uses JSON props. You'll have to convert this to use the same approach and use qemuBuildObjectCommandlineFromJSON() to convert the JSON definition to the commandline. Apart from symmetry, this'll provide validation of the generated properties against qemu's internal schemas.