The previous, already existing readdir test simply used a 'count'
parameter big enough to retrieve all directory entries with a
single Treaddir request.

In this new 'splitted' readdir test, directory entries are
retrieved, splitted over several Treaddir requests by picking small
'count' parameters which force the server to truncate the response.
So the test client sends as many Treaddir requests as necessary to
get all directory entries. Currently this test covers actually two
tests: a sequence of Treaddir requests with count=512 and then a
subsequent test with a sequence of Treaddir requests with count=256.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/virtio-9p-test.c | 91 ++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 2167322985..8b0d94546e 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -578,6 +578,7 @@ static bool fs_dirents_contain_name(struct V9fsDirent *e, const char* name)
     return false;
 }
 
+/* basic readdir test where reply fits into a single response message */
 static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     QVirtio9P *v9p = obj;
@@ -631,6 +632,95 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
     g_free(wnames[0]);
 }
 
+/* readdir test where overall request is splitted over several messages */
+static void fs_readdir_split(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+    QVirtio9P *v9p = obj;
+    alloc = t_alloc;
+    char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_READDIR_DIR) };
+    uint16_t nqid;
+    v9fs_qid qid;
+    uint32_t count, nentries, npartialentries;
+    struct V9fsDirent *entries, *tail, *partialentries;
+    P9Req *req;
+    int subtest;
+    int fid;
+    uint64_t offset;
+    /* the Treaddir 'count' parameter values to be tested */
+    const uint32_t vcount[] = { 512, 256 };
+    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) {
+        fid = subtest + 1;
+        offset = 0;
+        entries = NULL;
+        nentries = 0;
+        tail = NULL;
+
+        req = v9fs_twalk(v9p, 0, fid, 1, wnames, 0);
+        v9fs_req_wait_for_reply(req, NULL);
+        v9fs_rwalk(req, &nqid, NULL);
+        g_assert_cmpint(nqid, ==, 1);
+
+        req = v9fs_tlopen(v9p, fid, O_DIRECTORY, 0);
+        v9fs_req_wait_for_reply(req, NULL);
+        v9fs_rlopen(req, &qid, NULL);
+
+        /*
+         * send as many Treaddir requests as required to get all directory
+         * entries
+         */
+        while (true) {
+            npartialentries = 0;
+            partialentries = NULL;
+
+            req = v9fs_treaddir(v9p, fid, offset, vcount[subtest], 0);
+            v9fs_req_wait_for_reply(req, NULL);
+            v9fs_rreaddir(req, &count, &npartialentries, &partialentries);
+            if (npartialentries > 0 && partialentries) {
+                if (!entries) {
+                    entries = partialentries;
+                    nentries = npartialentries;
+                    tail = partialentries;
+                } else {
+                    tail->next = partialentries;
+                    nentries += npartialentries;
+                }
+                while (tail->next) {
+                    tail = tail->next;
+                }
+                offset = tail->offset;
+            } else {
+                break;
+            }
+        }
+
+        g_assert_cmpint(
+            nentries, ==,
+            QTEST_V9FS_SYNTH_READDIR_NFILES + 2 /* "." and ".." */
+        );
+
+        /*
+         * Check all file names exist in returned entries, ignore their order
+         * though.
+         */
+        g_assert_cmpint(fs_dirents_contain_name(entries, "."), ==, true);
+        g_assert_cmpint(fs_dirents_contain_name(entries, ".."), ==, true);
+        for (int i = 0; i < QTEST_V9FS_SYNTH_READDIR_NFILES; ++i) {
+            char *name = g_strdup_printf(QTEST_V9FS_SYNTH_READDIR_FILE, i);
+            g_assert_cmpint(fs_dirents_contain_name(entries, name), ==, true);
+            g_free(name);
+        }
+
+        v9fs_free_dirents(entries);
+    }
+
+    g_free(wnames[0]);
+}
+
 static void fs_walk_no_slash(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     QVirtio9P *v9p = obj;
@@ -810,6 +900,7 @@ static void register_virtio_9p_test(void)
     qos_add_test("fs/flush/ignored", "virtio-9p", fs_flush_ignored,
                  NULL);
     qos_add_test("fs/readdir/basic", "virtio-9p", fs_readdir, NULL);
+    qos_add_test("fs/readdir/split", "virtio-9p", fs_readdir_split, NULL);
 }
 
 libqos_init(register_virtio_9p_test);
