[PATCH 5/8] qemu: Generate acpi-generic-initiator command from acpi nodeset

Andrea Righi via Devel posted 8 patches 5 months ago
[PATCH 5/8] qemu: Generate acpi-generic-initiator command from acpi nodeset
Posted by Andrea Righi via Devel 5 months ago
Signed-off-by: Andrea Righi <arighi@nvidia.com>
---
 src/qemu/qemu_command.c | 45 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3f9b583985..9ca0847789 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5222,6 +5222,47 @@ qemuBuildHostdevSCSICommandLine(virCommand *cmd,
 }
 
 
+static int
+qemuBuildAcpiNodesetProps(virCommand *cmd,
+                          virDomainDeviceInfo *info,
+                          virQEMUCaps *qemuCaps)
+{
+    static unsigned int giIndex;
+    int node = -1;
+
+    if (!info->acpiNodeset)
+        return 0;
+
+    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_ACPI_GENERIC_INITIATOR))
+        return -1;
+
+    while ((node = virBitmapNextSetBit(info->acpiNodeset, node)) > -1) {
+        g_autoptr(virJSONValue) props = NULL;
+        g_autofree char *id = g_strdup_printf("gi%u", giIndex++);
+
+        if (virJSONValueObjectAdd(&props,
+                                  "s:qom-type", "acpi-generic-initiator",
+                                  "s:id", id,
+                                  "s:pci-dev", info->alias,
+                                  "i:node", node,
+                                  NULL) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("Failed to build acpi-generic-initiator properties"));
+
+            return -1;
+        }
+
+        if (qemuBuildObjectCommandlineFromJSON(cmd, props) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("Failed to build QEMU command line for acpi-generic-initiator"));
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+
 static int
 qemuBuildHostdevCommandLine(virCommand *cmd,
                             const virDomainDef *def,
@@ -5264,6 +5305,10 @@ qemuBuildHostdevCommandLine(virCommand *cmd,
 
             if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, def, qemuCaps) < 0)
                 return -1;
+
+            if (qemuBuildAcpiNodesetProps(cmd, hostdev->info, qemuCaps) < 0)
+                return -1;
+
             break;
 
         /* SCSI */
-- 
2.51.0
Re: [PATCH 5/8] qemu: Generate acpi-generic-initiator command from acpi nodeset
Posted by Daniel P. Berrangé via Devel 5 months ago
On Sat, Sep 06, 2025 at 03:09:00PM +0200, Andrea Righi wrote:
> Signed-off-by: Andrea Righi <arighi@nvidia.com>
> ---
>  src/qemu/qemu_command.c | 45 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 3f9b583985..9ca0847789 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -5222,6 +5222,47 @@ qemuBuildHostdevSCSICommandLine(virCommand *cmd,
>  }
>  
>  
> +static int
> +qemuBuildAcpiNodesetProps(virCommand *cmd,
> +                          virDomainDeviceInfo *info,
> +                          virQEMUCaps *qemuCaps)
> +{
> +    static unsigned int giIndex;
> +    int node = -1;
> +
> +    if (!info->acpiNodeset)
> +        return 0;
> +
> +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_ACPI_GENERIC_INITIATOR))
> +        return -1;

We can assume the validate function already ran, so we don't
need this check here, which is good as this would return an
error status without setting an error message.

> +
> +    while ((node = virBitmapNextSetBit(info->acpiNodeset, node)) > -1) {
> +        g_autoptr(virJSONValue) props = NULL;
> +        g_autofree char *id = g_strdup_printf("gi%u", giIndex++);
> +
> +        if (virJSONValueObjectAdd(&props,
> +                                  "s:qom-type", "acpi-generic-initiator",
> +                                  "s:id", id,
> +                                  "s:pci-dev", info->alias,
> +                                  "i:node", node,
> +                                  NULL) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("Failed to build acpi-generic-initiator properties"));
> +
> +            return -1;
> +        }
> +
> +        if (qemuBuildObjectCommandlineFromJSON(cmd, props) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("Failed to build QEMU command line for acpi-generic-initiator"));
> +            return -1;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +
>  static int
>  qemuBuildHostdevCommandLine(virCommand *cmd,
>                              const virDomainDef *def,
> @@ -5264,6 +5305,10 @@ qemuBuildHostdevCommandLine(virCommand *cmd,
>  
>              if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, def, qemuCaps) < 0)
>                  return -1;
> +
> +            if (qemuBuildAcpiNodesetProps(cmd, hostdev->info, qemuCaps) < 0)
> +                return -1;
> +
>              break;
>  
>          /* SCSI */
> -- 
> 2.51.0
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 5/8] qemu: Generate acpi-generic-initiator command from acpi nodeset
Posted by Andrea Righi via Devel 5 months ago
Hi Daniel,

