[PATCH 2/2] virDomainNetDefCheckABIStability: Consider virtio 'queues' ABI

Peter Krempa via Devel posted 2 patches 3 months, 4 weeks ago
[PATCH 2/2] virDomainNetDefCheckABIStability: Consider virtio 'queues' ABI
Posted by Peter Krempa via Devel 3 months, 4 weeks ago
From: Peter Krempa <pkrempa@redhat.com>

While the queue count itself is not a guest visible property, libvirt
uses it to calculate the 'vectors' property of the 'virtio-net' device
which is ABI.

Since we don't expose control of 'vectors' explicitly, consider 'queues'
ABI.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/conf/domain_conf.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 542d6ade91..b3b0bd7329 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -20801,6 +20801,17 @@ virDomainNetDefCheckABIStability(virDomainNetDef *src,
         return false;
     }

+    /* The number of queues is used to calculate the value for 'vectors'
+     * (see qemuBuildNicDevProps) which is machine ABI thus we need to ensure
+     * that the number of queues is kept in sync */
+    if (virDomainNetIsVirtioModel(src) &&
+        (src->driver.virtio.queues != dst->driver.virtio.queues)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Target virtio network queue count '%1$d' does not match source '%2$d'"),
+                       dst->driver.virtio.queues, src->driver.virtio.queues);
+        return false;
+    }
+
     if (!virDomainVirtioOptionsCheckABIStability(src->virtio, dst->virtio))
         return false;

-- 
2.49.0
Re: [PATCH 2/2] virDomainNetDefCheckABIStability: Consider virtio 'queues' ABI
Posted by Daniel P. Berrangé via Devel 3 months, 4 weeks ago
On Tue, May 13, 2025 at 02:25:51PM +0200, Peter Krempa via Devel wrote:
> From: Peter Krempa <pkrempa@redhat.com>
> 
> While the queue count itself is not a guest visible property, libvirt
> uses it to calculate the 'vectors' property of the 'virtio-net' device
> which is ABI.
> 
> Since we don't expose control of 'vectors' explicitly, consider 'queues'
> ABI.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/conf/domain_conf.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)

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


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 2/2] virDomainNetDefCheckABIStability: Consider virtio 'queues' ABI
Posted by Peter Krempa via Devel 3 months, 4 weeks ago
On Tue, May 13, 2025 at 13:28:58 +0100, Daniel P. Berrangé wrote:
> On Tue, May 13, 2025 at 02:25:51PM +0200, Peter Krempa via Devel wrote:
> > From: Peter Krempa <pkrempa@redhat.com>
> > 
> > While the queue count itself is not a guest visible property, libvirt
> > uses it to calculate the 'vectors' property of the 'virtio-net' device
> > which is ABI.
> > 
> > Since we don't expose control of 'vectors' explicitly, consider 'queues'
> > ABI.
> > 
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> >  src/conf/domain_conf.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)

Per your off-list suggestion I'll be adding a comment to the formula
in qemuBuildNicDevProps which calculates 'vectors' to state that it's
guest ABI and must not change: 


        if (net->driver.virtio.queues > 1) {
            if (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
                /* ccw provides a one to one relation of fds to queues and
                 * does not support the vectors option
                 */
                mq = VIR_TRISTATE_SWITCH_ON;
            } else {
                /* As advised at https://www.linux-kvm.org/page/Multiqueue
                 * we should add vectors=2*N+2 where N is the vhostfdSize
                 */
                mq = VIR_TRISTATE_SWITCH_ON;
+               /* As 'vectors' is a guest-OS visible property and thus
+                * guest ABI this formula *MUST NOT* change */
                vectors = 2 * net->driver.virtio.queues + 2;
            }
        }



> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> 
> 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 :|
>