On Tue, 21 Jan 2020 01:17:35 +0100
Christian Schoenebeck <qemu_...@crudebyte.com> wrote:
> This patch is not intended to be merged. It resembles
> an issue (with debug messages) where the splitted
> readdir test fails because server is interrupted with
> transport error "Failed to decode VirtFS request type 40",
Ok. When we send a new request, we call:
uint32_t qvirtqueue_add(QTestState *qts, QVirtQueue *vq, uint64_t data,
uint32_t len, bool write, bool next)
{
uint16_t flags = 0;
vq->num_free--;
[...]
return vq->free_head++; /* Return and increase, in this order */
}
where vq->num_free is the number of available buffers (aka. requests) in
the vq and vq->free_head the index of the next available buffer. The size
of the vq of the virtio-9p device is MAX_REQ (128) buffers. The driver
is very simple and doesn't care to handle the scenario of a full vq,
ie, num_free == 0 and free_head is past the vq->desc[] array. It seems
that count=128 generates enough extra requests to reach the end of the
vq. Hence the "decode" error you get. Maybe an assert(vq->num_free) in
qvirtqueue_add() would make that more clear ?
Not sure it is worth to address this limitation though. Especially since
count=128 isn't really a recommended choice in the first place. It has
more chances to cause a disconnect if the server needs to return a longer
file name (which is expected since most fs have 255 character long file
names).
> which BTW fails both with the unoptimized and with the
> optimized 9p readdir code.
>
Yes, this is the client's fault.
> Signed-off-by: Christian Schoenebeck <qemu_...@crudebyte.com>
> ---
> tests/qtest/virtio-9p-test.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> index 8b0d94546e..e47b286340 100644
> --- a/tests/qtest/virtio-9p-test.c
> +++ b/tests/qtest/virtio-9p-test.c
> @@ -647,13 +647,14 @@ static void fs_readdir_split(void *obj, void *data,
> QGuestAllocator *t_alloc)
> int fid;
> uint64_t offset;
> /* the Treaddir 'count' parameter values to be tested */
> - const uint32_t vcount[] = { 512, 256 };
> + const uint32_t vcount[] = { 512, 256, 128 };
> const int nvcount = sizeof(vcount) / sizeof(uint32_t);
>
> fs_attach(v9p, NULL, t_alloc);
>
> /* iterate over all 'count' parameter values to be tested with Treaddir
> */
> for (subtest = 0; subtest < nvcount; ++subtest) {
> + printf("\nsubtest[%d] with count=%d\n", subtest, vcount[subtest]);
> fid = subtest + 1;
> offset = 0;
> entries = NULL;
> @@ -674,12 +675,16 @@ static void fs_readdir_split(void *obj, void *data,
> QGuestAllocator *t_alloc)
> * entries
> */
> while (true) {
> + printf("\toffset=%ld\n", offset);
> npartialentries = 0;
> partialentries = NULL;
>
> + printf("Treaddir fid=%d offset=%ld count=%d\n",
> + fid, offset, vcount[subtest]);
> req = v9fs_treaddir(v9p, fid, offset, vcount[subtest], 0);
> v9fs_req_wait_for_reply(req, NULL);
> v9fs_rreaddir(req, &count, &npartialentries, &partialentries);
> + printf("\t\tnpartial=%d nentries=%d\n", npartialentries,
> nentries);
> if (npartialentries > 0 && partialentries) {
> if (!entries) {
> entries = partialentries;
> @@ -716,6 +721,8 @@ static void fs_readdir_split(void *obj, void *data,
> QGuestAllocator *t_alloc)
> }
>
> v9fs_free_dirents(entries);
> +
> + printf("PASSED subtest[%d]\n", subtest);
> }
>
> g_free(wnames[0]);
