Re: [PATCH v3 05/16] tests/virtio-blk-test: set up virtqueue after feature negotiation

[Thomas Huth <thuth@redhat.com>]

On 19/10/2019 08.37, Stefan Hajnoczi wrote:
> VIRTIO Device Initialization requires that feature negotiation has
> completed before virtqueues are set up.  This makes sense because the
> driver must know whether it is operating in Legacy or VIRTIO 1.0 mode
> before it can access vring fields with the correct endianness.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/virtio-blk-test.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> index 31680cc159..fe0dc4a896 100644
> --- a/tests/virtio-blk-test.c
> +++ b/tests/virtio-blk-test.c
> @@ -113,8 +113,8 @@ static uint64_t virtio_blk_request(QGuestAllocator *alloc, QVirtioDevice *d,
>      return addr;
>  }
>  
> -static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,
> -                       QVirtQueue *vq)
> +/* Returns the request virtqueue so the caller can perform further tests */
> +static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
>  {
>      QVirtioBlkReq req;
>      uint64_t req_addr;
> @@ -124,6 +124,7 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,
>      uint8_t status;
>      char *data;
>      QTestState *qts = global_qtest;
> +    QVirtQueue *vq;
>  
>      features = qvirtio_get_features(dev);
>      features = features & ~(QVIRTIO_F_BAD_FEATURE |
> @@ -135,6 +136,8 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,
>      capacity = qvirtio_config_readq(dev, 0);
>      g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
>  
> +    vq = qvirtqueue_setup(dev, alloc, 0);
> +
>      qvirtio_set_driver_ok(dev);
>  
>      /* Write and read with 3 descriptor layout */
> @@ -331,14 +334,16 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,
>  
>          guest_free(alloc, req_addr);
>      }
> +
> +    return vq;
>  }
>  
>  static void basic(void *obj, void *data, QGuestAllocator *t_alloc)
>  {
>      QVirtioBlk *blk_if = obj;
>      QVirtQueue *vq;
> -    vq = qvirtqueue_setup(blk_if->vdev, t_alloc, 0);
> -    test_basic(blk_if->vdev, t_alloc, vq);
> +
> +    vq = test_basic(blk_if->vdev, t_alloc);
>      qvirtqueue_cleanup(blk_if->vdev->bus, vq, t_alloc);
>  
>  }
> @@ -746,9 +751,7 @@ static void resize(void *obj, void *data, QGuestAllocator *t_alloc)
>      QVirtQueue *vq;
>      QTestState *qts = global_qtest;
>  
> -    vq = qvirtqueue_setup(dev, t_alloc, 0);
> -
> -    test_basic(dev, t_alloc, vq);
> +    vq = test_basic(dev, t_alloc);
>  
>      qmp_discard_response("{ 'execute': 'block_resize', "
>                           " 'arguments': { 'device': 'drive0', "
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>