[PULL 01/13] tests/9pfs: Factor out do_version() helper

[PULL 01/13] tests/9pfs: Factor out do_version() helper

[Greg Kurz]

fs_version() is a top level test function. Factor out the reusable
code to a separate helper instead of hijacking it in other tests.

Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <160321015403.266767.4533967728943968456.stgit@bahia.lan>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/virtio-9p-test.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index c15908f27b..59bcea4c30 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -567,10 +567,8 @@ static void v9fs_rflush(P9Req *req)
     v9fs_req_free(req);
 }
 
-static void fs_version(void *obj, void *data, QGuestAllocator *t_alloc)
+static void do_version(QVirtio9P *v9p)
 {
-    QVirtio9P *v9p = obj;
-    alloc = t_alloc;
     const char *version = "9P2000.L";
     uint16_t server_len;
     char *server_version;
@@ -585,13 +583,19 @@ static void fs_version(void *obj, void *data, QGuestAllocator *t_alloc)
     g_free(server_version);
 }
 
+static void fs_version(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+    alloc = t_alloc;
+    do_version(obj);
+}
+
 static void fs_attach(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     QVirtio9P *v9p = obj;
     alloc = t_alloc;
     P9Req *req;
 
-    fs_version(v9p, NULL, t_alloc);
+    do_version(v9p);
     req = v9fs_tattach(v9p, 0, getuid(), 0);
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rattach(req, NULL);
@@ -831,7 +835,7 @@ static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc)
     v9fs_qid root_qid, *wqid;
     P9Req *req;
 
-    fs_version(v9p, NULL, t_alloc);
+    do_version(v9p);
     req = v9fs_tattach(v9p, 0, getuid(), 0);
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rattach(req, &root_qid);
-- 
2.20.1

[PULL 04/13] tests/9pfs: Turn fs_readdir_split() into a helper

[Greg Kurz]

fs_readdir_split() isn't a top level test function and thus shouldn't
take the "void *obj, void *data, QGuestAllocator *t_alloc" arguments.
Turn it into a helper to be used by test functions.

Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <160321016084.266767.9501523425012383531.stgit@bahia.lan>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/virtio-9p-test.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index e07292bdb8..3c187cdc08 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -731,11 +731,8 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
 }
 
 /* readdir test where overall request is split over several messages */
-static void fs_readdir_split(void *obj, void *data, QGuestAllocator *t_alloc,
-                             uint32_t count)
+static void do_readdir_split(QVirtio9P *v9p, uint32_t count)
 {
-    QVirtio9P *v9p = obj;
-    alloc = t_alloc;
     char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_READDIR_DIR) };
     uint16_t nqid;
     v9fs_qid qid;
@@ -1002,19 +999,22 @@ static void fs_mkdir(void *obj, void *data, QGuestAllocator *t_alloc,
 static void fs_readdir_split_128(void *obj, void *data,
                                  QGuestAllocator *t_alloc)
 {
-    fs_readdir_split(obj, data, t_alloc, 128);
+    alloc = t_alloc;
+    do_readdir_split(obj, 128);
 }
 
 static void fs_readdir_split_256(void *obj, void *data,
                                  QGuestAllocator *t_alloc)
 {
-    fs_readdir_split(obj, data, t_alloc, 256);
+    alloc = t_alloc;
+    do_readdir_split(obj, 256);
 }
 
 static void fs_readdir_split_512(void *obj, void *data,
                                  QGuestAllocator *t_alloc)
 {
-    fs_readdir_split(obj, data, t_alloc, 512);
+    alloc = t_alloc;
+    do_readdir_split(obj, 512);
 }
 
 
-- 
2.20.1

[PULL 02/13] tests/9pfs: Set alloc in fs_create_dir()

[Greg Kurz]

fs_create_dir() is a top level test function. It should set alloc.

Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <160321016764.266767.3763279057643874020.stgit@bahia.lan>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/virtio-9p-test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 59bcea4c30..93a2a4cd76 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -1019,6 +1019,7 @@ static void fs_readdir_split_512(void *obj, void *data,
 static void fs_create_dir(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     QVirtio9P *v9p = obj;
+    alloc = t_alloc;
     struct stat st;
     char *root_path = virtio_9p_test_path("");
     char *new_dir = virtio_9p_test_path("01");
-- 
2.20.1

[PULL 03/13] tests/9pfs: Factor out do_attach() helper

[Greg Kurz]

fs_attach() is a top level test function. Factor out the reusable
code to a separate helper instead of hijacking it in other tests.

Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <160321017450.266767.17377192504263871186.stgit@bahia.lan>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/virtio-9p-test.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 93a2a4cd76..e07292bdb8 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -589,10 +589,8 @@ static void fs_version(void *obj, void *data, QGuestAllocator *t_alloc)
     do_version(obj);
 }
 
-static void fs_attach(void *obj, void *data, QGuestAllocator *t_alloc)
+static void do_attach(QVirtio9P *v9p)
 {
-    QVirtio9P *v9p = obj;
-    alloc = t_alloc;
     P9Req *req;
 
     do_version(v9p);
@@ -601,6 +599,12 @@ static void fs_attach(void *obj, void *data, QGuestAllocator *t_alloc)
     v9fs_rattach(req, NULL);
 }
 
+static void fs_attach(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+    alloc = t_alloc;
+    do_attach(obj);
+}
+
 static void fs_walk(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     QVirtio9P *v9p = obj;
@@ -615,7 +619,7 @@ static void fs_walk(void *obj, void *data, QGuestAllocator *t_alloc)
         wnames[i] = g_strdup_printf(QTEST_V9FS_SYNTH_WALK_FILE, i);
     }
 
-    fs_attach(v9p, NULL, t_alloc);
+    do_attach(v9p);
     req = v9fs_twalk(v9p, 0, 1, P9_MAXWELEM, wnames, 0);
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rwalk(req, &nwqid, &wqid);
@@ -684,7 +688,7 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
     struct V9fsDirent *entries = NULL;
     P9Req *req;
 
-    fs_attach(v9p, NULL, t_alloc);
+    do_attach(v9p);
     req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0);
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rwalk(req, &nqid, NULL);
@@ -741,7 +745,7 @@ static void fs_readdir_split(void *obj, void *data, QGuestAllocator *t_alloc,
     int fid;
     uint64_t offset;
 
-    fs_attach(v9p, NULL, t_alloc);
+    do_attach(v9p);
 
     fid = 1;
     offset = 0;
@@ -817,7 +821,7 @@ static void fs_walk_no_slash(void *obj, void *data, QGuestAllocator *t_alloc)
     P9Req *req;
     uint32_t err;
 
-    fs_attach(v9p, NULL, t_alloc);
+    do_attach(v9p);
     req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0);
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rlerror(req, &err);
@@ -857,7 +861,7 @@ static void fs_lopen(void *obj, void *data, QGuestAllocator *t_alloc)
     char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_LOPEN_FILE) };
     P9Req *req;
 
-    fs_attach(v9p, NULL, t_alloc);
+    do_attach(v9p);
     req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0);
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rwalk(req, NULL, NULL);
@@ -879,7 +883,7 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc)
     uint32_t count;
     P9Req *req;
 
-    fs_attach(v9p, NULL, t_alloc);
+    do_attach(v9p);
     req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0);
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rwalk(req, NULL, NULL);
@@ -906,7 +910,7 @@ static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc)
     uint32_t reply_len;
     uint8_t should_block;
 
-    fs_attach(v9p, NULL, t_alloc);
+    do_attach(v9p);
     req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0);
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rwalk(req, NULL, NULL);
@@ -943,7 +947,7 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc)
     uint32_t count;
     uint8_t should_block;
 
-    fs_attach(v9p, NULL, t_alloc);
+    do_attach(v9p);
     req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0);
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rwalk(req, NULL, NULL);
@@ -1026,7 +1030,7 @@ static void fs_create_dir(void *obj, void *data, QGuestAllocator *t_alloc)
 
     g_assert(root_path != NULL);
 
-    fs_attach(v9p, NULL, t_alloc);
+    do_attach(v9p);
     fs_mkdir(v9p, data, t_alloc, "/", "01");
 
     /* check if created directory really exists now ... */
-- 
2.20.1

[PULL 05/13] tests/9pfs: Turn fs_mkdir() into a helper

[Greg Kurz]

fs_mkdir() isn't a top level test function and thus shouldn't take
the "void *obj, void *data, QGuestAllocator *t_alloc" arguments.
Turn it into a helper to be used by test functions.

Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <160321018148.266767.15959608711038504029.stgit@bahia.lan>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/virtio-9p-test.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 3c187cdc08..2ea555fa04 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -972,11 +972,8 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc)
     g_free(wnames[0]);
 }
 
-static void fs_mkdir(void *obj, void *data, QGuestAllocator *t_alloc,
-                     const char *path, const char *cname)
+static void do_mkdir(QVirtio9P *v9p, const char *path, const char *cname)
 {
-    QVirtio9P *v9p = obj;
-    alloc = t_alloc;
     char **wnames;
     char *const name = g_strdup(cname);
     P9Req *req;
@@ -1031,7 +1028,7 @@ static void fs_create_dir(void *obj, void *data, QGuestAllocator *t_alloc)
     g_assert(root_path != NULL);
 
     do_attach(v9p);
-    fs_mkdir(v9p, data, t_alloc, "/", "01");
+    do_mkdir(v9p, "/", "01");
 
     /* check if created directory really exists now ... */
     g_assert(stat(new_dir, &st) == 0);
-- 
2.20.1

[PULL 06/13] tests/9pfs: simplify do_mkdir()

[Christian Schoenebeck]

Split out walking a directory path to a separate new utility function
do_walk() and use that function in do_mkdir().

The code difference saved this way is not much, but we'll use that new
do_walk() function in the upcoming patches, so it will avoid quite
some code duplication after all.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <4d7275b2363f122438a443ce079cbb355285e9d6.1603285620.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/virtio-9p-test.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 2ea555fa04..21807037df 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -583,6 +583,23 @@ static void do_version(QVirtio9P *v9p)
     g_free(server_version);
 }
 
+/* utility function: walk to requested dir and return fid for that dir */
+static uint32_t do_walk(QVirtio9P *v9p, const char *path)
+{
+    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, NULL, NULL);
+
+    split_free(&wnames);
+    return fid;
+}
+
 static void fs_version(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     alloc = t_alloc;
@@ -974,23 +991,17 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc)
 
 static void do_mkdir(QVirtio9P *v9p, const char *path, const char *cname)
 {
-    char **wnames;
     char *const name = g_strdup(cname);
+    uint32_t fid;
     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, NULL, NULL);
+    fid = do_walk(v9p, path);
 
     req = v9fs_tmkdir(v9p, fid, name, 0750, 0, 0);
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rmkdir(req, NULL);
 
     g_free(name);
-    split_free(&wnames);
 }
 
 static void fs_readdir_split_128(void *obj, void *data,
-- 
2.20.1

[PULL 07/13] tests/9pfs: add local Tunlinkat directory test

[Christian Schoenebeck]

This test case uses a Tunlinkat 9p request with flag AT_REMOVEDIR
(see 'man 2 unlink') to remove a directory from host's test directory.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <3c7c65b476ba44bea6afd0b378b5287e1c671a32.1603285620.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/virtio-9p-test.c | 71 ++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 21807037df..abd7e44648 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -258,6 +258,7 @@ static const char *rmessage_name(uint8_t id)
         id == P9_RLOPEN ? "RLOPEN" :
         id == P9_RWRITE ? "RWRITE" :
         id == P9_RMKDIR ? "RMKDIR" :
+        id == P9_RUNLINKAT ? "RUNLINKAT" :
         id == P9_RFLUSH ? "RFLUSH" :
         id == P9_RREADDIR ? "READDIR" :
         "<unknown>";
@@ -693,6 +694,33 @@ static void v9fs_rmkdir(P9Req *req, v9fs_qid *qid)
     v9fs_req_free(req);
 }
 