On Mon, Sep 08, 2025 at 06:02:20PM +0100, Daniel P. Berrangé wrote:
> On Sat, Sep 06, 2025 at 03:09:00PM +0200, Andrea Righi wrote:
> > Signed-off-by: Andrea Righi <arighi@nvidia.com>
> > ---
> >  src/qemu/qemu_command.c | 45 +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 45 insertions(+)
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> > 
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 3f9b583985..9ca0847789 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -5222,6 +5222,47 @@ qemuBuildHostdevSCSICommandLine(virCommand *cmd,
> >  }
> >  
> >  
> > +static int
> > +qemuBuildAcpiNodesetProps(virCommand *cmd,
> > +                          virDomainDeviceInfo *info,
> > +                          virQEMUCaps *qemuCaps)
> > +{
> > +    static unsigned int giIndex;
> > +    int node = -1;
> > +
> > +    if (!info->acpiNodeset)
> > +        return 0;
> > +
> > +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_ACPI_GENERIC_INITIATOR))
> > +        return -1;
> 
> We can assume the validate function already ran, so we don't
> need this check here, which is good as this would return an
> error status without setting an error message.

Ah yes, this check is redundant, we can definitely drop it.
Should I send an update patch just with this change?

Thanks,
-Andrea
Re: [PATCH 5/8] qemu: Generate acpi-generic-initiator command from acpi nodeset
Posted by Daniel P. Berrangé via Devel 5 months ago
On Mon, Sep 08, 2025 at 07:15:18PM +0200, Andrea Righi wrote:
> Hi Daniel,
> 
> On Mon, Sep 08, 2025 at 06:02:20PM +0100, Daniel P. Berrangé wrote:
> > On Sat, Sep 06, 2025 at 03:09:00PM +0200, Andrea Righi wrote:
> > > Signed-off-by: Andrea Righi <arighi@nvidia.com>
> > > ---
> > >  src/qemu/qemu_command.c | 45 +++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 45 insertions(+)
> > 
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > 
> > > 
> > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > > index 3f9b583985..9ca0847789 100644
> > > --- a/src/qemu/qemu_command.c
> > > +++ b/src/qemu/qemu_command.c
> > > @@ -5222,6 +5222,47 @@ qemuBuildHostdevSCSICommandLine(virCommand *cmd,
> > >  }
> > >  
> > >  
> > > +static int
> > > +qemuBuildAcpiNodesetProps(virCommand *cmd,
> > > +                          virDomainDeviceInfo *info,
> > > +                          virQEMUCaps *qemuCaps)
> > > +{
> > > +    static unsigned int giIndex;
> > > +    int node = -1;
> > > +
> > > +    if (!info->acpiNodeset)
> > > +        return 0;
> > > +
> > > +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_ACPI_GENERIC_INITIATOR))
> > > +        return -1;
> > 
> > We can assume the validate function already ran, so we don't
> > need this check here, which is good as this would return an
> > error status without setting an error message.
> 
> Ah yes, this check is redundant, we can definitely drop it.
> Should I send an update patch just with this change?

Don't bother. The rest of the series is fine, so I'll make the obvious
change and push this once I've validated it in CI.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 5/8] qemu: Generate acpi-generic-initiator command from acpi nodeset
Posted by Andrea Righi via Devel 5 months ago
On Mon, Sep 08, 2025 at 06:19:38PM +0100, Daniel P. Berrangé wrote:
> On Mon, Sep 08, 2025 at 07:15:18PM +0200, Andrea Righi wrote:
> > Hi Daniel,
> > 
> > On Mon, Sep 08, 2025 at 06:02:20PM +0100, Daniel P. Berrangé wrote:
> > > On Sat, Sep 06, 2025 at 03:09:00PM +0200, Andrea Righi wrote:
> > > > Signed-off-by: Andrea Righi <arighi@nvidia.com>
> > > > ---
> > > >  src/qemu/qemu_command.c | 45 +++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 45 insertions(+)
> > > 
> > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > > 
> > > > 
> > > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > > > index 3f9b583985..9ca0847789 100644
> > > > --- a/src/qemu/qemu_command.c
> > > > +++ b/src/qemu/qemu_command.c
> > > > @@ -5222,6 +5222,47 @@ qemuBuildHostdevSCSICommandLine(virCommand *cmd,
> > > >  }
> > > >  
> > > >  
> > > > +static int
> > > > +qemuBuildAcpiNodesetProps(virCommand *cmd,
> > > > +                          virDomainDeviceInfo *info,
> > > > +                          virQEMUCaps *qemuCaps)
> > > > +{
> > > > +    static unsigned int giIndex;
> > > > +    int node = -1;
> > > > +
> > > > +    if (!info->acpiNodeset)
> > > > +        return 0;
> > > > +
> > > > +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_ACPI_GENERIC_INITIATOR))
> > > > +        return -1;
> > > 
> > > We can assume the validate function already ran, so we don't
> > > need this check here, which is good as this would return an
> > > error status without setting an error message.
> > 
> > Ah yes, this check is redundant, we can definitely drop it.
> > Should I send an update patch just with this change?
> 
> Don't bother. The rest of the series is fine, so I'll make the obvious
> change and push this once I've validated it in CI.

Ok, thanks!

-Andrea