[PATCH 01/20] tests/9p: merge *walk*() functions

[PATCH 01/20] tests/9p: merge *walk*() functions

[Christian Schoenebeck]

Introduce declarative function calls.

There are currently 4 different functions for sending a 9p 'Twalk'
request: v9fs_twalk(), do_walk(), do_walk_rqids() and
do_walk_expect_error(). They are all doing the same thing, just in a
slightly different way and with slightly different function arguments.

Merge those 4 functions into a single function by using a struct for
function call arguments and use designated initializers when calling
this function to turn usage into a declarative approach, which is
better readable and easier to maintain.

Also move private functions genfid(), split() and split_free() from
virtio-9p-test.c to virtio-9p-client.c.

Based-on: <E1odrya-0004Fv-97@lizzy.crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio-9p-client.c | 114 ++++++++++++++--
 tests/qtest/libqos/virtio-9p-client.h |  37 ++++-
 tests/qtest/virtio-9p-test.c          | 187 +++++++++-----------------
 3 files changed, 198 insertions(+), 140 deletions(-)

diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c
index f5c35fd722..a95bbad9c8 100644
--- a/tests/qtest/libqos/virtio-9p-client.c
+++ b/tests/qtest/libqos/virtio-9p-client.c
@@ -23,6 +23,65 @@ void v9fs_set_allocator(QGuestAllocator *t_alloc)
     alloc = t_alloc;
 }
 
+/*
+ * Used to auto generate new fids. Start with arbitrary high value to avoid
+ * collision with hard coded fids in basic test code.
+ */
+static uint32_t fid_generator = 1000;
+
+static uint32_t genfid(void)
+{
+    return fid_generator++;
+}
+
+/**
+ * Splits the @a in string by @a delim into individual (non empty) strings
+ * and outputs them to @a out. The output array @a out is NULL terminated.
+ *
+ * Output array @a out must be freed by calling split_free().
+ *
+ * @returns number of individual elements in output array @a out (without the
+ *          final NULL terminating element)
+ */
+static int split(const char *in, const char *delim, char ***out)
+{
+    int n = 0, i = 0;
+    char *tmp, *p;
+
+    tmp = g_strdup(in);
+    for (p = strtok(tmp, delim); p != NULL; p = strtok(NULL, delim)) {
+        if (strlen(p) > 0) {
+            ++n;
+        }
+    }
+    g_free(tmp);
+
+    *out = g_new0(char *, n + 1); /* last element NULL delimiter */
+
+    tmp = g_strdup(in);
+    for (p = strtok(tmp, delim); p != NULL; p = strtok(NULL, delim)) {
+        if (strlen(p) > 0) {
+            (*out)[i++] = g_strdup(p);
+        }
+    }
+    g_free(tmp);
+
+    return n;
+}
+
+static void split_free(char ***out)
+{
+    int i;
+    if (!*out) {
+        return;
+    }
+    for (i = 0; (*out)[i]; ++i) {
+        g_free((*out)[i]);
+    }
+    g_free(*out);
+    *out = NULL;
+}
+
 void v9fs_memwrite(P9Req *req, const void *addr, size_t len)
 {
     qtest_memwrite(req->qts, req->t_msg + req->t_off, addr, len);
@@ -294,28 +353,61 @@ void v9fs_rattach(P9Req *req, v9fs_qid *qid)
 }
 
 /* size[4] Twalk tag[2] fid[4] newfid[4] nwname[2] nwname*(wname[s]) */
-P9Req *v9fs_twalk(QVirtio9P *v9p, uint32_t fid, uint32_t newfid,
-                  uint16_t nwname, char *const wnames[], uint16_t tag)
+TWalkRes v9fs_twalk(TWalkOpt opt)
 {
     P9Req *req;
     int i;
     uint32_t body_size = 4 + 4 + 2;
+    uint32_t err;
+    char **wnames = NULL;
 
-    for (i = 0; i < nwname; i++) {
-        uint16_t wname_size = v9fs_string_size(wnames[i]);
+    g_assert(opt.client);
+    /* expecting either high- or low-level path, both not both */
+    g_assert(!opt.path || !(opt.nwname || opt.wnames));
+    /* expecting either Rwalk or Rlerror, but obviously not both */
+    g_assert(!opt.expectErr || !(opt.rwalk.nwqid || opt.rwalk.wqid));
+
+    if (!opt.newfid) {
+        opt.newfid = genfid();
+    }
+
+    if (opt.path) {
+        opt.nwname = split(opt.path, "/", &wnames);
+        opt.wnames = wnames;
+    }
+
+    for (i = 0; i < opt.nwname; i++) {
+        uint16_t wname_size = v9fs_string_size(opt.wnames[i]);
 
         g_assert_cmpint(body_size, <=, UINT32_MAX - wname_size);
         body_size += wname_size;
     }
-    req = v9fs_req_init(v9p,  body_size, P9_TWALK, tag);
-    v9fs_uint32_write(req, fid);
-    v9fs_uint32_write(req, newfid);
-    v9fs_uint16_write(req, nwname);
-    for (i = 0; i < nwname; i++) {
-        v9fs_string_write(req, wnames[i]);
+    req = v9fs_req_init(opt.client, body_size, P9_TWALK, opt.tag);
+    v9fs_uint32_write(req, opt.fid);
+    v9fs_uint32_write(req, opt.newfid);
+    v9fs_uint16_write(req, opt.nwname);
+    for (i = 0; i < opt.nwname; i++) {
+        v9fs_string_write(req, opt.wnames[i]);
     }
     v9fs_req_send(req);
-    return req;
+
+    if (!opt.requestOnly) {
+        v9fs_req_wait_for_reply(req, NULL);
+        if (opt.expectErr) {
+            v9fs_rlerror(req, &err);
+            g_assert_cmpint(err, ==, opt.expectErr);
+        } else {
+            v9fs_rwalk(req, opt.rwalk.nwqid, opt.rwalk.wqid);
+        }
+        req = NULL; /* request was freed */
+    }
+
+    split_free(&wnames);
+
+    return (TWalkRes) {
+        .newfid = opt.newfid,
+        .req = req,
+    };
 }
 
 /* size[4] Rwalk tag[2] nwqid[2] nwqid*(wqid[13]) */
diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h
index c502d12a66..8c6abbb173 100644
--- a/tests/qtest/libqos/virtio-9p-client.h
+++ b/tests/qtest/libqos/virtio-9p-client.h
@@ -72,6 +72,40 @@ struct V9fsDirent {
     struct V9fsDirent *next;
 };
 
+/* options for 'Twalk' 9p request */
+typedef struct TWalkOpt {
+    /* 9P client being used (mandatory) */
+    QVirtio9P *client;
+    /* user supplied tag number being returned with response (optional) */
+    uint16_t tag;
+    /* file ID of directory from where walk should start (optional) */
+    uint32_t fid;
+    /* file ID for target directory being walked to (optional) */
+    uint32_t newfid;
+    /* low level variant of path to walk to (optional) */
+    uint16_t nwname;
+    char **wnames;
+    /* high level variant of path to walk to (optional) */
+    const char *path;
+    /* data being received from 9p server as 'Rwalk' response (optional) */
+    struct {
+        uint16_t *nwqid;
+        v9fs_qid **wqid;
+    } rwalk;
+    /* only send Twalk request but not wait for a reply? (optional) */
+    bool requestOnly;
+    /* do we expect an Rlerror response, if yes which error code? (optional) */
+    uint32_t expectErr;
+} TWalkOpt;
+
+/* result of 'Twalk' 9p request */
+typedef struct TWalkRes {
+    /* file ID of target directory been walked to */
+    uint32_t newfid;
+    /* if requestOnly was set: request object for further processing */
+    P9Req *req;
+} TWalkRes;
+
 void v9fs_set_allocator(QGuestAllocator *t_alloc);
 void v9fs_memwrite(P9Req *req, const void *addr, size_t len);
 void v9fs_memskip(P9Req *req, size_t len);
@@ -99,8 +133,7 @@ void v9fs_rversion(P9Req *req, uint16_t *len, char **version);
 P9Req *v9fs_tattach(QVirtio9P *v9p, uint32_t fid, uint32_t n_uname,
                     uint16_t tag);
 void v9fs_rattach(P9Req *req, v9fs_qid *qid);
-P9Req *v9fs_twalk(QVirtio9P *v9p, uint32_t fid, uint32_t newfid,
-                  uint16_t nwname, char *const wnames[], uint16_t tag);
+TWalkRes v9fs_twalk(TWalkOpt opt);
 void v9fs_rwalk(P9Req *req, uint16_t *nwqid, v9fs_qid **wqid);
 P9Req *v9fs_tgetattr(QVirtio9P *v9p, uint32_t fid, uint64_t request_mask,
                      uint16_t tag);
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 498c32e21b..cf5d6146ad 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -16,61 +16,7 @@
 #include "qemu/module.h"
 #include "libqos/virtio-9p-client.h"
 
-/*
- * Used to auto generate new fids. Start with arbitrary high value to avoid
- * collision with hard coded fids in basic test code.
- */
-static uint32_t fid_generator = 1000;
-
-static uint32_t genfid(void)
-{
-    return fid_generator++;
-}
-
-/**
- * Splits the @a in string by @a delim into individual (non empty) strings
- * and outputs them to @a out. The output array @a out is NULL terminated.
- *
- * Output array @a out must be freed by calling split_free().
- *
- * @returns number of individual elements in output array @a out (without the
- *          final NULL terminating element)
- */
-static int split(const char *in, const char *delim, char ***out)
-{
-    int n = 0, i = 0;
-    char *tmp, *p;
-
-    tmp = g_strdup(in);
-    for (p = strtok(tmp, delim); p != NULL; p = strtok(NULL, delim)) {
-        if (strlen(p) > 0) {
-            ++n;
-        }
-    }
-    g_free(tmp);
-
-    *out = g_new0(char *, n + 1); /* last element NULL delimiter */
-
-    tmp = g_strdup(in);
-    for (p = strtok(tmp, delim); p != NULL; p = strtok(NULL, delim)) {
-        if (strlen(p) > 0) {
-            (*out)[i++] = g_strdup(p);
-        }
-    }
-    g_free(tmp);
-
-    return n;
-}
-
-static void split_free(char ***out)
-{
-    int i;
-    for (i = 0; (*out)[i]; ++i) {
-        g_free((*out)[i]);
-    }
-    g_free(*out);
-    *out = NULL;
-}
+#define twalk(...) v9fs_twalk((TWalkOpt) __VA_ARGS__)
 
 static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc)
 {
@@ -109,52 +55,6 @@ static void do_version(QVirtio9P *v9p)
     g_assert_cmpmem(server_version, server_len, version, strlen(version));
 }
 
-/*
- * utility function: walk to requested dir and return fid for that dir and
- * the QIDs of server response
- */
-static uint32_t do_walk_rqids(QVirtio9P *v9p, const char *path, uint16_t *nwqid,
-                              v9fs_qid **wqid)
-{
-    char **wnames;
-    P9Req *req;
-    const uint32_t fid = genfid();
-
-    int nwnames = split(path, "/", &wnames);
-
-    req = v9fs_twalk(v9p, 0, fid, nwnames, wnames, 0);
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rwalk(req, nwqid, wqid);
-
-    split_free(&wnames);
-    return fid;
-}
-
-/* utility function: walk to requested dir and return fid for that dir */
-static uint32_t do_walk(QVirtio9P *v9p, const char *path)
-{
-    return do_walk_rqids(v9p, path, NULL, NULL);
-}
-
-/* utility function: walk to requested dir and expect passed error response */
-static void do_walk_expect_error(QVirtio9P *v9p, const char *path, uint32_t err)
-{
-    char **wnames;
-    P9Req *req;
-    uint32_t _err;
-    const uint32_t fid = genfid();
-
-    int nwnames = split(path, "/", &wnames);
-
-    req = v9fs_twalk(v9p, 0, fid, nwnames, wnames, 0);
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rlerror(req, &_err);
-
-    g_assert_cmpint(_err, ==, err);
-
-    split_free(&wnames);
-}
-
 static void fs_version(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     v9fs_set_allocator(t_alloc);
@@ -197,7 +97,10 @@ static void fs_walk(void *obj, void *data, QGuestAllocator *t_alloc)
     }
 
     do_attach(v9p);
-    req = v9fs_twalk(v9p, 0, 1, P9_MAXWELEM, wnames, 0);
+    req = twalk({
+        .client = v9p, .fid = 0, .newfid = 1,
+        .nwname = P9_MAXWELEM, .wnames = wnames, .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rwalk(req, &nwqid, &wqid);
 
@@ -223,7 +126,7 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     QVirtio9P *v9p = obj;
     v9fs_set_allocator(t_alloc);
-    char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_READDIR_DIR) };
+    char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_READDIR_DIR) };
     uint16_t nqid;
     v9fs_qid qid;
     uint32_t count, nentries;
@@ -231,7 +134,10 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
     P9Req *req;
 
     do_attach(v9p);
-    req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0);
+    req = twalk({
+        .client = v9p, .fid = 0, .newfid = 1,
+        .nwname = 1, .wnames = wnames, .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rwalk(req, &nqid, NULL);
     g_assert_cmpint(nqid, ==, 1);
@@ -275,7 +181,7 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
 /* readdir test where overall request is split over several messages */
 static void do_readdir_split(QVirtio9P *v9p, uint32_t count)
 {
-    char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_READDIR_DIR) };
+    char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_READDIR_DIR) };
     uint16_t nqid;
     v9fs_qid qid;
     uint32_t nentries, npartialentries;
@@ -292,7 +198,10 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count)
     nentries = 0;
     tail = NULL;
 
-    req = v9fs_twalk(v9p, 0, fid, 1, wnames, 0);
+    req = twalk({
+        .client = v9p, .fid = 0, .newfid = fid,
+        .nwname = 1, .wnames = wnames, .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rwalk(req, &nqid, NULL);
     g_assert_cmpint(nqid, ==, 1);
@@ -356,12 +265,15 @@ static void fs_walk_no_slash(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     QVirtio9P *v9p = obj;
     v9fs_set_allocator(t_alloc);
-    char *const wnames[] = { g_strdup(" /") };
+    char *wnames[] = { g_strdup(" /") };
     P9Req *req;
     uint32_t err;
 
     do_attach(v9p);
-    req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0);
+    req = twalk({
+        .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames,
+        .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rlerror(req, &err);
 
@@ -380,7 +292,7 @@ static void fs_walk_nonexistent(void *obj, void *data, QGuestAllocator *t_alloc)
      * The 9p2000 protocol spec says: "If the first element cannot be walked
      * for any reason, Rerror is returned."
      */
-    do_walk_expect_error(v9p, "non-existent", ENOENT);
+    twalk({ .client = v9p, .path = "non-existent", .expectErr = ENOENT });
 }
 
 static void fs_walk_2nd_nonexistent(void *obj, void *data,
@@ -398,7 +310,10 @@ static void fs_walk_2nd_nonexistent(void *obj, void *data,
     );
 
     do_attach_rqid(v9p, &root_qid);
-    fid = do_walk_rqids(v9p, path, &nwqid, &wqid);
+    fid = twalk({
+        .client = v9p, .path = path,
+        .rwalk.nwqid = &nwqid, .rwalk.wqid = &wqid
+    }).newfid;
     /*
      * The 9p2000 protocol spec says: "nwqid is therefore either nwname or the
      * index of the first elementwise walk that failed."
@@ -430,7 +345,10 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc)
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rattach(req, &root_qid);
 
-    req = v9fs_twalk(v9p, 0, 1, 0, NULL, 0);
+    req = twalk({
+        .client = v9p, .fid = 0, .newfid = 1, .nwname = 0, .wnames = NULL,
+        .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rwalk(req, NULL, &wqid);
 
@@ -448,7 +366,7 @@ static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     QVirtio9P *v9p = obj;
     v9fs_set_allocator(t_alloc);
-    char *const wnames[] = { g_strdup("..") };
+    char *wnames[] = { g_strdup("..") };
     v9fs_qid root_qid;
     g_autofree v9fs_qid *wqid = NULL;
     P9Req *req;
@@ -458,7 +376,10 @@ static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc)
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rattach(req, &root_qid);
 
-    req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0);
+    req = twalk({
+        .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames,
+        .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rwalk(req, NULL, &wqid); /* We now we'll get one qid */
 
@@ -471,11 +392,14 @@ static void fs_lopen(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     QVirtio9P *v9p = obj;
     v9fs_set_allocator(t_alloc);
-    char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_LOPEN_FILE) };
+    char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_LOPEN_FILE) };
     P9Req *req;
 
     do_attach(v9p);
-    req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0);
+    req = twalk({
+        .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames,
+        .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rwalk(req, NULL, NULL);
 
@@ -491,13 +415,16 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc)
     QVirtio9P *v9p = obj;
     v9fs_set_allocator(t_alloc);
     static const uint32_t write_count = P9_MAX_SIZE / 2;
-    char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_WRITE_FILE) };
+    char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_WRITE_FILE) };
     g_autofree char *buf = g_malloc0(write_count);
     uint32_t count;
     P9Req *req;
 
     do_attach(v9p);
-    req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0);
+    req = twalk({
+        .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames,
+        .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rwalk(req, NULL, NULL);
 
@@ -517,13 +444,16 @@ static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     QVirtio9P *v9p = obj;
     v9fs_set_allocator(t_alloc);
-    char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_FLUSH_FILE) };
+    char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_FLUSH_FILE) };
     P9Req *req, *flush_req;
     uint32_t reply_len;
     uint8_t should_block;
 
     do_attach(v9p);
-    req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0);
+    req = twalk({
+        .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames,
+        .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rwalk(req, NULL, NULL);
 
@@ -554,13 +484,16 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     QVirtio9P *v9p = obj;
     v9fs_set_allocator(t_alloc);
-    char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_FLUSH_FILE) };
+    char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_FLUSH_FILE) };
     P9Req *req, *flush_req;
     uint32_t count;
     uint8_t should_block;
 
     do_attach(v9p);