+/* size[4] Tunlinkat tag[2] dirfd[4] name[s] flags[4] */
+static P9Req *v9fs_tunlinkat(QVirtio9P *v9p, uint32_t dirfd, const char *name,
+                             uint32_t flags, uint16_t tag)
+{
+    P9Req *req;
+
+    uint32_t body_size = 4 + 4;
+    uint16_t string_size = v9fs_string_size(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);
+    v9fs_req_send(req);
+    return req;
+}
+
+/* size[4] Runlinkat tag[2] */
+static void v9fs_runlinkat(P9Req *req)
+{
+    v9fs_req_recv(req, P9_RUNLINKAT);
+    v9fs_req_free(req);
+}
+
 /* basic readdir test where reply fits into a single response message */
 static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc)
 {
@@ -1004,6 +1032,22 @@ static void do_mkdir(QVirtio9P *v9p, const char *path, const char *cname)
     g_free(name);
 }
 
+static void do_unlinkat(QVirtio9P *v9p, const char *atpath, const char *rpath,
+                        uint32_t flags)
+{
+    char *const name = g_strdup(rpath);
+    uint32_t fid;
+    P9Req *req;
+
+    fid = do_walk(v9p, atpath);
+
+    req = v9fs_tunlinkat(v9p, fid, name, flags, 0);
+    v9fs_req_wait_for_reply(req, NULL);
+    v9fs_runlinkat(req);
+
+    g_free(name);
+}
+
 static void fs_readdir_split_128(void *obj, void *data,
                                  QGuestAllocator *t_alloc)
 {
@@ -1050,6 +1094,32 @@ static void fs_create_dir(void *obj, void *data, QGuestAllocator *t_alloc)
     g_free(root_path);
 }
 
+static void fs_unlinkat_dir(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+    QVirtio9P *v9p = obj;
+    alloc = t_alloc;
+    struct stat st;
+    char *root_path = virtio_9p_test_path("");
+    char *new_dir = virtio_9p_test_path("02");
+
+    g_assert(root_path != NULL);
+
+    do_attach(v9p);
+    do_mkdir(v9p, "/", "02");
+
+    /* check if created directory really exists now ... */
+    g_assert(stat(new_dir, &st) == 0);
+    /* ... and is actually a directory */
+    g_assert((st.st_mode & S_IFMT) == S_IFDIR);
+
+    do_unlinkat(v9p, "/", "02", AT_REMOVEDIR);
+    /* directory should be gone now */
+    g_assert(stat(new_dir, &st) != 0);
+
+    g_free(new_dir);
+    g_free(root_path);
+}
+
 static void *assign_9p_local_driver(GString *cmd_line, void *arg)
 {
     virtio_9p_assign_local_driver(cmd_line, "security_model=mapped-xattr");
@@ -1090,6 +1160,7 @@ static void register_virtio_9p_test(void)
     opts.before = assign_9p_local_driver;
     qos_add_test("local/config", "virtio-9p", pci_config,  &opts);
     qos_add_test("local/create_dir", "virtio-9p", fs_create_dir, &opts);
+    qos_add_test("local/unlinkat_dir", "virtio-9p", fs_unlinkat_dir, &opts);
 }
 
 libqos_init(register_virtio_9p_test);
-- 
2.20.1

[PULL 08/13] tests/9pfs: add local Tlcreate test

[Christian Schoenebeck]

This test case uses a Tlcreate 9p request to create a regular file inside
host's test directory.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <269cae0c00af941a3a4ae78f1e319f93462a7eb4.1603285620.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/virtio-9p-test.c | 77 ++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index abd7e44648..c030bc2a6c 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -258,6 +258,7 @@ static const char *rmessage_name(uint8_t id)
         id == P9_RLOPEN ? "RLOPEN" :
         id == P9_RWRITE ? "RWRITE" :
         id == P9_RMKDIR ? "RMKDIR" :
+        id == P9_RLCREATE ? "RLCREATE" :
         id == P9_RUNLINKAT ? "RUNLINKAT" :
         id == P9_RFLUSH ? "RFLUSH" :
         id == P9_RREADDIR ? "READDIR" :
@@ -694,6 +695,44 @@ static void v9fs_rmkdir(P9Req *req, v9fs_qid *qid)
     v9fs_req_free(req);
 }
 
+/* size[4] Tlcreate tag[2] fid[4] name[s] flags[4] mode[4] gid[4] */
+static P9Req *v9fs_tlcreate(QVirtio9P *v9p, uint32_t fid, const char *name,
+                            uint32_t flags, uint32_t mode, uint32_t gid,
+                            uint16_t tag)
+{
+    P9Req *req;
+
+    uint32_t body_size = 4 + 4 + 4 + 4;
+    uint16_t string_size = v9fs_string_size(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);
+    v9fs_string_write(req, name);
+    v9fs_uint32_write(req, flags);
+    v9fs_uint32_write(req, mode);
+    v9fs_uint32_write(req, gid);
+    v9fs_req_send(req);
+    return req;
+}
+
+/* size[4] Rlcreate tag[2] qid[13] iounit[4] */
+static void v9fs_rlcreate(P9Req *req, v9fs_qid *qid, uint32_t *iounit)
+{
+    v9fs_req_recv(req, P9_RLCREATE);
+    if (qid) {
+        v9fs_memread(req, qid, 13);
+    } else {
+        v9fs_memskip(req, 13);
+    }
+    if (iounit) {
+        v9fs_uint32_read(req, iounit);
+    }
+    v9fs_req_free(req);
+}
+
 /* size[4] Tunlinkat tag[2] dirfd[4] name[s] flags[4] */
 static P9Req *v9fs_tunlinkat(QVirtio9P *v9p, uint32_t dirfd, const char *name,
                              uint32_t flags, uint16_t tag)
@@ -1032,6 +1071,24 @@ static void do_mkdir(QVirtio9P *v9p, const char *path, const char *cname)
     g_free(name);
 }
 
+/* create a regular file with Tlcreate and return file's fid */
+static uint32_t do_lcreate(QVirtio9P *v9p, const char *path,
+                           const char *cname)
+{
+    char *const name = g_strdup(cname);
+    uint32_t fid;
+    P9Req *req;
+
+    fid = do_walk(v9p, path);
+
+    req = v9fs_tlcreate(v9p, fid, name, 0, 0750, 0, 0);
+    v9fs_req_wait_for_reply(req, NULL);
+    v9fs_rlcreate(req, NULL, NULL);
+
+    g_free(name);
+    return fid;
+}
+
 static void do_unlinkat(QVirtio9P *v9p, const char *atpath, const char *rpath,
                         uint32_t flags)
 {
@@ -1120,6 +1177,25 @@ static void fs_unlinkat_dir(void *obj, void *data, QGuestAllocator *t_alloc)
     g_free(root_path);
 }
 
+static void fs_create_file(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+    QVirtio9P *v9p = obj;
+    alloc = t_alloc;
+    struct stat st;
+    char *new_file = virtio_9p_test_path("03/1st_file");
+
+    do_attach(v9p);
+    do_mkdir(v9p, "/", "03");
+    do_lcreate(v9p, "03", "1st_file");
+
+    /* check if created file exists now ... */
+    g_assert(stat(new_file, &st) == 0);
+    /* ... and is a regular file */
+    g_assert((st.st_mode & S_IFMT) == S_IFREG);
+
+    g_free(new_file);
+}
+
 static void *assign_9p_local_driver(GString *cmd_line, void *arg)
 {
     virtio_9p_assign_local_driver(cmd_line, "security_model=mapped-xattr");
@@ -1161,6 +1237,7 @@ static void register_virtio_9p_test(void)
     qos_add_test("local/config", "virtio-9p", pci_config,  &opts);
     qos_add_test("local/create_dir", "virtio-9p", fs_create_dir, &opts);
     qos_add_test("local/unlinkat_dir", "virtio-9p", fs_unlinkat_dir, &opts);
+    qos_add_test("local/create_file", "virtio-9p", fs_create_file, &opts);
 }
 
 libqos_init(register_virtio_9p_test);
-- 
2.20.1

[PULL 09/13] tests/9pfs: add local Tunlinkat file test

[Christian Schoenebeck]

This test case uses a Tunlinkat request to remove a regular file using
the 9pfs 'local' fs driver.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <4eabeed7f662721dd5664cb77fe36ea0aa08b1ec.1603285620.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/virtio-9p-test.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index c030bc2a6c..6b74a1fd7e 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -1196,6 +1196,29 @@ static void fs_create_file(void *obj, void *data, QGuestAllocator *t_alloc)
     g_free(new_file);
 }
 
+static void fs_unlinkat_file(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+    QVirtio9P *v9p = obj;
+    alloc = t_alloc;
+    struct stat st;
+    char *new_file = virtio_9p_test_path("04/doa_file");
+
+    do_attach(v9p);
+    do_mkdir(v9p, "/", "04");
+    do_lcreate(v9p, "04", "doa_file");
+
+    /* check if created file exists now ... */
+    g_assert(stat(new_file, &st) == 0);
+    /* ... and is a regular file */
+    g_assert((st.st_mode & S_IFMT) == S_IFREG);
+
+    do_unlinkat(v9p, "04", "doa_file", 0);
+    /* file should be gone now */
+    g_assert(stat(new_file, &st) != 0);
+
+    g_free(new_file);
+}
+
 static void *assign_9p_local_driver(GString *cmd_line, void *arg)
 {
     virtio_9p_assign_local_driver(cmd_line, "security_model=mapped-xattr");
@@ -1238,6 +1261,7 @@ static void register_virtio_9p_test(void)
     qos_add_test("local/create_dir", "virtio-9p", fs_create_dir, &opts);
     qos_add_test("local/unlinkat_dir", "virtio-9p", fs_unlinkat_dir, &opts);
     qos_add_test("local/create_file", "virtio-9p", fs_create_file, &opts);
+    qos_add_test("local/unlinkat_file", "virtio-9p", fs_unlinkat_file, &opts);
 }
 
 libqos_init(register_virtio_9p_test);
-- 
2.20.1

[PULL 10/13] tests/9pfs: add local Tsymlink test

[Christian Schoenebeck]

This test case uses a Tsymlink 9p request to create a symbolic link using
the 9pfs 'local' fs driver.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <84ac76937855bf441242372cc3e62df42f0a3dc4.1603285620.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/virtio-9p-test.c | 77 ++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 6b74a1fd7e..0c11417236 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -259,6 +259,7 @@ static const char *rmessage_name(uint8_t id)
         id == P9_RWRITE ? "RWRITE" :
         id == P9_RMKDIR ? "RMKDIR" :
         id == P9_RLCREATE ? "RLCREATE" :
+        id == P9_RSYMLINK ? "RSYMLINK" :
         id == P9_RUNLINKAT ? "RUNLINKAT" :
         id == P9_RFLUSH ? "RFLUSH" :
         id == P9_RREADDIR ? "READDIR" :
@@ -733,6 +734,39 @@ static void v9fs_rlcreate(P9Req *req, v9fs_qid *qid, uint32_t *iounit)
     v9fs_req_free(req);
 }
 
+/* size[4] Tsymlink tag[2] fid[4] name[s] symtgt[s] gid[4] */
+static P9Req *v9fs_tsymlink(QVirtio9P *v9p, uint32_t fid, const char *name,
+                            const char *symtgt, uint32_t gid, uint16_t tag)
+{
+    P9Req *req;
+
+    uint32_t body_size = 4 + 4;
+    uint16_t string_size = v9fs_string_size(name) + v9fs_string_size(symtgt);
+
+    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);
+    v9fs_string_write(req, name);
+    v9fs_string_write(req, symtgt);
+    v9fs_uint32_write(req, gid);
+    v9fs_req_send(req);
+    return req;
+}
+
+/* size[4] Rsymlink tag[2] qid[13] */
+static void v9fs_rsymlink(P9Req *req, v9fs_qid *qid)
+{
+    v9fs_req_recv(req, P9_RSYMLINK);
+    if (qid) {
+        v9fs_memread(req, qid, 13);
+    } else {
+        v9fs_memskip(req, 13);
+    }
+    v9fs_req_free(req);
+}
+
 /* size[4] Tunlinkat tag[2] dirfd[4] name[s] flags[4] */
 static P9Req *v9fs_tunlinkat(QVirtio9P *v9p, uint32_t dirfd, const char *name,
                              uint32_t flags, uint16_t tag)
@@ -1089,6 +1123,25 @@ static uint32_t do_lcreate(QVirtio9P *v9p, const char *path,
     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)
