virtio-fs: fix MSI-X nvectors calculation
diff mbox series

Message ID 20191209110759.35227-1-stefanha@redhat.com
State New
Headers show
Series
  • virtio-fs: fix MSI-X nvectors calculation
Related show

Commit Message

Stefan Hajnoczi Dec. 9, 2019, 11:07 a.m. UTC
The following MSI-X vectors are required:
 * VIRTIO Configuration Change
 * hiprio virtqueue
 * requests virtqueues

Fix the calculation to reserve enough MSI-X vectors.  Otherwise guest
drivers fall back to a sub-optional configuration where all virtqueues
share a single vector.

This change does not break live migration compatibility since
vhost-user-fs-pci devices are not migratable yet.

Reported-by: Vivek Goyal <vgoyal@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/virtio/vhost-user-fs-pci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Dr. David Alan Gilbert Dec. 9, 2019, 2:39 p.m. UTC | #1
* Stefan Hajnoczi (stefanha@redhat.com) wrote:
> The following MSI-X vectors are required:
>  * VIRTIO Configuration Change
>  * hiprio virtqueue
>  * requests virtqueues
> 
> Fix the calculation to reserve enough MSI-X vectors.  Otherwise guest
> drivers fall back to a sub-optional configuration where all virtqueues
> share a single vector.
> 
> This change does not break live migration compatibility since
> vhost-user-fs-pci devices are not migratable yet.
> 
> Reported-by: Vivek Goyal <vgoyal@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>



> ---
>  hw/virtio/vhost-user-fs-pci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/vhost-user-fs-pci.c b/hw/virtio/vhost-user-fs-pci.c
> index 933a3f265b..e3a649d4a6 100644
> --- a/hw/virtio/vhost-user-fs-pci.c
> +++ b/hw/virtio/vhost-user-fs-pci.c
> @@ -40,7 +40,8 @@ static void vhost_user_fs_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>      DeviceState *vdev = DEVICE(&dev->vdev);
>  
>      if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
> -        vpci_dev->nvectors = dev->vdev.conf.num_request_queues + 1;
> +        /* Also reserve config change and hiprio queue vectors */
> +        vpci_dev->nvectors = dev->vdev.conf.num_request_queues + 2;
>      }
>  
>      qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
> -- 
> 2.23.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
no-reply@patchew.org Dec. 9, 2019, 7:21 p.m. UTC | #2
Patchew URL: https://patchew.org/QEMU/20191209110759.35227-1-stefanha@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
remote: Counting objects: 5280, done.        
error: RPC failed; result=18, HTTP code = 200
fatal: The remote end hung up unexpectedly
fatal: protocol error: bad pack header
Clone of 'https://git.qemu.org/git/dtc.git' into submodule path 'dtc' failed
failed to update submodule dtc
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) unregistered for path 'dtc'
make[1]: *** [/var/tmp/patchew-tester-tmp-x3rpl899/src/docker-src.2019-12-09-14.16.12.15965] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-x3rpl899/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    5m24.090s
user    0m2.544s


The full log is available at
http://patchew.org/logs/20191209110759.35227-1-stefanha@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
no-reply@patchew.org Dec. 9, 2019, 7:26 p.m. UTC | #3
Patchew URL: https://patchew.org/QEMU/20191209110759.35227-1-stefanha@redhat.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
remote: Counting objects: 5280, done.        
error: RPC failed; result=18, HTTP code = 200
fatal: The remote end hung up unexpectedly
fatal: protocol error: bad pack header
Clone of 'https://git.qemu.org/git/dtc.git' into submodule path 'dtc' failed
failed to update submodule dtc
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) unregistered for path 'dtc'
make[1]: *** [/var/tmp/patchew-tester-tmp-qf8_kdog/src/docker-src.2019-12-09-14.22.17.17018] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-qf8_kdog/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    4m26.482s
user    0m2.275s


