All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/8] tests/9pfs: simplify fs_mkdir()
  2020-10-19 23:17 [PATCH 0/8] 9pfs: more local tests Christian Schoenebeck
  2020-10-19 23:13 ` [PATCH 2/8] tests/9pfs: add local unlinkat directory test Christian Schoenebeck
@ 2020-10-19 23:13 ` Christian Schoenebeck
  2020-10-20 13:35   ` Greg Kurz
  2020-10-19 23:13 ` [PATCH 8/8] tests/9pfs: add local unlinkat hard link test Christian Schoenebeck
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Christian Schoenebeck @ 2020-10-19 23:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz

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

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

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

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index c15908f27b..dc724bbb1e 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -967,13 +967,12 @@ 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)
+/* utility function: walk to requested dir and return fid for that dir */
+static uint32_t fs_walk_fid(void *obj, void *data, QGuestAllocator *t_alloc,
+                            const char *path)
 {
     QVirtio9P *v9p = obj;
-    alloc = t_alloc;
     char **wnames;
-    char *const name = g_strdup(cname);
     P9Req *req;
     const uint32_t fid = genfid();
 
@@ -983,12 +982,26 @@ static void fs_mkdir(void *obj, void *data, QGuestAllocator *t_alloc,
     v9fs_req_wait_for_reply(req, NULL);
     v9fs_rwalk(req, NULL, NULL);
 
+    split_free(&wnames);
+    return fid;
+}
+
+static void fs_mkdir(void *obj, void *data, QGuestAllocator *t_alloc,
+                     const char *path, const char *cname)
+{
+    QVirtio9P *v9p = obj;
+    alloc = t_alloc;
+    char *const name = g_strdup(cname);
+    uint32_t fid;
+    P9Req *req;
+
+    fid = fs_walk_fid(v9p, data, t_alloc, 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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/8] tests/9pfs: add local unlinkat directory test
  2020-10-19 23:17 [PATCH 0/8] 9pfs: more local tests Christian Schoenebeck
@ 2020-10-19 23:13 ` Christian Schoenebeck
  2020-10-19 23:13 ` [PATCH 1/8] tests/9pfs: simplify fs_mkdir() Christian Schoenebeck
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Christian Schoenebeck @ 2020-10-19 23:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz

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>
---
 tests/qtest/virtio-9p-test.c | 72 ++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index dc724bbb1e..990d074d33 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>";
@@ -668,6 +669,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,24 @@ static void fs_mkdir(void *obj, void *data, QGuestAllocator *t_alloc,
     g_free(name);
 }
 
+static void fs_unlinkat(void *obj, void *data, QGuestAllocator *t_alloc,
+                        const char *atpath, const char *rpath, uint32_t flags)
+{
+    QVirtio9P *v9p = obj;
+    alloc = t_alloc;
+    char *const name = g_strdup(rpath);
+    uint32_t fid;
+    P9Req *req;
+
+    fid = fs_walk_fid(v9p, data, t_alloc, 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)
 {
@@ -1046,6 +1092,31 @@ 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;
+    struct stat st;
+    char *root_path = virtio_9p_test_path("");
+    char *new_dir = virtio_9p_test_path("02");
+
+    g_assert(root_path != NULL);
+
+    fs_attach(v9p, NULL, t_alloc);
+    fs_mkdir(v9p, data, t_alloc, "/", "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);
+
+    fs_unlinkat(v9p, data, t_alloc, "/", "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");
@@ -1086,6 +1157,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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/8] tests/9pfs: add local Tsymlink test
  2020-10-19 23:17 [PATCH 0/8] 9pfs: more local tests Christian Schoenebeck
                   ` (4 preceding siblings ...)
  2020-10-19 23:13 ` [PATCH 3/8] tests/9pfs: add local Tlcreate test Christian Schoenebeck
@ 2020-10-19 23:13 ` Christian Schoenebeck
  2020-10-19 23:13 ` [PATCH 4/8] tests/9pfs: add local unlinkat file test Christian Schoenebeck
  2020-10-19 23:13 ` [PATCH 6/8] tests/9pfs: add local unlinkat symlink test Christian Schoenebeck
  7 siblings, 0 replies; 15+ messages in thread
From: Christian Schoenebeck @ 2020-10-19 23:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz

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>
---
 tests/qtest/virtio-9p-test.c | 78 ++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 06a9f10d34..78f4ed7e5f 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" :
@@ -708,6 +709,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)
@@ -1091,6 +1125,27 @@ static uint32_t fs_lcreate(void *obj, void *data, QGuestAllocator *t_alloc,
     return fid;
 }
 
+/* create symlink named @a clink in directory @a path pointing to @a to */
+static void fs_symlink(void *obj, void *data, QGuestAllocator *t_alloc,
+                       const char *path, const char *clink, const char *to)
+{
+    QVirtio9P *v9p = obj;
+    alloc = t_alloc;
+    char *const name = g_strdup(clink);
+    char *const dst = g_strdup(to);
+    uint32_t fid;
+    P9Req *req;
+
+    fid = fs_walk_fid(v9p, data, t_alloc, 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 fs_unlinkat(void *obj, void *data, QGuestAllocator *t_alloc,
                         const char *atpath, const char *rpath, uint32_t flags)
 {
@@ -1216,6 +1271,28 @@ 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;
+    struct stat st;
+    char *real_file = virtio_9p_test_path("05/real_file");
+    char *symlink_file = virtio_9p_test_path("05/symlink_file");
+
+    fs_attach(v9p, NULL, t_alloc);
+    fs_mkdir(v9p, data, t_alloc, "/", "05");
+    fs_lcreate(v9p, data, t_alloc, "05", "real_file");
+    g_assert(stat(real_file, &st) == 0);
+    g_assert((st.st_mode & S_IFMT) == S_IFREG);
+
+    fs_symlink(v9p, data, t_alloc, "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");
@@ -1259,6 +1336,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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/8] tests/9pfs: add local Tlcreate test
  2020-10-19 23:17 [PATCH 0/8] 9pfs: more local tests Christian Schoenebeck
                   ` (3 preceding siblings ...)
  2020-10-19 23:13 ` [PATCH 7/8] tests/9pfs: add local Tlink test Christian Schoenebeck
@ 2020-10-19 23:13 ` Christian Schoenebeck
  2020-10-19 23:13 ` [PATCH 5/8] tests/9pfs: add local Tsymlink test Christian Schoenebeck
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Christian Schoenebeck @ 2020-10-19 23:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz

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>
---
 tests/qtest/virtio-9p-test.c | 78 ++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 990d074d33..1b133f52bd 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" :
@@ -669,6 +670,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,26 @@ static void fs_mkdir(void *obj, void *data, QGuestAllocator *t_alloc,
     g_free(name);
 }
 
+/* create a regular file with Tlcreate and return file's fid */
+static uint32_t fs_lcreate(void *obj, void *data, QGuestAllocator *t_alloc,
+                           const char *path, const char *cname)
+{
+    QVirtio9P *v9p = obj;
+    alloc = t_alloc;
+    char *const name = g_strdup(cname);
+    uint32_t fid;
+    P9Req *req;
+
+    fid = fs_walk_fid(v9p, data, t_alloc, 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 fs_unlinkat(void *obj, void *data, QGuestAllocator *t_alloc,
                         const char *atpath, const char *rpath, uint32_t flags)
 {
@@ -1117,6 +1176,24 @@ 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;
+    struct stat st;
+    char *new_file = virtio_9p_test_path("03/1st_file");
+
+    fs_attach(v9p, NULL, t_alloc);
+    fs_mkdir(v9p, data, t_alloc, "/", "03");
+    fs_lcreate(v9p, data, t_alloc, "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");
@@ -1158,6 +1235,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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/8] tests/9pfs: add local unlinkat file test
  2020-10-19 23:17 [PATCH 0/8] 9pfs: more local tests Christian Schoenebeck
                   ` (5 preceding siblings ...)
  2020-10-19 23:13 ` [PATCH 5/8] tests/9pfs: add local Tsymlink test Christian Schoenebeck
@ 2020-10-19 23:13 ` Christian Schoenebeck
  2020-10-19 23:13 ` [PATCH 6/8] tests/9pfs: add local unlinkat symlink test Christian Schoenebeck
  7 siblings, 0 replies; 15+ messages in thread
From: Christian Schoenebeck @ 2020-10-19 23:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz

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>
---
 tests/qtest/virtio-9p-test.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 1b133f52bd..06a9f10d34 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -1194,6 +1194,28 @@ 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;
+    struct stat st;
+    char *new_file = virtio_9p_test_path("04/doa_file");
+
+    fs_attach(v9p, NULL, t_alloc);
+    fs_mkdir(v9p, data, t_alloc, "/", "04");
+    fs_lcreate(v9p, data, t_alloc, "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);
+
+    fs_unlinkat(v9p, data, t_alloc, "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");
@@ -1236,6 +1258,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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 6/8] tests/9pfs: add local unlinkat symlink test
  2020-10-19 23:17 [PATCH 0/8] 9pfs: more local tests Christian Schoenebeck
                   ` (6 preceding siblings ...)
  2020-10-19 23:13 ` [PATCH 4/8] tests/9pfs: add local unlinkat file test Christian Schoenebeck
@ 2020-10-19 23:13 ` Christian Schoenebeck
  7 siblings, 0 replies; 15+ messages in thread
From: Christian Schoenebeck @ 2020-10-19 23:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz

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>
---
 tests/qtest/virtio-9p-test.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 78f4ed7e5f..f7d18f6274 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -1293,6 +1293,31 @@ 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;
+    struct stat st;
+    char *real_file = virtio_9p_test_path("06/real_file");
+    char *symlink_file = virtio_9p_test_path("06/symlink_file");
+
+    fs_attach(v9p, NULL, t_alloc);
+    fs_mkdir(v9p, data, t_alloc, "/", "06");
+    fs_lcreate(v9p, data, t_alloc, "06", "real_file");
+    g_assert(stat(real_file, &st) == 0);
+    g_assert((st.st_mode & S_IFMT) == S_IFREG);
+
+    fs_symlink(v9p, data, t_alloc, "06", "symlink_file", "real_file");
+    g_assert(stat(symlink_file, &st) == 0);
+
+    fs_unlinkat(v9p, data, t_alloc, "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");
@@ -1337,6 +1362,7 @@ 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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 7/8] tests/9pfs: add local Tlink test
  2020-10-19 23:17 [PATCH 0/8] 9pfs: more local tests Christian Schoenebeck
                   ` (2 preceding siblings ...)
  2020-10-19 23:13 ` [PATCH 8/8] tests/9pfs: add local unlinkat hard link test Christian Schoenebeck
@ 2020-10-19 23:13 ` Christian Schoenebeck
  2020-10-20  0:01   ` Christian Schoenebeck
  2020-10-19 23:13 ` [PATCH 3/8] tests/9pfs: add local Tlcreate test Christian Schoenebeck
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Christian Schoenebeck @ 2020-10-19 23:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz

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>
---
 tests/qtest/virtio-9p-test.c | 61 ++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index f7d18f6274..447d8e3344 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" :
@@ -742,6 +743,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)
@@ -1318,6 +1346,38 @@ 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;
+    P9Req *req;
+    uint32_t dfid, fid;
+    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");
+
+    fs_attach(v9p, NULL, t_alloc);
+    fs_mkdir(v9p, data, t_alloc, "/", "07");
+    fid = fs_lcreate(v9p, data, t_alloc, "07", "real_file");
+    g_assert(stat(real_file, &st_real) == 0);
+    g_assert((st_real.st_mode & S_IFMT) == S_IFREG);
+
+    dfid = fs_walk_fid(v9p, data, t_alloc, "07");
+
+    req = v9fs_tlink(v9p, dfid, fid, "hardlink_file", 0);
+    v9fs_req_wait_for_reply(req, NULL);
+    v9fs_rlink(req);
+
+    /* 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");
@@ -1363,6 +1423,7 @@ static void register_virtio_9p_test(void)
     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);
+    qos_add_test("local/hardlink_file", "virtio-9p", fs_hardlink_file, &opts);
 }
 
 libqos_init(register_virtio_9p_test);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 8/8] tests/9pfs: add local unlinkat hard link test
  2020-10-19 23:17 [PATCH 0/8] 9pfs: more local tests Christian Schoenebeck
  2020-10-19 23:13 ` [PATCH 2/8] tests/9pfs: add local unlinkat directory test Christian Schoenebeck
  2020-10-19 23:13 ` [PATCH 1/8] tests/9pfs: simplify fs_mkdir() Christian Schoenebeck
@ 2020-10-19 23:13 ` Christian Schoenebeck
  2020-10-19 23:13 ` [PATCH 7/8] tests/9pfs: add local Tlink test Christian Schoenebeck
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Christian Schoenebeck @ 2020-10-19 23:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz

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>
---
 tests/qtest/virtio-9p-test.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 447d8e3344..2e50445745 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -1378,6 +1378,39 @@ 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;
+    P9Req *req;
+    uint32_t dfid, fid;
+    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");
+
+    fs_attach(v9p, NULL, t_alloc);
+    fs_mkdir(v9p, data, t_alloc, "/", "08");
+    fid = fs_lcreate(v9p, data, t_alloc, "08", "real_file");
+    g_assert(stat(real_file, &st_real) == 0);
+    g_assert((st_real.st_mode & S_IFMT) == S_IFREG);
+
+    dfid = fs_walk_fid(v9p, data, t_alloc, "08");
+
+    req = v9fs_tlink(v9p, dfid, fid, "hardlink_file", 0);
+    v9fs_req_wait_for_reply(req, NULL);
+    v9fs_rlink(req);
+    g_assert(stat(hardlink_file, &st_link) == 0);
+
+    fs_unlinkat(v9p, data, t_alloc, "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");
@@ -1424,6 +1457,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);
+    qos_add_test("local/unlinkat_hardlink", "virtio-9p", fs_unlinkat_hardlink, &opts);
 }
 
 libqos_init(register_virtio_9p_test);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 0/8] 9pfs: more local tests
@ 2020-10-19 23:17 Christian Schoenebeck
  2020-10-19 23:13 ` [PATCH 2/8] tests/9pfs: add local unlinkat directory test Christian Schoenebeck
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: Christian Schoenebeck @ 2020-10-19 23:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz

Just a bunch of more test case using the 9pfs 'local' fs driver backend,
namely for these 9p requests:

* Tunlinkat, Tlcreate, Tsymlink and Tlink.

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

 tests/qtest/virtio-9p-test.c | 395 ++++++++++++++++++++++++++++++++++-
 1 file changed, 390 insertions(+), 5 deletions(-)

-- 
2.20.1



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 7/8] tests/9pfs: add local Tlink test
  2020-10-19 23:13 ` [PATCH 7/8] tests/9pfs: add local Tlink test Christian Schoenebeck
@ 2020-10-20  0:01   ` Christian Schoenebeck
  0 siblings, 0 replies; 15+ messages in thread
From: Christian Schoenebeck @ 2020-10-20  0:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz

On Dienstag, 20. Oktober 2020 01:13:24 CEST Christian Schoenebeck wrote:
> 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>
> ---
>  tests/qtest/virtio-9p-test.c | 61 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> index f7d18f6274..447d8e3344 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" :
> @@ -742,6 +743,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)
> +{

This hard-link test was actually motived by an issue that I recently 
encountered on a machine: it fails to create any hard links with 9p. This 
particular test case succeeds though.

I think the problem is that recent libvirt versions enable qemu's sandbox 
feature by default which filters syscalls. Fact is, any linkat() call fails on 
that machine with EACCES now. I couldn't reproduce it on my development 
machine yet though. I guess it's a difference in white/black-list seccomp 
config or something. Not sure yet if there is some change required on 9p side 
or whether it's really just a seccomp config issue.

P.S. Noisy days from my side, but this is probably the last batch of patches 
from my side in a while, unless I really need to fix something for that hard 
link isssue. We'll see ...

Best regards,
Christian Schoenebeck




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/8] tests/9pfs: simplify fs_mkdir()
  2020-10-19 23:13 ` [PATCH 1/8] tests/9pfs: simplify fs_mkdir() Christian Schoenebeck
@ 2020-10-20 13:35   ` Greg Kurz
  2020-10-20 13:43     ` Christian Schoenebeck
  0 siblings, 1 reply; 15+ messages in thread
From: Greg Kurz @ 2020-10-20 13:35 UTC (permalink / raw)
  To: Christian Schoenebeck; +Cc: qemu-devel

On Tue, 20 Oct 2020 01:13:23 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> Split out walking a directory path to a separate new utility function
> fs_walk_fid() and use that function in fs_mkdir().
> 
> The code difference saved this way is not much, but we'll use that new
> fs_walk_fid() function in the upcoming patches, so it will avoid quite
> some code duplication after all.
> 
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---
>  tests/qtest/virtio-9p-test.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> index c15908f27b..dc724bbb1e 100644
> --- a/tests/qtest/virtio-9p-test.c
> +++ b/tests/qtest/virtio-9p-test.c
> @@ -967,13 +967,12 @@ 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)
> +/* utility function: walk to requested dir and return fid for that dir */
> +static uint32_t fs_walk_fid(void *obj, void *data, QGuestAllocator *t_alloc,
> +                            const char *path)
>  {

Since fs_walk_fid() is a helper function, ie. not passed to qos_add_test(),
any reason to keep the "void *obj, void *data, QGuestAllocator *t_alloc" based
signature ? data and t_alloc aren't used at all and it seems that the function
should rather take a QVirtio9P * directly instead of casting from a void *.

Something like:

static uint32_t fs_walk_fid(QVirtio9P *v9p, const char *path)
{
...
}


Same remark applies to fs_mkdir() which isn't a top level test function
either BTW (sorry for not having spotted this earlier).

>      QVirtio9P *v9p = obj;
> -    alloc = t_alloc;
>      char **wnames;
> -    char *const name = g_strdup(cname);
>      P9Req *req;
>      const uint32_t fid = genfid();
>  
> @@ -983,12 +982,26 @@ static void fs_mkdir(void *obj, void *data, QGuestAllocator *t_alloc,
>      v9fs_req_wait_for_reply(req, NULL);
>      v9fs_rwalk(req, NULL, NULL);
>  
> +    split_free(&wnames);
> +    return fid;
> +}
> +
> +static void fs_mkdir(void *obj, void *data, QGuestAllocator *t_alloc,
> +                     const char *path, const char *cname)
> +{
> +    QVirtio9P *v9p = obj;
> +    alloc = t_alloc;
> +    char *const name = g_strdup(cname);
> +    uint32_t fid;
> +    P9Req *req;
> +
> +    fid = fs_walk_fid(v9p, data, t_alloc, 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,



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/8] tests/9pfs: simplify fs_mkdir()
  2020-10-20 13:35   ` Greg Kurz
@ 2020-10-20 13:43     ` Christian Schoenebeck
  2020-10-20 13:59       ` Greg Kurz
  2020-10-20 18:03       ` Greg Kurz
  0 siblings, 2 replies; 15+ messages in thread
From: Christian Schoenebeck @ 2020-10-20 13:43 UTC (permalink / raw)
  To: Greg Kurz; +Cc: qemu-devel

On Dienstag, 20. Oktober 2020 15:35:36 CEST Greg Kurz wrote:
> On Tue, 20 Oct 2020 01:13:23 +0200
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > Split out walking a directory path to a separate new utility function
> > fs_walk_fid() and use that function in fs_mkdir().
> > 
> > The code difference saved this way is not much, but we'll use that new
> > fs_walk_fid() function in the upcoming patches, so it will avoid quite
> > some code duplication after all.
> > 
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > ---
> > 
> >  tests/qtest/virtio-9p-test.c | 23 ++++++++++++++++++-----
> >  1 file changed, 18 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> > index c15908f27b..dc724bbb1e 100644
> > --- a/tests/qtest/virtio-9p-test.c
> > +++ b/tests/qtest/virtio-9p-test.c
> > @@ -967,13 +967,12 @@ 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)
> > +/* utility function: walk to requested dir and return fid for that dir */
> > +static uint32_t fs_walk_fid(void *obj, void *data, QGuestAllocator
> > *t_alloc, +                            const char *path)
> > 
> >  {
> 
> Since fs_walk_fid() is a helper function, ie. not passed to qos_add_test(),
> any reason to keep the "void *obj, void *data, QGuestAllocator *t_alloc"
> based signature ? data and t_alloc aren't used at all and it seems that the
> function should rather take a QVirtio9P * directly instead of casting from
> a void *.
> 
> Something like:
> 
> static uint32_t fs_walk_fid(QVirtio9P *v9p, const char *path)
> {
> ...
> }
> 
> 
> Same remark applies to fs_mkdir() which isn't a top level test function
> either BTW (sorry for not having spotted this earlier).

Good point. Typical case of being copy & waste induced. I'll change that.

Best regards,
Christian Schoenebeck




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/8] tests/9pfs: simplify fs_mkdir()
  2020-10-20 13:43     ` Christian Schoenebeck
@ 2020-10-20 13:59       ` Greg Kurz
  2020-10-20 18:03       ` Greg Kurz
  1 sibling, 0 replies; 15+ messages in thread
From: Greg Kurz @ 2020-10-20 13:59 UTC (permalink / raw)
  To: Christian Schoenebeck; +Cc: qemu-devel

On Tue, 20 Oct 2020 15:43:21 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Dienstag, 20. Oktober 2020 15:35:36 CEST Greg Kurz wrote:
> > On Tue, 20 Oct 2020 01:13:23 +0200
> > 
> > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > Split out walking a directory path to a separate new utility function
> > > fs_walk_fid() and use that function in fs_mkdir().
> > > 
> > > The code difference saved this way is not much, but we'll use that new
> > > fs_walk_fid() function in the upcoming patches, so it will avoid quite
> > > some code duplication after all.
> > > 
> > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > ---
> > > 
> > >  tests/qtest/virtio-9p-test.c | 23 ++++++++++++++++++-----
> > >  1 file changed, 18 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> > > index c15908f27b..dc724bbb1e 100644
> > > --- a/tests/qtest/virtio-9p-test.c
> > > +++ b/tests/qtest/virtio-9p-test.c
> > > @@ -967,13 +967,12 @@ 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)
> > > +/* utility function: walk to requested dir and return fid for that dir */
> > > +static uint32_t fs_walk_fid(void *obj, void *data, QGuestAllocator
> > > *t_alloc, +                            const char *path)
> > > 
> > >  {
> > 
> > Since fs_walk_fid() is a helper function, ie. not passed to qos_add_test(),
> > any reason to keep the "void *obj, void *data, QGuestAllocator *t_alloc"
> > based signature ? data and t_alloc aren't used at all and it seems that the
> > function should rather take a QVirtio9P * directly instead of casting from
> > a void *.
> > 
> > Something like:
> > 
> > static uint32_t fs_walk_fid(QVirtio9P *v9p, const char *path)
> > {
> > ...
> > }
> > 
> > 
> > Same remark applies to fs_mkdir() which isn't a top level test function
> > either BTW (sorry for not having spotted this earlier).
> 
> Good point. Typical case of being copy & waste induced. I'll change that.
> 

Well I guess I haven't set a good exemple by calling fs_attach() everywhere
instead of factoring out an helper in the first place... I have some spare
time, I'll fix that.

> Best regards,
> Christian Schoenebeck
> 
> 



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/8] tests/9pfs: simplify fs_mkdir()
  2020-10-20 13:43     ` Christian Schoenebeck
  2020-10-20 13:59       ` Greg Kurz