-- 
2.20.1

On Tue, 21 Jan 2020 01:16:21 +0100
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> The previous, already existing readdir test simply used a 'count'
> parameter big enough to retrieve all directory entries with a
> single Treaddir request.
> 
> In this new 'splitted' readdir test, directory entries are
> retrieved, splitted over several Treaddir requests by picking small
> 'count' parameters which force the server to truncate the response.
> So the test client sends as many Treaddir requests as necessary to
> get all directory entries. Currently this test covers actually two
> tests: a sequence of Treaddir requests with count=512 and then a
> subsequent test with a sequence of Treaddir requests with count=256.
> 
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---

Not sure it is really needed to check with multiple values for 'count',
but it doesn't eat too many cycles so I guess it doesn't hurt.

Applied as well.

>  tests/qtest/virtio-9p-test.c | 91 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 91 insertions(+)
> 
> diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> index 2167322985..8b0d94546e 100644
> --- a/tests/qtest/virtio-9p-test.c
> +++ b/tests/qtest/virtio-9p-test.c
> @@ -578,6 +578,7 @@ static bool fs_dirents_contain_name(struct V9fsDirent *e, const char* name)
>      return false;
>  }
>  
> +/* basic readdir test where reply fits into a single response message */
>  static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
>  {
>      QVirtio9P *v9p = obj;
> @@ -631,6 +632,95 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
>      g_free(wnames[0]);
>  }
>  
> +/* readdir test where overall request is splitted over several messages */
> +static void fs_readdir_split(void *obj, void *data, QGuestAllocator *t_alloc)
> +{
> +    QVirtio9P *v9p = obj;
> +    alloc = t_alloc;
> +    char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_READDIR_DIR) };
> +    uint16_t nqid;
> +    v9fs_qid qid;
> +    uint32_t count, nentries, npartialentries;
> +    struct V9fsDirent *entries, *tail, *partialentries;
> +    P9Req *req;
> +    int subtest;
> +    int fid;
> +    uint64_t offset;
> +    /* the Treaddir 'count' parameter values to be tested */
> +    const uint32_t vcount[] = { 512, 256 };
> +    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) {
> +        fid = subtest + 1;
> +        offset = 0;
> +        entries = NULL;
> +        nentries = 0;
> +        tail = NULL;
> +
> +        req = v9fs_twalk(v9p, 0, fid, 1, wnames, 0);
> +        v9fs_req_wait_for_reply(req, NULL);
> +        v9fs_rwalk(req, &nqid, NULL);
> +        g_assert_cmpint(nqid, ==, 1);
> +
> +        req = v9fs_tlopen(v9p, fid, O_DIRECTORY, 0);
> +        v9fs_req_wait_for_reply(req, NULL);
> +        v9fs_rlopen(req, &qid, NULL);
> +
> +        /*
> +         * send as many Treaddir requests as required to get all directory
> +         * entries
> +         */
> +        while (true) {
> +            npartialentries = 0;
> +            partialentries = NULL;
> +
> +            req = v9fs_treaddir(v9p, fid, offset, vcount[subtest], 0);
> +            v9fs_req_wait_for_reply(req, NULL);
> +            v9fs_rreaddir(req, &count, &npartialentries, &partialentries);
> +            if (npartialentries > 0 && partialentries) {
> +                if (!entries) {
> +                    entries = partialentries;
> +                    nentries = npartialentries;
> +                    tail = partialentries;
> +                } else {
> +                    tail->next = partialentries;
> +                    nentries += npartialentries;
> +                }
> +                while (tail->next) {
> +                    tail = tail->next;
> +                }
> +                offset = tail->offset;
> +            } else {
> +                break;
> +            }
> +        }
> +
> +        g_assert_cmpint(
> +            nentries, ==,
> +            QTEST_V9FS_SYNTH_READDIR_NFILES + 2 /* "." and ".." */
> +        );
> +
> +        /*
> +         * Check all file names exist in returned entries, ignore their order
> +         * though.
> +         */
> +        g_assert_cmpint(fs_dirents_contain_name(entries, "."), ==, true);
> +        g_assert_cmpint(fs_dirents_contain_name(entries, ".."), ==, true);
> +        for (int i = 0; i < QTEST_V9FS_SYNTH_READDIR_NFILES; ++i) {
> +            char *name = g_strdup_printf(QTEST_V9FS_SYNTH_READDIR_FILE, i);
> +            g_assert_cmpint(fs_dirents_contain_name(entries, name), ==, true);
> +            g_free(name);
> +        }
> +
> +        v9fs_free_dirents(entries);
> +    }
> +
> +    g_free(wnames[0]);
> +}
> +
>  static void fs_walk_no_slash(void *obj, void *data, QGuestAllocator *t_alloc)
>  {
>      QVirtio9P *v9p = obj;
> @@ -810,6 +900,7 @@ static void register_virtio_9p_test(void)
>      qos_add_test("fs/flush/ignored", "virtio-9p", fs_flush_ignored,
>                   NULL);
>      qos_add_test("fs/readdir/basic", "virtio-9p", fs_readdir, NULL);
> +    qos_add_test("fs/readdir/split", "virtio-9p", fs_readdir_split, NULL);
>  }
>  
>  libqos_init(register_virtio_9p_test);