-    req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0);
+    req = twalk({
+        .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames,
+        .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rwalk(req, NULL, NULL);
 
@@ -593,7 +526,7 @@ static void do_mkdir(QVirtio9P *v9p, const char *path, const char *cname)
     uint32_t fid;
     P9Req *req;
 
-    fid = do_walk(v9p, path);
+    fid = twalk({ .client = v9p, .path = path }).newfid;
 
     req = v9fs_tmkdir(v9p, fid, name, 0750, 0, 0);
     v9fs_req_wait_for_reply(req, NULL);
@@ -608,7 +541,7 @@ static uint32_t do_lcreate(QVirtio9P *v9p, const char *path,
     uint32_t fid;
     P9Req *req;
 
-    fid = do_walk(v9p, path);
+    fid = twalk({ .client = v9p, .path = path }).newfid;
 
     req = v9fs_tlcreate(v9p, fid, name, 0, 0750, 0, 0);
     v9fs_req_wait_for_reply(req, NULL);
@@ -626,7 +559,7 @@ static void do_symlink(QVirtio9P *v9p, const char *path, const char *clink,
     uint32_t fid;
     P9Req *req;
 
-    fid = do_walk(v9p, path);
+    fid = twalk({ .client = v9p, .path = path }).newfid;
 
     req = v9fs_tsymlink(v9p, fid, name, dst, 0, 0);
     v9fs_req_wait_for_reply(req, NULL);
@@ -640,8 +573,8 @@ static void do_hardlink(QVirtio9P *v9p, const char *path, const char *clink,
     uint32_t dfid, fid;
     P9Req *req;
 
-    dfid = do_walk(v9p, path);
-    fid = do_walk(v9p, to);
+    dfid = twalk({ .client = v9p, .path = path }).newfid;
+    fid = twalk({ .client = v9p, .path = to }).newfid;
 
     req = v9fs_tlink(v9p, dfid, fid, clink, 0);
     v9fs_req_wait_for_reply(req, NULL);
@@ -655,7 +588,7 @@ static void do_unlinkat(QVirtio9P *v9p, const char *atpath, const char *rpath,
     uint32_t fid;
     P9Req *req;
 
-    fid = do_walk(v9p, atpath);
+    fid = twalk({ .client = v9p, .path = atpath }).newfid;
 
     req = v9fs_tunlinkat(v9p, fid, name, flags, 0);
     v9fs_req_wait_for_reply(req, NULL);
-- 
2.30.2

[PATCH 02/20] tests/9p: simplify callers of twalk()

[Christian Schoenebeck]

Now as twalk() is using a declarative approach, simplify the
code of callers of this function.

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

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index cf5d6146ad..3c326451b1 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -90,19 +90,17 @@ static void fs_walk(void *obj, void *data, QGuestAllocator *t_alloc)
     uint16_t nwqid;
     g_autofree v9fs_qid *wqid = NULL;
     int i;
-    P9Req *req;
 
     for (i = 0; i < P9_MAXWELEM; i++) {
         wnames[i] = g_strdup_printf(QTEST_V9FS_SYNTH_WALK_FILE, i);
     }
 
     do_attach(v9p);
-    req = twalk({
+    twalk({
         .client = v9p, .fid = 0, .newfid = 1,
-        .nwname = P9_MAXWELEM, .wnames = wnames, .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rwalk(req, &nwqid, &wqid);
+        .nwname = P9_MAXWELEM, .wnames = wnames,
+        .rwalk = { .nwqid = &nwqid, .wqid = &wqid }
+    });
 
     g_assert_cmpint(nwqid, ==, P9_MAXWELEM);
 
@@ -134,12 +132,10 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
     P9Req *req;
 
     do_attach(v9p);
-    req = twalk({
+    twalk({
         .client = v9p, .fid = 0, .newfid = 1,
-        .nwname = 1, .wnames = wnames, .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rwalk(req, &nqid, NULL);
+        .nwname = 1, .wnames = wnames, .rwalk.nwqid = &nqid
+    });
     g_assert_cmpint(nqid, ==, 1);
 
     req = v9fs_tlopen(v9p, 1, O_DIRECTORY, 0);
@@ -198,12 +194,10 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count)
     nentries = 0;
     tail = NULL;
 
-    req = twalk({
+    twalk({
         .client = v9p, .fid = 0, .newfid = fid,
-        .nwname = 1, .wnames = wnames, .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rwalk(req, &nqid, NULL);
+        .nwname = 1, .wnames = wnames, .rwalk.nwqid = &nqid
+    });
     g_assert_cmpint(nqid, ==, 1);
 
     req = v9fs_tlopen(v9p, fid, O_DIRECTORY, 0);
@@ -266,18 +260,12 @@ static void fs_walk_no_slash(void *obj, void *data, QGuestAllocator *t_alloc)
     QVirtio9P *v9p = obj;
     v9fs_set_allocator(t_alloc);
     char *wnames[] = { g_strdup(" /") };
-    P9Req *req;
-    uint32_t err;
 
     do_attach(v9p);
-    req = twalk({
+    twalk({
         .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames,
-        .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rlerror(req, &err);
-
-    g_assert_cmpint(err, ==, ENOENT);
+        .expectErr = ENOENT
+    });
 
     g_free(wnames[0]);
 }
@@ -312,7 +300,7 @@ static void fs_walk_2nd_nonexistent(void *obj, void *data,
     do_attach_rqid(v9p, &root_qid);
     fid = twalk({
         .client = v9p, .path = path,
-        .rwalk.nwqid = &nwqid, .rwalk.wqid = &wqid
+        .rwalk = { .nwqid = &nwqid, .wqid = &wqid }
     }).newfid;
     /*
      * The 9p2000 protocol spec says: "nwqid is therefore either nwname or the
@@ -345,12 +333,10 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc)
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rattach(req, &root_qid);
 
-    req = twalk({
+    twalk({
         .client = v9p, .fid = 0, .newfid = 1, .nwname = 0, .wnames = NULL,
-        .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rwalk(req, NULL, &wqid);
+        .rwalk.wqid = &wqid
+    });
 
     /* special case: no QID is returned if nwname=0 was sent */
     g_assert(wqid == NULL);
@@ -376,12 +362,10 @@ static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc)
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rattach(req, &root_qid);
 
-    req = twalk({
+    twalk({
         .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames,
-        .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rwalk(req, NULL, &wqid); /* We now we'll get one qid */
+        .rwalk.wqid = &wqid /* We now we'll get one qid */
+    });
 
     g_assert_cmpmem(&root_qid, 13, wqid[0], 13);
 
@@ -396,12 +380,9 @@ static void fs_lopen(void *obj, void *data, QGuestAllocator *t_alloc)
     P9Req *req;
 
     do_attach(v9p);
-    req = twalk({
-        .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames,
-        .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rwalk(req, NULL, NULL);
+    twalk({
+        .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames
+    });
 
     req = v9fs_tlopen(v9p, 1, O_WRONLY, 0);
     v9fs_req_wait_for_reply(req, NULL);
@@ -421,12 +402,9 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc)
     P9Req *req;
 
     do_attach(v9p);
-    req = twalk({
-        .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames,
-        .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rwalk(req, NULL, NULL);
+    twalk({
+        .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames
+    });
 
     req = v9fs_tlopen(v9p, 1, O_WRONLY, 0);
     v9fs_req_wait_for_reply(req, NULL);
@@ -450,12 +428,9 @@ static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc)
     uint8_t should_block;
 
     do_attach(v9p);
-    req = twalk({
-        .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames,
-        .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rwalk(req, NULL, NULL);
+    twalk({
+        .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames
+    });
 
     req = v9fs_tlopen(v9p, 1, O_WRONLY, 0);
     v9fs_req_wait_for_reply(req, NULL);
@@ -490,12 +465,9 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc)
     uint8_t should_block;
 
     do_attach(v9p);
-    req = twalk({
-        .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames,
-        .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rwalk(req, NULL, NULL);
+    twalk({
+        .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames
+    });
 
     req = v9fs_tlopen(v9p, 1, O_WRONLY, 0);
     v9fs_req_wait_for_reply(req, NULL);
-- 
2.30.2

[PATCH 03/20] tests/9p: merge v9fs_tversion() and do_version()

[Christian Schoenebeck]

As with previous patches, unify functions v9fs_tversion() and do_version()
into a single function v9fs_tversion() by using a declarative function
arguments approach.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio-9p-client.c | 47 +++++++++++++++++++++++----
 tests/qtest/libqos/virtio-9p-client.h | 25 ++++++++++++--
 tests/qtest/virtio-9p-test.c          | 23 +++----------
 3 files changed, 68 insertions(+), 27 deletions(-)

diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c
index a95bbad9c8..e8364f8d64 100644
--- a/tests/qtest/libqos/virtio-9p-client.c
+++ b/tests/qtest/libqos/virtio-9p-client.c
@@ -291,21 +291,54 @@ void v9fs_rlerror(P9Req *req, uint32_t *err)
 }
 
 /* size[4] Tversion tag[2] msize[4] version[s] */
-P9Req *v9fs_tversion(QVirtio9P *v9p, uint32_t msize, const char *version,
-                     uint16_t tag)
+TVersionRes v9fs_tversion(TVersionOpt opt)
 {
     P9Req *req;
+    uint32_t err;
     uint32_t body_size = 4;
-    uint16_t string_size = v9fs_string_size(version);
+    uint16_t string_size;
+    uint16_t server_len;
+    g_autofree char *server_version = NULL;
 
+    g_assert(opt.client);
+
+    if (!opt.msize) {
+        opt.msize = P9_MAX_SIZE;
+    }
+
+    if (!opt.tag) {
+        opt.tag = P9_NOTAG;
+    }
+
+    if (!opt.version) {
+        opt.version = "9P2000.L";
+    }
+
+    string_size = v9fs_string_size(opt.version);
     g_assert_cmpint(body_size, <=, UINT32_MAX - string_size);
     body_size += string_size;
-    req = v9fs_req_init(v9p, body_size, P9_TVERSION, tag);
+    req = v9fs_req_init(opt.client, body_size, P9_TVERSION, opt.tag);
 
-    v9fs_uint32_write(req, msize);
-    v9fs_string_write(req, version);
+    v9fs_uint32_write(req, opt.msize);
+    v9fs_string_write(req, opt.version);
     v9fs_req_send(req);
-    return req;
+
+    if (!opt.requestOnly) {
+        v9fs_req_wait_for_reply(req, NULL);
+        if (opt.expectErr) {
+            v9fs_rlerror(req, &err);
+            g_assert_cmpint(err, ==, opt.expectErr);
+        } else {
+            v9fs_rversion(req, &server_len, &server_version);
+            g_assert_cmpmem(server_version, server_len,
+                            opt.version, strlen(opt.version));
+        }
+        req = NULL; /* request was freed */
+    }
+
+    return (TVersionRes) {
+        .req = req,
+    };
 }
 
 /* size[4] Rversion tag[2] msize[4] version[s] */
diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h
index 8c6abbb173..fcde849b5d 100644
--- a/tests/qtest/libqos/virtio-9p-client.h
+++ b/tests/qtest/libqos/virtio-9p-client.h
@@ -106,6 +106,28 @@ typedef struct TWalkRes {
     P9Req *req;
 } TWalkRes;
 
+/* options for 'Tversion' 9p request */
+typedef struct TVersionOpt {
+    /* 9P client being used (mandatory) */
+    QVirtio9P *client;
+    /* user supplied tag number being returned with response (optional) */
+    uint16_t tag;
+    /* maximum message size that can be handled by client (optional) */
+    uint32_t msize;
+    /* protocol version (optional) */
+    const char *version;
+    /* only send Tversion request but not wait for a reply? (optional) */
+    bool requestOnly;
+    /* do we expect an Rlerror response, if yes which error code? (optional) */
+    uint32_t expectErr;
+} TVersionOpt;
+
+/* result of 'Tversion' 9p request */
+typedef struct TVersionRes {
+    /* if requestOnly was set: request object for further processing */
+    P9Req *req;
+} TVersionRes;
+
 void v9fs_set_allocator(QGuestAllocator *t_alloc);
 void v9fs_memwrite(P9Req *req, const void *addr, size_t len);
 void v9fs_memskip(P9Req *req, size_t len);
@@ -127,8 +149,7 @@ void v9fs_req_wait_for_reply(P9Req *req, uint32_t *len);
 void v9fs_req_recv(P9Req *req, uint8_t id);
 void v9fs_req_free(P9Req *req);
 void v9fs_rlerror(P9Req *req, uint32_t *err);
-P9Req *v9fs_tversion(QVirtio9P *v9p, uint32_t msize, const char *version,
-                     uint16_t tag);
+TVersionRes v9fs_tversion(TVersionOpt);
 void v9fs_rversion(P9Req *req, uint16_t *len, char **version);
 P9Req *v9fs_tattach(QVirtio9P *v9p, uint32_t fid, uint32_t n_uname,
                     uint16_t tag);
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 3c326451b1..f2907c8026 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -17,6 +17,7 @@
 #include "libqos/virtio-9p-client.h"
 
 #define twalk(...) v9fs_twalk((TWalkOpt) __VA_ARGS__)
+#define tversion(...) v9fs_tversion((TVersionOpt) __VA_ARGS__)
 
 static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc)
 {
@@ -41,31 +42,17 @@ static inline bool is_same_qid(v9fs_qid a, v9fs_qid b)
     return a[0] == b[0] && memcmp(&a[5], &b[5], 8) == 0;
 }
 
-static void do_version(QVirtio9P *v9p)
-{
-    const char *version = "9P2000.L";
-    uint16_t server_len;
-    g_autofree char *server_version = NULL;
-    P9Req *req;
-
-    req = v9fs_tversion(v9p, P9_MAX_SIZE, version, P9_NOTAG);
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rversion(req, &server_len, &server_version);
-
-    g_assert_cmpmem(server_version, server_len, version, strlen(version));
-}
-
 static void fs_version(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     v9fs_set_allocator(t_alloc);
-    do_version(obj);
+    tversion({ .client = obj });
 }
 
 static void do_attach_rqid(QVirtio9P *v9p, v9fs_qid *qid)
 {
     P9Req *req;
 
-    do_version(v9p);
+    tversion({ .client = v9p });
     req = v9fs_tattach(v9p, 0, getuid(), 0);
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rattach(req, qid);
@@ -328,7 +315,7 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc)
     P9Req *req;
     struct v9fs_attr attr;
 
-    do_version(v9p);
+    tversion({ .client = v9p });
     req = v9fs_tattach(v9p, 0, getuid(), 0);
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rattach(req, &root_qid);
@@ -357,7 +344,7 @@ static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc)
     g_autofree v9fs_qid *wqid = NULL;
     P9Req *req;
 
-    do_version(v9p);
+    tversion({ .client = v9p });
     req = v9fs_tattach(v9p, 0, getuid(), 0);
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rattach(req, &root_qid);
-- 
2.30.2

[PATCH 04/20] tests/9p: merge v9fs_tattach(), do_attach(), do_attach_rqid()

[Christian Schoenebeck]

As with previous patches, unify those 3 functions into a single function
v9fs_tattach() by using a declarative function arguments approach.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio-9p-client.c | 40 ++++++++++++++---
 tests/qtest/libqos/virtio-9p-client.h | 30 ++++++++++++-
 tests/qtest/virtio-9p-test.c          | 62 +++++++++++----------------
 3 files changed, 88 insertions(+), 44 deletions(-)

diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c
index e8364f8d64..5e6bd6120c 100644
--- a/tests/qtest/libqos/virtio-9p-client.c
+++ b/tests/qtest/libqos/virtio-9p-client.c
@@ -359,20 +359,48 @@ void v9fs_rversion(P9Req *req, uint16_t *len, char **version)
 }
 
 /* size[4] Tattach tag[2] fid[4] afid[4] uname[s] aname[s] n_uname[4] */
-P9Req *v9fs_tattach(QVirtio9P *v9p, uint32_t fid, uint32_t n_uname,
-                    uint16_t tag)
+TAttachRes v9fs_tattach(TAttachOpt opt)
 {
+    uint32_t err;
     const char *uname = ""; /* ignored by QEMU */
     const char *aname = ""; /* ignored by QEMU */
-    P9Req *req = v9fs_req_init(v9p, 4 + 4 + 2 + 2 + 4, P9_TATTACH, tag);
 
-    v9fs_uint32_write(req, fid);
+    g_assert(opt.client);
+    /* expecting either Rattach or Rlerror, but obviously not both */
+    g_assert(!opt.expectErr || !opt.rattach.qid);
+
+    if (!opt.requestOnly) {
+        v9fs_tversion((TVersionOpt) { .client = opt.client });
+    }
+
+    if (!opt.n_uname) {
+        opt.n_uname = getuid();
+    }
+
+    P9Req *req = v9fs_req_init(opt.client, 4 + 4 + 2 + 2 + 4, P9_TATTACH,
+                               opt.tag);
+
+    v9fs_uint32_write(req, opt.fid);
     v9fs_uint32_write(req, P9_NOFID);
     v9fs_string_write(req, uname);
     v9fs_string_write(req, aname);
-    v9fs_uint32_write(req, n_uname);
+    v9fs_uint32_write(req, opt.n_uname);
     v9fs_req_send(req);
-    return req;
+
+    if (!opt.requestOnly) {
+        v9fs_req_wait_for_reply(req, NULL);
+        if (opt.expectErr) {
+            v9fs_rlerror(req, &err);
+            g_assert_cmpint(err, ==, opt.expectErr);
+        } else {
+            v9fs_rattach(req, opt.rattach.qid);
+        }
+        req = NULL; /* request was freed */
+    }
+
+    return (TAttachRes) {
+        .req = req,
+    };
 }
 
 /* size[4] Rattach tag[2] qid[13] */
diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h
index fcde849b5d..64b97b229b 100644
--- a/tests/qtest/libqos/virtio-9p-client.h
+++ b/tests/qtest/libqos/virtio-9p-client.h
@@ -128,6 +128,33 @@ typedef struct TVersionRes {
     P9Req *req;
 } TVersionRes;
 
+/* options for 'Tattach' 9p request */
+typedef struct TAttachOpt {
+    /* 9P client being used (mandatory) */
+    QVirtio9P *client;
+    /* user supplied tag number being returned with response (optional) */
+    uint16_t tag;
+    /* file ID to be associated with root of file tree (optional) */
+    uint32_t fid;
+    /* numerical uid of user being introduced to server (optional) */
+    uint32_t n_uname;
+    /* data being received from 9p server as 'Rattach' response (optional) */
+    struct {
+        /* server's idea of the root of the file tree */
+        v9fs_qid *qid;
+    } rattach;
+    /* only send Tattach request but not wait for a reply? (optional) */
+    bool requestOnly;
+    /* do we expect an Rlerror response, if yes which error code? (optional) */
+    uint32_t expectErr;
+} TAttachOpt;
+
+/* result of 'Tattach' 9p request */
+typedef struct TAttachRes {
+    /* if requestOnly was set: request object for further processing */
+    P9Req *req;
+} TAttachRes;
+
 void v9fs_set_allocator(QGuestAllocator *t_alloc);
 void v9fs_memwrite(P9Req *req, const void *addr, size_t len);
 void v9fs_memskip(P9Req *req, size_t len);
@@ -151,8 +178,7 @@ void v9fs_req_free(P9Req *req);
 void v9fs_rlerror(P9Req *req, uint32_t *err);
 TVersionRes v9fs_tversion(TVersionOpt);
 void v9fs_rversion(P9Req *req, uint16_t *len, char **version);
-P9Req *v9fs_tattach(QVirtio9P *v9p, uint32_t fid, uint32_t n_uname,
-                    uint16_t tag);
+TAttachRes v9fs_tattach(TAttachOpt);
 void v9fs_rattach(P9Req *req, v9fs_qid *qid);
 TWalkRes v9fs_twalk(TWalkOpt opt);
 void v9fs_rwalk(P9Req *req, uint16_t *nwqid, v9fs_qid **wqid);
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index f2907c8026..271c42f6f9 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -18,6 +18,7 @@
 
 #define twalk(...) v9fs_twalk((TWalkOpt) __VA_ARGS__)
 #define tversion(...) v9fs_tversion((TVersionOpt) __VA_ARGS__)
+#define tattach(...) v9fs_tattach((TAttachOpt) __VA_ARGS__)
 
 static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc)
 {
@@ -48,25 +49,10 @@ static void fs_version(void *obj, void *data, QGuestAllocator *t_alloc)
     tversion({ .client = obj });
 }
 
-static void do_attach_rqid(QVirtio9P *v9p, v9fs_qid *qid)
-{
-    P9Req *req;
-
-    tversion({ .client = v9p });
-    req = v9fs_tattach(v9p, 0, getuid(), 0);
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rattach(req, qid);
-}
-
-static void do_attach(QVirtio9P *v9p)
-{
-    do_attach_rqid(v9p, NULL);
-}
-
 static void fs_attach(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     v9fs_set_allocator(t_alloc);
-    do_attach(obj);
+    tattach({ .client = obj });
 }
 
 static void fs_walk(void *obj, void *data, QGuestAllocator *t_alloc)
@@ -82,7 +68,7 @@ static void fs_walk(void *obj, void *data, QGuestAllocator *t_alloc)
         wnames[i] = g_strdup_printf(QTEST_V9FS_SYNTH_WALK_FILE, i);
     }
 
-    do_attach(v9p);
+    tattach({ .client = v9p });
     twalk({
         .client = v9p, .fid = 0, .newfid = 1,
         .nwname = P9_MAXWELEM, .wnames = wnames,
@@ -118,7 +104,7 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
     struct V9fsDirent *entries = NULL;
     P9Req *req;
 
-    do_attach(v9p);
+    tattach({ .client = v9p });
     twalk({
         .client = v9p, .fid = 0, .newfid = 1,
         .nwname = 1, .wnames = wnames, .rwalk.nwqid = &nqid
@@ -173,7 +159,7 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count)
     int fid;
     uint64_t offset;
 
-    do_attach(v9p);
+    tattach({ .client = v9p });
 
     fid = 1;
     offset = 0;
@@ -248,7 +234,7 @@ static void fs_walk_no_slash(void *obj, void *data, QGuestAllocator *t_alloc)
     v9fs_set_allocator(t_alloc);
     char *wnames[] = { g_strdup(" /") };
 
-    do_attach(v9p);
+    tattach({ .client = v9p });
     twalk({
         .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames,
         .expectErr = ENOENT
@@ -262,7 +248,7 @@ static void fs_walk_nonexistent(void *obj, void *data, QGuestAllocator *t_alloc)
     QVirtio9P *v9p = obj;
     v9fs_set_allocator(t_alloc);
 
-    do_attach(v9p);
+    tattach({ .client = v9p });
     /*
      * The 9p2000 protocol spec says: "If the first element cannot be walked
      * for any reason, Rerror is returned."
@@ -284,7 +270,7 @@ static void fs_walk_2nd_nonexistent(void *obj, void *data,
         QTEST_V9FS_SYNTH_WALK_FILE "/non-existent", 0
     );
 
-    do_attach_rqid(v9p, &root_qid);
+    tattach({ .client = v9p, .rattach.qid = &root_qid });
     fid = twalk({
         .client = v9p, .path = path,
         .rwalk = { .nwqid = &nwqid, .wqid = &wqid }
@@ -316,7 +302,9 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc)
     struct v9fs_attr attr;
 
     tversion({ .client = v9p });
-    req = v9fs_tattach(v9p, 0, getuid(), 0);
+    req = tattach({
+        .client = v9p, .fid = 0, .n_uname = getuid(), .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rattach(req, &root_qid);
 
@@ -345,7 +333,9 @@ static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc)
     P9Req *req;
 
     tversion({ .client = v9p });
-    req = v9fs_tattach(v9p, 0, getuid(), 0);
+    req = tattach((TAttachOpt) {
+        .client = v9p, .fid = 0, .n_uname = getuid(), .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rattach(req, &root_qid);
 
@@ -366,7 +356,7 @@ static void fs_lopen(void *obj, void *data, QGuestAllocator *t_alloc)
     char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_LOPEN_FILE) };
     P9Req *req;
 
-    do_attach(v9p);
+    tattach({ .client = v9p });
     twalk({
         .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames
     });
@@ -388,7 +378,7 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc)
     uint32_t count;
     P9Req *req;
 
-    do_attach(v9p);
+    tattach({ .client = v9p });
     twalk({
         .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames
     });
@@ -414,7 +404,7 @@ static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc)
     uint32_t reply_len;
     uint8_t should_block;
 
-    do_attach(v9p);
+    tattach({ .client = v9p });
     twalk({
         .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames
     });
@@ -451,7 +441,7 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc)
     uint32_t count;
     uint8_t should_block;
 
-    do_attach(v9p);
+    tattach({ .client = v9p });
     twalk({
         .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames
     });
@@ -588,7 +578,7 @@ static void fs_create_dir(void *obj, void *data, QGuestAllocator *t_alloc)
 
     g_assert(root_path != NULL);
 
-    do_attach(v9p);
+    tattach({ .client = v9p });
     do_mkdir(v9p, "/", "01");
 
     /* check if created directory really exists now ... */
@@ -607,7 +597,7 @@ static void fs_unlinkat_dir(void *obj, void *data, QGuestAllocator *t_alloc)
 
     g_assert(root_path != NULL);
 
-    do_attach(v9p);
+    tattach({ .client = v9p });
     do_mkdir(v9p, "/", "02");
 
     /* check if created directory really exists now ... */
@@ -627,7 +617,7 @@ static void fs_create_file(void *obj, void *data, QGuestAllocator *t_alloc)
     struct stat st;
     g_autofree char *new_file = virtio_9p_test_path("03/1st_file");
 
-    do_attach(v9p);
+    tattach({ .client = v9p });
     do_mkdir(v9p, "/", "03");
     do_lcreate(v9p, "03", "1st_file");
 
@@ -644,7 +634,7 @@ static void fs_unlinkat_file(void *obj, void *data, QGuestAllocator *t_alloc)
     struct stat st;
     g_autofree char *new_file = virtio_9p_test_path("04/doa_file");
 
-    do_attach(v9p);
+    tattach({ .client = v9p });
     do_mkdir(v9p, "/", "04");
     do_lcreate(v9p, "04", "doa_file");
 
@@ -666,7 +656,7 @@ static void fs_symlink_file(void *obj, void *data, QGuestAllocator *t_alloc)
     g_autofree char *real_file = virtio_9p_test_path("05/real_file");
     g_autofree char *symlink_file = virtio_9p_test_path("05/symlink_file");
 
-    do_attach(v9p);
+    tattach({ .client = v9p });
     do_mkdir(v9p, "/", "05");
     do_lcreate(v9p, "05", "real_file");
     g_assert(stat(real_file, &st) == 0);
@@ -687,7 +677,7 @@ static void fs_unlinkat_symlink(void *obj, void *data,
     g_autofree char *real_file = virtio_9p_test_path("06/real_file");
     g_autofree char *symlink_file = virtio_9p_test_path("06/symlink_file");
 
-    do_attach(v9p);
+    tattach({ .client = v9p });
     do_mkdir(v9p, "/", "06");
     do_lcreate(v9p, "06", "real_file");
     g_assert(stat(real_file, &st) == 0);
@@ -709,7 +699,7 @@ static void fs_hardlink_file(void *obj, void *data, QGuestAllocator *t_alloc)
     g_autofree char *real_file = virtio_9p_test_path("07/real_file");
     g_autofree char *hardlink_file = virtio_9p_test_path("07/hardlink_file");
 
-    do_attach(v9p);
+    tattach({ .client = v9p });
     do_mkdir(v9p, "/", "07");
     do_lcreate(v9p, "07", "real_file");
     g_assert(stat(real_file, &st_real) == 0);
@@ -734,7 +724,7 @@ static void fs_unlinkat_hardlink(void *obj, void *data,
     g_autofree char *real_file = virtio_9p_test_path("08/real_file");
     g_autofree char *hardlink_file = virtio_9p_test_path("08/hardlink_file");
 
-    do_attach(v9p);
+    tattach({ .client = v9p });
     do_mkdir(v9p, "/", "08");
     do_lcreate(v9p, "08", "real_file");
     g_assert(stat(real_file, &st_real) == 0);
-- 
2.30.2

[PATCH 05/20] tests/9p: simplify callers of tattach()

[Christian Schoenebeck]

Now as tattach() is using a declarative approach, simplify the
code of callers of this function.

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

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 271c42f6f9..46bb189b81 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -302,11 +302,10 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc)
     struct v9fs_attr attr;
 
     tversion({ .client = v9p });
-    req = tattach({
-        .client = v9p, .fid = 0, .n_uname = getuid(), .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rattach(req, &root_qid);
+    tattach({
+        .client = v9p, .fid = 0, .n_uname = getuid(),
+        .rattach.qid = &root_qid
+    });
 
     twalk({
         .client = v9p, .fid = 0, .newfid = 1, .nwname = 0, .wnames = NULL,
@@ -330,14 +329,12 @@ static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc)
     char *wnames[] = { g_strdup("..") };
     v9fs_qid root_qid;
     g_autofree v9fs_qid *wqid = NULL;
-    P9Req *req;
 
     tversion({ .client = v9p });
-    req = tattach((TAttachOpt) {
-        .client = v9p, .fid = 0, .n_uname = getuid(), .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rattach(req, &root_qid);
+    tattach({
+        .client = v9p, .fid = 0, .n_uname = getuid(),
+        .rattach.qid = &root_qid
+    });
 
     twalk({
         .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames,
-- 
2.30.2

[PATCH 06/20] tests/9p: convert v9fs_tgetattr() to declarative arguments

[Christian Schoenebeck]

Use declarative function arguments for function v9fs_tgetattr().

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio-9p-client.c | 32 ++++++++++++++++++++++-----
 tests/qtest/libqos/virtio-9p-client.h | 30 +++++++++++++++++++++++--
 tests/qtest/virtio-9p-test.c          | 11 +++++++--
 3 files changed, 63 insertions(+), 10 deletions(-)

diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c
index 5e6bd6120c..29916a23b5 100644
--- a/tests/qtest/libqos/virtio-9p-client.c
+++ b/tests/qtest/libqos/virtio-9p-client.c
@@ -489,16 +489,36 @@ void v9fs_rwalk(P9Req *req, uint16_t *nwqid, v9fs_qid **wqid)
 }
 
 /* size[4] Tgetattr tag[2] fid[4] request_mask[8] */
-P9Req *v9fs_tgetattr(QVirtio9P *v9p, uint32_t fid, uint64_t request_mask,
-                     uint16_t tag)
+TGetAttrRes v9fs_tgetattr(TGetAttrOpt opt)
 {
     P9Req *req;
+    uint32_t err;
 
-    req = v9fs_req_init(v9p, 4 + 8, P9_TGETATTR, tag);
-    v9fs_uint32_write(req, fid);
-    v9fs_uint64_write(req, request_mask);
+    g_assert(opt.client);
+    /* expecting either Rgetattr or Rlerror, but obviously not both */
+    g_assert(!opt.expectErr || !opt.rgetattr.attr);
+
+    if (!opt.request_mask) {
+        opt.request_mask = P9_GETATTR_ALL;
+    }
+
+    req = v9fs_req_init(opt.client, 4 + 8, P9_TGETATTR, opt.tag);
+    v9fs_uint32_write(req, opt.fid);
+    v9fs_uint64_write(req, opt.request_mask);
     v9fs_req_send(req);
-    return req;
+
+    if (!opt.requestOnly) {
+        v9fs_req_wait_for_reply(req, NULL);
+        if (opt.expectErr) {
+            v9fs_rlerror(req, &err);
+            g_assert_cmpint(err, ==, opt.expectErr);
+        } else {
+            v9fs_rgetattr(req, opt.rgetattr.attr);
+        }
+        req = NULL; /* request was freed */
+    }
+
+    return (TGetAttrRes) { .req = req };
 }
 
 /*
diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h
index 64b97b229b..f7b1bfc79a 100644
--- a/tests/qtest/libqos/virtio-9p-client.h
+++ b/tests/qtest/libqos/virtio-9p-client.h
@@ -63,6 +63,7 @@ typedef struct v9fs_attr {
 } v9fs_attr;
 
 #define P9_GETATTR_BASIC    0x000007ffULL /* Mask for fields up to BLOCKS */
+#define P9_GETATTR_ALL      0x00003fffULL /* Mask for ALL fields */
 
 struct V9fsDirent {
     v9fs_qid qid;
@@ -155,6 +156,32 @@ typedef struct TAttachRes {
     P9Req *req;
 } TAttachRes;
 
+/* options for 'Tgetattr' 9p request */
+typedef struct TGetAttrOpt {
+    /* 9P client being used (mandatory) */
+    QVirtio9P *client;
+    /* user supplied tag number being returned with response (optional) */
+    uint16_t tag;
+    /* file ID of file/dir whose attributes shall be retrieved (required) */
+    uint32_t fid;
+    /* bitmask indicating attribute fields to be retrieved (optional) */
+    uint64_t request_mask;
+    /* data being received from 9p server as 'Rgetattr' response (optional) */
+    struct {
+        v9fs_attr *attr;
+    } rgetattr;
+    /* only send Tgetattr request but not wait for a reply? (optional) */
+    bool requestOnly;
+    /* do we expect an Rlerror response, if yes which error code? (optional) */
+    uint32_t expectErr;
+} TGetAttrOpt;
+
+/* result of 'Tgetattr' 9p request */
+typedef struct TGetAttrRes {
+    /* if requestOnly was set: request object for further processing */
+    P9Req *req;
+} TGetAttrRes;
+
 void v9fs_set_allocator(QGuestAllocator *t_alloc);
 void v9fs_memwrite(P9Req *req, const void *addr, size_t len);
 void v9fs_memskip(P9Req *req, size_t len);
@@ -182,8 +209,7 @@ TAttachRes v9fs_tattach(TAttachOpt);
 void v9fs_rattach(P9Req *req, v9fs_qid *qid);
 TWalkRes v9fs_twalk(TWalkOpt opt);
 void v9fs_rwalk(P9Req *req, uint16_t *nwqid, v9fs_qid **wqid);
-P9Req *v9fs_tgetattr(QVirtio9P *v9p, uint32_t fid, uint64_t request_mask,
-                     uint16_t tag);
+TGetAttrRes v9fs_tgetattr(TGetAttrOpt);
 void v9fs_rgetattr(P9Req *req, v9fs_attr *attr);
 P9Req *v9fs_treaddir(QVirtio9P *v9p, uint32_t fid, uint64_t offset,
                      uint32_t count, uint16_t tag);
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 46bb189b81..9c1219db33 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -19,6 +19,7 @@
 #define twalk(...) v9fs_twalk((TWalkOpt) __VA_ARGS__)
 #define tversion(...) v9fs_tversion((TVersionOpt) __VA_ARGS__)
 #define tattach(...) v9fs_tattach((TAttachOpt) __VA_ARGS__)
+#define tgetattr(...) v9fs_tgetattr((TGetAttrOpt) __VA_ARGS__)
 
 static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc)
 {
@@ -285,7 +286,10 @@ static void fs_walk_2nd_nonexistent(void *obj, void *data,
     g_assert(wqid && wqid[0] && !is_same_qid(root_qid, wqid[0]));
 
     /* expect fid being unaffected by walk above */
-    req = v9fs_tgetattr(v9p, fid, P9_GETATTR_BASIC, 0);
+    req = tgetattr({
+        .client = v9p, .fid = fid, .request_mask = P9_GETATTR_BASIC,
+        .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rlerror(req, &err);
 
@@ -315,7 +319,10 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc)
     /* special case: no QID is returned if nwname=0 was sent */
     g_assert(wqid == NULL);
 
-    req = v9fs_tgetattr(v9p, 1, P9_GETATTR_BASIC, 0);
+    req = tgetattr({
+        .client = v9p, .fid = 1, .request_mask = P9_GETATTR_BASIC,
+        .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rgetattr(req, &attr);
 
-- 
2.30.2

[PATCH 07/20] tests/9p: simplify callers of tgetattr()

[Christian Schoenebeck]

Now as tgetattr() is using a declarative approach, simplify the
code of callers of this function.

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

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 9c1219db33..ae1220d0cb 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -264,8 +264,7 @@ static void fs_walk_2nd_nonexistent(void *obj, void *data,
     v9fs_set_allocator(t_alloc);
     v9fs_qid root_qid;
     uint16_t nwqid;
-    uint32_t fid, err;
-    P9Req *req;
+    uint32_t fid;
     g_autofree v9fs_qid *wqid = NULL;
     g_autofree char *path = g_strdup_printf(
         QTEST_V9FS_SYNTH_WALK_FILE "/non-existent", 0
@@ -286,14 +285,10 @@ static void fs_walk_2nd_nonexistent(void *obj, void *data,
     g_assert(wqid && wqid[0] && !is_same_qid(root_qid, wqid[0]));
 
     /* expect fid being unaffected by walk above */
-    req = tgetattr({
+    tgetattr({
         .client = v9p, .fid = fid, .request_mask = P9_GETATTR_BASIC,
-        .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rlerror(req, &err);
-
-    g_assert_cmpint(err, ==, ENOENT);
+        .expectErr = ENOENT
+    });
 }
 
 static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc)
@@ -302,7 +297,6 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc)
     v9fs_set_allocator(t_alloc);
     v9fs_qid root_qid;
     g_autofree v9fs_qid *wqid = NULL;
-    P9Req *req;
     struct v9fs_attr attr;
 
     tversion({ .client = v9p });
@@ -319,12 +313,10 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc)
     /* special case: no QID is returned if nwname=0 was sent */
     g_assert(wqid == NULL);
 
-    req = tgetattr({
+    tgetattr({
         .client = v9p, .fid = 1, .request_mask = P9_GETATTR_BASIC,
-        .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rgetattr(req, &attr);
+        .rgetattr.attr = &attr
+    });
 
     g_assert(is_same_qid(root_qid, attr.qid));
 }
-- 
2.30.2

[PATCH 08/20] tests/9p: convert v9fs_treaddir() to declarative arguments

[Christian Schoenebeck]

Use declarative function arguments for function v9fs_treaddir().

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio-9p-client.c | 32 ++++++++++++++++++++------
 tests/qtest/libqos/virtio-9p-client.h | 33 +++++++++++++++++++++++++--
 tests/qtest/virtio-9p-test.c          | 11 +++++++--
 3 files changed, 65 insertions(+), 11 deletions(-)

diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c
index 29916a23b5..047c8993b6 100644
--- a/tests/qtest/libqos/virtio-9p-client.c
+++ b/tests/qtest/libqos/virtio-9p-client.c
@@ -557,17 +557,35 @@ void v9fs_rgetattr(P9Req *req, v9fs_attr *attr)
 }
 
 /* size[4] Treaddir tag[2] fid[4] offset[8] count[4] */
-P9Req *v9fs_treaddir(QVirtio9P *v9p, uint32_t fid, uint64_t offset,
-                     uint32_t count, uint16_t tag)
+TReadDirRes v9fs_treaddir(TReadDirOpt opt)
 {
     P9Req *req;
+    uint32_t err;
 
-    req = v9fs_req_init(v9p, 4 + 8 + 4, P9_TREADDIR, tag);
-    v9fs_uint32_write(req, fid);
-    v9fs_uint64_write(req, offset);
-    v9fs_uint32_write(req, count);
+    g_assert(opt.client);
+    /* expecting either Rreaddir or Rlerror, but obviously not both */
+    g_assert(!opt.expectErr || !(opt.rreaddir.count ||
+             opt.rreaddir.nentries || opt.rreaddir.entries));
+
+    req = v9fs_req_init(opt.client, 4 + 8 + 4, P9_TREADDIR, opt.tag);
+    v9fs_uint32_write(req, opt.fid);
+    v9fs_uint64_write(req, opt.offset);
+    v9fs_uint32_write(req, opt.count);
     v9fs_req_send(req);
-    return req;
+
+    if (!opt.requestOnly) {
+        v9fs_req_wait_for_reply(req, NULL);
+        if (opt.expectErr) {
+            v9fs_rlerror(req, &err);
+            g_assert_cmpint(err, ==, opt.expectErr);
+        } else {
+            v9fs_rreaddir(req, opt.rreaddir.count, opt.rreaddir.nentries,
+                          opt.rreaddir.entries);
+        }
+        req = NULL; /* request was freed */
+    }
+
+    return (TReadDirRes) { .req = req };
 }
 
 /* size[4] Rreaddir tag[2] count[4] data[count] */
diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h
index f7b1bfc79a..2bf649085f 100644
--- a/tests/qtest/libqos/virtio-9p-client.h
+++ b/tests/qtest/libqos/virtio-9p-client.h
@@ -182,6 +182,36 @@ typedef struct TGetAttrRes {
     P9Req *req;
 } TGetAttrRes;
 
+/* options for 'Treaddir' 9p request */
+typedef struct TReadDirOpt {
+    /* 9P client being used (mandatory) */
+    QVirtio9P *client;
+    /* user supplied tag number being returned with response (optional) */
+    uint16_t tag;
+    /* file ID of directory whose entries shall be retrieved (required) */
+    uint32_t fid;
+    /* offset in entries stream, i.e. for multiple requests (optional) */
+    uint64_t offset;
+    /* maximum bytes to be returned by server (required) */
+    uint32_t count;
+    /* data being received from 9p server as 'Rreaddir' response (optional) */
+    struct {
+        uint32_t *count;
+        uint32_t *nentries;
+        struct V9fsDirent **entries;
+    } rreaddir;
+    /* only send Treaddir request but not wait for a reply? (optional) */
+    bool requestOnly;
+    /* do we expect an Rlerror response, if yes which error code? (optional) */
+    uint32_t expectErr;
+} TReadDirOpt;
+
+/* result of 'Treaddir' 9p request */
+typedef struct TReadDirRes {
+    /* if requestOnly was set: request object for further processing */
+    P9Req *req;
+} TReadDirRes;
+
 void v9fs_set_allocator(QGuestAllocator *t_alloc);
 void v9fs_memwrite(P9Req *req, const void *addr, size_t len);
 void v9fs_memskip(P9Req *req, size_t len);
@@ -211,8 +241,7 @@ TWalkRes v9fs_twalk(TWalkOpt opt);
 void v9fs_rwalk(P9Req *req, uint16_t *nwqid, v9fs_qid **wqid);
 TGetAttrRes v9fs_tgetattr(TGetAttrOpt);
 void v9fs_rgetattr(P9Req *req, v9fs_attr *attr);
-P9Req *v9fs_treaddir(QVirtio9P *v9p, uint32_t fid, uint64_t offset,
-                     uint32_t count, uint16_t tag);
+TReadDirRes v9fs_treaddir(TReadDirOpt);
 void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t *nentries,
                    struct V9fsDirent **entries);
 void v9fs_free_dirents(struct V9fsDirent *e);
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index ae1220d0cb..e5c174c218 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -20,6 +20,7 @@
 #define tversion(...) v9fs_tversion((TVersionOpt) __VA_ARGS__)
 #define tattach(...) v9fs_tattach((TAttachOpt) __VA_ARGS__)
 #define tgetattr(...) v9fs_tgetattr((TGetAttrOpt) __VA_ARGS__)
+#define treaddir(...) v9fs_treaddir((TReadDirOpt) __VA_ARGS__)
 
 static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc)
 {
@@ -119,7 +120,10 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
     /*
      * submit count = msize - 11, because 11 is the header size of Rreaddir
      */
-    req = v9fs_treaddir(v9p, 1, 0, P9_MAX_SIZE - 11, 0);
+    req = treaddir({
+        .client = v9p, .fid = 1, .offset = 0, .count = P9_MAX_SIZE - 11,
+        .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rreaddir(req, &count, &nentries, &entries);
 
@@ -186,7 +190,10 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count)
         npartialentries = 0;
         partialentries = NULL;
 
-        req = v9fs_treaddir(v9p, fid, offset, count, 0);
+        req = treaddir({
+            .client = v9p, .fid = fid, .offset = offset, .count = count,
+            .requestOnly = true
+        }).req;
         v9fs_req_wait_for_reply(req, NULL);
         v9fs_rreaddir(req, &count, &npartialentries, &partialentries);
         if (npartialentries > 0 && partialentries) {
-- 
2.30.2

[PATCH 09/20] tests/9p: simplify callers of treaddir()

[Christian Schoenebeck]

Now as treaddir() is using a declarative approach, simplify the
code of callers of this function.

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

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index e5c174c218..99e24fce0b 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -120,12 +120,12 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
     /*
      * submit count = msize - 11, because 11 is the header size of Rreaddir
      */
-    req = treaddir({
+    treaddir({
         .client = v9p, .fid = 1, .offset = 0, .count = P9_MAX_SIZE - 11,
-        .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rreaddir(req, &count, &nentries, &entries);
+        .rreaddir = {
+            .count = &count, .nentries = &nentries, .entries = &entries
+        }
+    });
 
     /*
      * Assuming msize (P9_MAX_SIZE) is large enough so we can retrieve all
@@ -190,12 +190,13 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count)
         npartialentries = 0;
         partialentries = NULL;
 
-        req = treaddir({
+        treaddir({
             .client = v9p, .fid = fid, .offset = offset, .count = count,
-            .requestOnly = true
-        }).req;
-        v9fs_req_wait_for_reply(req, NULL);
-        v9fs_rreaddir(req, &count, &npartialentries, &partialentries);
+            .rreaddir = {
+                .count = &count, .nentries = &npartialentries,
+                .entries = &partialentries
+            }
+        });
         if (npartialentries > 0 && partialentries) {
             if (!entries) {
                 entries = partialentries;
-- 
2.30.2

[PATCH 10/20] tests/9p: convert v9fs_tlopen() to declarative arguments

[Christian Schoenebeck]

Use declarative function arguments for function v9fs_tlopen().

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio-9p-client.c | 28 +++++++++++++++++++------
 tests/qtest/libqos/virtio-9p-client.h | 30 +++++++++++++++++++++++++--
 tests/qtest/virtio-9p-test.c          | 25 ++++++++++++++++------
 3 files changed, 69 insertions(+), 14 deletions(-)

diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c
index 047c8993b6..15fde54d63 100644
--- a/tests/qtest/libqos/virtio-9p-client.c
+++ b/tests/qtest/libqos/virtio-9p-client.c
@@ -643,16 +643,32 @@ void v9fs_free_dirents(struct V9fsDirent *e)
 }
 
 /* size[4] Tlopen tag[2] fid[4] flags[4] */
-P9Req *v9fs_tlopen(QVirtio9P *v9p, uint32_t fid, uint32_t flags,
-                   uint16_t tag)
+TLOpenRes v9fs_tlopen(TLOpenOpt opt)
 {
     P9Req *req;
+    uint32_t err;
 
-    req = v9fs_req_init(v9p,  4 + 4, P9_TLOPEN, tag);
-    v9fs_uint32_write(req, fid);
-    v9fs_uint32_write(req, flags);
+    g_assert(opt.client);
+    /* expecting either Rlopen or Rlerror, but obviously not both */
+    g_assert(!opt.expectErr || !(opt.rlopen.qid || opt.rlopen.iounit));
+
+    req = v9fs_req_init(opt.client,  4 + 4, P9_TLOPEN, opt.tag);
+    v9fs_uint32_write(req, opt.fid);
+    v9fs_uint32_write(req, opt.flags);
     v9fs_req_send(req);
-    return req;
+
+    if (!opt.requestOnly) {
+        v9fs_req_wait_for_reply(req, NULL);
+        if (opt.expectErr) {
+            v9fs_rlerror(req, &err);
+            g_assert_cmpint(err, ==, opt.expectErr);
+        } else {
+            v9fs_rlopen(req, opt.rlopen.qid, opt.rlopen.iounit);
+        }
+        req = NULL; /* request was freed */
+    }
+
+    return (TLOpenRes) { .req = req };
 }
 
 /* size[4] Rlopen tag[2] qid[13] iounit[4] */
diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h
index 2bf649085f..3b70aef51e 100644
--- a/tests/qtest/libqos/virtio-9p-client.h
+++ b/tests/qtest/libqos/virtio-9p-client.h
@@ -212,6 +212,33 @@ typedef struct TReadDirRes {
     P9Req *req;
 } TReadDirRes;
 
+/* options for 'Tlopen' 9p request */
+typedef struct TLOpenOpt {
+    /* 9P client being used (mandatory) */
+    QVirtio9P *client;
+    /* user supplied tag number being returned with response (optional) */
+    uint16_t tag;
+    /* file ID of file / directory to be opened (required) */
+    uint32_t fid;
+    /* Linux open(2) flags such as O_RDONLY, O_RDWR, O_WRONLY (optional) */
+    uint32_t flags;
+    /* data being received from 9p server as 'Rlopen' response (optional) */
+    struct {
+        v9fs_qid *qid;
+        uint32_t *iounit;
+    } rlopen;
+    /* only send Tlopen request but not wait for a reply? (optional) */
+    bool requestOnly;
+    /* do we expect an Rlerror response, if yes which error code? (optional) */
+    uint32_t expectErr;
+} TLOpenOpt;
+
+/* result of 'Tlopen' 9p request */
+typedef struct TLOpenRes {
+    /* if requestOnly was set: request object for further processing */
+    P9Req *req;
+} TLOpenRes;
+
 void v9fs_set_allocator(QGuestAllocator *t_alloc);
 void v9fs_memwrite(P9Req *req, const void *addr, size_t len);
 void v9fs_memskip(P9Req *req, size_t len);
@@ -245,8 +272,7 @@ TReadDirRes v9fs_treaddir(TReadDirOpt);
 void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t *nentries,
                    struct V9fsDirent **entries);
 void v9fs_free_dirents(struct V9fsDirent *e);
-P9Req *v9fs_tlopen(QVirtio9P *v9p, uint32_t fid, uint32_t flags,
-                   uint16_t tag);
+TLOpenRes v9fs_tlopen(TLOpenOpt);
 void v9fs_rlopen(P9Req *req, v9fs_qid *qid, uint32_t *iounit);
 P9Req *v9fs_twrite(QVirtio9P *v9p, uint32_t fid, uint64_t offset,
                    uint32_t count, const void *data, uint16_t tag);
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 99e24fce0b..0455c3a094 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -21,6 +21,7 @@
 #define tattach(...) v9fs_tattach((TAttachOpt) __VA_ARGS__)
 #define tgetattr(...) v9fs_tgetattr((TGetAttrOpt) __VA_ARGS__)
 #define treaddir(...) v9fs_treaddir((TReadDirOpt) __VA_ARGS__)
+#define tlopen(...) v9fs_tlopen((TLOpenOpt) __VA_ARGS__)
 
 static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc)
 {
@@ -113,7 +114,9 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
     });
     g_assert_cmpint(nqid, ==, 1);
 
-    req = v9fs_tlopen(v9p, 1, O_DIRECTORY, 0);
+    req = tlopen({
+        .client = v9p, .fid = 1, .flags = O_DIRECTORY, .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rlopen(req, &qid, NULL);
 
@@ -178,7 +181,9 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count)
     });
     g_assert_cmpint(nqid, ==, 1);
 
-    req = v9fs_tlopen(v9p, fid, O_DIRECTORY, 0);
+    req = tlopen({
+        .client = v9p, .fid = fid, .flags = O_DIRECTORY, .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rlopen(req, &qid, NULL);
 
@@ -365,7 +370,9 @@ static void fs_lopen(void *obj, void *data, QGuestAllocator *t_alloc)
         .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames
     });
 
-    req = v9fs_tlopen(v9p, 1, O_WRONLY, 0);
+    req = tlopen({
+        .client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rlopen(req, NULL, NULL);
 
@@ -387,7 +394,9 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc)
         .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames
     });
 
-    req = v9fs_tlopen(v9p, 1, O_WRONLY, 0);
+    req = tlopen({
+        .client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rlopen(req, NULL, NULL);
 
@@ -413,7 +422,9 @@ static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc)
         .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames
     });
 
-    req = v9fs_tlopen(v9p, 1, O_WRONLY, 0);
+    req = tlopen({
+        .client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rlopen(req, NULL, NULL);
 
@@ -450,7 +461,9 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc)
         .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames
     });
 
-    req = v9fs_tlopen(v9p, 1, O_WRONLY, 0);
+    req = tlopen({
+        .client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rlopen(req, NULL, NULL);
 
-- 
2.30.2

[PATCH 11/20] tests/9p: simplify callers of tlopen()

[Christian Schoenebeck]

Now as tlopen() is using a declarative approach, simplify the
code of callers of this function.

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

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 0455c3a094..60a030b877 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -105,7 +105,6 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
     v9fs_qid qid;
     uint32_t count, nentries;
     struct V9fsDirent *entries = NULL;
-    P9Req *req;
 
     tattach({ .client = v9p });
     twalk({
@@ -114,11 +113,9 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
     });
     g_assert_cmpint(nqid, ==, 1);
 
-    req = tlopen({
-        .client = v9p, .fid = 1, .flags = O_DIRECTORY, .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rlopen(req, &qid, NULL);
+    tlopen({
+        .client = v9p, .fid = 1, .flags = O_DIRECTORY, .rlopen.qid = &qid
+    });
 
     /*
      * submit count = msize - 11, because 11 is the header size of Rreaddir
@@ -163,7 +160,6 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count)
     v9fs_qid qid;
     uint32_t nentries, npartialentries;
     struct V9fsDirent *entries, *tail, *partialentries;
-    P9Req *req;
     int fid;
     uint64_t offset;
 
@@ -181,11 +177,9 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count)
     });
     g_assert_cmpint(nqid, ==, 1);
 
-    req = tlopen({
-        .client = v9p, .fid = fid, .flags = O_DIRECTORY, .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rlopen(req, &qid, NULL);
+    tlopen({
+        .client = v9p, .fid = fid, .flags = O_DIRECTORY, .rlopen.qid = &qid
+    });
 
     /*
      * send as many Treaddir requests as required to get all directory
@@ -363,18 +357,13 @@ static void fs_lopen(void *obj, void *data, QGuestAllocator *t_alloc)
     QVirtio9P *v9p = obj;
     v9fs_set_allocator(t_alloc);
     char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_LOPEN_FILE) };
-    P9Req *req;
 
     tattach({ .client = v9p });
     twalk({
         .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames
     });
 
-    req = tlopen({
-        .client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rlopen(req, NULL, NULL);
+    tlopen({ .client = v9p, .fid = 1, .flags = O_WRONLY });
 
     g_free(wnames[0]);
 }
@@ -394,11 +383,7 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc)
         .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames
     });
 
-    req = tlopen({
-        .client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rlopen(req, NULL, NULL);
+    tlopen({ .client = v9p, .fid = 1, .flags = O_WRONLY });
 
     req = v9fs_twrite(v9p, 1, 0, write_count, buf, 0);
     v9fs_req_wait_for_reply(req, NULL);
@@ -422,11 +407,7 @@ static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc)
         .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames
     });
 
-    req = tlopen({
-        .client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rlopen(req, NULL, NULL);
+    tlopen({ .client = v9p, .fid = 1, .flags = O_WRONLY });
 
     /* This will cause the 9p server to try to write data to the backend,
      * until the write request gets cancelled.
@@ -461,11 +442,7 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc)
         .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames
     });
 
-    req = tlopen({
-        .client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rlopen(req, NULL, NULL);
+    tlopen({ .client = v9p, .fid = 1, .flags = O_WRONLY });
 
     /* This will cause the write request to complete right away, before it
      * could be actually cancelled.
-- 
2.30.2

[PATCH 12/20] tests/9p: convert v9fs_twrite() to declarative arguments

[Christian Schoenebeck]

Use declarative function arguments for function v9fs_twrite().

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio-9p-client.c | 38 ++++++++++++++++++++-------
 tests/qtest/libqos/virtio-9p-client.h | 31 ++++++++++++++++++++--
 tests/qtest/virtio-9p-test.c          | 18 ++++++++++---
 3 files changed, 72 insertions(+), 15 deletions(-)

diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c
index 15fde54d63..9ae347fad5 100644
--- a/tests/qtest/libqos/virtio-9p-client.c
+++ b/tests/qtest/libqos/virtio-9p-client.c
@@ -687,21 +687,39 @@ void v9fs_rlopen(P9Req *req, v9fs_qid *qid, uint32_t *iounit)
 }
 
 /* size[4] Twrite tag[2] fid[4] offset[8] count[4] data[count] */
-P9Req *v9fs_twrite(QVirtio9P *v9p, uint32_t fid, uint64_t offset,
-                   uint32_t count, const void *data, uint16_t tag)
+TWriteRes v9fs_twrite(TWriteOpt opt)
 {
     P9Req *req;
+    uint32_t err;
     uint32_t body_size = 4 + 8 + 4;
+    uint32_t written = 0;
 
-    g_assert_cmpint(body_size, <=, UINT32_MAX - count);
-    body_size += count;
-    req = v9fs_req_init(v9p,  body_size, P9_TWRITE, tag);
-    v9fs_uint32_write(req, fid);
-    v9fs_uint64_write(req, offset);
-    v9fs_uint32_write(req, count);
-    v9fs_memwrite(req, data, count);
+    g_assert(opt.client);
+
+    g_assert_cmpint(body_size, <=, UINT32_MAX - opt.count);
+    body_size += opt.count;
+    req = v9fs_req_init(opt.client, body_size, P9_TWRITE, opt.tag);
+    v9fs_uint32_write(req, opt.fid);
+    v9fs_uint64_write(req, opt.offset);
+    v9fs_uint32_write(req, opt.count);
+    v9fs_memwrite(req, opt.data, opt.count);
     v9fs_req_send(req);
-    return req;
+
+    if (!opt.requestOnly) {
+        v9fs_req_wait_for_reply(req, NULL);
+        if (opt.expectErr) {
+            v9fs_rlerror(req, &err);
+            g_assert_cmpint(err, ==, opt.expectErr);
+        } else {
+            v9fs_rwrite(req, &written);
+        }
+        req = NULL; /* request was freed */
+    }
+
+    return (TWriteRes) {
+        .req = req,
+        .count = written
+    };
 }
 
 /* size[4] Rwrite tag[2] count[4] */
diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h
index 3b70aef51e..dda371c054 100644
--- a/tests/qtest/libqos/virtio-9p-client.h
+++ b/tests/qtest/libqos/virtio-9p-client.h
@@ -239,6 +239,34 @@ typedef struct TLOpenRes {
     P9Req *req;
 } TLOpenRes;
 
+/* options for 'Twrite' 9p request */
+typedef struct TWriteOpt {
+    /* 9P client being used (mandatory) */
+    QVirtio9P *client;
+    /* user supplied tag number being returned with response (optional) */
+    uint16_t tag;
+    /* file ID of file to write to (required) */
+    uint32_t fid;
+    /* start position of write from beginning of file (optional) */
+    uint64_t offset;
+    /* how many bytes to write */
+    uint32_t count;
+    /* data to be written */
+    const void *data;
+    /* only send Twrite request but not wait for a reply? (optional) */
+    bool requestOnly;
+    /* do we expect an Rlerror response, if yes which error code? (optional) */
+    uint32_t expectErr;
+} TWriteOpt;
+
+/* result of 'Twrite' 9p request */
+typedef struct TWriteRes {
+    /* if requestOnly was set: request object for further processing */
+    P9Req *req;
+    /* amount of bytes written */
+    uint32_t count;
+} TWriteRes;
+
 void v9fs_set_allocator(QGuestAllocator *t_alloc);
 void v9fs_memwrite(P9Req *req, const void *addr, size_t len);
 void v9fs_memskip(P9Req *req, size_t len);
@@ -274,8 +302,7 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t *nentries,
 void v9fs_free_dirents(struct V9fsDirent *e);
 TLOpenRes v9fs_tlopen(TLOpenOpt);
 void v9fs_rlopen(P9Req *req, v9fs_qid *qid, uint32_t *iounit);
-P9Req *v9fs_twrite(QVirtio9P *v9p, uint32_t fid, uint64_t offset,
-                   uint32_t count, const void *data, uint16_t tag);
+TWriteRes v9fs_twrite(TWriteOpt);
 void v9fs_rwrite(P9Req *req, uint32_t *count);
 P9Req *v9fs_tflush(QVirtio9P *v9p, uint16_t oldtag, uint16_t tag);
 void v9fs_rflush(P9Req *req);
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 60a030b877..a5b9284acb 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -22,6 +22,7 @@
 #define tgetattr(...) v9fs_tgetattr((TGetAttrOpt) __VA_ARGS__)
 #define treaddir(...) v9fs_treaddir((TReadDirOpt) __VA_ARGS__)
 #define tlopen(...) v9fs_tlopen((TLOpenOpt) __VA_ARGS__)
+#define twrite(...) v9fs_twrite((TWriteOpt) __VA_ARGS__)
 
 static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc)
 {
@@ -385,7 +386,10 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc)
 
     tlopen({ .client = v9p, .fid = 1, .flags = O_WRONLY });
 
-    req = v9fs_twrite(v9p, 1, 0, write_count, buf, 0);
+    req = twrite({
+        .client = v9p, .fid = 1, .offset = 0, .count = write_count,
+        .data = buf, .requestOnly = true
+    }).req;
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rwrite(req, &count);
     g_assert_cmpint(count, ==, write_count);
@@ -413,7 +417,11 @@ static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc)
      * until the write request gets cancelled.
      */
     should_block = 1;
-    req = v9fs_twrite(v9p, 1, 0, sizeof(should_block), &should_block, 0);
+    req = twrite({
+        .client = v9p, .fid = 1, .offset = 0,
+        .count = sizeof(should_block), .data = &should_block,
+        .requestOnly = true
+    }).req;
 
     flush_req = v9fs_tflush(v9p, req->tag, 1);
 
@@ -448,7 +456,11 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc)
      * could be actually cancelled.
      */
     should_block = 0;
-    req = v9fs_twrite(v9p, 1, 0, sizeof(should_block), &should_block, 0);
+    req = twrite({
+        .client = v9p, .fid = 1, .offset = 0,
+        .count = sizeof(should_block), .data = &should_block,
+        .requestOnly = true
+    }).req;
 
     flush_req = v9fs_tflush(v9p, req->tag, 1);
 
-- 
2.30.2

[PATCH 13/20] tests/9p: simplify callers of twrite()

[Christian Schoenebeck]

Now as twrite() is using a declarative approach, simplify the
code of callers of this function.

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

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index a5b9284acb..5ad7bebec7 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -377,7 +377,6 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc)
     char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_WRITE_FILE) };
     g_autofree char *buf = g_malloc0(write_count);
     uint32_t count;
-    P9Req *req;
 
     tattach({ .client = v9p });
     twalk({
@@ -386,12 +385,10 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc)
 
     tlopen({ .client = v9p, .fid = 1, .flags = O_WRONLY });
 
-    req = twrite({
+    count = twrite({
         .client = v9p, .fid = 1, .offset = 0, .count = write_count,
-        .data = buf, .requestOnly = true
-    }).req;
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rwrite(req, &count);
+        .data = buf
+    }).count;
     g_assert_cmpint(count, ==, write_count);
 
     g_free(wnames[0]);
-- 
2.30.2

[PATCH 14/20] tests/9p: convert v9fs_tflush() to declarative arguments

[Christian Schoenebeck]

Use declarative function arguments for function v9fs_tflush().

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio-9p-client.c | 23 +++++++++++++++++++----
 tests/qtest/libqos/virtio-9p-client.h | 22 +++++++++++++++++++++-
 tests/qtest/virtio-9p-test.c          |  9 +++++++--
 3 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c
index 9ae347fad5..3be0ffc7da 100644
--- a/tests/qtest/libqos/virtio-9p-client.c
+++ b/tests/qtest/libqos/virtio-9p-client.c
@@ -733,14 +733,29 @@ void v9fs_rwrite(P9Req *req, uint32_t *count)
 }
 
 /* size[4] Tflush tag[2] oldtag[2] */
-P9Req *v9fs_tflush(QVirtio9P *v9p, uint16_t oldtag, uint16_t tag)
+TFlushRes v9fs_tflush(TFlushOpt opt)
 {
     P9Req *req;
+    uint32_t err;
 
-    req = v9fs_req_init(v9p,  2, P9_TFLUSH, tag);
-    v9fs_uint32_write(req, oldtag);
+    g_assert(opt.client);
+
+    req = v9fs_req_init(opt.client, 2, P9_TFLUSH, opt.tag);
+    v9fs_uint32_write(req, opt.oldtag);
     v9fs_req_send(req);
-    return req;
+
+    if (!opt.requestOnly) {
+        v9fs_req_wait_for_reply(req, NULL);
+        if (opt.expectErr) {
+            v9fs_rlerror(req, &err);
+            g_assert_cmpint(err, ==, opt.expectErr);
+        } else {
+            v9fs_rflush(req);
+        }
+        req = NULL; /* request was freed */
+    }
+
+    return (TFlushRes) { .req = req };
 }
 
 /* size[4] Rflush tag[2] */
diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h
index dda371c054..b22b54c720 100644
--- a/tests/qtest/libqos/virtio-9p-client.h
+++ b/tests/qtest/libqos/virtio-9p-client.h
@@ -267,6 +267,26 @@ typedef struct TWriteRes {
     uint32_t count;
 } TWriteRes;
 
+/* options for 'Tflush' 9p request */
+typedef struct TFlushOpt {
+    /* 9P client being used (mandatory) */
+    QVirtio9P *client;
+    /* user supplied tag number being returned with response (optional) */
+    uint16_t tag;
+    /* message to flush (required) */
+    uint16_t oldtag;
+    /* only send Tflush request but not wait for a reply? (optional) */
+    bool requestOnly;
+    /* do we expect an Rlerror response, if yes which error code? (optional) */
+    uint32_t expectErr;
+} TFlushOpt;
+
+/* result of 'Tflush' 9p request */
+typedef struct TFlushRes {
+    /* if requestOnly was set: request object for further processing */
+    P9Req *req;
+} TFlushRes;
+
 void v9fs_set_allocator(QGuestAllocator *t_alloc);
 void v9fs_memwrite(P9Req *req, const void *addr, size_t len);
 void v9fs_memskip(P9Req *req, size_t len);
@@ -304,7 +324,7 @@ TLOpenRes v9fs_tlopen(TLOpenOpt);
 void v9fs_rlopen(P9Req *req, v9fs_qid *qid, uint32_t *iounit);
 TWriteRes v9fs_twrite(TWriteOpt);
 void v9fs_rwrite(P9Req *req, uint32_t *count);
-P9Req *v9fs_tflush(QVirtio9P *v9p, uint16_t oldtag, uint16_t tag);
+TFlushRes v9fs_tflush(TFlushOpt);
 void v9fs_rflush(P9Req *req);
 P9Req *v9fs_tmkdir(QVirtio9P *v9p, uint32_t dfid, const char *name,
                    uint32_t mode, uint32_t gid, uint16_t tag);
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 5ad7bebec7..5544998bac 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -23,6 +23,7 @@
 #define treaddir(...) v9fs_treaddir((TReadDirOpt) __VA_ARGS__)
 #define tlopen(...) v9fs_tlopen((TLOpenOpt) __VA_ARGS__)
 #define twrite(...) v9fs_twrite((TWriteOpt) __VA_ARGS__)
+#define tflush(...) v9fs_tflush((TFlushOpt) __VA_ARGS__)
 
 static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc)
 {
@@ -420,7 +421,9 @@ static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc)
         .requestOnly = true
     }).req;
 
-    flush_req = v9fs_tflush(v9p, req->tag, 1);
+    flush_req = tflush({
+        .client = v9p, .oldtag = req->tag, .tag = 1, .requestOnly = true
+    }).req;
 
     /* The write request is supposed to be flushed: the server should just
      * mark the write request as used and reply to the flush request.
@@ -459,7 +462,9 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc)
         .requestOnly = true
     }).req;
 
-    flush_req = v9fs_tflush(v9p, req->tag, 1);
+    flush_req = tflush({
+        .client = v9p, .oldtag = req->tag, .tag = 1, .requestOnly = true
+    }).req;
 
     /* The write request is supposed to complete. The server should
      * reply to the write request and the flush request.
-- 
2.30.2

[PATCH 15/20] tests/9p: merge v9fs_tmkdir() and do_mkdir()

[Christian Schoenebeck]

As with previous patches, unify those 2 functions into a single function
v9fs_tmkdir() by using a declarative function arguments approach.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio-9p-client.c | 42 ++++++++++++++++++++++-----
 tests/qtest/libqos/virtio-9p-client.h | 36 +++++++++++++++++++++--
 tests/qtest/virtio-9p-test.c          | 30 ++++++-------------
 3 files changed, 78 insertions(+), 30 deletions(-)

diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c
index 3be0ffc7da..c374ba2048 100644
--- a/tests/qtest/libqos/virtio-9p-client.c
+++ b/tests/qtest/libqos/virtio-9p-client.c
@@ -766,10 +766,26 @@ void v9fs_rflush(P9Req *req)
 }
 
 /* size[4] Tmkdir tag[2] dfid[4] name[s] mode[4] gid[4] */
-P9Req *v9fs_tmkdir(QVirtio9P *v9p, uint32_t dfid, const char *name,
-                   uint32_t mode, uint32_t gid, uint16_t tag)
+TMkdirRes v9fs_tmkdir(TMkdirOpt opt)
 {
     P9Req *req;
+    uint32_t err;
+    g_autofree char *name = g_strdup(opt.name);
+
+    g_assert(opt.client);
+    /* expecting either hi-level atPath or low-level dfid, but not both */
+    g_assert(!opt.atPath || !opt.dfid);
+    /* expecting either Rmkdir or Rlerror, but obviously not both */
+    g_assert(!opt.expectErr || !opt.rmkdir.qid);
+
+    if (opt.atPath) {
+        opt.dfid = v9fs_twalk((TWalkOpt) { .client = opt.client,
+                                           .path = opt.atPath }).newfid;
+    }
+
+    if (!opt.mode) {
+        opt.mode = 0750;
+    }
 
     uint32_t body_size = 4 + 4 + 4;
     uint16_t string_size = v9fs_string_size(name);
@@ -777,13 +793,25 @@ P9Req *v9fs_tmkdir(QVirtio9P *v9p, uint32_t dfid, const char *name,
     g_assert_cmpint(body_size, <=, UINT32_MAX - string_size);
     body_size += string_size;
 
-    req = v9fs_req_init(v9p, body_size, P9_TMKDIR, tag);
-    v9fs_uint32_write(req, dfid);
+    req = v9fs_req_init(opt.client, body_size, P9_TMKDIR, opt.tag);
+    v9fs_uint32_write(req, opt.dfid);
     v9fs_string_write(req, name);
-    v9fs_uint32_write(req, mode);
-    v9fs_uint32_write(req, gid);
+    v9fs_uint32_write(req, opt.mode);
+    v9fs_uint32_write(req, opt.gid);
     v9fs_req_send(req);
-    return req;
+
+    if (!opt.requestOnly) {
+        v9fs_req_wait_for_reply(req, NULL);
+        if (opt.expectErr) {
+            v9fs_rlerror(req, &err);
+            g_assert_cmpint(err, ==, opt.expectErr);
+        } else {
+            v9fs_rmkdir(req, opt.rmkdir.qid);
+        }
+        req = NULL; /* request was freed */
+    }
+
+    return (TMkdirRes) { .req = req };
 }
 
 /* size[4] Rmkdir tag[2] qid[13] */
diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h
index b22b54c720..ae44f95a4d 100644
--- a/tests/qtest/libqos/virtio-9p-client.h
+++ b/tests/qtest/libqos/virtio-9p-client.h
@@ -287,6 +287,39 @@ typedef struct TFlushRes {
     P9Req *req;
 } TFlushRes;
 
+/* options for 'Tmkdir' 9p request */
+typedef struct TMkdirOpt {
+    /* 9P client being used (mandatory) */
+    QVirtio9P *client;
+    /* user supplied tag number being returned with response (optional) */
+    uint16_t tag;
+    /* low level variant of directory where new one shall be created */
+    uint32_t dfid;
+    /* high-level variant of directory where new one shall be created */
+    const char *atPath;
+    /* New directory's name (required) */
+    const char *name;
+    /* Linux mkdir(2) mode bits (optional) */
+    uint32_t mode;
+    /* effective group ID of caller */
+    uint32_t gid;
+    /* data being received from 9p server as 'Rmkdir' response (optional) */
+    struct {
+        /* QID of newly created directory */
+        v9fs_qid *qid;
+    } rmkdir;
+    /* only send Tmkdir request but not wait for a reply? (optional) */
+    bool requestOnly;
+    /* do we expect an Rlerror response, if yes which error code? (optional) */
+    uint32_t expectErr;
+} TMkdirOpt;
+
+/* result of 'TMkdir' 9p request */
+typedef struct TMkdirRes {
+    /* if requestOnly was set: request object for further processing */
+    P9Req *req;
+} TMkdirRes;
+
 void v9fs_set_allocator(QGuestAllocator *t_alloc);
 void v9fs_memwrite(P9Req *req, const void *addr, size_t len);
 void v9fs_memskip(P9Req *req, size_t len);
@@ -326,8 +359,7 @@ TWriteRes v9fs_twrite(TWriteOpt);
 void v9fs_rwrite(P9Req *req, uint32_t *count);
 TFlushRes v9fs_tflush(TFlushOpt);
 void v9fs_rflush(P9Req *req);
-P9Req *v9fs_tmkdir(QVirtio9P *v9p, uint32_t dfid, const char *name,
-                   uint32_t mode, uint32_t gid, uint16_t tag);
+TMkdirRes v9fs_tmkdir(TMkdirOpt);
 void v9fs_rmkdir(P9Req *req, v9fs_qid *qid);
 P9Req *v9fs_tlcreate(QVirtio9P *v9p, uint32_t fid, const char *name,
                      uint32_t flags, uint32_t mode, uint32_t gid,
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 5544998bac..6d75afee87 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -24,6 +24,7 @@
 #define tlopen(...) v9fs_tlopen((TLOpenOpt) __VA_ARGS__)
 #define twrite(...) v9fs_twrite((TWriteOpt) __VA_ARGS__)
 #define tflush(...) v9fs_tflush((TFlushOpt) __VA_ARGS__)
+#define tmkdir(...) v9fs_tmkdir((TMkdirOpt) __VA_ARGS__)
 
 static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc)
 {
@@ -477,19 +478,6 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc)
     g_free(wnames[0]);
 }
 
-static void do_mkdir(QVirtio9P *v9p, const char *path, const char *cname)
-{
-    g_autofree char *name = g_strdup(cname);
-    uint32_t fid;
-    P9Req *req;
-
-    fid = twalk({ .client = v9p, .path = path }).newfid;
-
-    req = v9fs_tmkdir(v9p, fid, name, 0750, 0, 0);
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rmkdir(req, NULL);
-}
-
 /* create a regular file with Tlcreate and return file's fid */
 static uint32_t do_lcreate(QVirtio9P *v9p, const char *path,
                            const char *cname)
@@ -587,7 +575,7 @@ static void fs_create_dir(void *obj, void *data, QGuestAllocator *t_alloc)
     g_assert(root_path != NULL);
 
     tattach({ .client = v9p });
-    do_mkdir(v9p, "/", "01");
+    tmkdir({ .client = v9p, .atPath = "/", .name = "01" });
 
     /* check if created directory really exists now ... */
     g_assert(stat(new_dir, &st) == 0);
@@ -606,7 +594,7 @@ static void fs_unlinkat_dir(void *obj, void *data, QGuestAllocator *t_alloc)
     g_assert(root_path != NULL);
 
     tattach({ .client = v9p });
-    do_mkdir(v9p, "/", "02");
+    tmkdir({ .client = v9p, .atPath = "/", .name = "02" });
 
     /* check if created directory really exists now ... */
     g_assert(stat(new_dir, &st) == 0);
@@ -626,7 +614,7 @@ static void fs_create_file(void *obj, void *data, QGuestAllocator *t_alloc)
     g_autofree char *new_file = virtio_9p_test_path("03/1st_file");
 
     tattach({ .client = v9p });
-    do_mkdir(v9p, "/", "03");
+    tmkdir({ .client = v9p, .atPath = "/", .name = "03" });
     do_lcreate(v9p, "03", "1st_file");
 
     /* check if created file exists now ... */
@@ -643,7 +631,7 @@ static void fs_unlinkat_file(void *obj, void *data, QGuestAllocator *t_alloc)
     g_autofree char *new_file = virtio_9p_test_path("04/doa_file");
 
     tattach({ .client = v9p });
-    do_mkdir(v9p, "/", "04");
+    tmkdir({ .client = v9p, .atPath = "/", .name = "04" });
     do_lcreate(v9p, "04", "doa_file");
 
     /* check if created file exists now ... */
@@ -665,7 +653,7 @@ static void fs_symlink_file(void *obj, void *data, QGuestAllocator *t_alloc)
     g_autofree char *symlink_file = virtio_9p_test_path("05/symlink_file");
 
     tattach({ .client = v9p });
-    do_mkdir(v9p, "/", "05");
+    tmkdir({ .client = v9p, .atPath = "/", .name = "05" });
     do_lcreate(v9p, "05", "real_file");
     g_assert(stat(real_file, &st) == 0);
     g_assert((st.st_mode & S_IFMT) == S_IFREG);
@@ -686,7 +674,7 @@ static void fs_unlinkat_symlink(void *obj, void *data,
     g_autofree char *symlink_file = virtio_9p_test_path("06/symlink_file");
 
     tattach({ .client = v9p });
-    do_mkdir(v9p, "/", "06");
+    tmkdir({ .client = v9p, .atPath = "/", .name = "06" });
     do_lcreate(v9p, "06", "real_file");
     g_assert(stat(real_file, &st) == 0);
     g_assert((st.st_mode & S_IFMT) == S_IFREG);
@@ -708,7 +696,7 @@ static void fs_hardlink_file(void *obj, void *data, QGuestAllocator *t_alloc)
     g_autofree char *hardlink_file = virtio_9p_test_path("07/hardlink_file");
 
     tattach({ .client = v9p });
-    do_mkdir(v9p, "/", "07");
+    tmkdir({ .client = v9p, .atPath = "/", .name = "07" });
     do_lcreate(v9p, "07", "real_file");
     g_assert(stat(real_file, &st_real) == 0);
     g_assert((st_real.st_mode & S_IFMT) == S_IFREG);
@@ -733,7 +721,7 @@ static void fs_unlinkat_hardlink(void *obj, void *data,
     g_autofree char *hardlink_file = virtio_9p_test_path("08/hardlink_file");
 
     tattach({ .client = v9p });
-    do_mkdir(v9p, "/", "08");
+    tmkdir({ .client = v9p, .atPath = "/", .name = "08" });
     do_lcreate(v9p, "08", "real_file");
     g_assert(stat(real_file, &st_real) == 0);
     g_assert((st_real.st_mode & S_IFMT) == S_IFREG);
-- 
2.30.2

[PATCH 16/20] tests/9p: merge v9fs_tlcreate() and do_lcreate()

[Christian Schoenebeck]

As with previous patches, unify those 2 functions into a single function
v9fs_tlcreate() by using a declarative function arguments approach.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio-9p-client.c | 45 +++++++++++++++++++++------
 tests/qtest/libqos/virtio-9p-client.h | 39 +++++++++++++++++++++--
 tests/qtest/virtio-9p-test.c          | 30 +++++-------------
 3 files changed, 79 insertions(+), 35 deletions(-)

diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c
index c374ba2048..5c805a133c 100644
--- a/tests/qtest/libqos/virtio-9p-client.c
+++ b/tests/qtest/libqos/virtio-9p-client.c
@@ -827,11 +827,26 @@ void v9fs_rmkdir(P9Req *req, v9fs_qid *qid)
 }
 
 /* size[4] Tlcreate tag[2] fid[4] name[s] flags[4] mode[4] gid[4] */
-P9Req *v9fs_tlcreate(QVirtio9P *v9p, uint32_t fid, const char *name,
-                     uint32_t flags, uint32_t mode, uint32_t gid,
-                     uint16_t tag)
+TlcreateRes v9fs_tlcreate(TlcreateOpt opt)
 {
     P9Req *req;
+    uint32_t err;
+    g_autofree char *name = g_strdup(opt.name);
+
+    g_assert(opt.client);
+    /* expecting either hi-level atPath or low-level fid, but not both */
+    g_assert(!opt.atPath || !opt.fid);
+    /* expecting either Rlcreate or Rlerror, but obviously not both */
+    g_assert(!opt.expectErr || !(opt.rlcreate.qid || opt.rlcreate.iounit));
+
+    if (opt.atPath) {
+        opt.fid = v9fs_twalk((TWalkOpt) { .client = opt.client,
+                                          .path = opt.atPath }).newfid;
+    }
+
+    if (!opt.mode) {
+        opt.mode = 0750;
+    }
 
     uint32_t body_size = 4 + 4 + 4 + 4;
     uint16_t string_size = v9fs_string_size(name);
@@ -839,14 +854,26 @@ P9Req *v9fs_tlcreate(QVirtio9P *v9p, uint32_t fid, const char *name,
     g_assert_cmpint(body_size, <=, UINT32_MAX - string_size);
     body_size += string_size;
 
-    req = v9fs_req_init(v9p, body_size, P9_TLCREATE, tag);
-    v9fs_uint32_write(req, fid);
+    req = v9fs_req_init(opt.client, body_size, P9_TLCREATE, opt.tag);
+    v9fs_uint32_write(req, opt.fid);
     v9fs_string_write(req, name);
-    v9fs_uint32_write(req, flags);
-    v9fs_uint32_write(req, mode);
-    v9fs_uint32_write(req, gid);
+    v9fs_uint32_write(req, opt.flags);
+    v9fs_uint32_write(req, opt.mode);
+    v9fs_uint32_write(req, opt.gid);
     v9fs_req_send(req);
-    return req;
+
+    if (!opt.requestOnly) {
+        v9fs_req_wait_for_reply(req, NULL);
+        if (opt.expectErr) {
+            v9fs_rlerror(req, &err);
+            g_assert_cmpint(err, ==, opt.expectErr);
+        } else {
+            v9fs_rlcreate(req, opt.rlcreate.qid, opt.rlcreate.iounit);
+        }
+        req = NULL; /* request was freed */
+    }
+
+    return (TlcreateRes) { .req = req };
 }
 
 /* size[4] Rlcreate tag[2] qid[13] iounit[4] */
diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h
index ae44f95a4d..8916b1c7aa 100644
--- a/tests/qtest/libqos/virtio-9p-client.h
+++ b/tests/qtest/libqos/virtio-9p-client.h
@@ -320,6 +320,41 @@ typedef struct TMkdirRes {
     P9Req *req;
 } TMkdirRes;
 
+/* options for 'Tlcreate' 9p request */
+typedef struct TlcreateOpt {
+    /* 9P client being used (mandatory) */
+    QVirtio9P *client;
+    /* user supplied tag number being returned with response (optional) */
+    uint16_t tag;
+    /* low-level variant of directory where new file shall be created */
+    uint32_t fid;
+    /* high-level variant of directory where new file shall be created */
+    const char *atPath;
+    /* name of new file (required) */
+    const char *name;
+    /* Linux kernel intent bits */
+    uint32_t flags;
+    /* Linux create(2) mode bits */
+    uint32_t mode;
+    /* effective group ID of caller */
+    uint32_t gid;
+    /* data being received from 9p server as 'Rlcreate' response (optional) */
+    struct {
+        v9fs_qid *qid;
+        uint32_t *iounit;
+    } rlcreate;
+    /* only send Tlcreate request but not wait for a reply? (optional) */
+    bool requestOnly;
+    /* do we expect an Rlerror response, if yes which error code? (optional) */
+    uint32_t expectErr;
+} TlcreateOpt;
+
+/* result of 'Tlcreate' 9p request */
+typedef struct TlcreateRes {
+    /* if requestOnly was set: request object for further processing */
+    P9Req *req;
+} TlcreateRes;
+
 void v9fs_set_allocator(QGuestAllocator *t_alloc);
 void v9fs_memwrite(P9Req *req, const void *addr, size_t len);
 void v9fs_memskip(P9Req *req, size_t len);
@@ -361,9 +396,7 @@ TFlushRes v9fs_tflush(TFlushOpt);
 void v9fs_rflush(P9Req *req);
 TMkdirRes v9fs_tmkdir(TMkdirOpt);
 void v9fs_rmkdir(P9Req *req, v9fs_qid *qid);
-P9Req *v9fs_tlcreate(QVirtio9P *v9p, uint32_t fid, const char *name,
-                     uint32_t flags, uint32_t mode, uint32_t gid,
-                     uint16_t tag);
+TlcreateRes v9fs_tlcreate(TlcreateOpt);
 void v9fs_rlcreate(P9Req *req, v9fs_qid *qid, uint32_t *iounit);
 P9Req *v9fs_tsymlink(QVirtio9P *v9p, uint32_t fid, const char *name,
                      const char *symtgt, uint32_t gid, uint16_t tag);
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 6d75afee87..d13b27bd2e 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -25,6 +25,7 @@
 #define twrite(...) v9fs_twrite((TWriteOpt) __VA_ARGS__)
 #define tflush(...) v9fs_tflush((TFlushOpt) __VA_ARGS__)
 #define tmkdir(...) v9fs_tmkdir((TMkdirOpt) __VA_ARGS__)
+#define tlcreate(...) v9fs_tlcreate((TlcreateOpt) __VA_ARGS__)
 
 static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc)
 {
@@ -478,23 +479,6 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc)
     g_free(wnames[0]);
 }
 
-/* create a regular file with Tlcreate and return file's fid */
-static uint32_t do_lcreate(QVirtio9P *v9p, const char *path,
-                           const char *cname)
-{
-    g_autofree char *name = g_strdup(cname);
-    uint32_t fid;
-    P9Req *req;
-
-    fid = twalk({ .client = v9p, .path = path }).newfid;
-
-    req = v9fs_tlcreate(v9p, fid, name, 0, 0750, 0, 0);
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rlcreate(req, NULL, NULL);
-
-    return fid;
-}
-
 /* create symlink named @a clink in directory @a path pointing to @a to */
 static void do_symlink(QVirtio9P *v9p, const char *path, const char *clink,
                        const char *to)
@@ -615,7 +599,7 @@ static void fs_create_file(void *obj, void *data, QGuestAllocator *t_alloc)
 
     tattach({ .client = v9p });
     tmkdir({ .client = v9p, .atPath = "/", .name = "03" });
-    do_lcreate(v9p, "03", "1st_file");
+    tlcreate({ .client = v9p, .atPath = "03", .name = "1st_file" });
 
     /* check if created file exists now ... */
     g_assert(stat(new_file, &st) == 0);
@@ -632,7 +616,7 @@ static void fs_unlinkat_file(void *obj, void *data, QGuestAllocator *t_alloc)
 
     tattach({ .client = v9p });
     tmkdir({ .client = v9p, .atPath = "/", .name = "04" });
-    do_lcreate(v9p, "04", "doa_file");
+    tlcreate({ .client = v9p, .atPath = "04", .name = "doa_file" });
 
     /* check if created file exists now ... */
     g_assert(stat(new_file, &st) == 0);
@@ -654,7 +638,7 @@ static void fs_symlink_file(void *obj, void *data, QGuestAllocator *t_alloc)
 
     tattach({ .client = v9p });
     tmkdir({ .client = v9p, .atPath = "/", .name = "05" });
-    do_lcreate(v9p, "05", "real_file");
+    tlcreate({ .client = v9p, .atPath = "05", .name = "real_file" });
     g_assert(stat(real_file, &st) == 0);
     g_assert((st.st_mode & S_IFMT) == S_IFREG);
 
@@ -675,7 +659,7 @@ static void fs_unlinkat_symlink(void *obj, void *data,
 
     tattach({ .client = v9p });
     tmkdir({ .client = v9p, .atPath = "/", .name = "06" });
-    do_lcreate(v9p, "06", "real_file");
+    tlcreate({ .client = v9p, .atPath = "06", .name = "real_file" });
     g_assert(stat(real_file, &st) == 0);
     g_assert((st.st_mode & S_IFMT) == S_IFREG);
 
@@ -697,7 +681,7 @@ static void fs_hardlink_file(void *obj, void *data, QGuestAllocator *t_alloc)
 
     tattach({ .client = v9p });
     tmkdir({ .client = v9p, .atPath = "/", .name = "07" });
-    do_lcreate(v9p, "07", "real_file");
+    tlcreate({ .client = v9p, .atPath = "07", .name = "real_file" });
     g_assert(stat(real_file, &st_real) == 0);
     g_assert((st_real.st_mode & S_IFMT) == S_IFREG);
 
@@ -722,7 +706,7 @@ static void fs_unlinkat_hardlink(void *obj, void *data,
 
     tattach({ .client = v9p });
     tmkdir({ .client = v9p, .atPath = "/", .name = "08" });
-    do_lcreate(v9p, "08", "real_file");
+    tlcreate({ .client = v9p, .atPath = "08", .name = "real_file" });
     g_assert(stat(real_file, &st_real) == 0);
     g_assert((st_real.st_mode & S_IFMT) == S_IFREG);
 
-- 
2.30.2

[PATCH 17/20] tests/9p: merge v9fs_tsymlink() and do_symlink()

[Christian Schoenebeck]

As with previous patches, unify those 2 functions into a single function
v9fs_tsymlink() by using a declarative function arguments approach.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio-9p-client.c | 37 ++++++++++++++++++++++-----
 tests/qtest/libqos/virtio-9p-client.h | 35 +++++++++++++++++++++++--
 tests/qtest/virtio-9p-test.c          | 27 +++++++------------
 3 files changed, 73 insertions(+), 26 deletions(-)

diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c
index 5c805a133c..89eaf50355 100644
--- a/tests/qtest/libqos/virtio-9p-client.c
+++ b/tests/qtest/libqos/virtio-9p-client.c
@@ -892,10 +892,23 @@ void v9fs_rlcreate(P9Req *req, v9fs_qid *qid, uint32_t *iounit)
 }
 
 /* size[4] Tsymlink tag[2] fid[4] name[s] symtgt[s] gid[4] */
-P9Req *v9fs_tsymlink(QVirtio9P *v9p, uint32_t fid, const char *name,
-                     const char *symtgt, uint32_t gid, uint16_t tag)
+TsymlinkRes v9fs_tsymlink(TsymlinkOpt opt)
 {
     P9Req *req;
+    uint32_t err;
+    g_autofree char *name = g_strdup(opt.name);
+    g_autofree char *symtgt = g_strdup(opt.symtgt);
+
+    g_assert(opt.client);
+    /* expecting either hi-level atPath or low-level fid, but not both */
+    g_assert(!opt.atPath || !opt.fid);
+    /* expecting either Rsymlink or Rlerror, but obviously not both */
+    g_assert(!opt.expectErr || !opt.rsymlink.qid);
+
+    if (opt.atPath) {
+        opt.fid = v9fs_twalk((TWalkOpt) { .client = opt.client,
+                                          .path = opt.atPath }).newfid;
+    }
 
     uint32_t body_size = 4 + 4;
     uint16_t string_size = v9fs_string_size(name) + v9fs_string_size(symtgt);
@@ -903,13 +916,25 @@ P9Req *v9fs_tsymlink(QVirtio9P *v9p, uint32_t fid, const char *name,
     g_assert_cmpint(body_size, <=, UINT32_MAX - string_size);
     body_size += string_size;
 
-    req = v9fs_req_init(v9p, body_size, P9_TSYMLINK, tag);
-    v9fs_uint32_write(req, fid);
+    req = v9fs_req_init(opt.client, body_size, P9_TSYMLINK, opt.tag);
+    v9fs_uint32_write(req, opt.fid);
     v9fs_string_write(req, name);
     v9fs_string_write(req, symtgt);
-    v9fs_uint32_write(req, gid);
+    v9fs_uint32_write(req, opt.gid);
     v9fs_req_send(req);
-    return req;
+
+    if (!opt.requestOnly) {
+        v9fs_req_wait_for_reply(req, NULL);
+        if (opt.expectErr) {
+            v9fs_rlerror(req, &err);
+            g_assert_cmpint(err, ==, opt.expectErr);
+        } else {
+            v9fs_rsymlink(req, opt.rsymlink.qid);
+        }
+        req = NULL; /* request was freed */
+    }
+
+    return (TsymlinkRes) { .req = req };
 }
 
 /* size[4] Rsymlink tag[2] qid[13] */
diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h
index 8916b1c7aa..b905a54966 100644
--- a/tests/qtest/libqos/virtio-9p-client.h
+++ b/tests/qtest/libqos/virtio-9p-client.h
@@ -355,6 +355,38 @@ typedef struct TlcreateRes {
     P9Req *req;
 } TlcreateRes;
 
+/* options for 'Tsymlink' 9p request */
+typedef struct TsymlinkOpt {
+    /* 9P client being used (mandatory) */
+    QVirtio9P *client;
+    /* user supplied tag number being returned with response (optional) */
+    uint16_t tag;
+    /* low-level variant of directory where symlink shall be created */
+    uint32_t fid;
+    /* high-level variant of directory where symlink shall be created */
+    const char *atPath;
+    /* name of symlink (required) */
+    const char *name;
+    /* where symlink will point to (required) */
+    const char *symtgt;
+    /* effective group ID of caller */
+    uint32_t gid;
+    /* data being received from 9p server as 'Rsymlink' response (optional) */
+    struct {
+        v9fs_qid *qid;
+    } rsymlink;
+    /* only send Tsymlink request but not wait for a reply? (optional) */
+    bool requestOnly;
+    /* do we expect an Rlerror response, if yes which error code? (optional) */
+    uint32_t expectErr;
+} TsymlinkOpt;
+
+/* result of 'Tsymlink' 9p request */
+typedef struct TsymlinkRes {
+    /* if requestOnly was set: request object for further processing */
+    P9Req *req;
+} TsymlinkRes;
+
 void v9fs_set_allocator(QGuestAllocator *t_alloc);
 void v9fs_memwrite(P9Req *req, const void *addr, size_t len);
 void v9fs_memskip(P9Req *req, size_t len);
@@ -398,8 +430,7 @@ TMkdirRes v9fs_tmkdir(TMkdirOpt);
 void v9fs_rmkdir(P9Req *req, v9fs_qid *qid);
 TlcreateRes v9fs_tlcreate(TlcreateOpt);
 void v9fs_rlcreate(P9Req *req, v9fs_qid *qid, uint32_t *iounit);
-P9Req *v9fs_tsymlink(QVirtio9P *v9p, uint32_t fid, const char *name,
-                     const char *symtgt, uint32_t gid, uint16_t tag);
+TsymlinkRes v9fs_tsymlink(TsymlinkOpt);
 void v9fs_rsymlink(P9Req *req, v9fs_qid *qid);
 P9Req *v9fs_tlink(QVirtio9P *v9p, uint32_t dfid, uint32_t fid,
                   const char *name, uint16_t tag);
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index d13b27bd2e..c7213d6caf 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -26,6 +26,7 @@
 #define tflush(...) v9fs_tflush((TFlushOpt) __VA_ARGS__)
 #define tmkdir(...) v9fs_tmkdir((TMkdirOpt) __VA_ARGS__)
 #define tlcreate(...) v9fs_tlcreate((TlcreateOpt) __VA_ARGS__)
+#define tsymlink(...) v9fs_tsymlink((TsymlinkOpt) __VA_ARGS__)
 
 static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc)
 {
@@ -479,22 +480,6 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc)
     g_free(wnames[0]);
 }
 
-/* create symlink named @a clink in directory @a path pointing to @a to */
-static void do_symlink(QVirtio9P *v9p, const char *path, const char *clink,
-                       const char *to)
-{
-    g_autofree char *name = g_strdup(clink);
-    g_autofree char *dst = g_strdup(to);
-    uint32_t fid;
-    P9Req *req;
-
-    fid = twalk({ .client = v9p, .path = path }).newfid;
-
-    req = v9fs_tsymlink(v9p, fid, name, dst, 0, 0);
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rsymlink(req, NULL);
-}
-
 /* create a hard link named @a clink in directory @a path pointing to @a to */
 static void do_hardlink(QVirtio9P *v9p, const char *path, const char *clink,
                         const char *to)
@@ -642,7 +627,10 @@ static void fs_symlink_file(void *obj, void *data, QGuestAllocator *t_alloc)
     g_assert(stat(real_file, &st) == 0);
     g_assert((st.st_mode & S_IFMT) == S_IFREG);
 
-    do_symlink(v9p, "05", "symlink_file", "real_file");
+    tsymlink({
+        .client = v9p, .atPath = "05", .name = "symlink_file",
+        .symtgt = "real_file"
+    });
 
     /* check if created link exists now */
     g_assert(stat(symlink_file, &st) == 0);
@@ -663,7 +651,10 @@ static void fs_unlinkat_symlink(void *obj, void *data,
     g_assert(stat(real_file, &st) == 0);
     g_assert((st.st_mode & S_IFMT) == S_IFREG);
 
-    do_symlink(v9p, "06", "symlink_file", "real_file");
+    tsymlink({
+        .client = v9p, .atPath = "06", .name = "symlink_file",
+        .symtgt = "real_file"
+    });
     g_assert(stat(symlink_file, &st) == 0);
 
     do_unlinkat(v9p, "06", "symlink_file", 0);
-- 
2.30.2

[PATCH 18/20] tests/9p: merge v9fs_tlink() and do_hardlink()

[Christian Schoenebeck]

As with previous patches, unify those 2 functions into a single function
v9fs_tlink() by using a declarative function arguments approach.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio-9p-client.c | 43 ++++++++++++++++++++++-----
 tests/qtest/libqos/virtio-9p-client.h | 31 +++++++++++++++++--
 tests/qtest/virtio-9p-test.c          | 26 ++++++----------
 3 files changed, 73 insertions(+), 27 deletions(-)

diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c
index 89eaf50355..a2770719b9 100644
--- a/tests/qtest/libqos/virtio-9p-client.c
+++ b/tests/qtest/libqos/virtio-9p-client.c
@@ -950,23 +950,50 @@ void v9fs_rsymlink(P9Req *req, v9fs_qid *qid)
 }
 
 /* size[4] Tlink tag[2] dfid[4] fid[4] name[s] */
-P9Req *v9fs_tlink(QVirtio9P *v9p, uint32_t dfid, uint32_t fid,
-                  const char *name, uint16_t tag)
+TlinkRes v9fs_tlink(TlinkOpt opt)
 {
     P9Req *req;
+    uint32_t err;
+
+    g_assert(opt.client);
+    /* expecting either hi-level atPath or low-level dfid, but not both */
+    g_assert(!opt.atPath || !opt.dfid);
+    /* expecting either hi-level toPath or low-level fid, but not both */
+    g_assert(!opt.toPath || !opt.fid);
+
+    if (opt.atPath) {
+        opt.dfid = v9fs_twalk((TWalkOpt) { .client = opt.client,
+                                           .path = opt.atPath }).newfid;
+    }
+    if (opt.toPath) {
+        opt.fid = v9fs_twalk((TWalkOpt) { .client = opt.client,
+                                          .path = opt.toPath }).newfid;
+    }
 
     uint32_t body_size = 4 + 4;
-    uint16_t string_size = v9fs_string_size(name);
+    uint16_t string_size = v9fs_string_size(opt.name);
 
     g_assert_cmpint(body_size, <=, UINT32_MAX - string_size);
     body_size += string_size;
 
-    req = v9fs_req_init(v9p, body_size, P9_TLINK, tag);
-    v9fs_uint32_write(req, dfid);
-    v9fs_uint32_write(req, fid);
-    v9fs_string_write(req, name);
+    req = v9fs_req_init(opt.client, body_size, P9_TLINK, opt.tag);
+    v9fs_uint32_write(req, opt.dfid);
+    v9fs_uint32_write(req, opt.fid);
+    v9fs_string_write(req, opt.name);
     v9fs_req_send(req);
-    return req;
+
+    if (!opt.requestOnly) {
+        v9fs_req_wait_for_reply(req, NULL);
+        if (opt.expectErr) {
+            v9fs_rlerror(req, &err);
+            g_assert_cmpint(err, ==, opt.expectErr);
+        } else {
+            v9fs_rlink(req);
+        }
+        req = NULL; /* request was freed */
+    }
+
+    return (TlinkRes) { .req = req };
 }
 
 /* size[4] Rlink tag[2] */
diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h
index b905a54966..49ffd0fc51 100644
--- a/tests/qtest/libqos/virtio-9p-client.h
+++ b/tests/qtest/libqos/virtio-9p-client.h
@@ -387,6 +387,34 @@ typedef struct TsymlinkRes {
     P9Req *req;
 } TsymlinkRes;
 
+/* options for 'Tlink' 9p request */
+typedef struct TlinkOpt {
+    /* 9P client being used (mandatory) */
+    QVirtio9P *client;
+    /* user supplied tag number being returned with response (optional) */
+    uint16_t tag;
+    /* low-level variant of directory where hard link shall be created */
+    uint32_t dfid;
+    /* high-level variant of directory where hard link shall be created */
+    const char *atPath;
+    /* low-level variant of target referenced by new hard link */
+    uint32_t fid;
+    /* high-level variant of target referenced by new hard link */
+    const char *toPath;
+    /* name of hard link (required) */
+    const char *name;
+    /* only send Tlink request but not wait for a reply? (optional) */
+    bool requestOnly;
+    /* do we expect an Rlerror response, if yes which error code? (optional) */
+    uint32_t expectErr;
+} TlinkOpt;
+
+/* result of 'Tlink' 9p request */
+typedef struct TlinkRes {
+    /* if requestOnly was set: request object for further processing */
+    P9Req *req;
+} TlinkRes;
+
 void v9fs_set_allocator(QGuestAllocator *t_alloc);
 void v9fs_memwrite(P9Req *req, const void *addr, size_t len);
 void v9fs_memskip(P9Req *req, size_t len);
@@ -432,8 +460,7 @@ TlcreateRes v9fs_tlcreate(TlcreateOpt);
 void v9fs_rlcreate(P9Req *req, v9fs_qid *qid, uint32_t *iounit);
 TsymlinkRes v9fs_tsymlink(TsymlinkOpt);
 void v9fs_rsymlink(P9Req *req, v9fs_qid *qid);
-P9Req *v9fs_tlink(QVirtio9P *v9p, uint32_t dfid, uint32_t fid,
-                  const char *name, uint16_t tag);
+TlinkRes v9fs_tlink(TlinkOpt);
 void v9fs_rlink(P9Req *req);
 P9Req *v9fs_tunlinkat(QVirtio9P *v9p, uint32_t dirfd, const char *name,
                       uint32_t flags, uint16_t tag);
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index c7213d6caf..185eaf8b1e 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -27,6 +27,7 @@
 #define tmkdir(...) v9fs_tmkdir((TMkdirOpt) __VA_ARGS__)
 #define tlcreate(...) v9fs_tlcreate((TlcreateOpt) __VA_ARGS__)
 #define tsymlink(...) v9fs_tsymlink((TsymlinkOpt) __VA_ARGS__)
+#define tlink(...) v9fs_tlink((TlinkOpt) __VA_ARGS__)
 
 static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc)
 {
@@ -480,21 +481,6 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc)
     g_free(wnames[0]);
 }
 
-/* create a hard link named @a clink in directory @a path pointing to @a to */
-static void do_hardlink(QVirtio9P *v9p, const char *path, const char *clink,
-                        const char *to)
-{
-    uint32_t dfid, fid;
-    P9Req *req;
-
-    dfid = twalk({ .client = v9p, .path = path }).newfid;
-    fid = twalk({ .client = v9p, .path = to }).newfid;
-
-    req = v9fs_tlink(v9p, dfid, fid, clink, 0);
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_rlink(req);
-}
-
 static void do_unlinkat(QVirtio9P *v9p, const char *atpath, const char *rpath,
                         uint32_t flags)
 {
@@ -676,7 +662,10 @@ static void fs_hardlink_file(void *obj, void *data, QGuestAllocator *t_alloc)
     g_assert(stat(real_file, &st_real) == 0);
     g_assert((st_real.st_mode & S_IFMT) == S_IFREG);
 
-    do_hardlink(v9p, "07", "hardlink_file", "07/real_file");
+    tlink({
+        .client = v9p, .atPath = "07", .name = "hardlink_file",
+        .toPath = "07/real_file"
+    });
 
     /* check if link exists now ... */
     g_assert(stat(hardlink_file, &st_link) == 0);
@@ -701,7 +690,10 @@ static void fs_unlinkat_hardlink(void *obj, void *data,
     g_assert(stat(real_file, &st_real) == 0);
     g_assert((st_real.st_mode & S_IFMT) == S_IFREG);
 
-    do_hardlink(v9p, "08", "hardlink_file", "08/real_file");
+    tlink({
+        .client = v9p, .atPath = "08", .name = "hardlink_file",
+        .toPath = "08/real_file"
+    });
     g_assert(stat(hardlink_file, &st_link) == 0);
 
     do_unlinkat(v9p, "08", "hardlink_file", 0);
-- 
2.30.2

[PATCH 19/20] tests/9p: merge v9fs_tunlinkat() and do_unlinkat()

[Christian Schoenebeck]

As with previous patches, unify those 2 functions into a single function
v9fs_tunlinkat() by using a declarative function arguments approach.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio-9p-client.c | 37 +++++++++++++++++++++------
 tests/qtest/libqos/virtio-9p-client.h | 29 +++++++++++++++++++--
 tests/qtest/virtio-9p-test.c          | 26 ++++++-------------
 3 files changed, 64 insertions(+), 28 deletions(-)

diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c
index a2770719b9..e017e030ec 100644
--- a/tests/qtest/libqos/virtio-9p-client.c
+++ b/tests/qtest/libqos/virtio-9p-client.c
@@ -1004,23 +1004,44 @@ void v9fs_rlink(P9Req *req)
 }
 
 /* size[4] Tunlinkat tag[2] dirfd[4] name[s] flags[4] */
-P9Req *v9fs_tunlinkat(QVirtio9P *v9p, uint32_t dirfd, const char *name,
-                      uint32_t flags, uint16_t tag)
+TunlinkatRes v9fs_tunlinkat(TunlinkatOpt opt)
 {
     P9Req *req;
+    uint32_t err;
+
+    g_assert(opt.client);
+    /* expecting either hi-level atPath or low-level dirfd, but not both */
+    g_assert(!opt.atPath || !opt.dirfd);
+
+    if (opt.atPath) {
+        opt.dirfd = v9fs_twalk((TWalkOpt) { .client = opt.client,
+                                            .path = opt.atPath }).newfid;
+    }
 
     uint32_t body_size = 4 + 4;
-    uint16_t string_size = v9fs_string_size(name);
+    uint16_t string_size = v9fs_string_size(opt.name);
 
     g_assert_cmpint(body_size, <=, UINT32_MAX - string_size);
     body_size += string_size;
 
-    req = v9fs_req_init(v9p, body_size, P9_TUNLINKAT, tag);
-    v9fs_uint32_write(req, dirfd);
-    v9fs_string_write(req, name);
-    v9fs_uint32_write(req, flags);
+    req = v9fs_req_init(opt.client, body_size, P9_TUNLINKAT, opt.tag);
+    v9fs_uint32_write(req, opt.dirfd);
+    v9fs_string_write(req, opt.name);
+    v9fs_uint32_write(req, opt.flags);
     v9fs_req_send(req);
-    return req;
+
+    if (!opt.requestOnly) {
+        v9fs_req_wait_for_reply(req, NULL);
+        if (opt.expectErr) {
+            v9fs_rlerror(req, &err);
+            g_assert_cmpint(err, ==, opt.expectErr);
+        } else {
+            v9fs_runlinkat(req);
+        }
+        req = NULL; /* request was freed */
+    }
+
+    return (TunlinkatRes) { .req = req };
 }
 
 /* size[4] Runlinkat tag[2] */
diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h
index 49ffd0fc51..78228eb97d 100644
--- a/tests/qtest/libqos/virtio-9p-client.h
+++ b/tests/qtest/libqos/virtio-9p-client.h
@@ -415,6 +415,32 @@ typedef struct TlinkRes {
     P9Req *req;
 } TlinkRes;
 
+/* options for 'Tunlinkat' 9p request */
+typedef struct TunlinkatOpt {
+    /* 9P client being used (mandatory) */
+    QVirtio9P *client;
+    /* user supplied tag number being returned with response (optional) */
+    uint16_t tag;
+    /* low-level variant of directory where name shall be unlinked */
+    uint32_t dirfd;
+    /* high-level variant of directory where name shall be unlinked */
+    const char *atPath;
+    /* name of directory entry to be unlinked (required) */
+    const char *name;
+    /* Linux unlinkat(2) flags */
+    uint32_t flags;
+    /* only send Tunlinkat request but not wait for a reply? (optional) */
+    bool requestOnly;
+    /* do we expect an Rlerror response, if yes which error code? (optional) */
+    uint32_t expectErr;
+} TunlinkatOpt;
+
+/* result of 'Tunlinkat' 9p request */
+typedef struct TunlinkatRes {
+    /* if requestOnly was set: request object for further processing */
+    P9Req *req;
+} TunlinkatRes;
+
 void v9fs_set_allocator(QGuestAllocator *t_alloc);
 void v9fs_memwrite(P9Req *req, const void *addr, size_t len);
 void v9fs_memskip(P9Req *req, size_t len);
@@ -462,8 +488,7 @@ TsymlinkRes v9fs_tsymlink(TsymlinkOpt);
 void v9fs_rsymlink(P9Req *req, v9fs_qid *qid);
 TlinkRes v9fs_tlink(TlinkOpt);
 void v9fs_rlink(P9Req *req);
-P9Req *v9fs_tunlinkat(QVirtio9P *v9p, uint32_t dirfd, const char *name,
-                      uint32_t flags, uint16_t tag);
+TunlinkatRes v9fs_tunlinkat(TunlinkatOpt);
 void v9fs_runlinkat(P9Req *req);
 
 #endif
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 185eaf8b1e..65e69491e5 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -28,6 +28,7 @@
 #define tlcreate(...) v9fs_tlcreate((TlcreateOpt) __VA_ARGS__)
 #define tsymlink(...) v9fs_tsymlink((TsymlinkOpt) __VA_ARGS__)
 #define tlink(...) v9fs_tlink((TlinkOpt) __VA_ARGS__)
+#define tunlinkat(...) v9fs_tunlinkat((TunlinkatOpt) __VA_ARGS__)
 
 static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc)
 {
@@ -481,20 +482,6 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc)
     g_free(wnames[0]);
 }
 
-static void do_unlinkat(QVirtio9P *v9p, const char *atpath, const char *rpath,
-                        uint32_t flags)
-{
-    g_autofree char *name = g_strdup(rpath);
-    uint32_t fid;
-    P9Req *req;
-
-    fid = twalk({ .client = v9p, .path = atpath }).newfid;
-
-    req = v9fs_tunlinkat(v9p, fid, name, flags, 0);
-    v9fs_req_wait_for_reply(req, NULL);
-    v9fs_runlinkat(req);
-}
-
 static void fs_readdir_split_128(void *obj, void *data,
                                  QGuestAllocator *t_alloc)
 {
@@ -556,7 +543,10 @@ static void fs_unlinkat_dir(void *obj, void *data, QGuestAllocator *t_alloc)
     /* ... and is actually a directory */
     g_assert((st.st_mode & S_IFMT) == S_IFDIR);
 
-    do_unlinkat(v9p, "/", "02", P9_DOTL_AT_REMOVEDIR);
+    tunlinkat({
+        .client = v9p, .atPath = "/", .name = "02",
+        .flags = P9_DOTL_AT_REMOVEDIR
+    });
     /* directory should be gone now */
     g_assert(stat(new_dir, &st) != 0);
 }
@@ -594,7 +584,7 @@ static void fs_unlinkat_file(void *obj, void *data, QGuestAllocator *t_alloc)
     /* ... and is a regular file */
     g_assert((st.st_mode & S_IFMT) == S_IFREG);
 
-    do_unlinkat(v9p, "04", "doa_file", 0);
+    tunlinkat({ .client = v9p, .atPath = "04", .name = "doa_file" });
     /* file should be gone now */
     g_assert(stat(new_file, &st) != 0);
 }
@@ -643,7 +633,7 @@ static void fs_unlinkat_symlink(void *obj, void *data,
     });
     g_assert(stat(symlink_file, &st) == 0);
 
-    do_unlinkat(v9p, "06", "symlink_file", 0);
+    tunlinkat({ .client = v9p, .atPath = "06", .name = "symlink_file" });
     /* symlink should be gone now */
     g_assert(stat(symlink_file, &st) != 0);
 }
@@ -696,7 +686,7 @@ static void fs_unlinkat_hardlink(void *obj, void *data,
     });
     g_assert(stat(hardlink_file, &st_link) == 0);
 
-    do_unlinkat(v9p, "08", "hardlink_file", 0);
+    tunlinkat({ .client = v9p, .atPath = "08", .name = "hardlink_file" });
     /* symlink should be gone now */
     g_assert(stat(hardlink_file, &st_link) != 0);
     /* and old file should still exist */
-- 
2.30.2

[PATCH 20/20] tests/9p: remove unnecessary g_strdup() calls

[Christian Schoenebeck]

This is a leftover from before the recent function merge and
refactoring patches:

As these functions do not return control to the caller in
between, it is not necessary to duplicate strings passed to them.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio-9p-client.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c
index e017e030ec..e4a368e036 100644
--- a/tests/qtest/libqos/virtio-9p-client.c
+++ b/tests/qtest/libqos/virtio-9p-client.c
@@ -770,7 +770,6 @@ TMkdirRes v9fs_tmkdir(TMkdirOpt opt)
 {
     P9Req *req;
     uint32_t err;
-    g_autofree char *name = g_strdup(opt.name);
 
     g_assert(opt.client);
     /* expecting either hi-level atPath or low-level dfid, but not both */
@@ -788,14 +787,14 @@ TMkdirRes v9fs_tmkdir(TMkdirOpt opt)
     }
 
     uint32_t body_size = 4 + 4 + 4;
-    uint16_t string_size = v9fs_string_size(name);
+    uint16_t string_size = v9fs_string_size(opt.name);
 
     g_assert_cmpint(body_size, <=, UINT32_MAX - string_size);
     body_size += string_size;
 
     req = v9fs_req_init(opt.client, body_size, P9_TMKDIR, opt.tag);
     v9fs_uint32_write(req, opt.dfid);
-    v9fs_string_write(req, name);
+    v9fs_string_write(req, opt.name);
     v9fs_uint32_write(req, opt.mode);
     v9fs_uint32_write(req, opt.gid);
     v9fs_req_send(req);
@@ -831,7 +830,6 @@ TlcreateRes v9fs_tlcreate(TlcreateOpt opt)
 {
     P9Req *req;
     uint32_t err;
-    g_autofree char *name = g_strdup(opt.name);
 
     g_assert(opt.client);
     /* expecting either hi-level atPath or low-level fid, but not both */
@@ -849,14 +847,14 @@ TlcreateRes v9fs_tlcreate(TlcreateOpt opt)
     }
 
     uint32_t body_size = 4 + 4 + 4 + 4;
-    uint16_t string_size = v9fs_string_size(name);
+    uint16_t string_size = v9fs_string_size(opt.name);
 
     g_assert_cmpint(body_size, <=, UINT32_MAX - string_size);
     body_size += string_size;
 
     req = v9fs_req_init(opt.client, body_size, P9_TLCREATE, opt.tag);
     v9fs_uint32_write(req, opt.fid);
-    v9fs_string_write(req, name);
+    v9fs_string_write(req, opt.name);
     v9fs_uint32_write(req, opt.flags);
     v9fs_uint32_write(req, opt.mode);
     v9fs_uint32_write(req, opt.gid);
@@ -896,8 +894,6 @@ TsymlinkRes v9fs_tsymlink(TsymlinkOpt opt)
 {
     P9Req *req;
     uint32_t err;
-    g_autofree char *name = g_strdup(opt.name);
-    g_autofree char *symtgt = g_strdup(opt.symtgt);
 
     g_assert(opt.client);
     /* expecting either hi-level atPath or low-level fid, but not both */
@@ -911,15 +907,16 @@ TsymlinkRes v9fs_tsymlink(TsymlinkOpt opt)
     }
 
     uint32_t body_size = 4 + 4;
-    uint16_t string_size = v9fs_string_size(name) + v9fs_string_size(symtgt);
+    uint16_t string_size = v9fs_string_size(opt.name) +
+                           v9fs_string_size(opt.symtgt);
 
     g_assert_cmpint(body_size, <=, UINT32_MAX - string_size);
     body_size += string_size;
 
     req = v9fs_req_init(opt.client, body_size, P9_TSYMLINK, opt.tag);
     v9fs_uint32_write(req, opt.fid);
-    v9fs_string_write(req, name);
-    v9fs_string_write(req, symtgt);
+    v9fs_string_write(req, opt.name);
+    v9fs_string_write(req, opt.symtgt);
     v9fs_uint32_write(req, opt.gid);
     v9fs_req_send(req);
 
-- 
2.30.2

[PATCH 00/20] tests/9p: introduce declarative function calls

[Christian Schoenebeck]

This series converts relevant 9p (test) client functions to use named
function arguments. For instance

    do_walk_expect_error(v9p, "non-existent", ENOENT);

becomes

    twalk({
        .client = v9p, .path = "non-existent", .expectErr = ENOENT
    });

The intention is to make the actual 9p test code more readable, and easier
to maintain on the long-term.

Not only makes it clear what a literal passed to a function is supposed to
do, it also makes the order and selection of arguments very liberal, and
allows to merge multiple, similar functions into one single function.

This is basically just refactoring, it does not change behaviour.

PREREQUISITES
=============

This series requires the following additional patch to work correctly:

https://lore.kernel.org/all/E1odrya-0004Fv-97@lizzy.crudebyte.com/
https://github.com/cschoenebeck/qemu/commit/23d01367fc7a4f27be323ed6d195c527bec9ede1

Christian Schoenebeck (20):
  tests/9p: merge *walk*() functions
  tests/9p: simplify callers of twalk()
  tests/9p: merge v9fs_tversion() and do_version()
  tests/9p: merge v9fs_tattach(), do_attach(), do_attach_rqid()
  tests/9p: simplify callers of tattach()
  tests/9p: convert v9fs_tgetattr() to declarative arguments
  tests/9p: simplify callers of tgetattr()
  tests/9p: convert v9fs_treaddir() to declarative arguments
  tests/9p: simplify callers of treaddir()
  tests/9p: convert v9fs_tlopen() to declarative arguments
  tests/9p: simplify callers of tlopen()
  tests/9p: convert v9fs_twrite() to declarative arguments
  tests/9p: simplify callers of twrite()
  tests/9p: convert v9fs_tflush() to declarative arguments
  tests/9p: merge v9fs_tmkdir() and do_mkdir()
  tests/9p: merge v9fs_tlcreate() and do_lcreate()
  tests/9p: merge v9fs_tsymlink() and do_symlink()
  tests/9p: merge v9fs_tlink() and do_hardlink()
  tests/9p: merge v9fs_tunlinkat() and do_unlinkat()
  tests/9p: remove unnecessary g_strdup() calls

 tests/qtest/libqos/virtio-9p-client.c | 569 +++++++++++++++++++++-----
 tests/qtest/libqos/virtio-9p-client.h | 408 ++++++++++++++++--
 tests/qtest/virtio-9p-test.c          | 529 ++++++++----------------
 3 files changed, 1031 insertions(+), 475 deletions(-)

-- 
2.30.2

Re: [PATCH 00/20] tests/9p: introduce declarative function calls

[Christian Schoenebeck]

On Dienstag, 4. Oktober 2022 22:56:44 CEST Christian Schoenebeck wrote:
> This series converts relevant 9p (test) client functions to use named
> function arguments. For instance
> 
>     do_walk_expect_error(v9p, "non-existent", ENOENT);
> 
> becomes
> 
>     twalk({
>         .client = v9p, .path = "non-existent", .expectErr = ENOENT
>     });
> 
> The intention is to make the actual 9p test code more readable, and easier
> to maintain on the long-term.
> 
> Not only makes it clear what a literal passed to a function is supposed to
> do, it also makes the order and selection of arguments very liberal, and
> allows to merge multiple, similar functions into one single function.
> 
> This is basically just refactoring, it does not change behaviour.

Too massive for review?

If so, then I'll probably just go ahead and prepare a PR early next week with 
this queued as well. It's just test code refactoring, so I am quite painless 
about these changes.

Best regards,
Christian Schoenebeck

> 
> PREREQUISITES
> =============
> 
> This series requires the following additional patch to work correctly:
> 
> https://lore.kernel.org/all/E1odrya-0004Fv-97@lizzy.crudebyte.com/
> https://github.com/cschoenebeck/qemu/commit/23d01367fc7a4f27be323ed6d195c527
> bec9ede1
> 
> Christian Schoenebeck (20):
>   tests/9p: merge *walk*() functions
>   tests/9p: simplify callers of twalk()
>   tests/9p: merge v9fs_tversion() and do_version()
>   tests/9p: merge v9fs_tattach(), do_attach(), do_attach_rqid()
>   tests/9p: simplify callers of tattach()
>   tests/9p: convert v9fs_tgetattr() to declarative arguments
>   tests/9p: simplify callers of tgetattr()
>   tests/9p: convert v9fs_treaddir() to declarative arguments
>   tests/9p: simplify callers of treaddir()
>   tests/9p: convert v9fs_tlopen() to declarative arguments
>   tests/9p: simplify callers of tlopen()
>   tests/9p: convert v9fs_twrite() to declarative arguments
>   tests/9p: simplify callers of twrite()
>   tests/9p: convert v9fs_tflush() to declarative arguments
>   tests/9p: merge v9fs_tmkdir() and do_mkdir()
>   tests/9p: merge v9fs_tlcreate() and do_lcreate()
>   tests/9p: merge v9fs_tsymlink() and do_symlink()
>   tests/9p: merge v9fs_tlink() and do_hardlink()
>   tests/9p: merge v9fs_tunlinkat() and do_unlinkat()
>   tests/9p: remove unnecessary g_strdup() calls
> 
>  tests/qtest/libqos/virtio-9p-client.c | 569 +++++++++++++++++++++-----
>  tests/qtest/libqos/virtio-9p-client.h | 408 ++++++++++++++++--
>  tests/qtest/virtio-9p-test.c          | 529 ++++++++----------------
>  3 files changed, 1031 insertions(+), 475 deletions(-)

Re: [PATCH 00/20] tests/9p: introduce declarative function calls

[Greg Kurz]

On Wed, 12 Oct 2022 12:00:40 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Dienstag, 4. Oktober 2022 22:56:44 CEST Christian Schoenebeck wrote:
> > This series converts relevant 9p (test) client functions to use named
> > function arguments. For instance
> > 
> >     do_walk_expect_error(v9p, "non-existent", ENOENT);
> > 
> > becomes
> > 
> >     twalk({
> >         .client = v9p, .path = "non-existent", .expectErr = ENOENT
> >     });
> > 
> > The intention is to make the actual 9p test code more readable, and easier
> > to maintain on the long-term.
> > 
> > Not only makes it clear what a literal passed to a function is supposed to
> > do, it also makes the order and selection of arguments very liberal, and
> > allows to merge multiple, similar functions into one single function.
> > 
> > This is basically just refactoring, it does not change behaviour.
> 
> Too massive for review?
> 

Yeah, sorry :-(

But since the approach you're taking here may be valuable elsewhere,
and this is qtest, it seems fair to ask Thomas and Laurent to have
a look :-)

> If so, then I'll probably just go ahead and prepare a PR early next week with 
> this queued as well. It's just test code refactoring, so I am quite painless 
> about these changes.
> 
> Best regards,
> Christian Schoenebeck
> 
> > 
> > PREREQUISITES
> > =============
> > 
> > This series requires the following additional patch to work correctly:
> > 
> > https://lore.kernel.org/all/E1odrya-0004Fv-97@lizzy.crudebyte.com/
> > https://github.com/cschoenebeck/qemu/commit/23d01367fc7a4f27be323ed6d195c527
> > bec9ede1
> > 
> > Christian Schoenebeck (20):
> >   tests/9p: merge *walk*() functions
> >   tests/9p: simplify callers of twalk()
> >   tests/9p: merge v9fs_tversion() and do_version()
> >   tests/9p: merge v9fs_tattach(), do_attach(), do_attach_rqid()
> >   tests/9p: simplify callers of tattach()
> >   tests/9p: convert v9fs_tgetattr() to declarative arguments
> >   tests/9p: simplify callers of tgetattr()
> >   tests/9p: convert v9fs_treaddir() to declarative arguments
> >   tests/9p: simplify callers of treaddir()
> >   tests/9p: convert v9fs_tlopen() to declarative arguments
> >   tests/9p: simplify callers of tlopen()
> >   tests/9p: convert v9fs_twrite() to declarative arguments
> >   tests/9p: simplify callers of twrite()
> >   tests/9p: convert v9fs_tflush() to declarative arguments
> >   tests/9p: merge v9fs_tmkdir() and do_mkdir()
> >   tests/9p: merge v9fs_tlcreate() and do_lcreate()
> >   tests/9p: merge v9fs_tsymlink() and do_symlink()
> >   tests/9p: merge v9fs_tlink() and do_hardlink()
> >   tests/9p: merge v9fs_tunlinkat() and do_unlinkat()
> >   tests/9p: remove unnecessary g_strdup() calls
> > 
> >  tests/qtest/libqos/virtio-9p-client.c | 569 +++++++++++++++++++++-----
> >  tests/qtest/libqos/virtio-9p-client.h | 408 ++++++++++++++++--
> >  tests/qtest/virtio-9p-test.c          | 529 ++++++++----------------
> >  3 files changed, 1031 insertions(+), 475 deletions(-)
> 
> 
> 

Re: [PATCH 00/20] tests/9p: introduce declarative function calls

[Christian Schoenebeck]

On Mittwoch, 12. Oktober 2022 15:58:06 CEST Greg Kurz wrote:
> On Wed, 12 Oct 2022 12:00:40 +0200
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> 
> > On Dienstag, 4. Oktober 2022 22:56:44 CEST Christian Schoenebeck wrote:
> > > This series converts relevant 9p (test) client functions to use named
> > > function arguments. For instance
> > > 
> > >     do_walk_expect_error(v9p, "non-existent", ENOENT);
> > > 
> > > becomes
> > > 
> > >     twalk({
> > >         .client = v9p, .path = "non-existent", .expectErr = ENOENT
> > >     });
> > > 
> > > The intention is to make the actual 9p test code more readable, and 
easier
> > > to maintain on the long-term.
> > > 
> > > Not only makes it clear what a literal passed to a function is supposed 
to
> > > do, it also makes the order and selection of arguments very liberal, and
> > > allows to merge multiple, similar functions into one single function.
> > > 
> > > This is basically just refactoring, it does not change behaviour.
> > 
> > Too massive for review?
> > 
> 
> Yeah, sorry :-(
> 
> But since the approach you're taking here may be valuable elsewhere,
> and this is qtest, it seems fair to ask Thomas and Laurent to have
> a look :-)

Thomas, Laurent, if you had some spare cycles to look at this, that would be 
much appreciated!

Otherwise if not possible then I'll go ahead with PR next week as said.

Best regards,
Christian Schoenebeck

Re: [PATCH 00/20] tests/9p: introduce declarative function calls

[Christian Schoenebeck]

On Tuesday, October 4, 2022 10:56:44 PM CEST Christian Schoenebeck wrote:
> This series converts relevant 9p (test) client functions to use named
> function arguments. For instance
> 
>     do_walk_expect_error(v9p, "non-existent", ENOENT);
> 
> becomes
> 
>     twalk({
>         .client = v9p, .path = "non-existent", .expectErr = ENOENT
>     });
> 
> The intention is to make the actual 9p test code more readable, and easier
> to maintain on the long-term.
> 
> Not only makes it clear what a literal passed to a function is supposed to
> do, it also makes the order and selection of arguments very liberal, and
> allows to merge multiple, similar functions into one single function.
> 
> This is basically just refactoring, it does not change behaviour.

Queued on 9p.next:
https://github.com/cschoenebeck/qemu/commits/9p.next

Thanks!

Next 9p PR end of this week.

Best regards,
Christian Schoenebeck