@ 2020-10-20 18:03       ` Greg Kurz
  2020-10-20 18:26         ` Christian Schoenebeck
  1 sibling, 1 reply; 15+ messages in thread
From: Greg Kurz @ 2020-10-20 18:03 UTC (permalink / raw)
  To: Christian Schoenebeck; +Cc: qemu-devel

On Tue, 20 Oct 2020 15:43:21 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Dienstag, 20. Oktober 2020 15:35:36 CEST Greg Kurz wrote:
> > On Tue, 20 Oct 2020 01:13:23 +0200
> > 
> > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > Split out walking a directory path to a separate new utility function
> > > fs_walk_fid() and use that function in fs_mkdir().
> > > 
> > > The code difference saved this way is not much, but we'll use that new
> > > fs_walk_fid() function in the upcoming patches, so it will avoid quite
> > > some code duplication after all.
> > > 
> > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > ---
> > > 
> > >  tests/qtest/virtio-9p-test.c | 23 ++++++++++++++++++-----
> > >  1 file changed, 18 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> > > index c15908f27b..dc724bbb1e 100644
> > > --- a/tests/qtest/virtio-9p-test.c
> > > +++ b/tests/qtest/virtio-9p-test.c
> > > @@ -967,13 +967,12 @@ 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)
> > > +/* utility function: walk to requested dir and return fid for that dir */
> > > +static uint32_t fs_walk_fid(void *obj, void *data, QGuestAllocator
> > > *t_alloc, +                            const char *path)
> > > 
> > >  {
> > 
> > Since fs_walk_fid() is a helper function, ie. not passed to qos_add_test(),
> > any reason to keep the "void *obj, void *data, QGuestAllocator *t_alloc"
> > based signature ? data and t_alloc aren't used at all and it seems that the
> > function should rather take a QVirtio9P * directly instead of casting from
> > a void *.
> > 
> > Something like:
> > 
> > static uint32_t fs_walk_fid(QVirtio9P *v9p, const char *path)
> > {
> > ...
> > }
> > 
> > 
> > Same remark applies to fs_mkdir() which isn't a top level test function
> > either BTW (sorry for not having spotted this earlier).
> 
> Good point. Typical case of being copy & waste induced. I'll change that.
> 

Since this also affects other patches in this series and this might
have a substantial impact, I'll wait for v2 to review if you don't
mind.

> Best regards,
> Christian Schoenebeck
> 
> 



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/8] tests/9pfs: simplify fs_mkdir()
  2020-10-20 18:03       ` Greg Kurz
@ 2020-10-20 18:26         ` Christian Schoenebeck
  0 siblings, 0 replies; 15+ messages in thread
