Re: [Qemu-devel] [PATCH v6 16/29] libqos: Use explicit QTestState for virtio operations

[Thomas Huth <thuth@redhat.com>]

On 01.09.2017 20:03, Eric Blake wrote:
> Drop one more client of global_qtest by teaching all virtio test
> functionality to pass in an explicit QTestState in constructors,
> where it is then reused for later access.  Adjust all callers.
> This gets us one step closer to eliminating implicit use of
> global_qtest.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  tests/libqos/virtio-mmio.h |  3 +-
>  tests/libqos/virtio.h      |  5 ++-
>  tests/libqos/virtio-mmio.c | 54 +++++++++++++++++---------------
>  tests/libqos/virtio-pci.c  |  3 ++
>  tests/libqos/virtio.c      | 77 ++++++++++++++++++++++++++--------------------
>  tests/virtio-blk-test.c    |  3 +-
>  6 files changed, 84 insertions(+), 61 deletions(-)
> 
> diff --git a/tests/libqos/virtio-mmio.h b/tests/libqos/virtio-mmio.h
> index e3e52b9ce1..bd01386054 100644
> --- a/tests/libqos/virtio-mmio.h
> +++ b/tests/libqos/virtio-mmio.h
> @@ -41,6 +41,7 @@ typedef struct QVirtioMMIODevice {
> 
>  extern const QVirtioBus qvirtio_mmio;
> 
> -QVirtioMMIODevice *qvirtio_mmio_init_device(uint64_t addr, uint32_t page_size);
> +QVirtioMMIODevice *qvirtio_mmio_init_device(QTestState *qts, uint64_t addr,
> +                                            uint32_t page_size);
> 
>  #endif
> diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
> index 8fbcd1869c..d180d54fc4 100644
> --- a/tests/libqos/virtio.h
> +++ b/tests/libqos/virtio.h
> @@ -19,6 +19,7 @@ typedef struct QVirtioBus QVirtioBus;
> 
>  typedef struct QVirtioDevice {
>      const QVirtioBus *bus;
> +    QTestState *qts;
>      /* Device type */
>      uint16_t device_type;
>  } QVirtioDevice;
> @@ -35,12 +36,14 @@ typedef struct QVirtQueue {
>      uint16_t last_used_idx;
>      bool indirect;
>      bool event;
> +    QTestState *qts;
>  } QVirtQueue;
> 
>  typedef struct QVRingIndirectDesc {
>      uint64_t desc; /* This points to an array fo struct vring_desc */
>      uint16_t index;
>      uint16_t elem;
> +    QTestState *qts;
>  } QVRingIndirectDesc;

Adding QTestState to QVRingIndirectDesc and QVirtQueue sounds somewhat
ugly to me. I think they should either rather have a pointer to the
associated QVirtioDevice, or the functions where this is needed
(qvring_init() for example) should get a "QTestState *" parameter instead.

Just my 0.02 €.

 Thomas