[PULL v3 2/6] tests/9pfs: change qtest name prefix to synth

[PULL v3 2/6] tests/9pfs: change qtest name prefix to synth

[Christian Schoenebeck]

All existing 9pfs test cases are using the 'synth' fs driver so far, which
means they are not accessing real files, but a purely simulated (in RAM
only) file system.

Let's make this clear by changing the prefix of the individual qtest case
names from 'fs/' to 'synth/'. That way they'll be easily distinguishable
from upcoming new 9pfs test cases supposed to be using a different fs
driver.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Message-Id: <e04e75acb849b085c6d6320b2433a15fa935bcff.1602182956.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/virtio-9p-test.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index de30b717b6..3281153b9c 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -897,26 +897,26 @@ static void fs_readdir_split_512(void *obj, void *data,
 
 static void register_virtio_9p_test(void)
 {
-    qos_add_test("config", "virtio-9p", pci_config, NULL);
-    qos_add_test("fs/version/basic", "virtio-9p", fs_version, NULL);
-    qos_add_test("fs/attach/basic", "virtio-9p", fs_attach, NULL);
-    qos_add_test("fs/walk/basic", "virtio-9p", fs_walk, NULL);
-    qos_add_test("fs/walk/no_slash", "virtio-9p", fs_walk_no_slash,
+    qos_add_test("synth/config", "virtio-9p", pci_config, NULL);
+    qos_add_test("synth/version/basic", "virtio-9p", fs_version, NULL);
+    qos_add_test("synth/attach/basic", "virtio-9p", fs_attach, NULL);
+    qos_add_test("synth/walk/basic", "virtio-9p", fs_walk, NULL);
+    qos_add_test("synth/walk/no_slash", "virtio-9p", fs_walk_no_slash,
                  NULL);
-    qos_add_test("fs/walk/dotdot_from_root", "virtio-9p",
+    qos_add_test("synth/walk/dotdot_from_root", "virtio-9p",
                  fs_walk_dotdot, NULL);
-    qos_add_test("fs/lopen/basic", "virtio-9p", fs_lopen, NULL);
-    qos_add_test("fs/write/basic", "virtio-9p", fs_write, NULL);
-    qos_add_test("fs/flush/success", "virtio-9p", fs_flush_success,
+    qos_add_test("synth/lopen/basic", "virtio-9p", fs_lopen, NULL);
+    qos_add_test("synth/write/basic", "virtio-9p", fs_write, NULL);
+    qos_add_test("synth/flush/success", "virtio-9p", fs_flush_success,
                  NULL);
-    qos_add_test("fs/flush/ignored", "virtio-9p", fs_flush_ignored,
+    qos_add_test("synth/flush/ignored", "virtio-9p", fs_flush_ignored,
                  NULL);
-    qos_add_test("fs/readdir/basic", "virtio-9p", fs_readdir, NULL);
-    qos_add_test("fs/readdir/split_512", "virtio-9p",
+    qos_add_test("synth/readdir/basic", "virtio-9p", fs_readdir, NULL);
+    qos_add_test("synth/readdir/split_512", "virtio-9p",
                  fs_readdir_split_512, NULL);
-    qos_add_test("fs/readdir/split_256", "virtio-9p",
+    qos_add_test("synth/readdir/split_256", "virtio-9p",
                  fs_readdir_split_256, NULL);
-    qos_add_test("fs/readdir/split_128", "virtio-9p",
+    qos_add_test("synth/readdir/split_128", "virtio-9p",
                  fs_readdir_split_128, NULL);
 }
 
-- 
2.20.1

[PULL v3 6/6] tests/9pfs: add local Tmkdir test

[Christian Schoenebeck]

This test case uses the 9pfs 'local' driver to create a directory
and then checks if the expected directory was actually created
(as real directory) on host side.

This patch introduces a custom split() implementation, because
the test code requires non empty array elements as result. For
that reason g_strsplit() would not be a good alternative, as
it would require additional filter code for reshuffling the
array, and the resulting code would be even more complex than
this split() function.

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

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index af7e169d3a..c15908f27b 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -18,6 +18,62 @@
 #define QVIRTIO_9P_TIMEOUT_US (10 * 1000 * 1000)
 static QGuestAllocator *alloc;
 
+/*
+ * Used to auto generate new fids. Start with arbitrary high value to avoid
+ * collision with hard coded fids in basic test code.
+ */
+static uint32_t fid_generator = 1000;
+
+static uint32_t genfid(void)
+{
+    return fid_generator++;
+}
+
+/**
+ * Splits the @a in string by @a delim into individual (non empty) strings
+ * and outputs them to @a out. The output array @a out is NULL terminated.
+ *
+ * Output array @a out must be freed by calling split_free().
+ *
+ * @returns number of individual elements in output array @a out (without the
+ *          final NULL terminating element)
+ */
+static int split(const char *in, const char *delim, char ***out)
+{
+    int n = 0, i = 0;
+    char *tmp, *p;
+
+    tmp = g_strdup(in);
+    for (p = strtok(tmp, delim); p != NULL; p = strtok(NULL, delim)) {
+        if (strlen(p) > 0) {
+            ++n;
+        }
+    }
+    g_free(tmp);
+
+    *out = g_new0(char *, n + 1); /* last element NULL delimiter */
+
+    tmp = g_strdup(in);
+    for (p = strtok(tmp, delim); p != NULL; p = strtok(NULL, delim)) {
+        if (strlen(p) > 0) {
+            (*out)[i++] = g_strdup(p);
+        }
+    }
+    g_free(tmp);
+
+    return n;
+}
+
+static void split_free(char ***out)
+{
+    int i;
+    for (i = 0; (*out)[i]; ++i) {
+        g_free((*out)[i]);
+    }
+    g_free(*out);
+    *out = NULL;
+}
+
 static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     QVirtio9P *v9p = obj;
@@ -201,6 +257,7 @@ static const char *rmessage_name(uint8_t id)
         id == P9_RWALK ? "RWALK" :
         id == P9_RLOPEN ? "RLOPEN" :
         id == P9_RWRITE ? "RWRITE" :
+        id == P9_RMKDIR ? "RMKDIR" :
         id == P9_RFLUSH ? "RFLUSH" :
         id == P9_RREADDIR ? "READDIR" :
         "<unknown>";
@@ -578,6 +635,39 @@ static bool fs_dirents_contain_name(struct V9fsDirent *e, const char* name)
     return false;
 }
 
+/* size[4] Tmkdir tag[2] dfid[4] name[s] mode[4] gid[4] */
+static P9Req *v9fs_tmkdir(QVirtio9P *v9p, uint32_t dfid, const char *name,
+                          uint32_t mode, uint32_t gid, uint16_t tag)
+{
+    P9Req *req;
+
+    uint32_t body_size = 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_TMKDIR, tag);
+    v9fs_uint32_write(req, dfid);
+    v9fs_string_write(req, name);
+    v9fs_uint32_write(req, mode);
+    v9fs_uint32_write(req, gid);
+    v9fs_req_send(req);
+    return req;
+}
+
+/* size[4] Rmkdir tag[2] qid[13] */
+static void v9fs_rmkdir(P9Req *req, v9fs_qid *qid)
+{
+    v9fs_req_recv(req, P9_RMKDIR);
+    if (qid) {
+        v9fs_memread(req, qid, 13);
+    } else {
+        v9fs_memskip(req, 13);
+    }
+    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)
 {
@@ -877,6 +967,30 @@ 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)
+{
+    QVirtio9P *v9p = obj;
+    alloc = t_alloc;
+    char **wnames;
+    char *const name = g_strdup(cname);
+    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);
+
+    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,
                                  QGuestAllocator *t_alloc)
 {
@@ -895,6 +1009,30 @@ static void fs_readdir_split_512(void *obj, void *data,
     fs_readdir_split(obj, data, t_alloc, 512);
 }
 
+
+/* tests using the 9pfs 'local' fs driver */
+
+static void fs_create_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("01");
+
+    g_assert(root_path != NULL);
+
+    fs_attach(v9p, NULL, t_alloc);
+    fs_mkdir(v9p, data, t_alloc, "/", "01");
+
+    /* 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);
+
+    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");
@@ -934,6 +1072,7 @@ static void register_virtio_9p_test(void)
     /* 9pfs test cases using the 'local' filesystem driver */
     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);
 }
 
 libqos_init(register_virtio_9p_test);
-- 
2.20.1

[PULL v3 3/6] tests/9pfs: introduce local tests

[Christian Schoenebeck]

This patch introduces 9pfs test cases using the 9pfs 'local'
filesystem driver which reads/writes/creates/deletes real files
and directories.

In this initial version, there is only one local test which actually
only checks if the 9pfs 'local' device was created successfully.

Before the 9pfs 'local' tests are run, a test directory 'qtest-9p-local'
is created (with world rwx permissions) under the current working
directory. At this point that test directory is not auto deleted yet.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Message-Id: <81fc4b3b6b6c9bf7999e79f5e7cbc364a5f09ddb.1602182956.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio-9p.c | 81 ++++++++++++++++++++++++++++++++++
 tests/qtest/libqos/virtio-9p.h |  5 +++
 tests/qtest/virtio-9p-test.c   | 44 ++++++++++++------
 3 files changed, 116 insertions(+), 14 deletions(-)

diff --git a/tests/qtest/libqos/virtio-9p.c b/tests/qtest/libqos/virtio-9p.c
index 2e300063e3..ee331166de 100644
--- a/tests/qtest/libqos/virtio-9p.c
+++ b/tests/qtest/libqos/virtio-9p.c
@@ -24,6 +24,34 @@
 #include "qgraph.h"
 
 static QGuestAllocator *alloc;
+static char *local_test_path;
+
+/* Concatenates the passed 2 pathes. Returned result must be freed. */
+static char *concat_path(const char* a, const char* b)
+{
+    return g_build_filename(a, b, NULL);
+}
+
+static void init_local_test_path(void)
+{
+    char *pwd = g_get_current_dir();
+    local_test_path = concat_path(pwd, "qtest-9p-local");
+    g_free(pwd);
+}
+
+/* Creates the directory for the 9pfs 'local' filesystem driver to access. */
+static void create_local_test_dir(void)
+{
+    struct stat st;
+
+    g_assert(local_test_path != NULL);
+    mkdir(local_test_path, 0777);
+
+    /* ensure test directory exists now ... */
+    g_assert(stat(local_test_path, &st) == 0);
+    /* ... and is actually a directory */
+    g_assert((st.st_mode & S_IFMT) == S_IFDIR);
+}
 
 static void virtio_9p_cleanup(QVirtio9P *interface)
 {
@@ -146,11 +174,64 @@ static void *virtio_9p_pci_create(void *pci_bus, QGuestAllocator *t_alloc,
     return obj;
 }
 
+/**
+ * Performs regular expression based search and replace on @a haystack.
+ *
+ * @param haystack - input string to be parsed, result of replacement is
+ *                   stored back to @a haystack
+ * @param pattern - the regular expression pattern for scanning @a haystack
+ * @param replace_fmt - matches of supplied @a pattern are replaced by this,
+ *                      if necessary glib printf format can be used to add
+ *                      variable arguments of this function to this
+ *                      replacement string
+ */
+static void regex_replace(GString *haystack, const char *pattern,
+                          const char *replace_fmt, ...)
+{
+    GRegex *regex;
+    char *replace, *s;
+    va_list argp;
+
+    va_start(argp, replace_fmt);
+    replace = g_strdup_vprintf(replace_fmt, argp);
+    va_end(argp);
+
+    regex = g_regex_new(pattern, 0, 0, NULL);
+    s = g_regex_replace(regex, haystack->str, -1, 0, replace, 0, NULL);
+    g_string_assign(haystack, s);
+    g_free(s);
+    g_regex_unref(regex);
+    g_free(replace);
+}
+
+void virtio_9p_assign_local_driver(GString *cmd_line, const char *args)
+{
+    g_assert_nonnull(local_test_path);
+
+    /* replace 'synth' driver by 'local' driver */
+    regex_replace(cmd_line, "-fsdev synth,", "-fsdev local,");
+
+    /* append 'path=...' to '-fsdev ...' group */
+    regex_replace(cmd_line, "(-fsdev \\w[^ ]*)", "\\1,path='%s'",
+                  local_test_path);
+
+    if (!args) {
+        return;
+    }
+
+    /* append passed args to '-fsdev ...' group */
+    regex_replace(cmd_line, "(-fsdev \\w[^ ]*)", "\\1,%s", args);
+}
+
 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();
+    create_local_test_dir();
+
     QPCIAddress addr = {
         .devfn = QPCI_DEVFN(4, 0),
     };
diff --git a/tests/qtest/libqos/virtio-9p.h b/tests/qtest/libqos/virtio-9p.h
index b1e6badc4a..326a603f72 100644
--- a/tests/qtest/libqos/virtio-9p.h
+++ b/tests/qtest/libqos/virtio-9p.h
@@ -44,4 +44,9 @@ struct QVirtio9PDevice {
     QVirtio9P v9p;
 };
 
+/**
+ * Prepares QEMU command line for 9pfs tests using the 'local' fs driver.
+ */
+void virtio_9p_assign_local_driver(GString *cmd_line, const char *args);
+
 #endif
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 3281153b9c..af7e169d3a 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -895,29 +895,45 @@ static void fs_readdir_split_512(void *obj, void *data,
     fs_readdir_split(obj, data, t_alloc, 512);
 }
 
+static void *assign_9p_local_driver(GString *cmd_line, void *arg)
+{
+    virtio_9p_assign_local_driver(cmd_line, "security_model=mapped-xattr");
+    return arg;
+}
+
 static void register_virtio_9p_test(void)
 {
-    qos_add_test("synth/config", "virtio-9p", pci_config, NULL);
-    qos_add_test("synth/version/basic", "virtio-9p", fs_version, NULL);
-    qos_add_test("synth/attach/basic", "virtio-9p", fs_attach, NULL);
-    qos_add_test("synth/walk/basic", "virtio-9p", fs_walk, NULL);
+
+    QOSGraphTestOptions opts = {
+    };
+
+    /* 9pfs test cases using the 'synth' filesystem driver */
+    qos_add_test("synth/config", "virtio-9p", pci_config, &opts);
+    qos_add_test("synth/version/basic", "virtio-9p", fs_version,  &opts);
+    qos_add_test("synth/attach/basic", "virtio-9p", fs_attach,  &opts);
+    qos_add_test("synth/walk/basic", "virtio-9p", fs_walk,  &opts);
     qos_add_test("synth/walk/no_slash", "virtio-9p", fs_walk_no_slash,
-                 NULL);
+                  &opts);
     qos_add_test("synth/walk/dotdot_from_root", "virtio-9p",
-                 fs_walk_dotdot, NULL);
-    qos_add_test("synth/lopen/basic", "virtio-9p", fs_lopen, NULL);
-    qos_add_test("synth/write/basic", "virtio-9p", fs_write, NULL);
+                 fs_walk_dotdot,  &opts);
+    qos_add_test("synth/lopen/basic", "virtio-9p", fs_lopen,  &opts);
+    qos_add_test("synth/write/basic", "virtio-9p", fs_write,  &opts);
     qos_add_test("synth/flush/success", "virtio-9p", fs_flush_success,
-                 NULL);
+                  &opts);
     qos_add_test("synth/flush/ignored", "virtio-9p", fs_flush_ignored,
-                 NULL);
-    qos_add_test("synth/readdir/basic", "virtio-9p", fs_readdir, NULL);
+                  &opts);
+    qos_add_test("synth/readdir/basic", "virtio-9p", fs_readdir,  &opts);
     qos_add_test("synth/readdir/split_512", "virtio-9p",
-                 fs_readdir_split_512, NULL);
+                 fs_readdir_split_512,  &opts);
     qos_add_test("synth/readdir/split_256", "virtio-9p",
-                 fs_readdir_split_256, NULL);
+                 fs_readdir_split_256,  &opts);
     qos_add_test("synth/readdir/split_128", "virtio-9p",
-                 fs_readdir_split_128, NULL);
+                 fs_readdir_split_128,  &opts);
+
+
+    /* 9pfs test cases using the 'local' filesystem driver */
+    opts.before = assign_9p_local_driver;
+    qos_add_test("local/config", "virtio-9p", pci_config,  &opts);
 }
 
 libqos_init(register_virtio_9p_test);