From: Christian Schoenebeck @ 2020-10-20 18:26 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz

On Dienstag, 20. Oktober 2020 20:03:09 CEST Greg Kurz wrote:
> On Tue, 20 Oct 2020 15:43:21 +0200
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > On Dienstag, 20. Oktober 2020 15:35:36 CEST Greg Kurz wrote:
> > > On Tue, 20 Oct 2020 01:13:23 +0200
> > > 
> > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > > Split out walking a directory path to a separate new utility function
> > > > fs_walk_fid() and use that function in fs_mkdir().
> > > > 
> > > > The code difference saved this way is not much, but we'll use that new
> > > > fs_walk_fid() function in the upcoming patches, so it will avoid quite
> > > > some code duplication after all.
> > > > 
> > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > > ---
> > > > 
> > > >  tests/qtest/virtio-9p-test.c | 23 ++++++++++++++++++-----
> > > >  1 file changed, 18 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/tests/qtest/virtio-9p-test.c
> > > > b/tests/qtest/virtio-9p-test.c
> > > > index c15908f27b..dc724bbb1e 100644
> > > > --- a/tests/qtest/virtio-9p-test.c
> > > > +++ b/tests/qtest/virtio-9p-test.c
> > > > @@ -967,13 +967,12 @@ 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)
> > > > +/* utility function: walk to requested dir and return fid for that
> > > > dir */
> > > > +static uint32_t fs_walk_fid(void *obj, void *data, QGuestAllocator
> > > > *t_alloc, +                            const char *path)
> > > > 
> > > >  {
> > > 
> > > Since fs_walk_fid() is a helper function, ie. not passed to
> > > qos_add_test(),
> > > any reason to keep the "void *obj, void *data, QGuestAllocator *t_alloc"
> > > based signature ? data and t_alloc aren't used at all and it seems that
> > > the
> > > function should rather take a QVirtio9P * directly instead of casting
> > > from
> > > a void *.
> > > 
> > > Something like:
> > > 
> > > static uint32_t fs_walk_fid(QVirtio9P *v9p, const char *path)
> > > {
> > > ...
> > > }
> > > 
> > > 
> > > Same remark applies to fs_mkdir() which isn't a top level test function
> > > either BTW (sorry for not having spotted this earlier).
> > 
> > Good point. Typical case of being copy & waste induced. I'll change that.
> 
> Since this also affects other patches in this series and this might
> have a substantial impact, I'll wait for v2 to review if you don't
> mind.

Sure!

There is probably no need to hurry anyway; since these are just test case 
changes, I think it will also be fine if I send the PR during freeze.

You never had any issue with 9p hard links, right? I'm still investigating 
this in parallel. I already can rule out QEMU's sandbox (seccomp) feature, 
even after having disabled the latter that mentioned box is failing any 
link()/linkat() calls. Maybe it's a SELinux issue ...

Best regards,
Christian Schoenebeck




^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-10-20 18:27 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-19 23:17 [PATCH 0/8] 9pfs: more local tests Christian Schoenebeck
2020-10-19 23:13 ` [PATCH 2/8] tests/9pfs: add local unlinkat directory test Christian Schoenebeck
2020-10-19 23:13 ` [PATCH 1/8] tests/9pfs: simplify fs_mkdir() Christian Schoenebeck
2020-10-20 13:35   ` Greg Kurz
2020-10-20 13:43     ` Christian Schoenebeck
2020-10-20 13:59       ` Greg Kurz
2020-10-20 18:03       ` Greg Kurz
2020-10-20 18:26         ` Christian Schoenebeck
2020-10-19 23:13 ` [PATCH 8/8] tests/9pfs: add local unlinkat hard link test Christian Schoenebeck
2020-10-19 23:13 ` [PATCH 7/8] tests/9pfs: add local Tlink test Christian Schoenebeck
2020-10-20  0:01   ` Christian Schoenebeck
2020-10-19 23:13 ` [PATCH 3/8] tests/9pfs: add local Tlcreate test Christian Schoenebeck
2020-10-19 23:13 ` [PATCH 5/8] tests/9pfs: add local Tsymlink test Christian Schoenebeck
2020-10-19 23:13 ` [PATCH 4/8] tests/9pfs: add local unlinkat file test Christian Schoenebeck
2020-10-19 23:13 ` [PATCH 6/8] tests/9pfs: add local unlinkat symlink test Christian Schoenebeck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.