+{
+    char *const name = g_strdup(clink);
+    char *const dst = g_strdup(to);
+    uint32_t fid;
+    P9Req *req;
+
+    fid = do_walk(v9p, path);
+
+    req = v9fs_tsymlink(v9p, fid, name, dst, 0, 0);
+    v9fs_req_wait_for_reply(req, NULL);
+    v9fs_rsymlink(req, NULL);
+
+    g_free(dst);
+    g_free(name);
+}
+
 static void do_unlinkat(QVirtio9P *v9p, const char *atpath, const char *rpath,
                         uint32_t flags)
 {
@@ -1219,6 +1272,29 @@ static void fs_unlinkat_file(void *obj, void *data, QGuestAllocator *t_alloc)
     g_free(new_file);
 }
 
+static void fs_symlink_file(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+    QVirtio9P *v9p = obj;
+    alloc = t_alloc;
+    struct stat st;
+    char *real_file = virtio_9p_test_path("05/real_file");
+    char *symlink_file = virtio_9p_test_path("05/symlink_file");
+
+    do_attach(v9p);
+    do_mkdir(v9p, "/", "05");
+    do_lcreate(v9p, "05", "real_file");
+    g_assert(stat(real_file, &st) == 0);
+    g_assert((st.st_mode & S_IFMT) == S_IFREG);
+
+    do_symlink(v9p, "05", "symlink_file", "real_file");
+
+    /* check if created link exists now */
+    g_assert(stat(symlink_file, &st) == 0);
+
+    g_free(symlink_file);
+    g_free(real_file);
+}
+
 static void *assign_9p_local_driver(GString *cmd_line, void *arg)
 {
     virtio_9p_assign_local_driver(cmd_line, "security_model=mapped-xattr");
@@ -1262,6 +1338,7 @@ static void register_virtio_9p_test(void)
     qos_add_test("local/unlinkat_dir", "virtio-9p", fs_unlinkat_dir, &opts);
     qos_add_test("local/create_file", "virtio-9p", fs_create_file, &opts);
     qos_add_test("local/unlinkat_file", "virtio-9p", fs_unlinkat_file, &opts);
+    qos_add_test("local/symlink_file", "virtio-9p", fs_symlink_file, &opts);
 }
 
 libqos_init(register_virtio_9p_test);
-- 
2.20.1

[PULL 11/13] tests/9pfs: add local Tunlinkat symlink test

[Christian Schoenebeck]

This test case uses a Tunlinkat request to remove a symlink using
the 9pfs 'local' fs driver.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <a23cd4d2ab6d8d3048addab8cbf0416fe5ead43e.1603285620.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/virtio-9p-test.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 0c11417236..33cba24b18 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -1295,6 +1295,32 @@ static void fs_symlink_file(void *obj, void *data, QGuestAllocator *t_alloc)
     g_free(real_file);
 }
 
+static void fs_unlinkat_symlink(void *obj, void *data,
+                                QGuestAllocator *t_alloc)
+{
+    QVirtio9P *v9p = obj;
+    alloc = t_alloc;
+    struct stat st;
+    char *real_file = virtio_9p_test_path("06/real_file");
+    char *symlink_file = virtio_9p_test_path("06/symlink_file");
+
+    do_attach(v9p);
+    do_mkdir(v9p, "/", "06");
+    do_lcreate(v9p, "06", "real_file");
+    g_assert(stat(real_file, &st) == 0);
+    g_assert((st.st_mode & S_IFMT) == S_IFREG);
+
+    do_symlink(v9p, "06", "symlink_file", "real_file");
+    g_assert(stat(symlink_file, &st) == 0);
+
+    do_unlinkat(v9p, "06", "symlink_file", 0);
+    /* symlink should be gone now */
+    g_assert(stat(symlink_file, &st) != 0);
+
+    g_free(symlink_file);
+    g_free(real_file);
+}
+
 static void *assign_9p_local_driver(GString *cmd_line, void *arg)
 {
     virtio_9p_assign_local_driver(cmd_line, "security_model=mapped-xattr");
@@ -1339,6 +1365,8 @@ static void register_virtio_9p_test(void)
     qos_add_test("local/create_file", "virtio-9p", fs_create_file, &opts);
     qos_add_test("local/unlinkat_file", "virtio-9p", fs_unlinkat_file, &opts);
     qos_add_test("local/symlink_file", "virtio-9p", fs_symlink_file, &opts);
+    qos_add_test("local/unlinkat_symlink", "virtio-9p", fs_unlinkat_symlink,
+                 &opts);
 }
 
 libqos_init(register_virtio_9p_test);
-- 
2.20.1

[PULL 12/13] tests/9pfs: add local Tlink test

[Christian Schoenebeck]

This test case uses a Tlink request to create a hard link to a regular
file using the 9pfs 'local' fs driver.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <f0d869770ad23ee5ce10f7da90fdb742cadcad72.1603285620.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/virtio-9p-test.c | 71 ++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 33cba24b18..460fa49fe3 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -260,6 +260,7 @@ static const char *rmessage_name(uint8_t id)
         id == P9_RMKDIR ? "RMKDIR" :
         id == P9_RLCREATE ? "RLCREATE" :
         id == P9_RSYMLINK ? "RSYMLINK" :
+        id == P9_RLINK ? "RLINK" :
         id == P9_RUNLINKAT ? "RUNLINKAT" :
         id == P9_RFLUSH ? "RFLUSH" :
         id == P9_RREADDIR ? "READDIR" :
@@ -767,6 +768,33 @@ static void v9fs_rsymlink(P9Req *req, v9fs_qid *qid)
     v9fs_req_free(req);
 }
 
+/* size[4] Tlink tag[2] dfid[4] fid[4] name[s] */
+static P9Req *v9fs_tlink(QVirtio9P *v9p, uint32_t dfid, uint32_t fid,
+                         const char *name, uint16_t tag)
+{
+    P9Req *req;
+
+    uint32_t body_size = 4 + 4;
+    uint16_t string_size = v9fs_string_size(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);
+    v9fs_req_send(req);
+    return req;
+}
+
+/* size[4] Rlink tag[2] */
+static void v9fs_rlink(P9Req *req)
+{
+    v9fs_req_recv(req, P9_RLINK);
+    v9fs_req_free(req);
+}
+
 /* size[4] Tunlinkat tag[2] dirfd[4] name[s] flags[4] */
 static P9Req *v9fs_tunlinkat(QVirtio9P *v9p, uint32_t dirfd, const char *name,
                              uint32_t flags, uint16_t tag)
@@ -1142,6 +1170,21 @@ static void do_symlink(QVirtio9P *v9p, const char *path, const char *clink,
     g_free(name);
 }
 
+/* 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 = do_walk(v9p, path);
+    fid = do_walk(v9p, to);
+
+    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)
 {
@@ -1321,6 +1364,33 @@ static void fs_unlinkat_symlink(void *obj, void *data,
     g_free(real_file);
 }
 
+static void fs_hardlink_file(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+    QVirtio9P *v9p = obj;
+    alloc = t_alloc;
+    struct stat st_real, st_link;
+    char *real_file = virtio_9p_test_path("07/real_file");
+    char *hardlink_file = virtio_9p_test_path("07/hardlink_file");
+
+    do_attach(v9p);
+    do_mkdir(v9p, "/", "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);
+
+    do_hardlink(v9p, "07", "hardlink_file", "07/real_file");
+
+    /* check if link exists now ... */
+    g_assert(stat(hardlink_file, &st_link) == 0);
+    /* ... and it's a hard link, right? */
+    g_assert((st_link.st_mode & S_IFMT) == S_IFREG);
+    g_assert(st_link.st_dev == st_real.st_dev);
+    g_assert(st_link.st_ino == st_real.st_ino);
+
+    g_free(hardlink_file);
+    g_free(real_file);
+}
+
 static void *assign_9p_local_driver(GString *cmd_line, void *arg)
 {
     virtio_9p_assign_local_driver(cmd_line, "security_model=mapped-xattr");
@@ -1367,6 +1437,7 @@ static void register_virtio_9p_test(void)
     qos_add_test("local/symlink_file", "virtio-9p", fs_symlink_file, &opts);
     qos_add_test("local/unlinkat_symlink", "virtio-9p", fs_unlinkat_symlink,
                  &opts);
+    qos_add_test("local/hardlink_file", "virtio-9p", fs_hardlink_file, &opts);
 }
 
 libqos_init(register_virtio_9p_test);
-- 
2.20.1

[PULL 13/13] tests/9pfs: add local Tunlinkat hard link test

[Christian Schoenebeck]

This test case uses a Tunlinkat request to remove a previously hard
linked file by using the 9pfs 'local' fs driver.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <9bec33a7d8f006ef8f80517985d0d6ac48650d53.1603285620.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/virtio-9p-test.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 460fa49fe3..23433913bb 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -1391,6 +1391,34 @@ static void fs_hardlink_file(void *obj, void *data, QGuestAllocator *t_alloc)
     g_free(real_file);
 }
 
+static void fs_unlinkat_hardlink(void *obj, void *data,
+                                 QGuestAllocator *t_alloc)
+{
+    QVirtio9P *v9p = obj;
+    alloc = t_alloc;
+    struct stat st_real, st_link;
+    char *real_file = virtio_9p_test_path("08/real_file");
+    char *hardlink_file = virtio_9p_test_path("08/hardlink_file");
+
+    do_attach(v9p);
+    do_mkdir(v9p, "/", "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);
+
+    do_hardlink(v9p, "08", "hardlink_file", "08/real_file");
+    g_assert(stat(hardlink_file, &st_link) == 0);
+
+    do_unlinkat(v9p, "08", "hardlink_file", 0);
+    /* symlink should be gone now */
+    g_assert(stat(hardlink_file, &st_link) != 0);
+    /* and old file should still exist */
+    g_assert(stat(real_file, &st_real) == 0);
+
+    g_free(hardlink_file);
+    g_free(real_file);
+}
+
 static void *assign_9p_local_driver(GString *cmd_line, void *arg)
 {
     virtio_9p_assign_local_driver(cmd_line, "security_model=mapped-xattr");
@@ -1438,6 +1466,8 @@ static void register_virtio_9p_test(void)
     qos_add_test("local/unlinkat_symlink", "virtio-9p", fs_unlinkat_symlink,
                  &opts);
     qos_add_test("local/hardlink_file", "virtio-9p", fs_hardlink_file, &opts);
+    qos_add_test("local/unlinkat_hardlink", "virtio-9p", fs_unlinkat_hardlink,
+                 &opts);
 }
 
 libqos_init(register_virtio_9p_test);
-- 
2.20.1

[PULL 00/13] 9p queue 2020-10-23

[Christian Schoenebeck]

The following changes since commit 4c5b97bfd0dd54dc27717ae8d1cd10e14eef1430:

  Merge remote-tracking branch 'remotes/kraxel/tags/modules-20201022-pull-request' into staging (2020-10-22 12:33:21 +0100)

are available in the Git repository at:

  https://github.com/cschoenebeck/qemu.git tags/pull-9p-20201023

for you to fetch changes up to ee01926a11b1f9bffcd6cdec0961dd9d1882da71:

  tests/9pfs: add local Tunlinkat hard link test (2020-10-22 20:26:33 +0200)

----------------------------------------------------------------
9pfs: more tests using local fs driver

Only 9pfs test case changes this time:

* Refactor: Rename functions to make top-level test functions fs_*()
  easily distinguishable from utility test functions do_*().

* Refactor: Drop unnecessary function arguments in utility test
  functions.

* More test cases using the 9pfs 'local' filesystem driver backend,
  namely for the following 9p requests: Tunlinkat, Tlcreate, Tsymlink
  and Tlink.

----------------------------------------------------------------
Christian Schoenebeck (8):
      tests/9pfs: simplify do_mkdir()
      tests/9pfs: add local Tunlinkat directory test
      tests/9pfs: add local Tlcreate test
      tests/9pfs: add local Tunlinkat file test
      tests/9pfs: add local Tsymlink test
      tests/9pfs: add local Tunlinkat symlink test
      tests/9pfs: add local Tlink test
      tests/9pfs: add local Tunlinkat hard link test

Greg Kurz (5):
      tests/9pfs: Factor out do_version() helper
      tests/9pfs: Set alloc in fs_create_dir()
      tests/9pfs: Factor out do_attach() helper
      tests/9pfs: Turn fs_readdir_split() into a helper
      tests/9pfs: Turn fs_mkdir() into a helper

 tests/qtest/virtio-9p-test.c | 467 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 431 insertions(+), 36 deletions(-)

Re: [PULL 01/13] tests/9pfs: Factor out do_version() helper

[Greg Kurz]

It feels weird to receive a mail I didn't send with my address
in the top From: header. I would have expected yours instead and
another From: with my address in the changelog...


On Tue, 20 Oct 2020 18:09:14 +0200
Greg Kurz <groug@kaod.org> wrote:

... here.

> fs_version() is a top level test function. Factor out the reusable
> code to a separate helper instead of hijacking it in other tests.
> 

> Signed-off-by: Greg Kurz <groug@kaod.org>
> Message-Id: <160321015403.266767.4533967728943968456.stgit@bahia.lan>
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---
>  tests/qtest/virtio-9p-test.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> index c15908f27b..59bcea4c30 100644
> --- a/tests/qtest/virtio-9p-test.c
> +++ b/tests/qtest/virtio-9p-test.c
> @@ -567,10 +567,8 @@ static void v9fs_rflush(P9Req *req)
>      v9fs_req_free(req);
>  }
>  
> -static void fs_version(void *obj, void *data, QGuestAllocator *t_alloc)
> +static void do_version(QVirtio9P *v9p)
>  {
> -    QVirtio9P *v9p = obj;
> -    alloc = t_alloc;
>      const char *version = "9P2000.L";
>      uint16_t server_len;
>      char *server_version;
> @@ -585,13 +583,19 @@ static void fs_version(void *obj, void *data, QGuestAllocator *t_alloc)
>      g_free(server_version);
>  }
>  
> +static void fs_version(void *obj, void *data, QGuestAllocator *t_alloc)
> +{
> +    alloc = t_alloc;
> +    do_version(obj);
> +}
> +
>  static void fs_attach(void *obj, void *data, QGuestAllocator *t_alloc)
>  {
>      QVirtio9P *v9p = obj;
>      alloc = t_alloc;
>      P9Req *req;
>  
> -    fs_version(v9p, NULL, t_alloc);
> +    do_version(v9p);
>      req = v9fs_tattach(v9p, 0, getuid(), 0);
>      v9fs_req_wait_for_reply(req, NULL);
>      v9fs_rattach(req, NULL);
> @@ -831,7 +835,7 @@ static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc)
>      v9fs_qid root_qid, *wqid;
>      P9Req *req;
>  
> -    fs_version(v9p, NULL, t_alloc);
> +    do_version(v9p);
>      req = v9fs_tattach(v9p, 0, getuid(), 0);
>      v9fs_req_wait_for_reply(req, NULL);
>      v9fs_rattach(req, &root_qid);

