[PATCH 0/2] virtio-net queue handling fixes
Peter Krempa (2): virNetDevTapCreate: Use error message hinting to multiqueue use only when opening multiple queues virDomainNetDefCheckABIStability: Consider virtio 'queues' ABI src/conf/domain_conf.c | 11 +++++++++++ src/util/virnetdevtap.c | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) -- 2.49.0
From: Peter Krempa <pkrempa@redhat.com> Due to a logic bug the error message mentioning mult_queue operation would be mentioned also when a single queue would be opened on an externally managed tap device. Adjust the condition to trigger only when multiple queues are in use. Fixes: f6fb097e11a Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virnetdevtap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 1dc77f0f5c..648d095dfc 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -231,7 +231,7 @@ int virNetDevTapCreate(char **ifname, if (ioctl(fd, TUNSETIFF, &ifr) < 0) { if (flags & VIR_NETDEV_TAP_CREATE_ALLOW_EXISTING && - tapfdSize > 0) { + tapfdSize > 1) { virReportSystemError(errno, _("Unable to create multiple fds for tap device %1$s (maybe existing device was created without multi_queue flag)"), *ifname); -- 2.49.0
On Tue, May 13, 2025 at 02:25:50PM +0200, Peter Krempa via Devel wrote:
From: Peter Krempa <pkrempa@redhat.com>
Due to a logic bug the error message mentioning mult_queue operation would be mentioned also when a single queue would be opened on an externally managed tap device.
Adjust the condition to trigger only when multiple queues are in use.
Fixes: f6fb097e11a Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virnetdevtap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
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 :|
On a Tuesday in 2025, Peter Krempa via Devel wrote:
From: Peter Krempa <pkrempa@redhat.com>
Due to a logic bug the error message mentioning mult_queue operation
multi_queue
would be mentioned also when a single queue would be opened on an externally managed tap device.
Adjust the condition to trigger only when multiple queues are in use.
Fixes: f6fb097e11a Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virnetdevtap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Jano
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
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 :|
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 :|
participants (3)
-
Daniel P. Berrangé -
Ján Tomko -
Peter Krempa