On Wed, 29 Jul 2020 10:10:23 +0200
Christian Schoenebeck <qemu_...@crudebyte.com> wrote:
> The previous, already existing 'basic' readdir test simply used a
> 'count' parameter big enough to retrieve all directory entries with a
> single Treaddir request.
>
> In the 3 new 'split' readdir tests added by this patch, directory
> entries are retrieved, split 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.
>
> The following 3 new tests are added (executed in this sequence):
>
> 1. Split readdir test with count=512
> 2. Split readdir test with count=256
> 3. Split readdir test with count=128
>
> This test case sequence is chosen because the smaller the 'count' value,
> the higher the chance of errors in case of implementation bugs on server
> side.
>
> Signed-off-by: Christian Schoenebeck <qemu_...@crudebyte.com>
> ---
The existing fs_readdir() function for the 'basic' test is a subset
of the new fs_readdir_split() introduced by this patch (quite visible
if you sdiff the code).
To avoid code duplication, I would have probably tried to do the changes
in fs_readdir() and implement the 'basic' test as:
static void fs_readdir_basic(void *obj, void *data,
QGuestAllocator *t_alloc)
{
/*
* submit count = msize - 11, because 11 is the header size of Rreaddir
*/
fs_readdir(obj, data, t_alloc, P9_MAX_SIZE - 11);
}
but anyway this looks good to me so:
Reviewed-by: Greg Kurz <gr...@kaod.org>
> tests/qtest/virtio-9p-test.c | 108 +++++++++++++++++++++++++++++++++++
> 1 file changed, 108 insertions(+)
>
> diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> index 2167322985..de30b717b6 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,89 @@ static void fs_readdir(void *obj, void *data,
> QGuestAllocator *t_alloc)
> g_free(wnames[0]);
> }
>
> +/* readdir test where overall request is split over several messages */
> +static void fs_readdir_split(void *obj, void *data, QGuestAllocator *t_alloc,
> + uint32_t count)
> +{
> + QVirtio9P *v9p = obj;
> + alloc = t_alloc;
> + char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_READDIR_DIR) };
> + uint16_t nqid;
> + v9fs_qid qid;
> + uint32_t nentries, npartialentries;
> + struct V9fsDirent *entries, *tail, *partialentries;
> + P9Req *req;
> + int fid;
> + uint64_t offset;
> +
> + fs_attach(v9p, NULL, t_alloc);
> +
> + fid = 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, count, 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;
> @@ -793,6 +877,24 @@ static void fs_flush_ignored(void *obj, void *data,
> QGuestAllocator *t_alloc)
> g_free(wnames[0]);
> }
>
> +static void fs_readdir_split_128(void *obj, void *data,
> + QGuestAllocator *t_alloc)
> +{
> + fs_readdir_split(obj, data, t_alloc, 128);
> +}
> +
> +static void fs_readdir_split_256(void *obj, void *data,
> + QGuestAllocator *t_alloc)
> +{
> + fs_readdir_split(obj, data, t_alloc, 256);
> +}
> +
> +static void fs_readdir_split_512(void *obj, void *data,
> + QGuestAllocator *t_alloc)
> +{
> + fs_readdir_split(obj, data, t_alloc, 512);
> +}
> +
> static void register_virtio_9p_test(void)
> {
> qos_add_test("config", "virtio-9p", pci_config, NULL);
> @@ -810,6 +912,12 @@ 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_512", "virtio-9p",
> + fs_readdir_split_512, NULL);
> + qos_add_test("fs/readdir/split_256", "virtio-9p",
> + fs_readdir_split_256, NULL);
> + qos_add_test("fs/readdir/split_128", "virtio-9p",
> + fs_readdir_split_128, NULL);
> }
>
> libqos_init(register_virtio_9p_test);