Re: [PULL 01/13] tests/9pfs: Factor out do_version() helper

[Christian Schoenebeck]

On Freitag, 23. Oktober 2020 17:32:02 CEST Greg Kurz wrote:
> It feels weird to receive a mail I didn't send with my address
> in the top From: header. I would have expected yours instead and
> another From: with my address in the changelog...
> 
> 
> On Tue, 20 Oct 2020 18:09:14 +0200
> Greg Kurz <groug@kaod.org> wrote:
> 
> ... here.

Yep, I noticed that too late. I'll take care about it next time. Sorry!

Best regards,
Christian Schoenebeck

Re: [PULL 00/13] 9p queue 2020-10-23

[Peter Maydell]

On Fri, 23 Oct 2020 at 12:46, Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
>
> The following changes since commit 4c5b97bfd0dd54dc27717ae8d1cd10e14eef1430:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/modules-20201022-pull-request' into staging (2020-10-22 12:33:21 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/cschoenebeck/qemu.git tags/pull-9p-20201023
>
> for you to fetch changes up to ee01926a11b1f9bffcd6cdec0961dd9d1882da71:
>
>   tests/9pfs: add local Tunlinkat hard link test (2020-10-22 20:26:33 +0200)
>
> ----------------------------------------------------------------
> 9pfs: more tests using local fs driver
>
> Only 9pfs test case changes this time:
>
> * Refactor: Rename functions to make top-level test functions fs_*()
>   easily distinguishable from utility test functions do_*().
>
> * Refactor: Drop unnecessary function arguments in utility test
>   functions.
>
> * More test cases using the 9pfs 'local' filesystem driver backend,
>   namely for the following 9p requests: Tunlinkat, Tlcreate, Tsymlink
>   and Tlink.
>
> ----------------------------------------------------------------

I get a 'make check' failure on x86-64 Linux host:

PASS 54 qtest-x86_64: qos-test
/x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/config
PASS 55 qtest-x86_64: qos-test
/x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/create_dir
PASS 56 qtest-x86_64: qos-test
/x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/unlinkat_dir
PASS 57 qtest-x86_64: qos-test
/x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/create_file
PASS 58 qtest-x86_64: qos-test
/x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/unlinkat_file
PASS 59 qtest-x86_64: qos-test
/x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/symlink_file
Received response 7 (RLERROR) instead of 73 (RMKDIR)
Rlerror has errno 2 (No such file or directory)
**
ERROR:../../tests/qtest/virtio-9p-test.c:300:v9fs_req_recv: assertion
failed (hdr.id == id): (7 == 73)
ERROR qtest-x86_64: qos-test - Bail out!
ERROR:../../tests/qtest/virtio-9ptest.c:300:v9fs_req_recv: assertion
failed (hdr.id == id): (7 == 73)
Makefile.mtest:3953: recipe for target 'run-test-492' failed


thanks
-- PMM

Re: [PULL 00/13] 9p queue 2020-10-23

[Christian Schoenebeck]

On Montag, 26. Oktober 2020 11:33:42 CET Peter Maydell wrote:
> On Fri, 23 Oct 2020 at 12:46, Christian Schoenebeck
> 
> <qemu_oss@crudebyte.com> wrote:
> > The following changes since commit 4c5b97bfd0dd54dc27717ae8d1cd10e14eef1430:
> >   Merge remote-tracking branch
> >   'remotes/kraxel/tags/modules-20201022-pull-request' into staging
> >   (2020-10-22 12:33:21 +0100)> 
> > are available in the Git repository at:
> >   https://github.com/cschoenebeck/qemu.git tags/pull-9p-20201023
> > 
> > for you to fetch changes up to ee01926a11b1f9bffcd6cdec0961dd9d1882da71:
> >   tests/9pfs: add local Tunlinkat hard link test (2020-10-22 20:26:33
> >   +0200)
> > 
> > ----------------------------------------------------------------
> > 9pfs: more tests using local fs driver
> > 
> > Only 9pfs test case changes this time:
> > 
> > * Refactor: Rename functions to make top-level test functions fs_*()
> > 
> >   easily distinguishable from utility test functions do_*().
> > 
> > * Refactor: Drop unnecessary function arguments in utility test
> > 
> >   functions.
> > 
> > * More test cases using the 9pfs 'local' filesystem driver backend,
> > 
> >   namely for the following 9p requests: Tunlinkat, Tlcreate, Tsymlink
> >   and Tlink.
> > 
> > ----------------------------------------------------------------
> 
> I get a 'make check' failure on x86-64 Linux host:
> 
> PASS 54 qtest-x86_64: qos-test
> /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> 9p-tests/local/config PASS 55 qtest-x86_64: qos-test
> /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> 9p-tests/local/create_dir PASS 56 qtest-x86_64: qos-test
> /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> 9p-tests/local/unlinkat_dir PASS 57 qtest-x86_64: qos-test
> /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> 9p-tests/local/create_file PASS 58 qtest-x86_64: qos-test
> /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> 9p-tests/local/unlinkat_file PASS 59 qtest-x86_64: qos-test
> /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> 9p-tests/local/symlink_file Received response 7 (RLERROR) instead of 73
> (RMKDIR)
> Rlerror has errno 2 (No such file or directory)
> **
> ERROR:../../tests/qtest/virtio-9p-test.c:300:v9fs_req_recv: assertion
> failed (hdr.id == id): (7 == 73)
> ERROR qtest-x86_64: qos-test - Bail out!
> ERROR:../../tests/qtest/virtio-9ptest.c:300:v9fs_req_recv: assertion
> failed (hdr.id == id): (7 == 73)
> Makefile.mtest:3953: recipe for target 'run-test-492' failed
> 
> 
> thanks
> -- PMM

So the 9p server is already failing to create the test case directory
"./qtest-9p-local/05/" relative to your current working directory.

I would appreciate to get more info when you have some free cycles, as I'm
unable to reproduce this on any system unfortunately. But no hurry as
these tests only become relevant actually for QEMU 6.

What puzzles me is that the previous test cases succeeded there, which all
create their own test directory in the same way:

	./qtest-9p-local/01/
	./qtest-9p-local/02/  (<-- dir vanishes after that test completed)
	./qtest-9p-local/03/
	./qtest-9p-local/04/
	...

How does the "./qtest-9p-local/" directory look like after that
"local/symlink_file" test failed there? You can use this shortcut:

export QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
cd build
tests/qtest/qos-test --verbose
ls -l qtest-9p-local

That latter qos-test run will also output the assembled qemu command
line the 9p local tests would run with, which might also be helpful,
e.g. the relevant output would be something like this:

GTest: run: /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/config
(MSG: starting QEMU: exec x86_64-softmmu/qemu-system-x86_64 -qtest unix:/tmp/qtest-7428.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-7428.qmp,id=char0 -mon chardev=char0,mode=control -display none -M pc  -fsdev local,id=fsdev0,path='/home/me/git/qemu/build/qtest-9p-local',security_model=mapped-xattr -device virtio-9p-pci,fsdev=fsdev0,addr=04.0,mount_tag=qtest -accel qtest)

Would probably the test succeed if run alone?

tests/qtest/qos-test -p /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/symlink_file

Best regards,
Christian Schoenebeck

Re: [PULL 00/13] 9p queue 2020-10-23

[Greg Kurz]

On Mon, 26 Oct 2020 13:48:37 +0100
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Montag, 26. Oktober 2020 11:33:42 CET Peter Maydell wrote:
> > On Fri, 23 Oct 2020 at 12:46, Christian Schoenebeck
> > 
> > <qemu_oss@crudebyte.com> wrote:
> > > The following changes since commit 4c5b97bfd0dd54dc27717ae8d1cd10e14eef1430:
> > >   Merge remote-tracking branch
> > >   'remotes/kraxel/tags/modules-20201022-pull-request' into staging
> > >   (2020-10-22 12:33:21 +0100)> 
> > > are available in the Git repository at:
> > >   https://github.com/cschoenebeck/qemu.git tags/pull-9p-20201023
> > > 
> > > for you to fetch changes up to ee01926a11b1f9bffcd6cdec0961dd9d1882da71:
> > >   tests/9pfs: add local Tunlinkat hard link test (2020-10-22 20:26:33
> > >   +0200)
> > > 
> > > ----------------------------------------------------------------
> > > 9pfs: more tests using local fs driver
> > > 
> > > Only 9pfs test case changes this time:
> > > 
> > > * Refactor: Rename functions to make top-level test functions fs_*()
> > > 
> > >   easily distinguishable from utility test functions do_*().
> > > 
> > > * Refactor: Drop unnecessary function arguments in utility test
> > > 
> > >   functions.
> > > 
> > > * More test cases using the 9pfs 'local' filesystem driver backend,
> > > 
> > >   namely for the following 9p requests: Tunlinkat, Tlcreate, Tsymlink
> > >   and Tlink.
> > > 
> > > ----------------------------------------------------------------
> > 
> > I get a 'make check' failure on x86-64 Linux host:
> > 
> > PASS 54 qtest-x86_64: qos-test
> > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> > 9p-tests/local/config PASS 55 qtest-x86_64: qos-test
> > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> > 9p-tests/local/create_dir PASS 56 qtest-x86_64: qos-test
> > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> > 9p-tests/local/unlinkat_dir PASS 57 qtest-x86_64: qos-test
> > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> > 9p-tests/local/create_file PASS 58 qtest-x86_64: qos-test
> > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> > 9p-tests/local/unlinkat_file PASS 59 qtest-x86_64: qos-test
> > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> > 9p-tests/local/symlink_file Received response 7 (RLERROR) instead of 73
> > (RMKDIR)
> > Rlerror has errno 2 (No such file or directory)
> > **
> > ERROR:../../tests/qtest/virtio-9p-test.c:300:v9fs_req_recv: assertion
> > failed (hdr.id == id): (7 == 73)