The full log is available at
http://patchew.org/logs/20191209110759.35227-1-stefanha@redhat.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Michael S. Tsirkin Dec. 11, 2019, 4:05 p.m. UTC | #4
On Mon, Dec 09, 2019 at 11:07:59AM +0000, Stefan Hajnoczi wrote:
> The following MSI-X vectors are required:
>  * VIRTIO Configuration Change
>  * hiprio virtqueue
>  * requests virtqueues
> 
> Fix the calculation to reserve enough MSI-X vectors.  Otherwise guest
> drivers fall back to a sub-optional configuration where all virtqueues
> share a single vector.
> 
> This change does not break live migration compatibility since
> vhost-user-fs-pci devices are not migratable yet.
> 
> Reported-by: Vivek Goyal <vgoyal@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>


> ---
>  hw/virtio/vhost-user-fs-pci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/vhost-user-fs-pci.c b/hw/virtio/vhost-user-fs-pci.c
> index 933a3f265b..e3a649d4a6 100644
> --- a/hw/virtio/vhost-user-fs-pci.c
> +++ b/hw/virtio/vhost-user-fs-pci.c
> @@ -40,7 +40,8 @@ static void vhost_user_fs_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>      DeviceState *vdev = DEVICE(&dev->vdev);
>  
>      if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
> -        vpci_dev->nvectors = dev->vdev.conf.num_request_queues + 1;
> +        /* Also reserve config change and hiprio queue vectors */
> +        vpci_dev->nvectors = dev->vdev.conf.num_request_queues + 2;
>      }
>  
>      qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
> -- 
> 2.23.0
Dr. David Alan Gilbert Dec. 13, 2019, 10:55 a.m. UTC | #5
* Stefan Hajnoczi (stefanha@redhat.com) wrote:
> The following MSI-X vectors are required:
>  * VIRTIO Configuration Change
>  * hiprio virtqueue
>  * requests virtqueues
> 
> Fix the calculation to reserve enough MSI-X vectors.  Otherwise guest
> drivers fall back to a sub-optional configuration where all virtqueues
> share a single vector.
> 
> This change does not break live migration compatibility since
> vhost-user-fs-pci devices are not migratable yet.
> 
> Reported-by: Vivek Goyal <vgoyal@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Queued for virtiofs

> ---
>  hw/virtio/vhost-user-fs-pci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/vhost-user-fs-pci.c b/hw/virtio/vhost-user-fs-pci.c
> index 933a3f265b..e3a649d4a6 100644
> --- a/hw/virtio/vhost-user-fs-pci.c
> +++ b/hw/virtio/vhost-user-fs-pci.c
> @@ -40,7 +40,8 @@ static void vhost_user_fs_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>      DeviceState *vdev = DEVICE(&dev->vdev);
>  
>      if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
> -        vpci_dev->nvectors = dev->vdev.conf.num_request_queues + 1;
> +        /* Also reserve config change and hiprio queue vectors */
> +        vpci_dev->nvectors = dev->vdev.conf.num_request_queues + 2;
>      }
>  
>      qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
> -- 
> 2.23.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Stefan Hajnoczi Dec. 13, 2019, 11:29 a.m. UTC | #6
On Mon, Dec 09, 2019 at 11:07:59AM +0000, Stefan Hajnoczi wrote:
> The following MSI-X vectors are required:
>  * VIRTIO Configuration Change
>  * hiprio virtqueue
>  * requests virtqueues
> 
> Fix the calculation to reserve enough MSI-X vectors.  Otherwise guest
> drivers fall back to a sub-optional configuration where all virtqueues
> share a single vector.
> 
> This change does not break live migration compatibility since
> vhost-user-fs-pci devices are not migratable yet.
> 
> Reported-by: Vivek Goyal <vgoyal@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  hw/virtio/vhost-user-fs-pci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

Patch
diff mbox series

diff --git a/hw/virtio/vhost-user-fs-pci.c b/hw/virtio/vhost-user-fs-pci.c
index 933a3f265b..e3a649d4a6 100644
--- a/hw/virtio/vhost-user-fs-pci.c
+++ b/hw/virtio/vhost-user-fs-pci.c
@@ -40,7 +40,8 @@  static void vhost_user_fs_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
     DeviceState *vdev = DEVICE(&dev->vdev);
 
     if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
-        vpci_dev->nvectors = dev->vdev.conf.num_request_queues + 1;
+        /* Also reserve config change and hiprio queue vectors */
+        vpci_dev->nvectors = dev->vdev.conf.num_request_queues + 2;
     }
 
     qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));