On 8/13/19 4:30 AM, Thomas Huth wrote: > The libqos library functions should never depend on global_qtest, > since these functions might be used in tests that track multiple > test states. Pass around a pointer to the QTestState instead.
Thanks for picking up on my initial RFC along these lines a couple years
ago now; it's taking us a while, but these cleanups are worthwhile.
>
> Signed-off-by: Thomas Huth <th...@redhat.com>
> ---
> @@ -193,7 +193,7 @@ void qvring_init(QTestState *qts, const QGuestAllocator
> *alloc, QVirtQueue *vq,
> 0);
> }
>
> -QVRingIndirectDesc *qvring_indirect_desc_setup(QVirtioDevice *d,
> +QVRingIndirectDesc *qvring_indirect_desc_setup(QTestState *qs, QVirtioDevice
> *d,
> QGuestAllocator *alloc, uint16_t
> elem)
> {
Pre-existing, but indentation is off; maybe you want to fix that while here.
> -void qvirtqueue_kick(QVirtioDevice *d, QVirtQueue *vq, uint32_t free_head)
> +void qvirtqueue_kick(QTestState *qts, QVirtioDevice *d, QVirtQueue *vq,
> + uint32_t free_head)
> {
> /* vq->avail->idx */
> - uint16_t idx = readw(vq->avail + 2);
> + uint16_t idx = qtest_readw(qts, vq->avail + 2);
> /* vq->used->flags */
> uint16_t flags;
> /* vq->used->avail_event */
> uint16_t avail_event;
>
> /* vq->avail->ring[idx % vq->size] */
> - writew(vq->avail + 4 + (2 * (idx % vq->size)), free_head);
> + qtest_writew(qts, vq->avail + 4 + (2 * (idx % vq->size)), free_head);
> /* vq->avail->idx */
> - writew(vq->avail + 2, idx + 1);
> + qtest_writew(qts, vq->avail + 2, idx + 1);
>
> /* Must read after idx is updated */
> - flags = readw(vq->avail);
> - avail_event = readw(vq->used + 4 +
> + flags = qtest_readw(qts, vq->avail);
> + avail_event = qtest_readw(qts, vq->used + 4 +
> sizeof(struct vring_used_elem) * vq->size);
Here as well.
> @@ -333,23 +337,25 @@ bool qvirtqueue_get_buf(QVirtQueue *vq, uint32_t
> *desc_idx, uint32_t *len)
> sizeof(struct vring_used_elem);
>
> if (desc_idx) {
> - *desc_idx = readl(elem_addr + offsetof(struct vring_used_elem, id));
> + *desc_idx = qtest_readl(qts, elem_addr +
> + offsetof(struct vring_used_elem, id));
This one works as-is, although it gives me a double-take when the
indentation is not right after a ( in the line above. I probably would
have written either:
*desc_idx = qtest_readl(qts,
elem_addr + offsetof(struct
vring_used_elem, id));
or
*desc_idx = qtest_readl(qts, (elem_addr +
offsetof(struct vring_used_elem,
id)));
or even introduce a temp variable to bring down the line length.
> @@ -131,17 +131,20 @@ void qvirtqueue_cleanup(const QVirtioBus *bus,
> QVirtQueue *vq,
>
> void qvring_init(QTestState *qts, const QGuestAllocator *alloc, QVirtQueue
> *vq,
> uint64_t addr);
> -QVRingIndirectDesc *qvring_indirect_desc_setup(QVirtioDevice *d,
> +QVRingIndirectDesc *qvring_indirect_desc_setup(QTestState *qs, QVirtioDevice
> *d,
> QGuestAllocator *alloc, uint16_t
> elem);
Another pre-existing odd indentation (here, it looks like line length
was the reason).
Indentation is minor, so:
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