On Mittwoch, 22. Januar 2020 22:19:05 CET Greg Kurz wrote:
> On Tue, 21 Jan 2020 01:16:21 +0100
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > The previous, already existing readdir test simply used a 'count'
> > parameter big enough to retrieve all directory entries with a
> > single Treaddir request.
> > 
> > In this new 'splitted' readdir test, directory entries are
> > retrieved, splitted over several Treaddir requests by picking small
> > 'count' parameters which force the server to truncate the response.
> > So the test client sends as many Treaddir requests as necessary to
> > get all directory entries. Currently this test covers actually two
> > tests: a sequence of Treaddir requests with count=512 and then a
> > subsequent test with a sequence of Treaddir requests with count=256.
> > 
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > ---
> 
> Not sure it is really needed to check with multiple values for 'count',
> but it doesn't eat too many cycles so I guess it doesn't hurt.

Yes, it is a cheap test, nobody will feel the difference. One could argue 
about the precise 'count' values being used ...

> 
> Applied as well.
> 
> >  tests/qtest/virtio-9p-test.c | 91 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 91 insertions(+)
> > 
> > diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> > index 2167322985..8b0d94546e 100644
> > --- a/tests/qtest/virtio-9p-test.c
> > +++ b/tests/qtest/virtio-9p-test.c
> > @@ -578,6 +578,7 @@ static bool fs_dirents_contain_name(struct V9fsDirent
> > *e, const char* name)> 
> >      return false;
> >  
> >  }
> > 
> > +/* basic readdir test where reply fits into a single response message */
> > 
> >  static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
> >  {
> >  
> >      QVirtio9P *v9p = obj;
> > 
> > @@ -631,6 +632,95 @@ static void fs_readdir(void *obj, void *data,
> > QGuestAllocator *t_alloc)> 
> >      g_free(wnames[0]);
> >  
> >  }
> > 
> > +/* readdir test where overall request is splitted over several messages
> > */
> > +static void fs_readdir_split(void *obj, void *data, QGuestAllocator
> > *t_alloc) +{
> > +    QVirtio9P *v9p = obj;
> > +    alloc = t_alloc;
> > +    char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_READDIR_DIR) };
> > +    uint16_t nqid;
> > +    v9fs_qid qid;
> > +    uint32_t count, nentries, npartialentries;
> > +    struct V9fsDirent *entries, *tail, *partialentries;
> > +    P9Req *req;
> > +    int subtest;
> > +    int fid;
> > +    uint64_t offset;
> > +    /* the Treaddir 'count' parameter values to be tested */
> > +    const uint32_t vcount[] = { 512, 256 };

