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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---

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.



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux