Re: [PATCH 1/6] libqos/virtio.c: init all elems in qvring_indirect_desc_setup()

[Thomas Huth <thuth@redhat.com>]

On 13/02/2024 20.17, Daniel Henrique Barboza wrote:
> The loop isn't setting the values for the last element. Every other
> element is being initialized with addr = 0, flags = VRING_DESC_F_NEXT
> and next = i + 1. The last elem is never touched.
> 
> This became a problem when enabling a RISC-V 'virt' libqos machine in
> the 'indirect' test of virti-blk-test.c. The 'flags' for the last
> element will end up being an odd number (since we didn't touch it).
> Being an odd number it will be mistaken by VRING_DESC_F_NEXT, which
> happens to be 1.
> 
> Deep into hw/virt/virtio.c, in virtqueue_split_pop(), into
> virtqueue_split_read_next_desc(), a check for VRING_DESC_F_NEXT will be
> made to see if we're supposed to chain. The code will keep up chaining
> in the last element because the unintialized value happens to be odd.

s/unintialized/uninitialized/

> We'll error out right after that because desc->next (which is also
> uninitialized) will be >= max. A VIRTQUEUE_READ_DESC_ERROR will be
> returned, with an error message like this in the stderr:
> 
> qemu-system-riscv64: Desc next is 49391
> 
> Since we never returned, w'll end up timing out at qvirtio_wait_used_elem():

s/w'll/we'll/

> ERROR:../tests/qtest/libqos/virtio.c:236:qvirtio_wait_used_elem:
>      assertion failed: (g_get_monotonic_time() - start_time <= timeout_us)
> 
> The root cause is using unintialized values from guest_alloc() in

s/unintialized/uninitialized/

With the typos fixed:
Reviewed-by: Thomas Huth <thuth@redhat.com>