... here. But IMO it does make sense preserving the function's overall 
structure to allow testing with different 'count' values if necessary. Because 
that way this test could e.g. guard potential bugs when server's Treaddir 
handler is rolling back (or not) the dir offset for instance, which server has 
to do (or not) according to this 'count' value and the precise file name 
length of the individual directory entries.

Whatever precise 'count' tests are desired, it would only mean a one line 
change here.

Best regards,
Christian Schoenebeck

On Wed, 22 Jan 2020 23:36:22 +0100
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Mittwoch, 22. Januar 2020 22:19:05 CET Greg Kurz wrote:
> > On Tue, 21 Jan 2020 01:16:21 +0100
> > 
> > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > The previous, already existing readdir test simply used a 'count'
> > > parameter big enough to retrieve all directory entries with a
> > > single Treaddir request.
> > > 
> > > In this new 'splitted' readdir test, directory entries are
> > > retrieved, splitted over several Treaddir requests by picking small
> > > 'count' parameters which force the server to truncate the response.
> > > So the test client sends as many Treaddir requests as necessary to
> > > get all directory entries. Currently this test covers actually two
> > > tests: a sequence of Treaddir requests with count=512 and then a
> > > subsequent test with a sequence of Treaddir requests with count=256.
> > > 
> > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > ---
> > 
> > Not sure it is really needed to check with multiple values for 'count',
> > but it doesn't eat too many cycles so I guess it doesn't hurt.
> 
> Yes, it is a cheap test, nobody will feel the difference. One could argue 
> about the precise 'count' values being used ...
> 
> > 
> > Applied as well.
> > 
> > >  tests/qtest/virtio-9p-test.c | 91 ++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 91 insertions(+)
> > > 
> > > diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> > > index 2167322985..8b0d94546e 100644
> > > --- a/tests/qtest/virtio-9p-test.c
> > > +++ b/tests/qtest/virtio-9p-test.c
> > > @@ -578,6 +578,7 @@ static bool fs_dirents_contain_name(struct V9fsDirent
> > > *e, const char* name)> 
> > >      return false;
> > >  
> > >  }
> > > 
> > > +/* basic readdir test where reply fits into a single response message */
> > > 
> > >  static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
> > >  {
> > >  
> > >      QVirtio9P *v9p = obj;
> > > 
> > > @@ -631,6 +632,95 @@ static void fs_readdir(void *obj, void *data,
> > > QGuestAllocator *t_alloc)> 
> > >      g_free(wnames[0]);
> > >  
> > >  }
> > > 
> > > +/* readdir test where overall request is splitted over several messages
> > > */
> > > +static void fs_readdir_split(void *obj, void *data, QGuestAllocator
> > > *t_alloc) +{
> > > +    QVirtio9P *v9p = obj;
> > > +    alloc = t_alloc;
> > > +    char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_READDIR_DIR) };
> > > +    uint16_t nqid;
> > > +    v9fs_qid qid;
> > > +    uint32_t count, nentries, npartialentries;
> > > +    struct V9fsDirent *entries, *tail, *partialentries;
> > > +    P9Req *req;
> > > +    int subtest;
> > > +    int fid;
> > > +    uint64_t offset;
> > > +    /* the Treaddir 'count' parameter values to be tested */
> > > +    const uint32_t vcount[] = { 512, 256 };
> 
> ... here. But IMO it does make sense preserving the function's overall 
> structure to allow testing with different 'count' values if necessary. Because 
> that way this test could e.g. guard potential bugs when server's Treaddir 
> handler is rolling back (or not) the dir offset for instance, which server has 
> to do (or not) according to this 'count' value and the precise file name 
> length of the individual directory entries.
> 

I still agree it is useful to be able to check different values but I
now realize that it shouldn't be done like this because adding new
values to vcount[] doesn't scale well with the MAX_REQ limitation I
mentioned in another mail. More values, especially small ones, are
likely to trigger "Failed to decode VirtFS request type 40" at some
point. 

I now think that fs_readdir_split() should rather get count as
an argument and only do one run. By default we would only call
this with an appropriate value, ideally derived from the test
environment (number of files, size of filenames... etc...).
If someone needs to test a specific value, it is easy as adding
a new qos_add_test() line.