-- 
2.20.1

[PULL v3 4/6] tests/9pfs: wipe local 9pfs test directory

[Christian Schoenebeck]

Before running the first 9pfs test case, make sure the test directory
for running the 9pfs 'local' tests on is entirely empty. For that
reason simply delete the test directory (if any) before (re)creating
it on test suite startup.

Note: The preferable precise behaviour would be the test directory
only being wiped once *before* a test suite run. Right now the test
directory is also wiped at the *end* of a test suite run because
libqos is calling the virtio_9p_register_nodes() callback for some
reason also when a test suite completed. This is suboptimal as
developers cannot immediately see what files and directories the
9pfs local tests created precisely after the test suite completed.
But fortunately the test directory is not wiped if some test failed.
So it is probably not worth it drilling another hole into libqos
for this issue.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Message-Id: <b30776ea3289dc40dabc7d0063d825d21d9a65bf.1602182956.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio-9p.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tests/qtest/libqos/virtio-9p.c b/tests/qtest/libqos/virtio-9p.c
index ee331166de..8ee2a134bc 100644
--- a/tests/qtest/libqos/virtio-9p.c
+++ b/tests/qtest/libqos/virtio-9p.c
@@ -53,6 +53,18 @@ static void create_local_test_dir(void)
     g_assert((st.st_mode & S_IFMT) == S_IFDIR);
 }
 
+/* Deletes directory previously created by create_local_test_dir(). */
+static void remove_local_test_dir(void)
+{
+    g_assert(local_test_path != NULL);
+    char *cmd = g_strdup_printf("rm -r '%s'\n", local_test_path);
+    int res = system(cmd);
+    if (res < 0) {
+        /* ignore error, dummy check to prevent compiler error */
+    }
+    g_free(cmd);
+}
+
 static void virtio_9p_cleanup(QVirtio9P *interface)
 {
     qvirtqueue_cleanup(interface->vdev->bus, interface->vq, alloc);
@@ -230,6 +242,7 @@ static void virtio_9p_register_nodes(void)
 
     /* 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 = {
-- 
2.20.1

[PULL v3 5/6] tests/9pfs: add virtio_9p_test_path()

[Christian Schoenebeck]

This new public function virtio_9p_test_path() allows 9pfs
'local' tests to translate a path from guest scope to host
scope. For instance by passing an empty string it would
return the root path on host of the exported 9pfs tree.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Message-Id: <b563d3c73c6391ec927a2622c9f65c09ca56bd83.1602182956.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio-9p.c | 6 ++++++
 tests/qtest/libqos/virtio-9p.h | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/tests/qtest/libqos/virtio-9p.c b/tests/qtest/libqos/virtio-9p.c
index 8ee2a134bc..d43647b3b7 100644
--- a/tests/qtest/libqos/virtio-9p.c
+++ b/tests/qtest/libqos/virtio-9p.c
@@ -65,6 +65,12 @@ static void remove_local_test_dir(void)
     g_free(cmd);
 }
 
+char *virtio_9p_test_path(const char *path)
+{
+    g_assert(local_test_path);
+    return concat_path(local_test_path, path);
+}
+
 static void virtio_9p_cleanup(QVirtio9P *interface)
 {
     qvirtqueue_cleanup(interface->vdev->bus, interface->vq, alloc);
diff --git a/tests/qtest/libqos/virtio-9p.h b/tests/qtest/libqos/virtio-9p.h
index 326a603f72..19a4d97454 100644
--- a/tests/qtest/libqos/virtio-9p.h
+++ b/tests/qtest/libqos/virtio-9p.h
@@ -49,4 +49,9 @@ struct QVirtio9PDevice {
  */
 void virtio_9p_assign_local_driver(GString *cmd_line, const char *args);
 
+/**
+ * Returns path on host to the passed guest path. Result must be freed.
+ */
+char *virtio_9p_test_path(const char *path);
+
 #endif
-- 
2.20.1

[PULL v3 1/6] 9pfs: suppress performance warnings on qtest runs

[Christian Schoenebeck]

Don't trigger any performance warning if we're just running test cases,
because tests intentionally run for edge cases.

So far performance warnings were suppressed for the 'synth' fs driver
backend only. This patch suppresses them for all 9p fs driver backends.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <a2d2ff2163f8853ea782a7a1d4e6f2afd7c29ffe.1603106145.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/9p-synth.c         | 2 --
 hw/9pfs/virtio-9p-device.c | 6 ++++++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index cec8c0eefc..7eb210ffa8 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -541,8 +541,6 @@ static int synth_init(FsContext *ctx, Error **errp)
     QLIST_INIT(&synth_root.child);
     qemu_mutex_init(&synth_mutex);
 
-    ctx->export_flags |= V9FS_NO_PERF_WARN;
-
     /* Add "." and ".." entries for root */
     v9fs_add_dir_node(&synth_root, synth_root.attr->mode,
                       "..", synth_root.attr, synth_root.attr->inode);
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 36f3aa9352..14371a78ef 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -21,6 +21,7 @@
 #include "hw/virtio/virtio-access.h"
 #include "qemu/iov.h"
 #include "qemu/module.h"
+#include "sysemu/qtest.h"
 
 static void virtio_9p_push_and_notify(V9fsPDU *pdu)
 {
@@ -199,6 +200,11 @@ static void virtio_9p_device_realize(DeviceState *dev, Error **errp)
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     V9fsVirtioState *v = VIRTIO_9P(dev);
     V9fsState *s = &v->state;
+    FsDriverEntry *fse = get_fsdev_fsentry(s->fsconf.fsdev_id);
+
+    if (qtest_enabled() && fse) {
+        fse->export_flags |= V9FS_NO_PERF_WARN;
+    }
 
     if (v9fs_device_realize_common(s, &virtio_9p_transport, errp)) {
         return;
-- 
2.20.1

[PULL v3 0/6] 9p queue (previous 2020-10-17)

[Christian Schoenebeck]

The following changes since commit ba2a9a9e6318bfd93a2306dec40137e198205b86:

  Merge remote-tracking branch 'remotes/mcayland/tags/qemu-macppc-20201019' into staging (2020-10-19 11:46:03 +0100)

are available in the Git repository at:

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

for you to fetch changes up to 653daf38978d101d8810f96b9337ebc6b7b1423f:

  tests/9pfs: add local Tmkdir test (2020-10-19 14:25:40 +0200)

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

The currently existing 9pfs test cases are all solely using the 9pfs 'synth'
fileystem driver, which is a very simple and purely simulated (in RAM only)
filesystem. There are issues though where the 'synth' fs driver is not
sufficient. For example the following two bugs need test cases running the
9pfs 'local' fs driver:

https://bugs.launchpad.net/qemu/+bug/1336794
https://bugs.launchpad.net/qemu/+bug/1877384

This patch set for that reason introduces 9pfs test cases using the 9pfs
'local' filesystem driver along to the already existing tests on 'synth'.

----------------------------------------------------------------
Christian Schoenebeck (6):
      9pfs: suppress performance warnings on qtest runs
      tests/9pfs: change qtest name prefix to synth
      tests/9pfs: introduce local tests
      tests/9pfs: wipe local 9pfs test directory
      tests/9pfs: add virtio_9p_test_path()
      tests/9pfs: add local Tmkdir test

 hw/9pfs/9p-synth.c             |   2 -
 hw/9pfs/virtio-9p-device.c     |   6 ++
 tests/qtest/libqos/virtio-9p.c | 100 +++++++++++++++++++++
 tests/qtest/libqos/virtio-9p.h |  10 +++
 tests/qtest/virtio-9p-test.c   | 197 ++++++++++++++++++++++++++++++++++++-----
 5 files changed, 292 insertions(+), 23 deletions(-)

Re: [PULL v3 0/6] 9p queue (previous 2020-10-17)

[Peter Maydell]

On Mon, 19 Oct 2020 at 13:55, Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
>
> The following changes since commit ba2a9a9e6318bfd93a2306dec40137e198205b86:
>
>   Merge remote-tracking branch 'remotes/mcayland/tags/qemu-macppc-20201019' into staging (2020-10-19 11:46:03 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/cschoenebeck/qemu.git tags/pull-9p-20201019
>
> for you to fetch changes up to 653daf38978d101d8810f96b9337ebc6b7b1423f:
>
>   tests/9pfs: add local Tmkdir test (2020-10-19 14:25:40 +0200)
>
> ----------------------------------------------------------------
> 9pfs: add tests using local fs driver
>
> The currently existing 9pfs test cases are all solely using the 9pfs 'synth'
> fileystem driver, which is a very simple and purely simulated (in RAM only)
> filesystem. There are issues though where the 'synth' fs driver is not
> sufficient. For example the following two bugs need test cases running the
> 9pfs 'local' fs driver:
>
> https://bugs.launchpad.net/qemu/+bug/1336794
> https://bugs.launchpad.net/qemu/+bug/1877384
>
> This patch set for that reason introduces 9pfs test cases using the 9pfs
> 'local' filesystem driver along to the already existing tests on 'synth'.
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM

Re: [PULL v3 2/6] tests/9pfs: change qtest name prefix to synth

[Philippe Mathieu-Daudé]

On 10/8/20 8:34 PM, Christian Schoenebeck wrote:
> All existing 9pfs test cases are using the 'synth' fs driver so far, which
> means they are not accessing real files, but a purely simulated (in RAM
> only) file system.
> 
> Let's make this clear by changing the prefix of the individual qtest case
> names from 'fs/' to 'synth/'. That way they'll be easily distinguishable
> from upcoming new 9pfs test cases supposed to be using a different fs
> driver.
> 
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> Message-Id: <e04e75acb849b085c6d6320b2433a15fa935bcff.1602182956.git.qemu_oss@crudebyte.com>
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>

Harmless, but don't need to sign twice ;)

Re: [PULL v3 2/6] tests/9pfs: change qtest name prefix to synth

[Christian Schoenebeck]

On Dienstag, 20. Oktober 2020 09:36:10 CEST Philippe Mathieu-Daudé wrote:
> On 10/8/20 8:34 PM, Christian Schoenebeck wrote:
> > All existing 9pfs test cases are using the 'synth' fs driver so far, which
> > means they are not accessing real files, but a purely simulated (in RAM
> > only) file system.
> > 
> > Let's make this clear by changing the prefix of the individual qtest case
> > names from 'fs/' to 'synth/'. That way they'll be easily distinguishable
> > from upcoming new 9pfs test cases supposed to be using a different fs
> > driver.
> > 
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > Message-Id:
> > <e04e75acb849b085c6d6320b2433a15fa935bcff.1602182956.git.qemu_oss@crudeby
> > te.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> 
> Harmless, but don't need to sign twice ;)

Ah, I thought that's the common way, as Greg's PRs contained 2 SOBs as well, 
i.e. I thought this was intended to outline the patch author and submaintainer 
were the same person.

BTW I actually did not explicitly add the 2nd SOB. It was rather added by the 
patchwork client automatically. So maybe it should be fixed in the client to 
detect an already existing SOB line? Or am missing something here?

Best regards,
Christian Schoenebeck

Re: [PULL v3 2/6] tests/9pfs: change qtest name prefix to synth

[Greg Kurz]

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

> On Dienstag, 20. Oktober 2020 09:36:10 CEST Philippe Mathieu-Daudé wrote:
> > On 10/8/20 8:34 PM, Christian Schoenebeck wrote:
> > > All existing 9pfs test cases are using the 'synth' fs driver so far, which
> > > means they are not accessing real files, but a purely simulated (in RAM
> > > only) file system.
> > > 
> > > Let's make this clear by changing the prefix of the individual qtest case
> > > names from 'fs/' to 'synth/'. That way they'll be easily distinguishable
> > > from upcoming new 9pfs test cases supposed to be using a different fs
> > > driver.
> > > 
> > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > Message-Id:
> > > <e04e75acb849b085c6d6320b2433a15fa935bcff.1602182956.git.qemu_oss@crudeby
> > > te.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > 
> > Harmless, but don't need to sign twice ;)
> 
> Ah, I thought that's the common way, as Greg's PRs contained 2 SOBs as well, 
> i.e. I thought this was intended to outline the patch author and submaintainer 
> were the same person.
> 
> BTW I actually did not explicitly add the 2nd SOB. It was rather added by the 
> patchwork client automatically. So maybe it should be fixed in the client to 
> detect an already existing SOB line? Or am missing something here?
> 

Yeah this is the reason why my sob appears twice on patches authored by
me, and since this is harmless I never really investigated how to fix
pwclient :)

> Best regards,
> Christian Schoenebeck
> 
> 

Re: [PULL v3 2/6] tests/9pfs: change qtest name prefix to synth

[Christian Schoenebeck]

On Dienstag, 20. Oktober 2020 12:00:57 CEST Greg Kurz wrote:
> On Tue, 20 Oct 2020 11:43:18 +0200
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > On Dienstag, 20. Oktober 2020 09:36:10 CEST Philippe Mathieu-Daudé wrote:
> > > On 10/8/20 8:34 PM, Christian Schoenebeck wrote:
> > > > All existing 9pfs test cases are using the 'synth' fs driver so far,
> > > > which
> > > > means they are not accessing real files, but a purely simulated (in
> > > > RAM
> > > > only) file system.
> > > > 
> > > > Let's make this clear by changing the prefix of the individual qtest
> > > > case
> > > > names from 'fs/' to 'synth/'. That way they'll be easily
> > > > distinguishable
> > > > from upcoming new 9pfs test cases supposed to be using a different fs
> > > > driver.
> > > > 
> > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > > Message-Id:
> > > > <e04e75acb849b085c6d6320b2433a15fa935bcff.1602182956.git.qemu_oss@crud
> > > > eby
> > > > te.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > 
> > > Harmless, but don't need to sign twice ;)
> > 
> > Ah, I thought that's the common way, as Greg's PRs contained 2 SOBs as
> > well, i.e. I thought this was intended to outline the patch author and
> > submaintainer were the same person.
> > 
> > BTW I actually did not explicitly add the 2nd SOB. It was rather added by
> > the patchwork client automatically. So maybe it should be fixed in the
> > client to detect an already existing SOB line? Or am missing something
> > here?
> Yeah this is the reason why my sob appears twice on patches authored by
> me, and since this is harmless I never really investigated how to fix
> pwclient :)

Well, I would usually offer my 'I can look at it' at this point, but I am 
reluctant this time as I assume it will end up as my recently suggested libqos 
patches where I did not get any response from the officially assigned 
maintainers; not even a simple 'nack'.

Best regards,
Christian Schoenebeck

Re: [PULL v3 2/6] tests/9pfs: change qtest name prefix to synth

[Philippe Mathieu-Daudé]

Hi Cristian,

On 10/20/20 1:54 PM, Christian Schoenebeck wrote:
> On Dienstag, 20. Oktober 2020 12:00:57 CEST Greg Kurz wrote:
>> On Tue, 20 Oct 2020 11:43:18 +0200
>>
>> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
>>> On Dienstag, 20. Oktober 2020 09:36:10 CEST Philippe Mathieu-Daudé wrote:
>>>> On 10/8/20 8:34 PM, Christian Schoenebeck wrote:
>>>>> All existing 9pfs test cases are using the 'synth' fs driver so far,
>>>>> which
>>>>> means they are not accessing real files, but a purely simulated (in
>>>>> RAM
>>>>> only) file system.
>>>>>
>>>>> Let's make this clear by changing the prefix of the individual qtest
>>>>> case
>>>>> names from 'fs/' to 'synth/'. That way they'll be easily
>>>>> distinguishable
>>>>> from upcoming new 9pfs test cases supposed to be using a different fs
>>>>> driver.
>>>>>
>>>>> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
>>>>> Message-Id:
>>>>> <e04e75acb849b085c6d6320b2433a15fa935bcff.1602182956.git.qemu_oss@crud
>>>>> eby
>>>>> te.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
>>>>
>>>> Harmless, but don't need to sign twice ;)
>>>
>>> Ah, I thought that's the common way, as Greg's PRs contained 2 SOBs as
>>> well, i.e. I thought this was intended to outline the patch author and
>>> submaintainer were the same person.
>>>
>>> BTW I actually did not explicitly add the 2nd SOB. It was rather added by
>>> the patchwork client automatically. So maybe it should be fixed in the
>>> client to detect an already existing SOB line? Or am missing something
>>> here?
>> Yeah this is the reason why my sob appears twice on patches authored by
>> me, and since this is harmless I never really investigated how to fix
>> pwclient :)
> 
> Well, I would usually offer my 'I can look at it' at this point, but I am
> reluctant this time as I assume it will end up as my recently suggested libqos
> patches where I did not get any response from the officially assigned
> maintainers; not even a simple 'nack'.

I was just watching your contributions and suggested an improvement
because something in your new workflow seems mis-configured (other
maintainers don't have this problem). I didn't asked you to fix a
bug in a different tool. I apologize if I was unclear and you
understood it this way.

Regarding your issue with a different series, I suppose you already
read:
https://wiki.qemu.org/Contribute/SubmitAPatch#If_your_patch_seems_to_have_been_ignored
and
https://wiki.qemu.org/Contribute/SubmitAPatch#Return_the_favor

You'll see that maintenance can be very time consuming, and we are
overcrowded from time to time when there is rush. I doubt maintainers
are ignoring your patches, as most of them try to do their best.
You might help them by reviewing patches for them, so they have time
to process your series.

Regards,

Phil.

Re: [PULL v3 2/6] tests/9pfs: change qtest name prefix to synth

[Christian Schoenebeck]

On Mittwoch, 21. Oktober 2020 08:15:55 CEST Philippe Mathieu-Daudé wrote:
> Hi Cristian,
> 
> On 10/20/20 1:54 PM, Christian Schoenebeck wrote:
> > On Dienstag, 20. Oktober 2020 12:00:57 CEST Greg Kurz wrote:
> >> On Tue, 20 Oct 2020 11:43:18 +0200
> >> 
> >> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> >>> On Dienstag, 20. Oktober 2020 09:36:10 CEST Philippe Mathieu-Daudé 
wrote:
> >>>> On 10/8/20 8:34 PM, Christian Schoenebeck wrote:
> >>>>> All existing 9pfs test cases are using the 'synth' fs driver so far,
> >>>>> which
> >>>>> means they are not accessing real files, but a purely simulated (in
> >>>>> RAM
> >>>>> only) file system.
> >>>>> 
> >>>>> Let's make this clear by changing the prefix of the individual qtest
> >>>>> case
> >>>>> names from 'fs/' to 'synth/'. That way they'll be easily
> >>>>> distinguishable
> >>>>> from upcoming new 9pfs test cases supposed to be using a different fs
> >>>>> driver.
> >>>>> 
> >>>>> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> >>>>> Message-Id:
> >>>>> <e04e75acb849b085c6d6320b2433a15fa935bcff.1602182956.git.qemu_oss@crud
> >>>>> eby
> >>>>> te.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> >>>> 
> >>>> Harmless, but don't need to sign twice ;)
> >>> 
> >>> Ah, I thought that's the common way, as Greg's PRs contained 2 SOBs as
> >>> well, i.e. I thought this was intended to outline the patch author and
> >>> submaintainer were the same person.
> >>> 
> >>> BTW I actually did not explicitly add the 2nd SOB. It was rather added
> >>> by
> >>> the patchwork client automatically. So maybe it should be fixed in the
> >>> client to detect an already existing SOB line? Or am missing something
> >>> here?
> >> 
> >> Yeah this is the reason why my sob appears twice on patches authored by
> >> me, and since this is harmless I never really investigated how to fix
> >> pwclient :)
> > 
> > Well, I would usually offer my 'I can look at it' at this point, but I am
> > reluctant this time as I assume it will end up as my recently suggested
> > libqos patches where I did not get any response from the officially
> > assigned maintainers; not even a simple 'nack'.
> 
> I was just watching your contributions and suggested an improvement
> because something in your new workflow seems mis-configured (other
> maintainers don't have this problem). I didn't asked you to fix a
> bug in a different tool. I apologize if I was unclear and you
> understood it this way.

You actually did not raise that expectation to me Philippe, so definitely no 
need to apologize. But I appreciate it!

Correct me if I'm wrong, but AFAICS I'm actually not the only one being 
affected by this double-SOB issue. A short glimpse at the logs and I see for 
instance 3e7e134d827790c3714cae1d5b8aff8612000116 having it as well.

So I guess everyone having the following two options enabled in pwclientrc:

msgid=on
signoff=on

will have that issue.

> Regarding your issue with a different series, I suppose you already
> read:
> https://wiki.qemu.org/Contribute/SubmitAPatch#If_your_patch_seems_to_have_be
> en_ignored and
> https://wiki.qemu.org/Contribute/SubmitAPatch#Return_the_favor
> 
> You'll see that maintenance can be very time consuming, and we are
> overcrowded from time to time when there is rush. I doubt maintainers
> are ignoring your patches, as most of them try to do their best.
> You might help them by reviewing patches for them, so they have time
> to process your series.

Yes, I am aware of these. And once I got used to a new code base tree I also 
look at other ones' patches there.

I've recently been thinking whether it would be possible for QEMU 
submaintainers to make use of patchwork's status feature:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg737917.html

Maybe that could help preventing patches of high traffic components ending up 
unseen.

Best regards,
Christian Schoenebeck

Re: [PULL v3 3/6] tests/9pfs: introduce local tests

[Greg Kurz]

On Thu, 8 Oct 2020 20:34:56 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> This patch introduces 9pfs test cases using the 9pfs 'local'
> filesystem driver which reads/writes/creates/deletes real files
> and directories.
> 
> In this initial version, there is only one local test which actually
> only checks if the 9pfs 'local' device was created successfully.
> 
> Before the 9pfs 'local' tests are run, a test directory 'qtest-9p-local'
> is created (with world rwx permissions) under the current working
> directory. At this point that test directory is not auto deleted yet.
> 
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> Message-Id: <81fc4b3b6b6c9bf7999e79f5e7cbc364a5f09ddb.1602182956.git.qemu_oss@crudebyte.com>
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---
>  tests/qtest/libqos/virtio-9p.c | 81 ++++++++++++++++++++++++++++++++++
>  tests/qtest/libqos/virtio-9p.h |  5 +++
>  tests/qtest/virtio-9p-test.c   | 44 ++++++++++++------
>  3 files changed, 116 insertions(+), 14 deletions(-)
> 
> diff --git a/tests/qtest/libqos/virtio-9p.c b/tests/qtest/libqos/virtio-9p.c
> index 2e300063e3..ee331166de 100644
> --- a/tests/qtest/libqos/virtio-9p.c
> +++ b/tests/qtest/libqos/virtio-9p.c
> @@ -24,6 +24,34 @@
>  #include "qgraph.h"
>  
>  static QGuestAllocator *alloc;
> +static char *local_test_path;
> +
> +/* Concatenates the passed 2 pathes. Returned result must be freed. */
> +static char *concat_path(const char* a, const char* b)
> +{
> +    return g_build_filename(a, b, NULL);
> +}
> +
> +static void init_local_test_path(void)
> +{
> +    char *pwd = g_get_current_dir();
> +    local_test_path = concat_path(pwd, "qtest-9p-local");
> +    g_free(pwd);
> +}
> +
> +/* Creates the directory for the 9pfs 'local' filesystem driver to access. */
> +static void create_local_test_dir(void)
> +{
> +    struct stat st;
> +
> +    g_assert(local_test_path != NULL);
> +    mkdir(local_test_path, 0777);
> +

This makes coverity unhappy...

*** CID 1435963:  Error handling issues  (CHECKED_RETURN)
/qemu/tests/qtest/libqos/virtio-9p.c: 48 in create_local_test_dir()
42     /* Creates the directory for the 9pfs 'local' filesystem driver to access. */
43     static void create_local_test_dir(void)
44     {
45         struct stat st;
46     
47         g_assert(local_test_path != NULL);
>>>     CID 1435963:  Error handling issues  (CHECKED_RETURN)
>>>     Calling "mkdir(local_test_path, 511U)" without checking return value. This library function may fail and return an error code.  
48         mkdir(local_test_path, 0777);
49     
50         /* ensure test directory exists now ... */
51         g_assert(stat(local_test_path, &st) == 0);
52         /* ... and is actually a directory */
53         g_assert((st.st_mode & S_IFMT) == S_IFDIR);

> +    /* ensure test directory exists now ... */
> +    g_assert(stat(local_test_path, &st) == 0);
> +    /* ... and is actually a directory */
> +    g_assert((st.st_mode & S_IFMT) == S_IFDIR);
> +}
>  
>  static void virtio_9p_cleanup(QVirtio9P *interface)
>  {
> @@ -146,11 +174,64 @@ static void *virtio_9p_pci_create(void *pci_bus, QGuestAllocator *t_alloc,
>      return obj;
>  }
>  
> +/**
> + * Performs regular expression based search and replace on @a haystack.
> + *
> + * @param haystack - input string to be parsed, result of replacement is
> + *                   stored back to @a haystack
> + * @param pattern - the regular expression pattern for scanning @a haystack
> + * @param replace_fmt - matches of supplied @a pattern are replaced by this,
> + *                      if necessary glib printf format can be used to add
> + *                      variable arguments of this function to this
> + *                      replacement string
> + */
> +static void regex_replace(GString *haystack, const char *pattern,
> +                          const char *replace_fmt, ...)
> +{
> +    GRegex *regex;
> +    char *replace, *s;
> +    va_list argp;
> +
> +    va_start(argp, replace_fmt);
> +    replace = g_strdup_vprintf(replace_fmt, argp);
> +    va_end(argp);
> +
> +    regex = g_regex_new(pattern, 0, 0, NULL);
> +    s = g_regex_replace(regex, haystack->str, -1, 0, replace, 0, NULL);
> +    g_string_assign(haystack, s);
> +    g_free(s);
> +    g_regex_unref(regex);
> +    g_free(replace);
> +}
> +
> +void virtio_9p_assign_local_driver(GString *cmd_line, const char *args)
> +{
> +    g_assert_nonnull(local_test_path);
> +
> +    /* replace 'synth' driver by 'local' driver */
> +    regex_replace(cmd_line, "-fsdev synth,", "-fsdev local,");
> +
> +    /* append 'path=...' to '-fsdev ...' group */
> +    regex_replace(cmd_line, "(-fsdev \\w[^ ]*)", "\\1,path='%s'",
> +                  local_test_path);
> +
> +    if (!args) {
> +        return;
> +    }
> +
> +    /* append passed args to '-fsdev ...' group */
> +    regex_replace(cmd_line, "(-fsdev \\w[^ ]*)", "\\1,%s", args);
> +}
> +
>  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();
> +    create_local_test_dir();
> +
>      QPCIAddress addr = {
>          .devfn = QPCI_DEVFN(4, 0),
>      };
> diff --git a/tests/qtest/libqos/virtio-9p.h b/tests/qtest/libqos/virtio-9p.h
> index b1e6badc4a..326a603f72 100644
> --- a/tests/qtest/libqos/virtio-9p.h
> +++ b/tests/qtest/libqos/virtio-9p.h
> @@ -44,4 +44,9 @@ struct QVirtio9PDevice {
>      QVirtio9P v9p;
>  };
>  
> +/**
> + * Prepares QEMU command line for 9pfs tests using the 'local' fs driver.
> + */
> +void virtio_9p_assign_local_driver(GString *cmd_line, const char *args);
> +
>  #endif
> diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> index 3281153b9c..af7e169d3a 100644
> --- a/tests/qtest/virtio-9p-test.c
> +++ b/tests/qtest/virtio-9p-test.c
> @@ -895,29 +895,45 @@ static void fs_readdir_split_512(void *obj, void *data,
>      fs_readdir_split(obj, data, t_alloc, 512);
>  }
>  
> +static void *assign_9p_local_driver(GString *cmd_line, void *arg)
> +{
> +    virtio_9p_assign_local_driver(cmd_line, "security_model=mapped-xattr");
> +    return arg;
> +}
> +
>  static void register_virtio_9p_test(void)
>  {
> -    qos_add_test("synth/config", "virtio-9p", pci_config, NULL);
> -    qos_add_test("synth/version/basic", "virtio-9p", fs_version, NULL);
> -    qos_add_test("synth/attach/basic", "virtio-9p", fs_attach, NULL);
> -    qos_add_test("synth/walk/basic", "virtio-9p", fs_walk, NULL);
> +
> +    QOSGraphTestOptions opts = {
> +    };
> +
> +    /* 9pfs test cases using the 'synth' filesystem driver */
> +    qos_add_test("synth/config", "virtio-9p", pci_config, &opts);
> +    qos_add_test("synth/version/basic", "virtio-9p", fs_version,  &opts);
> +    qos_add_test("synth/attach/basic", "virtio-9p", fs_attach,  &opts);
> +    qos_add_test("synth/walk/basic", "virtio-9p", fs_walk,  &opts);
>      qos_add_test("synth/walk/no_slash", "virtio-9p", fs_walk_no_slash,
> -                 NULL);
> +                  &opts);
>      qos_add_test("synth/walk/dotdot_from_root", "virtio-9p",
> -                 fs_walk_dotdot, NULL);
> -    qos_add_test("synth/lopen/basic", "virtio-9p", fs_lopen, NULL);
> -    qos_add_test("synth/write/basic", "virtio-9p", fs_write, NULL);
> +                 fs_walk_dotdot,  &opts);
> +    qos_add_test("synth/lopen/basic", "virtio-9p", fs_lopen,  &opts);
> +    qos_add_test("synth/write/basic", "virtio-9p", fs_write,  &opts);
>      qos_add_test("synth/flush/success", "virtio-9p", fs_flush_success,
> -                 NULL);
> +                  &opts);
>      qos_add_test("synth/flush/ignored", "virtio-9p", fs_flush_ignored,
> -                 NULL);
> -    qos_add_test("synth/readdir/basic", "virtio-9p", fs_readdir, NULL);
> +                  &opts);
> +    qos_add_test("synth/readdir/basic", "virtio-9p", fs_readdir,  &opts);
>      qos_add_test("synth/readdir/split_512", "virtio-9p",
> -                 fs_readdir_split_512, NULL);
> +                 fs_readdir_split_512,  &opts);
>      qos_add_test("synth/readdir/split_256", "virtio-9p",
> -                 fs_readdir_split_256, NULL);
> +                 fs_readdir_split_256,  &opts);
>      qos_add_test("synth/readdir/split_128", "virtio-9p",
> -                 fs_readdir_split_128, NULL);
> +                 fs_readdir_split_128,  &opts);
> +
> +
> +    /* 9pfs test cases using the 'local' filesystem driver */
> +    opts.before = assign_9p_local_driver;
> +    qos_add_test("local/config", "virtio-9p", pci_config,  &opts);
>  }
>  
>  libqos_init(register_virtio_9p_test);

Re: [PULL v3 3/6] tests/9pfs: introduce local tests

[Christian Schoenebeck]

On Donnerstag, 29. Oktober 2020 19:02:34 CET Greg Kurz wrote:
> On Thu, 8 Oct 2020 20:34:56 +0200
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > This patch introduces 9pfs test cases using the 9pfs 'local'
> > filesystem driver which reads/writes/creates/deletes real files
> > and directories.
> > 
> > In this initial version, there is only one local test which actually
> > only checks if the 9pfs 'local' device was created successfully.
> > 
> > Before the 9pfs 'local' tests are run, a test directory 'qtest-9p-local'
> > is created (with world rwx permissions) under the current working
> > directory. At this point that test directory is not auto deleted yet.
> > 
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > Message-Id:
> > <81fc4b3b6b6c9bf7999e79f5e7cbc364a5f09ddb.1602182956.git.qemu_oss@crudeby
> > te.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > ---
> > 
> >  tests/qtest/libqos/virtio-9p.c | 81 ++++++++++++++++++++++++++++++++++
> >  tests/qtest/libqos/virtio-9p.h |  5 +++
> >  tests/qtest/virtio-9p-test.c   | 44 ++++++++++++------
> >  3 files changed, 116 insertions(+), 14 deletions(-)
> > 
> > diff --git a/tests/qtest/libqos/virtio-9p.c
> > b/tests/qtest/libqos/virtio-9p.c index 2e300063e3..ee331166de 100644
> > --- a/tests/qtest/libqos/virtio-9p.c
> > +++ b/tests/qtest/libqos/virtio-9p.c
> > @@ -24,6 +24,34 @@
> > 
> >  #include "qgraph.h"
> >  
> >  static QGuestAllocator *alloc;
> > 
> > +static char *local_test_path;
> > +
> > +/* Concatenates the passed 2 pathes. Returned result must be freed. */
> > +static char *concat_path(const char* a, const char* b)
> > +{
> > +    return g_build_filename(a, b, NULL);
> > +}
> > +
> > +static void init_local_test_path(void)
> > +{
> > +    char *pwd = g_get_current_dir();
> > +    local_test_path = concat_path(pwd, "qtest-9p-local");
> > +    g_free(pwd);
> > +}
> > +
> > +/* Creates the directory for the 9pfs 'local' filesystem driver to
> > access. */ +static void create_local_test_dir(void)
> > +{
> > +    struct stat st;
> > +
> > +    g_assert(local_test_path != NULL);
> > +    mkdir(local_test_path, 0777);
> > +
> 
> This makes coverity unhappy...
> 
> *** CID 1435963:  Error handling issues  (CHECKED_RETURN)
> /qemu/tests/qtest/libqos/virtio-9p.c: 48 in create_local_test_dir()
> 42     /* Creates the directory for the 9pfs 'local' filesystem driver to
> access. */ 43     static void create_local_test_dir(void)
> 44     {
> 45         struct stat st;
> 46
> 47         g_assert(local_test_path != NULL);
> 
> >>>     CID 1435963:  Error handling issues  (CHECKED_RETURN)
> >>>     Calling "mkdir(local_test_path, 511U)" without checking return
> >>>     value. This library function may fail and return an error code.
> 48         mkdir(local_test_path, 0777);
> 49
> 50         /* ensure test directory exists now ... */
> 51         g_assert(stat(local_test_path, &st) == 0);
> 52         /* ... and is actually a directory */
> 53         g_assert((st.st_mode & S_IFMT) == S_IFDIR);
> 
> > +    /* ensure test directory exists now ... */
> > +    g_assert(stat(local_test_path, &st) == 0);
> > +    /* ... and is actually a directory */
> > +    g_assert((st.st_mode & S_IFMT) == S_IFDIR);
> > +}
> > 

Ok, I'll fix that with tomorrow's patch(es) as well.

Best regards,
Christian Schoenebeck