Not sure this is related to this PR actually. Dave Gilbert reported on irc that
he encountered a similar issue with 'make -j check', likely without these patches.

> > ERROR qtest-x86_64: qos-test - Bail out!
> > ERROR:../../tests/qtest/virtio-9ptest.c:300:v9fs_req_recv: assertion
> > failed (hdr.id == id): (7 == 73)
> > Makefile.mtest:3953: recipe for target 'run-test-492' failed
> > 
> > 
> > thanks
> > -- PMM
> 
> So the 9p server is already failing to create the test case directory
> "./qtest-9p-local/05/" relative to your current working directory.
> 
> I would appreciate to get more info when you have some free cycles, as I'm
> unable to reproduce this on any system unfortunately. But no hurry as
> these tests only become relevant actually for QEMU 6.
> 
> What puzzles me is that the previous test cases succeeded there, which all
> create their own test directory in the same way:
> 
> 	./qtest-9p-local/01/
> 	./qtest-9p-local/02/  (<-- dir vanishes after that test completed)
> 	./qtest-9p-local/03/
> 	./qtest-9p-local/04/
> 	...
> 
> How does the "./qtest-9p-local/" directory look like after that
> "local/symlink_file" test failed there? You can use this shortcut:
> 
> export QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
> cd build
> tests/qtest/qos-test --verbose
> ls -l qtest-9p-local
> 
> That latter qos-test run will also output the assembled qemu command
> line the 9p local tests would run with, which might also be helpful,
> e.g. the relevant output would be something like this:
> 
> GTest: run: /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/config
> (MSG: starting QEMU: exec x86_64-softmmu/qemu-system-x86_64 -qtest unix:/tmp/qtest-7428.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-7428.qmp,id=char0 -mon chardev=char0,mode=control -display none -M pc  -fsdev local,id=fsdev0,path='/home/me/git/qemu/build/qtest-9p-local',security_model=mapped-xattr -device virtio-9p-pci,fsdev=fsdev0,addr=04.0,mount_tag=qtest -accel qtest)
> 
> Would probably the test succeed if run alone?
> 
> tests/qtest/qos-test -p /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/symlink_file
> 
> Best regards,
> Christian Schoenebeck
> 
> 

Re: [PULL 00/13] 9p queue 2020-10-23

[Dr. David Alan Gilbert]

* Greg Kurz (groug@kaod.org) wrote:
> On Mon, 26 Oct 2020 13:48:37 +0100
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> 
> > On Montag, 26. Oktober 2020 11:33:42 CET Peter Maydell wrote:
> > > On Fri, 23 Oct 2020 at 12:46, Christian Schoenebeck
> > > 
> > > <qemu_oss@crudebyte.com> wrote:
> > > > The following changes since commit 4c5b97bfd0dd54dc27717ae8d1cd10e14eef1430:
> > > >   Merge remote-tracking branch
> > > >   'remotes/kraxel/tags/modules-20201022-pull-request' into staging
> > > >   (2020-10-22 12:33:21 +0100)> 
> > > > are available in the Git repository at:
> > > >   https://github.com/cschoenebeck/qemu.git tags/pull-9p-20201023
> > > > 
> > > > for you to fetch changes up to ee01926a11b1f9bffcd6cdec0961dd9d1882da71:
> > > >   tests/9pfs: add local Tunlinkat hard link test (2020-10-22 20:26:33
> > > >   +0200)
> > > > 
> > > > ----------------------------------------------------------------
> > > > 9pfs: more tests using local fs driver
> > > > 
> > > > Only 9pfs test case changes this time:
> > > > 
> > > > * Refactor: Rename functions to make top-level test functions fs_*()
> > > > 
> > > >   easily distinguishable from utility test functions do_*().
> > > > 
> > > > * Refactor: Drop unnecessary function arguments in utility test
> > > > 
> > > >   functions.
> > > > 
> > > > * More test cases using the 9pfs 'local' filesystem driver backend,
> > > > 
> > > >   namely for the following 9p requests: Tunlinkat, Tlcreate, Tsymlink
> > > >   and Tlink.
> > > > 
> > > > ----------------------------------------------------------------
> > > 
> > > I get a 'make check' failure on x86-64 Linux host:
> > > 
> > > PASS 54 qtest-x86_64: qos-test
> > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> > > 9p-tests/local/config PASS 55 qtest-x86_64: qos-test
> > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> > > 9p-tests/local/create_dir PASS 56 qtest-x86_64: qos-test
> > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> > > 9p-tests/local/unlinkat_dir PASS 57 qtest-x86_64: qos-test
> > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> > > 9p-tests/local/create_file PASS 58 qtest-x86_64: qos-test
> > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> > > 9p-tests/local/unlinkat_file PASS 59 qtest-x86_64: qos-test
> > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> > > 9p-tests/local/symlink_file Received response 7 (RLERROR) instead of 73
> > > (RMKDIR)
> > > Rlerror has errno 2 (No such file or directory)
> > > **
> > > ERROR:../../tests/qtest/virtio-9p-test.c:300:v9fs_req_recv: assertion
> > > failed (hdr.id == id): (7 == 73)
> 
> Not sure this is related to this PR actually. Dave Gilbert reported on irc that
> he encountered a similar issue with 'make -j check', likely without these patches.

I was running on current master as of yesterday; no 9p specific patches.

Dave

> > > ERROR qtest-x86_64: qos-test - Bail out!
> > > ERROR:../../tests/qtest/virtio-9ptest.c:300:v9fs_req_recv: assertion
> > > failed (hdr.id == id): (7 == 73)
> > > Makefile.mtest:3953: recipe for target 'run-test-492' failed
> > > 
> > > 
> > > thanks
> > > -- PMM
> > 
> > So the 9p server is already failing to create the test case directory
> > "./qtest-9p-local/05/" relative to your current working directory.
> > 
> > I would appreciate to get more info when you have some free cycles, as I'm
> > unable to reproduce this on any system unfortunately. But no hurry as
> > these tests only become relevant actually for QEMU 6.
> > 
> > What puzzles me is that the previous test cases succeeded there, which all
> > create their own test directory in the same way:
> > 
> > 	./qtest-9p-local/01/
> > 	./qtest-9p-local/02/  (<-- dir vanishes after that test completed)
> > 	./qtest-9p-local/03/
> > 	./qtest-9p-local/04/
> > 	...
> > 
> > How does the "./qtest-9p-local/" directory look like after that
> > "local/symlink_file" test failed there? You can use this shortcut:
> > 
> > export QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
> > cd build
> > tests/qtest/qos-test --verbose
> > ls -l qtest-9p-local
> > 
> > That latter qos-test run will also output the assembled qemu command
> > line the 9p local tests would run with, which might also be helpful,
> > e.g. the relevant output would be something like this:
> > 
> > GTest: run: /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/config
> > (MSG: starting QEMU: exec x86_64-softmmu/qemu-system-x86_64 -qtest unix:/tmp/qtest-7428.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-7428.qmp,id=char0 -mon chardev=char0,mode=control -display none -M pc  -fsdev local,id=fsdev0,path='/home/me/git/qemu/build/qtest-9p-local',security_model=mapped-xattr -device virtio-9p-pci,fsdev=fsdev0,addr=04.0,mount_tag=qtest -accel qtest)
> > 
> > Would probably the test succeed if run alone?
> > 
> > tests/qtest/qos-test -p /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/symlink_file
> > 
> > Best regards,
> > Christian Schoenebeck
> > 
> > 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PULL 00/13] 9p queue 2020-10-23

[Christian Schoenebeck]

On Dienstag, 27. Oktober 2020 10:06:53 CET Dr. David Alan Gilbert wrote:
> * Greg Kurz (groug@kaod.org) wrote:
> > On Mon, 26 Oct 2020 13:48:37 +0100
> > 
> > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > On Montag, 26. Oktober 2020 11:33:42 CET Peter Maydell wrote:
> > > > On Fri, 23 Oct 2020 at 12:46, Christian Schoenebeck
> > > > 
> > > > <qemu_oss@crudebyte.com> wrote:
> > > > > The following changes since commit 
4c5b97bfd0dd54dc27717ae8d1cd10e14eef1430:
> > > > >   Merge remote-tracking branch
> > > > >   'remotes/kraxel/tags/modules-20201022-pull-request' into staging
> > > > >   (2020-10-22 12:33:21 +0100)>
> > > > > 
> > > > > are available in the Git repository at:
> > > > >   https://github.com/cschoenebeck/qemu.git tags/pull-9p-20201023
> > > > > 
> > > > > for you to fetch changes up to 
ee01926a11b1f9bffcd6cdec0961dd9d1882da71:
> > > > >   tests/9pfs: add local Tunlinkat hard link test (2020-10-22
> > > > >   20:26:33
> > > > >   +0200)
> > > > > 
> > > > > ----------------------------------------------------------------
> > > > > 9pfs: more tests using local fs driver
> > > > > 
> > > > > Only 9pfs test case changes this time:
> > > > > 
> > > > > * Refactor: Rename functions to make top-level test functions fs_*()
> > > > > 
> > > > >   easily distinguishable from utility test functions do_*().
> > > > > 
> > > > > * Refactor: Drop unnecessary function arguments in utility test
> > > > > 
> > > > >   functions.
> > > > > 
> > > > > * More test cases using the 9pfs 'local' filesystem driver backend,
> > > > > 
> > > > >   namely for the following 9p requests: Tunlinkat, Tlcreate,
> > > > >   Tsymlink
> > > > >   and Tlink.
> > > > > 
> > > > > ----------------------------------------------------------------
> > > > 
> > > > I get a 'make check' failure on x86-64 Linux host:
> > > > 
> > > > PASS 54 qtest-x86_64: qos-test
> > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/v
> > > > irtio- 9p-tests/local/config PASS 55 qtest-x86_64: qos-test
> > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/v
> > > > irtio- 9p-tests/local/create_dir PASS 56 qtest-x86_64: qos-test
> > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/v
> > > > irtio- 9p-tests/local/unlinkat_dir PASS 57 qtest-x86_64: qos-test
> > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/v
> > > > irtio- 9p-tests/local/create_file PASS 58 qtest-x86_64: qos-test
> > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/v
> > > > irtio- 9p-tests/local/unlinkat_file PASS 59 qtest-x86_64: qos-test
> > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/v
> > > > irtio- 9p-tests/local/symlink_file Received response 7 (RLERROR)
> > > > instead of 73 (RMKDIR)
> > > > Rlerror has errno 2 (No such file or directory)
> > > > **
> > > > ERROR:../../tests/qtest/virtio-9p-test.c:300:v9fs_req_recv: assertion
> > > > failed (hdr.id == id): (7 == 73)
> > 
> > Not sure this is related to this PR actually. Dave Gilbert reported on irc
> > that he encountered a similar issue with 'make -j check', likely without
> > these patches.
> I was running on current master as of yesterday; no 9p specific patches.
> 
> Dave

They might be related as the "local/create_dir" test is already merged, but 
hard to say reliably without any data.

How is reproducibility, sometimes / always?

> 
> > > > ERROR qtest-x86_64: qos-test - Bail out!
> > > > ERROR:../../tests/qtest/virtio-9ptest.c:300:v9fs_req_recv: assertion
> > > > failed (hdr.id == id): (7 == 73)
> > > > Makefile.mtest:3953: recipe for target 'run-test-492' failed
> > > > 
> > > > 
> > > > thanks
> > > > -- PMM
> > > 
> > > So the 9p server is already failing to create the test case directory
> > > "./qtest-9p-local/05/" relative to your current working directory.
> > > 
> > > I would appreciate to get more info when you have some free cycles, as
> > > I'm
> > > unable to reproduce this on any system unfortunately. But no hurry as
> > > these tests only become relevant actually for QEMU 6.
> > > 
> > > What puzzles me is that the previous test cases succeeded there, which
> > > all
> > > 
> > > create their own test directory in the same way:
> > > 	./qtest-9p-local/01/
> > > 	./qtest-9p-local/02/  (<-- dir vanishes after that test completed)
> > > 	./qtest-9p-local/03/
> > > 	./qtest-9p-local/04/
> > > 	...
> > > 
> > > How does the "./qtest-9p-local/" directory look like after that
> > > "local/symlink_file" test failed there? You can use this shortcut:
> > > 
> > > export QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
> > > cd build
> > > tests/qtest/qos-test --verbose
> > > ls -l qtest-9p-local
> > > 
> > > That latter qos-test run will also output the assembled qemu command
> > > line the 9p local tests would run with, which might also be helpful,
> > > e.g. the relevant output would be something like this:
> > > 
> > > GTest: run:
> > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/vi
> > > rtio-9p-tests/local/config (MSG: starting QEMU: exec
> > > x86_64-softmmu/qemu-system-x86_64 -qtest unix:/tmp/qtest-7428.sock
> > > -qtest-log /dev/null -chardev socket,path=/tmp/qtest-7428.qmp,id=char0
> > > -mon chardev=char0,mode=control -display none -M pc  -fsdev
> > > local,id=fsdev0,path='/home/me/git/qemu/build/qtest-9p-local',security_
> > > model=mapped-xattr -device
> > > virtio-9p-pci,fsdev=fsdev0,addr=04.0,mount_tag=qtest -accel qtest)
> > > 
> > > Would probably the test succeed if run alone?
> > > 
> > > tests/qtest/qos-test -p
> > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/vi
> > > rtio-9p-tests/local/symlink_file
> > > 
> > > Best regards,
> > > Christian Schoenebeck


Best regards,
Christian Schoenebeck

Re: [PULL 00/13] 9p queue 2020-10-23

[Dr. David Alan Gilbert]

* Christian Schoenebeck (qemu_oss@crudebyte.com) wrote:
> On Dienstag, 27. Oktober 2020 10:06:53 CET Dr. David Alan Gilbert wrote:
> > * Greg Kurz (groug@kaod.org) wrote:
> > > On Mon, 26 Oct 2020 13:48:37 +0100
> > > 
> > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > > On Montag, 26. Oktober 2020 11:33:42 CET Peter Maydell wrote:
> > > > > On Fri, 23 Oct 2020 at 12:46, Christian Schoenebeck
> > > > > 
> > > > > <qemu_oss@crudebyte.com> wrote:
> > > > > > The following changes since commit 
> 4c5b97bfd0dd54dc27717ae8d1cd10e14eef1430:
> > > > > >   Merge remote-tracking branch
> > > > > >   'remotes/kraxel/tags/modules-20201022-pull-request' into staging
> > > > > >   (2020-10-22 12:33:21 +0100)>
> > > > > > 
> > > > > > are available in the Git repository at:
> > > > > >   https://github.com/cschoenebeck/qemu.git tags/pull-9p-20201023
> > > > > > 
> > > > > > for you to fetch changes up to 
> ee01926a11b1f9bffcd6cdec0961dd9d1882da71:
> > > > > >   tests/9pfs: add local Tunlinkat hard link test (2020-10-22
> > > > > >   20:26:33
> > > > > >   +0200)
> > > > > > 
> > > > > > ----------------------------------------------------------------
> > > > > > 9pfs: more tests using local fs driver
> > > > > > 
> > > > > > Only 9pfs test case changes this time:
> > > > > > 
> > > > > > * Refactor: Rename functions to make top-level test functions fs_*()
> > > > > > 
> > > > > >   easily distinguishable from utility test functions do_*().
> > > > > > 
> > > > > > * Refactor: Drop unnecessary function arguments in utility test
> > > > > > 
> > > > > >   functions.
> > > > > > 
> > > > > > * More test cases using the 9pfs 'local' filesystem driver backend,
> > > > > > 
> > > > > >   namely for the following 9p requests: Tunlinkat, Tlcreate,
> > > > > >   Tsymlink
> > > > > >   and Tlink.
> > > > > > 
> > > > > > ----------------------------------------------------------------
> > > > > 
> > > > > I get a 'make check' failure on x86-64 Linux host:
> > > > > 
> > > > > PASS 54 qtest-x86_64: qos-test
> > > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/v
> > > > > irtio- 9p-tests/local/config PASS 55 qtest-x86_64: qos-test
> > > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/v
> > > > > irtio- 9p-tests/local/create_dir PASS 56 qtest-x86_64: qos-test
> > > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/v
> > > > > irtio- 9p-tests/local/unlinkat_dir PASS 57 qtest-x86_64: qos-test
> > > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/v
> > > > > irtio- 9p-tests/local/create_file PASS 58 qtest-x86_64: qos-test
> > > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/v
> > > > > irtio- 9p-tests/local/unlinkat_file PASS 59 qtest-x86_64: qos-test
> > > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/v
> > > > > irtio- 9p-tests/local/symlink_file Received response 7 (RLERROR)
> > > > > instead of 73 (RMKDIR)
> > > > > Rlerror has errno 2 (No such file or directory)
> > > > > **
> > > > > ERROR:../../tests/qtest/virtio-9p-test.c:300:v9fs_req_recv: assertion
> > > > > failed (hdr.id == id): (7 == 73)
> > > 
> > > Not sure this is related to this PR actually. Dave Gilbert reported on irc
> > > that he encountered a similar issue with 'make -j check', likely without
> > > these patches.
> > I was running on current master as of yesterday; no 9p specific patches.
> > 
> > Dave
> 
> They might be related as the "local/create_dir" test is already merged, but 
> hard to say reliably without any data.
> 
> How is reproducibility, sometimes / always?

I think I was seeing a few different errors; but I was running make
check -j 32 ish

Dave

> > 
> > > > > ERROR qtest-x86_64: qos-test - Bail out!
> > > > > ERROR:../../tests/qtest/virtio-9ptest.c:300:v9fs_req_recv: assertion
> > > > > failed (hdr.id == id): (7 == 73)
> > > > > Makefile.mtest:3953: recipe for target 'run-test-492' failed
> > > > > 
> > > > > 
> > > > > thanks
> > > > > -- PMM
> > > > 
> > > > So the 9p server is already failing to create the test case directory
> > > > "./qtest-9p-local/05/" relative to your current working directory.
> > > > 
> > > > I would appreciate to get more info when you have some free cycles, as
> > > > I'm
> > > > unable to reproduce this on any system unfortunately. But no hurry as
> > > > these tests only become relevant actually for QEMU 6.
> > > > 
> > > > What puzzles me is that the previous test cases succeeded there, which
> > > > all
> > > > 
> > > > create their own test directory in the same way:
> > > > 	./qtest-9p-local/01/
> > > > 	./qtest-9p-local/02/  (<-- dir vanishes after that test completed)
> > > > 	./qtest-9p-local/03/
> > > > 	./qtest-9p-local/04/
> > > > 	...
> > > > 
> > > > How does the "./qtest-9p-local/" directory look like after that
> > > > "local/symlink_file" test failed there? You can use this shortcut:
> > > > 
> > > > export QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
> > > > cd build
> > > > tests/qtest/qos-test --verbose
> > > > ls -l qtest-9p-local
> > > > 
> > > > That latter qos-test run will also output the assembled qemu command
> > > > line the 9p local tests would run with, which might also be helpful,
> > > > e.g. the relevant output would be something like this:
> > > > 
> > > > GTest: run:
> > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/vi
> > > > rtio-9p-tests/local/config (MSG: starting QEMU: exec
> > > > x86_64-softmmu/qemu-system-x86_64 -qtest unix:/tmp/qtest-7428.sock
> > > > -qtest-log /dev/null -chardev socket,path=/tmp/qtest-7428.qmp,id=char0
> > > > -mon chardev=char0,mode=control -display none -M pc  -fsdev
> > > > local,id=fsdev0,path='/home/me/git/qemu/build/qtest-9p-local',security_
> > > > model=mapped-xattr -device
> > > > virtio-9p-pci,fsdev=fsdev0,addr=04.0,mount_tag=qtest -accel qtest)
> > > > 
> > > > Would probably the test succeed if run alone?
> > > > 
> > > > tests/qtest/qos-test -p
> > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/vi
> > > > rtio-9p-tests/local/symlink_file
> > > > 
> > > > Best regards,
> > > > Christian Schoenebeck
> 
> 
> Best regards,
> Christian Schoenebeck
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PULL 00/13] 9p queue 2020-10-23

[Christian Schoenebeck]

On Dienstag, 27. Oktober 2020 11:26:53 CET Dr. David Alan Gilbert wrote:
> * Christian Schoenebeck (qemu_oss@crudebyte.com) wrote:
> > On Dienstag, 27. Oktober 2020 10:06:53 CET Dr. David Alan Gilbert wrote:
> > > * Greg Kurz (groug@kaod.org) wrote:
> > > > On Mon, 26 Oct 2020 13:48:37 +0100
> > > > 
> > > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > > > On Montag, 26. Oktober 2020 11:33:42 CET Peter Maydell wrote:
> > > > > > On Fri, 23 Oct 2020 at 12:46, Christian Schoenebeck
> > > > > > 
> > > > > > <qemu_oss@crudebyte.com> wrote:
> > > > > > > The following changes since commit
> > 
> > 4c5b97bfd0dd54dc27717ae8d1cd10e14eef1430:
> > > > > > >   Merge remote-tracking branch
> > > > > > >   'remotes/kraxel/tags/modules-20201022-pull-request' into
> > > > > > >   staging
> > > > > > >   (2020-10-22 12:33:21 +0100)>
> > > > > > > 
> > > > > > > are available in the Git repository at:
> > > > > > >   https://github.com/cschoenebeck/qemu.git tags/pull-9p-20201023
> > > > > > > 
> > > > > > > for you to fetch changes up to
> > 
> > ee01926a11b1f9bffcd6cdec0961dd9d1882da71:
> > > > > > >   tests/9pfs: add local Tunlinkat hard link test (2020-10-22
> > > > > > >   20:26:33
> > > > > > >   +0200)
> > > > > > > 
> > > > > > > ----------------------------------------------------------------
> > > > > > > 9pfs: more tests using local fs driver
> > > > > > > 
> > > > > > > Only 9pfs test case changes this time:
> > > > > > > 
> > > > > > > * Refactor: Rename functions to make top-level test functions
> > > > > > > fs_*()
> > > > > > > 
> > > > > > >   easily distinguishable from utility test functions do_*().
> > > > > > > 
> > > > > > > * Refactor: Drop unnecessary function arguments in utility test
> > > > > > > 
> > > > > > >   functions.
> > > > > > > 
> > > > > > > * More test cases using the 9pfs 'local' filesystem driver
> > > > > > > backend,
> > > > > > > 
> > > > > > >   namely for the following 9p requests: Tunlinkat, Tlcreate,
> > > > > > >   Tsymlink
> > > > > > >   and Tlink.
> > > > > > > 
> > > > > > > ----------------------------------------------------------------
> > > > > > 
> > > > > > I get a 'make check' failure on x86-64 Linux host:
> > > > > > 
> > > > > > PASS 54 qtest-x86_64: qos-test
> > > > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-
> > > > > > 9p/v
> > > > > > irtio- 9p-tests/local/config PASS 55 qtest-x86_64: qos-test
> > > > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-
> > > > > > 9p/v
> > > > > > irtio- 9p-tests/local/create_dir PASS 56 qtest-x86_64: qos-test
> > > > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-
> > > > > > 9p/v
> > > > > > irtio- 9p-tests/local/unlinkat_dir PASS 57 qtest-x86_64: qos-test
> > > > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-
> > > > > > 9p/v
> > > > > > irtio- 9p-tests/local/create_file PASS 58 qtest-x86_64: qos-test
> > > > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-
> > > > > > 9p/v
> > > > > > irtio- 9p-tests/local/unlinkat_file PASS 59 qtest-x86_64: qos-test
> > > > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-
> > > > > > 9p/v
> > > > > > irtio- 9p-tests/local/symlink_file Received response 7 (RLERROR)
> > > > > > instead of 73 (RMKDIR)
> > > > > > Rlerror has errno 2 (No such file or directory)
> > > > > > **
> > > > > > ERROR:../../tests/qtest/virtio-9p-test.c:300:v9fs_req_recv:
> > > > > > assertion
> > > > > > failed (hdr.id == id): (7 == 73)
> > > > 
> > > > Not sure this is related to this PR actually. Dave Gilbert reported on
> > > > irc
> > > > that he encountered a similar issue with 'make -j check', likely
> > > > without
> > > > these patches.
> > > 
> > > I was running on current master as of yesterday; no 9p specific patches.
> > > 
> > > Dave
> > 
> > They might be related as the "local/create_dir" test is already merged,
> > but
> > hard to say reliably without any data.
> > 
> > How is reproducibility, sometimes / always?
> 
> I think I was seeing a few different errors; but I was running make
> check -j 32 ish
> 
> Dave
> 

Ok, I understand, but how frequently are you able to trigger one of the test 
failures there? Does it happen like every time, or rather just once every xth 
run or so?

I'm running the qtests multi-threaded in a loop for several hours now, but so 
far I was unable to hit any test failure:

#/bin/sh
i=0
while true; do
  i=$[$i+1]
  echo '**************************************************'
  echo "* RUN $i                                         *"
  echo '**************************************************'
  make check-qtest -j32 V=1
  if [[ "$?" -ne 0 ]]; then
    echo "Aborted after $i runs due to failure"
    break
  fi
done

If you say it only happens once in a while, then I let it go for a day or 
more. However if it happens there quite frequently, then I guess I have to 
look into another aspect instead, like e.g. differences in the glib version.

> > > > > > ERROR qtest-x86_64: qos-test - Bail out!
> > > > > > ERROR:../../tests/qtest/virtio-9ptest.c:300:v9fs_req_recv:
> > > > > > assertion
> > > > > > failed (hdr.id == id): (7 == 73)
> > > > > > Makefile.mtest:3953: recipe for target 'run-test-492' failed
> > > > > > 
> > > > > > 
> > > > > > thanks
> > > > > > -- PMM
> > > > > 
> > > > > So the 9p server is already failing to create the test case
> > > > > directory
> > > > > "./qtest-9p-local/05/" relative to your current working directory.
> > > > > 
> > > > > I would appreciate to get more info when you have some free cycles,
> > > > > as
> > > > > I'm
> > > > > unable to reproduce this on any system unfortunately. But no hurry
> > > > > as
> > > > > these tests only become relevant actually for QEMU 6.
> > > > > 
> > > > > What puzzles me is that the previous test cases succeeded there,
> > > > > which
> > > > > all
> > > > > 
> > > > > create their own test directory in the same way:
> > > > > 	./qtest-9p-local/01/
> > > > > 	./qtest-9p-local/02/  (<-- dir vanishes after that test completed)
> > > > > 	./qtest-9p-local/03/
> > > > > 	./qtest-9p-local/04/
> > > > > 	...
> > > > > 
> > > > > How does the "./qtest-9p-local/" directory look like after that
> > > > > "local/symlink_file" test failed there? You can use this shortcut:
> > > > > 
> > > > > export QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
> > > > > cd build
> > > > > tests/qtest/qos-test --verbose
> > > > > ls -l qtest-9p-local
> > > > > 
> > > > > That latter qos-test run will also output the assembled qemu command
> > > > > line the 9p local tests would run with, which might also be helpful,
> > > > > e.g. the relevant output would be something like this:
> > > > > 
> > > > > GTest: run:
> > > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p
> > > > > /vi
> > > > > rtio-9p-tests/local/config (MSG: starting QEMU: exec
> > > > > x86_64-softmmu/qemu-system-x86_64 -qtest unix:/tmp/qtest-7428.sock
> > > > > -qtest-log /dev/null -chardev
> > > > > socket,path=/tmp/qtest-7428.qmp,id=char0
> > > > > -mon chardev=char0,mode=control -display none -M pc  -fsdev
> > > > > local,id=fsdev0,path='/home/me/git/qemu/build/qtest-9p-local',securi
> > > > > ty_
> > > > > model=mapped-xattr -device
> > > > > virtio-9p-pci,fsdev=fsdev0,addr=04.0,mount_tag=qtest -accel qtest)
> > > > > 
> > > > > Would probably the test succeed if run alone?
> > > > > 
> > > > > tests/qtest/qos-test -p
> > > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p
> > > > > /vi
> > > > > rtio-9p-tests/local/symlink_file
> > > > > 
> > > > > Best regards,
> > > > > Christian Schoenebeck
> > 
> > Best regards,
> > Christian Schoenebeck

Re: [PULL 00/13] 9p queue 2020-10-23

[Peter Maydell]

On Mon, 26 Oct 2020 at 12:48, Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
>
> On Montag, 26. Oktober 2020 11:33:42 CET Peter Maydell wrote:
> > I get a 'make check' failure on x86-64 Linux host:
> >
> > PASS 54 qtest-x86_64: qos-test
> > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> > 9p-tests/local/config PASS 55 qtest-x86_64: qos-test
> > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> > 9p-tests/local/create_dir PASS 56 qtest-x86_64: qos-test
> > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> > 9p-tests/local/unlinkat_dir PASS 57 qtest-x86_64: qos-test
> > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> > 9p-tests/local/create_file PASS 58 qtest-x86_64: qos-test
> > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> > 9p-tests/local/unlinkat_file PASS 59 qtest-x86_64: qos-test
> > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-
> > 9p-tests/local/symlink_file Received response 7 (RLERROR) instead of 73
> > (RMKDIR)
> > Rlerror has errno 2 (No such file or directory)
> > **
> > ERROR:../../tests/qtest/virtio-9p-test.c:300:v9fs_req_recv: assertion
> > failed (hdr.id == id): (7 == 73)
> > ERROR qtest-x86_64: qos-test - Bail out!
> > ERROR:../../tests/qtest/virtio-9ptest.c:300:v9fs_req_recv: assertion
> > failed (hdr.id == id): (7 == 73)
> > Makefile.mtest:3953: recipe for target 'run-test-492' failed

I just got this again on an entirely different pullreq so that
suggests that this is indeed an intermittent currently in master:

PASS 49 qtest-i386/qos-test
/i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/synth/flush/ignored
PASS 50 qtest-i386/qos-test
/i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/synth/readdir/basic
PASS 51 qtest-i386/qos-test
/i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/synth/readdir/split_512
PASS 52 qtest-i386/qos-test
/i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/synth/readdir/split_256
PASS 53 qtest-i386/qos-test
/i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/synth/readdir/split_128
PASS 54 qtest-i386/qos-test
/i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/config
Received response 7 (RLERROR) instead of 73 (RMKDIR)
Rlerror has errno 2 (No such file or directory)
**
ERROR:../../tests/qtest/virtio-9p-test.c:296:v9fs_req_recv: assertion
failed (hdr.id == id): (7 == 73)
ERROR qtest-i386/qos-test - Bail out!
ERROR:../../tests/qtest/virtio-9p-test.c:296:v9fs_req_recv: assertion
failed (hdr.id == id): (7 == 73)
Makefile.mtest:1857: recipe for target 'run-test-230' failed

> So the 9p server is already failing to create the test case directory
> "./qtest-9p-local/05/" relative to your current working directory.

This sounds suspicious, because there's nothing in that filename
that's specific to the test case being qtest-i386 and not
qtest-something-else. How does the test harness deal with the
possibility of the same virtio-9p-pci test being run in parallel
for multiple guest architectures under a make -jN setup ?

> What puzzles me is that the previous test cases succeeded there, which all
> create their own test directory in the same way:
>
>         ./qtest-9p-local/01/
>         ./qtest-9p-local/02/  (<-- dir vanishes after that test completed)
>         ./qtest-9p-local/03/
>         ./qtest-9p-local/04/
>         ...

After the build failed, the qtest-9p-local directory was empty.

thanks
-- PMM

Re: [PULL 00/13] 9p queue 2020-10-23

[Christian Schoenebeck]

On Donnerstag, 29. Oktober 2020 14:20:11 CET Peter Maydell wrote:
> On Mon, 26 Oct 2020 at 12:48, Christian Schoenebeck
> 
> <qemu_oss@crudebyte.com> wrote:
> > On Montag, 26. Oktober 2020 11:33:42 CET Peter Maydell wrote:
> > > I get a 'make check' failure on x86-64 Linux host:
> > > 
> > > PASS 54 qtest-x86_64: qos-test
> > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/vir
> > > tio- 9p-tests/local/config PASS 55 qtest-x86_64: qos-test
> > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/vir
> > > tio- 9p-tests/local/create_dir PASS 56 qtest-x86_64: qos-test
> > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/vir
> > > tio- 9p-tests/local/unlinkat_dir PASS 57 qtest-x86_64: qos-test
> > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/vir
> > > tio- 9p-tests/local/create_file PASS 58 qtest-x86_64: qos-test
> > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/vir
> > > tio- 9p-tests/local/unlinkat_file PASS 59 qtest-x86_64: qos-test
> > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/vir
> > > tio- 9p-tests/local/symlink_file Received response 7 (RLERROR) instead
> > > of 73 (RMKDIR)
> > > Rlerror has errno 2 (No such file or directory)
> > > **
> > > ERROR:../../tests/qtest/virtio-9p-test.c:300:v9fs_req_recv: assertion
> > > failed (hdr.id == id): (7 == 73)
> > > ERROR qtest-x86_64: qos-test - Bail out!
> > > ERROR:../../tests/qtest/virtio-9ptest.c:300:v9fs_req_recv: assertion
> > > failed (hdr.id == id): (7 == 73)
> > > Makefile.mtest:3953: recipe for target 'run-test-492' failed
> 
> I just got this again on an entirely different pullreq so that
> suggests that this is indeed an intermittent currently in master:
> 
> PASS 49 qtest-i386/qos-test
> /i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p
> -tests/synth/flush/ignored PASS 50 qtest-i386/qos-test
> /i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p
> -tests/synth/readdir/basic PASS 51 qtest-i386/qos-test
> /i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p
> -tests/synth/readdir/split_512 PASS 52 qtest-i386/qos-test
> /i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p
> -tests/synth/readdir/split_256 PASS 53 qtest-i386/qos-test
> /i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p
> -tests/synth/readdir/split_128 PASS 54 qtest-i386/qos-test
> /i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p
> -tests/local/config Received response 7 (RLERROR) instead of 73 (RMKDIR)
> Rlerror has errno 2 (No such file or directory)
> **
> ERROR:../../tests/qtest/virtio-9p-test.c:296:v9fs_req_recv: assertion
> failed (hdr.id == id): (7 == 73)
> ERROR qtest-i386/qos-test - Bail out!
> ERROR:../../tests/qtest/virtio-9p-test.c:296:v9fs_req_recv: assertion
> failed (hdr.id == id): (7 == 73)
> Makefile.mtest:1857: recipe for target 'run-test-230' failed
> 
> > So the 9p server is already failing to create the test case directory
> > "./qtest-9p-local/05/" relative to your current working directory.
> 
> This sounds suspicious, because there's nothing in that filename
> that's specific to the test case being qtest-i386 and not
> qtest-something-else. How does the test harness deal with the
> possibility of the same virtio-9p-pci test being run in parallel
> for multiple guest architectures under a make -jN setup ?

Aaaaah, now there we go!

I was actually running the tests for 2 days and >3000 test suite runs now 
without a single 9p test failure, but ... not for multiple architectures 
simultaniously.

Another point for centralizing test dir locations in future.

> 
> > What puzzles me is that the previous test cases succeeded there, which all
> > 
> > create their own test directory in the same way:
> >         ./qtest-9p-local/01/
> >         ./qtest-9p-local/02/  (<-- dir vanishes after that test completed)
> >         ./qtest-9p-local/03/
> >         ./qtest-9p-local/04/
> >         ...
> 
> After the build failed, the qtest-9p-local directory was empty.

Yes, that suggests a parallel test suite was wiping the test directory
'./qtest-9p-local'.

So I'll append the architecture to the test dir location. To nail it this 
time, anyting else that would come to your mind regarding test dirs?

> 
> thanks
> -- PMM

Best regards,
Christian Schoenebeck

Re: [PULL 00/13] 9p queue 2020-10-23

[Peter Maydell]

On Thu, 29 Oct 2020 at 13:48, Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
> So I'll append the architecture to the test dir location. To nail it this
> time, anyting else that would come to your mind regarding test dirs?

I think most tests that need a temp directory set one
up using mkdtemp().

thanks
-- PMM

Re: [PULL 00/13] 9p queue 2020-10-23

[Christian Schoenebeck]

On Donnerstag, 29. Oktober 2020 14:57:45 CET Peter Maydell wrote:
> On Thu, 29 Oct 2020 at 13:48, Christian Schoenebeck
> 
> <qemu_oss@crudebyte.com> wrote:
> > So I'll append the architecture to the test dir location. To nail it this
> > time, anyting else that would come to your mind regarding test dirs?
> 
> I think most tests that need a temp directory set one
> up using mkdtemp().
> 
> thanks
> -- PMM

Ok, I'll use mkdtemp() instead, that avoids other potential parallel config 
colissions that I may not have considered yet.

The original motivation against mkdtemp() was that /tmp is isually a ramfs, 
hence very limited regarding large file tests. But that's not an issue right 
now.

Thanks Peter!

Best regards,
Christian Schoenebeck

Re: [PULL 00/13] 9p queue 2020-10-23

[Greg Kurz]

On Thu, 29 Oct 2020 13:57:45 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Thu, 29 Oct 2020 at 13:48, Christian Schoenebeck
> <qemu_oss@crudebyte.com> wrote:
> > So I'll append the architecture to the test dir location. To nail it this
> > time, anyting else that would come to your mind regarding test dirs?
> 
> I think most tests that need a temp directory set one
> up using mkdtemp().
> 

Yeah, early versions of the 9p tests that were using the local fsdev
backend relied on mkdtemp().

> thanks
> -- PMM

Re: [PULL 00/13] 9p queue 2020-10-23

[Peter Maydell]

On Thu, 29 Oct 2020 at 14:06, Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
> Ok, I'll use mkdtemp() instead, that avoids other potential parallel config
> colissions that I may not have considered yet.
>
> The original motivation against mkdtemp() was that /tmp is isually a ramfs,
> hence very limited regarding large file tests. But that's not an issue right
> now.

How large is "large" here ?

thanks
-- PMM

Re: [PULL 00/13] 9p queue 2020-10-23

[Christian Schoenebeck]

On Donnerstag, 29. Oktober 2020 15:15:19 CET Peter Maydell wrote:
> On Thu, 29 Oct 2020 at 14:06, Christian Schoenebeck
> 
> <qemu_oss@crudebyte.com> wrote:
> > Ok, I'll use mkdtemp() instead, that avoids other potential parallel
> > config
> > colissions that I may not have considered yet.
> > 
> > The original motivation against mkdtemp() was that /tmp is isually a
> > ramfs,
> > hence very limited regarding large file tests. But that's not an issue
> > right now.
> 
> How large is "large" here ?
> 
> thanks
> -- PMM

E.g. ~10 GiB which might be a problem for cloud based CI platforms.

But again, we don't have any 9p test doing that yet. So mkdtemp() is just fine 
for now.

Best regards,
Christian Schoenebeck

Re: [PULL 00/13] 9p queue 2020-10-23

[Peter Maydell]

On Thu, 29 Oct 2020 at 14:31, Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
>
> On Donnerstag, 29. Oktober 2020 15:15:19 CET Peter Maydell wrote:
> > On Thu, 29 Oct 2020 at 14:06, Christian Schoenebeck
> >
> > <qemu_oss@crudebyte.com> wrote:
> > > Ok, I'll use mkdtemp() instead, that avoids other potential parallel
> > > config
> > > colissions that I may not have considered yet.
> > >
> > > The original motivation against mkdtemp() was that /tmp is isually a
> > > ramfs,
> > > hence very limited regarding large file tests. But that's not an issue
> > > right now.
> >
> > How large is "large" here ?

> E.g. ~10 GiB which might be a problem for cloud based CI platforms.

Yeah, 10GB is too big by an order of magnitude for anything in the
standard "make check" set. It could go in an optional "I'm the 9p
maintainer and I want to run a wider set of tests" target though.

thanks
-- PMM

Re: [PULL 00/13] 9p queue 2020-10-23

[Daniel P. Berrangé]

On Thu, Oct 29, 2020 at 02:52:16PM +0000, Peter Maydell wrote:
> On Thu, 29 Oct 2020 at 14:31, Christian Schoenebeck
> <qemu_oss@crudebyte.com> wrote:
> >
> > On Donnerstag, 29. Oktober 2020 15:15:19 CET Peter Maydell wrote:
> > > On Thu, 29 Oct 2020 at 14:06, Christian Schoenebeck
> > >
> > > <qemu_oss@crudebyte.com> wrote:
> > > > Ok, I'll use mkdtemp() instead, that avoids other potential parallel
> > > > config
> > > > colissions that I may not have considered yet.
> > > >
> > > > The original motivation against mkdtemp() was that /tmp is isually a
> > > > ramfs,
> > > > hence very limited regarding large file tests. But that's not an issue
> > > > right now.
> > >
> > > How large is "large" here ?
> 
> > E.g. ~10 GiB which might be a problem for cloud based CI platforms.
> 
> Yeah, 10GB is too big by an order of magnitude for anything in the
> standard "make check" set. It could go in an optional "I'm the 9p
> maintainer and I want to run a wider set of tests" target though.

I think as a rough rule of thumb, tests should not create files
that are larger than the size of the QEMU build dir, and if it is
creating non-trivially sized files, then they should be created in
the build dir, not /tmp.  This probably puts 1 GB as a upper bound
on size but bear in mind tests can run in parallel, so ideally biggest
file sizes should be more in the 100s of MB range


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PULL 00/13] 9p queue 2020-10-23

[Christian Schoenebeck]

On Donnerstag, 29. Oktober 2020 16:04:03 CET Daniel P. Berrangé wrote:
> On Thu, Oct 29, 2020 at 02:52:16PM +0000, Peter Maydell wrote:
> > On Thu, 29 Oct 2020 at 14:31, Christian Schoenebeck
> > 
> > <qemu_oss@crudebyte.com> wrote:
> > > On Donnerstag, 29. Oktober 2020 15:15:19 CET Peter Maydell wrote:
> > > > On Thu, 29 Oct 2020 at 14:06, Christian Schoenebeck
> > > > 
> > > > <qemu_oss@crudebyte.com> wrote:
> > > > > Ok, I'll use mkdtemp() instead, that avoids other potential parallel
> > > > > config
> > > > > colissions that I may not have considered yet.
> > > > > 
> > > > > The original motivation against mkdtemp() was that /tmp is isually a
> > > > > ramfs,
> > > > > hence very limited regarding large file tests. But that's not an
> > > > > issue
> > > > > right now.
> > > > 
> > > > How large is "large" here ?
> > > 
> > > E.g. ~10 GiB which might be a problem for cloud based CI platforms.
> > 
> > Yeah, 10GB is too big by an order of magnitude for anything in the
> > standard "make check" set. It could go in an optional "I'm the 9p
> > maintainer and I want to run a wider set of tests" target though.
> 
> I think as a rough rule of thumb, tests should not create files
> that are larger than the size of the QEMU build dir, and if it is
> creating non-trivially sized files, then they should be created in
> the build dir, not /tmp.  This probably puts 1 GB as a upper bound
> on size but bear in mind tests can run in parallel, so ideally biggest
> file sizes should be more in the 100s of MB range
> 
> Regards,
> Daniel

I definitely won't run such large-file tests uncaged, so it would not run by
default for sure. But sooner or later it would make sense to streamline test
case options in QEMU in general, so that certain tests would automatically run
or not depending on runner's capabilities, limits and preferences.

The TCG tests for instance periodically trigger test failures as they exceed
Travis-CI's runtime limit of 40mins once in a while.

Back to the actual 9p test failure issue, I think I will go for something like
this:

diff --git a/tests/qtest/libqos/virtio-9p.c b/tests/qtest/libqos/virtio-9p.c
index d43647b3b7..ebbacd5896 100644
--- a/tests/qtest/libqos/virtio-9p.c
+++ b/tests/qtest/libqos/virtio-9p.c
@@ -35,7 +35,12 @@ static char *concat_path(const char* a, const char* b)
 static void init_local_test_path(void)
 {
     char *pwd = g_get_current_dir();
-    local_test_path = concat_path(pwd, "qtest-9p-local");
+    char *template = concat_path(pwd, "qtest-9p-local-XXXXXX");
+    local_test_path = mkdtemp(template);
+    if (!local_test_path) {
+        g_test_message("mkdtemp('%s') failed: %s", template, strerror(errno));
+    }
+    g_assert(local_test_path);
     g_free(pwd);
 }
 
@@ -246,11 +251,6 @@ static void virtio_9p_register_nodes(void)
     const char *str_simple = "fsdev=fsdev0,mount_tag=" MOUNT_TAG;
     const char *str_addr = "fsdev=fsdev0,addr=04.0,mount_tag=" MOUNT_TAG;
 
-    /* make sure test dir for the 'local' tests exists and is clean */
-    init_local_test_path();
-    remove_local_test_dir();
-    create_local_test_dir();
-
     QPCIAddress addr = {
         .devfn = QPCI_DEVFN(4, 0),
     };
@@ -278,3 +278,14 @@ static void virtio_9p_register_nodes(void)
 }
 
 libqos_init(virtio_9p_register_nodes);
+
+static void __attribute__((constructor)) construct_virtio_9p(void) {
+    /* make sure test dir for the 'local' tests exists */
+    init_local_test_path();
+    create_local_test_dir();
+}
+
+static void __attribute__((destructor)) destruct_virtio_9p(void) {
+    /* remove previously created test dir when test suite completed */
+    remove_local_test_dir();
+}

So it would still use the current directory instead of /tmp/ and would
create and remove the test dir on process start/end. I wanted to avoid
using constructor/destructor atttributes, as some compilers don't
support them, but it seems it's already used in the code base and there
are currently no setup/teardown callbacks in libqos.

Another downside: with 'make check -jN' this will temporarily create a
bunch of unused, empty dirs. But I hope that's Ok to fix in QEMU 6
(by adding setup/teardown callbacks to libqos later).

Just hoping there are no glib versions that use threads instead of
processes.

Best regards,
Christian Schoenebeck

Re: [PULL 00/13] 9p queue 2020-10-23

[Daniel P. Berrangé]

On Thu, Oct 29, 2020 at 06:27:21PM +0100, Christian Schoenebeck wrote:
> On Donnerstag, 29. Oktober 2020 16:04:03 CET Daniel P. Berrangé wrote:
> > On Thu, Oct 29, 2020 at 02:52:16PM +0000, Peter Maydell wrote:
> > > On Thu, 29 Oct 2020 at 14:31, Christian Schoenebeck
> > > 
> > > <qemu_oss@crudebyte.com> wrote:
> > > > On Donnerstag, 29. Oktober 2020 15:15:19 CET Peter Maydell wrote:
> > > > > On Thu, 29 Oct 2020 at 14:06, Christian Schoenebeck
> > > > > 
> > > > > <qemu_oss@crudebyte.com> wrote:
> > > > > > Ok, I'll use mkdtemp() instead, that avoids other potential parallel
> > > > > > config
> > > > > > colissions that I may not have considered yet.
> > > > > > 
> > > > > > The original motivation against mkdtemp() was that /tmp is isually a
> > > > > > ramfs,
> > > > > > hence very limited regarding large file tests. But that's not an
> > > > > > issue
> > > > > > right now.

snip

> @@ -278,3 +278,14 @@ static void virtio_9p_register_nodes(void)
>  }
>  
>  libqos_init(virtio_9p_register_nodes);
> +
> +static void __attribute__((constructor)) construct_virtio_9p(void) {
> +    /* make sure test dir for the 'local' tests exists */
> +    init_local_test_path();
> +    create_local_test_dir();
> +}
> +
> +static void __attribute__((destructor)) destruct_virtio_9p(void) {
> +    /* remove previously created test dir when test suite completed */
> +    remove_local_test_dir();
> +}
> 
> So it would still use the current directory instead of /tmp/ and would
> create and remove the test dir on process start/end. I wanted to avoid
> using constructor/destructor atttributes, as some compilers don't
> support them, but it seems it's already used in the code base and there
> are currently no setup/teardown callbacks in libqos.

QEMU explicitly only supports CLang and GCC, and our code assumes
constructors/destructors work.

IOW, you should ignore the portability concern. We don't care about
other vendor compilers with poor feature sets.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|