This would ensure at least that each run starts with the same
fixed number of possible requests, ie. MAX_REQ minus the cost of
fs_attach().

So I'll revert this patch for now.

> Whatever precise 'count' tests are desired, it would only mean a one line 
> change here.
> 
> Best regards,
> Christian Schoenebeck
> 
> 

On Donnerstag, 23. Januar 2020 11:30:58 CET Greg Kurz wrote:
> On Wed, 22 Jan 2020 23:36:22 +0100
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > On Mittwoch, 22. Januar 2020 22:19:05 CET Greg Kurz wrote:
> > > On Tue, 21 Jan 2020 01:16:21 +0100
> > > 
> > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > > The previous, already existing readdir test simply used a 'count'
> > > > parameter big enough to retrieve all directory entries with a
> > > > single Treaddir request.
> > > > 
> > > > In this new 'splitted' readdir test, directory entries are
> > > > retrieved, splitted over several Treaddir requests by picking small
> > > > 'count' parameters which force the server to truncate the response.
> > > > So the test client sends as many Treaddir requests as necessary to
> > > > get all directory entries. Currently this test covers actually two
> > > > tests: a sequence of Treaddir requests with count=512 and then a
> > > > subsequent test with a sequence of Treaddir requests with count=256.
> > > > 
> > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > > ---
> > > 
> > > Not sure it is really needed to check with multiple values for 'count',
> > > but it doesn't eat too many cycles so I guess it doesn't hurt.
> > 
> > Yes, it is a cheap test, nobody will feel the difference. One could argue
> > about the precise 'count' values being used ...
> > 
> > > Applied as well.
> > > 
> > > >  tests/qtest/virtio-9p-test.c | 91
> > > >  ++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 91 insertions(+)
> > > > 
> > > > diff --git a/tests/qtest/virtio-9p-test.c
> > > > b/tests/qtest/virtio-9p-test.c
> > > > index 2167322985..8b0d94546e 100644
> > > > --- a/tests/qtest/virtio-9p-test.c
> > > > +++ b/tests/qtest/virtio-9p-test.c
> > > > @@ -578,6 +578,7 @@ static bool fs_dirents_contain_name(struct
> > > > V9fsDirent
> > > > *e, const char* name)>
> > > > 
> > > >      return false;
> > > >  
> > > >  }
> > > > 
> > > > +/* basic readdir test where reply fits into a single response message
> > > > */
> > > > 
> > > >  static void fs_readdir(void *obj, void *data, QGuestAllocator
> > > >  *t_alloc)
> > > >  {
> > > >  
> > > >      QVirtio9P *v9p = obj;
> > > > 
> > > > @@ -631,6 +632,95 @@ static void fs_readdir(void *obj, void *data,
> > > > QGuestAllocator *t_alloc)>
> > > > 
> > > >      g_free(wnames[0]);
> > > >  
> > > >  }
> > > > 
> > > > +/* readdir test where overall request is splitted over several
> > > > messages
> > > > */
> > > > +static void fs_readdir_split(void *obj, void *data, QGuestAllocator
> > > > *t_alloc) +{
> > > > +    QVirtio9P *v9p = obj;
> > > > +    alloc = t_alloc;
> > > > +    char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_READDIR_DIR)
> > > > };
> > > > +    uint16_t nqid;
> > > > +    v9fs_qid qid;
> > > > +    uint32_t count, nentries, npartialentries;
> > > > +    struct V9fsDirent *entries, *tail, *partialentries;
> > > > +    P9Req *req;
> > > > +    int subtest;
> > > > +    int fid;
> > > > +    uint64_t offset;
> > > > +    /* the Treaddir 'count' parameter values to be tested */
> > > > +    const uint32_t vcount[] = { 512, 256 };
> > 
> > ... here. But IMO it does make sense preserving the function's overall
> > structure to allow testing with different 'count' values if necessary.
> > Because that way this test could e.g. guard potential bugs when server's
> > Treaddir handler is rolling back (or not) the dir offset for instance,
> > which server has to do (or not) according to this 'count' value and the
> > precise file name length of the individual directory entries.
> 
> I still agree it is useful to be able to check different values but I
> now realize that it shouldn't be done like this because adding new
> values to vcount[] doesn't scale well with the MAX_REQ limitation I
> mentioned in another mail. More values, especially small ones, are
> likely to trigger "Failed to decode VirtFS request type 40" at some
> point.
> 
> I now think that fs_readdir_split() should rather get count as
> an argument and only do one run. By default we would only call
> this with an appropriate value, ideally derived from the test
> environment (number of files, size of filenames... etc...).
> If someone needs to test a specific value, it is easy as adding
> a new qos_add_test() line.

I actually had this variant in the exact same way as you're suggesting here as 
well before (not submitted to the list though), that is I had a version of 
this function with a count argument, and I had 3 separate qtest cases, but 
that variant failed in the exact same way.

Furthermore, I even get some strange "could not push stack" error messages 
from the qtest environment when I just add some empty noop tests to the 9pfs 
test suite. I am not sure if that's related to this ringbuffer issue here, but 
it was a show stopper for this 'separate test for each count' variant, so I 
reverted it to this suggested array solution for now.

So to avoid any misapprehension: it seems to me as if you were thinking that 
the ringbuffer is freed between every individual test. That's not what I am 
observing here. It rather seems as space in the ringbuffer is never freed 
(before the entire 9pfs test suite completed).

> This would ensure at least that each run starts with the same
> fixed number of possible requests, ie. MAX_REQ minus the cost of
> fs_attach().
> 
> So I'll revert this patch for now.
> 
> > Whatever precise 'count' tests are desired, it would only mean a one line
> > change here.
> > 
> > Best regards,
> > Christian Schoenebeck


Best regards,
Christian Schoenebeck

On 1/20/20 6:16 PM, Christian Schoenebeck wrote:
> The previous, already existing readdir test simply used a 'count'
> parameter big enough to retrieve all directory entries with a
> single Treaddir request.
> 
> In this new 'splitted' readdir test, directory entries are

English is weird; the past tense of 'split' is 'split', not 'splitted'

> retrieved, splitted over several Treaddir requests by picking small

and again

> 'count' parameters which force the server to truncate the response.
> So the test client sends as many Treaddir requests as necessary to
> get all directory entries. Currently this test covers actually two
> tests: a sequence of Treaddir requests with count=512 and then a
> subsequent test with a sequence of Treaddir requests with count=256.
> 
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---
>   tests/qtest/virtio-9p-test.c | 91 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 91 insertions(+)
> 

>   
> +/* readdir test where overall request is splitted over several messages */

and again

> +static void fs_readdir_split(void *obj, void *data, QGuestAllocator *t_alloc)
> +{
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

On Mittwoch, 22. Januar 2020 22:14:39 CET Eric Blake wrote:
> On 1/20/20 6:16 PM, Christian Schoenebeck wrote:
> > The previous, already existing readdir test simply used a 'count'
> > parameter big enough to retrieve all directory entries with a
> > single Treaddir request.
> > 
> > In this new 'splitted' readdir test, directory entries are
> 
> English is weird; the past tense of 'split' is 'split', not 'splitted'

Just an attempt to fix a language defect. You're right of course Eric.

Greg, should I resend that patch fixed or can you just awk it? Whatever is 
more comfortable to you.

Best regards,
Christian Schoenebeck

On Wed, 22 Jan 2020 22:29:12 +0100
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Mittwoch, 22. Januar 2020 22:14:39 CET Eric Blake wrote:
> > On 1/20/20 6:16 PM, Christian Schoenebeck wrote:
> > > The previous, already existing readdir test simply used a 'count'
> > > parameter big enough to retrieve all directory entries with a
> > > single Treaddir request.
> > > 
> > > In this new 'splitted' readdir test, directory entries are
> > 
> > English is weird; the past tense of 'split' is 'split', not 'splitted'
> 

Heh.. My irregular verb detector module is a bit tired. I should
replace it ;-)

> Just an attempt to fix a language defect. You're right of course Eric.
> 
> Greg, should I resend that patch fixed or can you just awk it? Whatever is 
> more comfortable to you.
> 

Yeah, I'll fix this in my tree.

> Best regards,
> Christian Schoenebeck
> 
>