All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 5/8] 9pfs: drop fid_to_qid()
  2021-07-05 11:13 [PULL 0/8] 9p queue 2021-07-05 Christian Schoenebeck
                   ` (3 preceding siblings ...)
  2021-07-05 11:13 ` [PULL 2/8] 9pfs: simplify v9fs_walk() Christian Schoenebeck
@ 2021-07-05 11:13 ` Christian Schoenebeck
  2021-07-05 11:13 ` [PULL 3/8] 9pfs: fix not_same_qid() Christian Schoenebeck
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Christian Schoenebeck @ 2021-07-05 11:13 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell; +Cc: Greg Kurz

There is only one user of fid_to_qid() which is v9fs_walk(). Let's
open-code fid_to_qid() directly within v9fs_walk(), because
fid_to_qid() hides the POSIX stat buffer which we are going to need
in the subsequent patch.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <e9a4c9c7a0792ed4db6578d105a0823ea05bc324.1622821729.git.qemu_oss@crudebyte.com>
---
 hw/9pfs/9p.c | 23 +++++------------------
 1 file changed, 5 insertions(+), 18 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index eb15ec2082..0e3857798d 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -971,23 +971,6 @@ static int stat_to_qid(V9fsPDU *pdu, const struct stat *stbuf, V9fsQID *qidp)
     return 0;
 }
 
-static int coroutine_fn fid_to_qid(V9fsPDU *pdu, V9fsFidState *fidp,
-                                   V9fsQID *qidp)
-{
-    struct stat stbuf;
-    int err;
-
-    err = v9fs_co_lstat(pdu, &fidp->path, &stbuf);
-    if (err < 0) {
-        return err;
-    }
-    err = stat_to_qid(pdu, &stbuf, qidp);
-    if (err < 0) {
-        return err;
-    }
-    return 0;
-}
-
 V9fsPDU *pdu_alloc(V9fsState *s)
 {
     V9fsPDU *pdu = NULL;
@@ -1772,7 +1755,11 @@ static void coroutine_fn v9fs_walk(void *opaque)
     v9fs_path_init(&dpath);
     v9fs_path_init(&path);
 
-    err = fid_to_qid(pdu, fidp, &qid);
+    err = v9fs_co_lstat(pdu, &fidp->path, &stbuf);
+    if (err < 0) {
+        goto out;
+    }
+    err = stat_to_qid(pdu, &stbuf, &qid);
     if (err < 0) {
         goto out;
     }
-- 
2.20.1



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

* [PULL 3/8] 9pfs: fix not_same_qid()
  2021-07-05 11:13 [PULL 0/8] 9p queue 2021-07-05 Christian Schoenebeck
                   ` (4 preceding siblings ...)
  2021-07-05 11:13 ` [PULL 5/8] 9pfs: drop fid_to_qid() Christian Schoenebeck
@ 2021-07-05 11:13 ` Christian Schoenebeck
  2021-07-05 11:13 ` [PULL 1/8] 9pfs: add link to 9p developer docs Christian Schoenebeck
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Christian Schoenebeck @ 2021-07-05 11:13 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell; +Cc: Greg Kurz

There is only one user of not_same_qid() which is v9fs_walk() and the
latter is using it for comparing a client supplied path with the 9p
export root path, for the sole purpose to prevent a Twalk request
from escaping from the exported 9p tree via "..".

However for that specific purpose the implementation of not_same_qid()
is wrong; if mtime of the 9p export root path changed between Tattach
and Twalk then not_same_qid() returns true when actually comparing
against the export root path.

To fix for the actual semantic being used, only compare QID path
members, but do not compare version or type members.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <ca0abae4a899d81c6e87f683732d6c1f56915232.1622821729.git.qemu_oss@crudebyte.com>
---
 hw/9pfs/9p.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 89aa07db78..e10a02f71d 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1707,10 +1707,7 @@ static bool name_is_illegal(const char *name)
 
 static bool not_same_qid(const V9fsQID *qid1, const V9fsQID *qid2)
 {
-    return
-        qid1->type != qid2->type ||
-        qid1->version != qid2->version ||
-        qid1->path != qid2->path;
+    return qid1->path != qid2->path;
 }
 
 static void coroutine_fn v9fs_walk(void *opaque)
-- 
2.20.1



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

* [PULL 8/8] 9pfs: reduce latency of Twalk
  2021-07-05 11:13 [PULL 0/8] 9p queue 2021-07-05 Christian Schoenebeck
@ 2021-07-05 11:13 ` Christian Schoenebeck
  2021-07-05 11:13 ` [PULL 7/8] 9pfs: drop root_qid Christian Schoenebeck
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Christian Schoenebeck @ 2021-07-05 11:13 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell; +Cc: Greg Kurz

As with previous performance optimization on Treaddir handling;
reduce the overall latency, i.e. overall time spent on processing
a Twalk request by reducing the amount of thread hops between the
9p server's main thread and fs worker thread(s).

In fact this patch even reduces the thread hops for Twalk handling
to its theoritical minimum of exactly 2 thread hops:

main thread -> fs worker thread -> main thread

This is achieved by doing all the required fs driver tasks altogether
in a single v9fs_co_run_in_worker({ ... }); code block.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <1a6701674afc4f08d40396e3aa2631e18a4dbb33.1622821729.git.qemu_oss@crudebyte.com>
---
 hw/9pfs/9p.c | 89 +++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 70 insertions(+), 19 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 7be07f2d68..2815257f42 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1705,9 +1705,9 @@ static void coroutine_fn v9fs_walk(void *opaque)
     int name_idx;
     V9fsQID *qids = NULL;
     int i, err = 0;
-    V9fsPath dpath, path;
+    V9fsPath dpath, path, *pathes = NULL;
     uint16_t nwnames;
-    struct stat stbuf;
+    struct stat stbuf, fidst, *stbufs = NULL;
     size_t offset = 7;
     int32_t fid, newfid;
     V9fsString *wnames = NULL;
@@ -1733,6 +1733,8 @@ static void coroutine_fn v9fs_walk(void *opaque)
     if (nwnames) {
         wnames = g_new0(V9fsString, nwnames);
         qids   = g_new0(V9fsQID, nwnames);
+        stbufs = g_new0(struct stat, nwnames);
+        pathes = g_new0(V9fsPath, nwnames);
         for (i = 0; i < nwnames; i++) {
             err = pdu_unmarshal(pdu, offset, "s", &wnames[i]);
             if (err < 0) {
@@ -1753,39 +1755,85 @@ static void coroutine_fn v9fs_walk(void *opaque)
 
     v9fs_path_init(&dpath);
     v9fs_path_init(&path);
+    /*
+     * Both dpath and path initially point to fidp.
+     * Needed to handle request with nwnames == 0
+     */
+    v9fs_path_copy(&dpath, &fidp->path);
+    v9fs_path_copy(&path, &fidp->path);
 
-    err = v9fs_co_lstat(pdu, &fidp->path, &stbuf);
+    /*
+     * To keep latency (i.e. overall execution time for processing this
+     * Twalk client request) as small as possible, run all the required fs
+     * driver code altogether inside the following block.
+     */
+    v9fs_co_run_in_worker({
+        if (v9fs_request_cancelled(pdu)) {
+            err = -EINTR;
+            break;
+        }
+        err = s->ops->lstat(&s->ctx, &dpath, &fidst);
+        if (err < 0) {
+            err = -errno;
+            break;
+        }
+        stbuf = fidst;
+        for (name_idx = 0; name_idx < nwnames; name_idx++) {
+            if (v9fs_request_cancelled(pdu)) {
+                err = -EINTR;
+                break;
+            }
+            if (!same_stat_id(&pdu->s->root_st, &stbuf) ||
+                strcmp("..", wnames[name_idx].data))
+            {
+                err = s->ops->name_to_path(&s->ctx, &dpath,
+                                        wnames[name_idx].data, &path);
+                if (err < 0) {
+                    err = -errno;
+                    break;
+                }
+                if (v9fs_request_cancelled(pdu)) {
+                    err = -EINTR;
+                    break;
+                }
+                err = s->ops->lstat(&s->ctx, &path, &stbuf);
+                if (err < 0) {
+                    err = -errno;
+                    break;
+                }
+                stbufs[name_idx] = stbuf;
+                v9fs_path_copy(&dpath, &path);
+                v9fs_path_copy(&pathes[name_idx], &path);
+            }
+        }
+    });
+    /*
+     * Handle all the rest of this Twalk request on main thread ...
+     */
     if (err < 0) {
         goto out;
     }
-    err = stat_to_qid(pdu, &stbuf, &qid);
+
+    err = stat_to_qid(pdu, &fidst, &qid);
     if (err < 0) {
         goto out;
     }
+    stbuf = fidst;
 
-    /*
-     * Both dpath and path initially poin to fidp.
-     * Needed to handle request with nwnames == 0
-     */
+    /* reset dpath and path */
     v9fs_path_copy(&dpath, &fidp->path);
     v9fs_path_copy(&path, &fidp->path);
+
     for (name_idx = 0; name_idx < nwnames; name_idx++) {
         if (!same_stat_id(&pdu->s->root_st, &stbuf) ||
-            strcmp("..", wnames[name_idx].data)) {
-            err = v9fs_co_name_to_path(pdu, &dpath, wnames[name_idx].data,
-                                       &path);
-            if (err < 0) {
-                goto out;
-            }
-
-            err = v9fs_co_lstat(pdu, &path, &stbuf);
-            if (err < 0) {
-                goto out;
-            }
+            strcmp("..", wnames[name_idx].data))
+        {
+            stbuf = stbufs[name_idx];
             err = stat_to_qid(pdu, &stbuf, &qid);
             if (err < 0) {
                 goto out;
             }
+            v9fs_path_copy(&path, &pathes[name_idx]);
             v9fs_path_copy(&dpath, &path);
         }
         memcpy(&qids[name_idx], &qid, sizeof(qid));
@@ -1821,9 +1869,12 @@ out_nofid:
     if (nwnames && nwnames <= P9_MAXWELEM) {
         for (name_idx = 0; name_idx < nwnames; name_idx++) {
             v9fs_string_free(&wnames[name_idx]);
+            v9fs_path_free(&pathes[name_idx]);
         }
         g_free(wnames);
         g_free(qids);
+        g_free(stbufs);
+        g_free(pathes);
     }
 }
 
-- 
2.20.1



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

* [PULL 6/8] 9pfs: replace not_same_qid() by same_stat_id()
  2021-07-05 11:13 [PULL 0/8] 9p queue 2021-07-05 Christian Schoenebeck
                   ` (6 preceding siblings ...)
  2021-07-05 11:13 ` [PULL 1/8] 9pfs: add link to 9p developer docs Christian Schoenebeck
@ 2021-07-05 11:13 ` Christian Schoenebeck
  2021-07-05 13:57 ` [PULL 0/8] 9p queue 2021-07-05 Peter Maydell
  2021-07-05 19:03 ` Peter Maydell
  9 siblings, 0 replies; 12+ messages in thread
From: Christian Schoenebeck @ 2021-07-05 11:13 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell; +Cc: Greg Kurz

As we are actually only comparing the filesystem ID (i.e. device number
and inode number pair) let's use the POSIX stat buffer instead of QIDs,
because resolving QIDs requires to be done on 9p server's main thread
only as it might mutate the server state if inode remapping is enabled.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <26aa465ff9cc9c07e053331554a02fdae3994417.1622821729.git.qemu_oss@crudebyte.com>
---
 hw/9pfs/9p.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 0e3857798d..47b000d3a9 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1696,9 +1696,9 @@ static bool name_is_illegal(const char *name)
     return !*name || strchr(name, '/') != NULL;
 }
 
-static bool not_same_qid(const V9fsQID *qid1, const V9fsQID *qid2)
+static bool same_stat_id(const struct stat *a, const struct stat *b)
 {
-    return qid1->path != qid2->path;
+    return a->st_dev == b->st_dev && a->st_ino == b->st_ino;
 }
 
 static void coroutine_fn v9fs_walk(void *opaque)
@@ -1771,7 +1771,7 @@ static void coroutine_fn v9fs_walk(void *opaque)
     v9fs_path_copy(&dpath, &fidp->path);
     v9fs_path_copy(&path, &fidp->path);
     for (name_idx = 0; name_idx < nwnames; name_idx++) {
-        if (not_same_qid(&pdu->s->root_qid, &qid) ||
+        if (!same_stat_id(&pdu->s->root_st, &stbuf) ||
             strcmp("..", wnames[name_idx].data)) {
             err = v9fs_co_name_to_path(pdu, &dpath, wnames[name_idx].data,
                                        &path);
-- 
2.20.1



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

* [PULL 2/8] 9pfs: simplify v9fs_walk()
  2021-07-05 11:13 [PULL 0/8] 9p queue 2021-07-05 Christian Schoenebeck
                   ` (2 preceding siblings ...)
  2021-07-05 11:13 ` [PULL 4/8] 9pfs: capture root stat Christian Schoenebeck
@ 2021-07-05 11:13 ` Christian Schoenebeck
  2021-07-05 11:13 ` [PULL 5/8] 9pfs: drop fid_to_qid() Christian Schoenebeck
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Christian Schoenebeck @ 2021-07-05 11:13 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell; +Cc: Greg Kurz

There is only one comparison between nwnames and P9_MAXWELEM required.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <E1liKiz-0006BC-Ja@lizzy.crudebyte.com>
---
 hw/9pfs/9p.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 0fa776af09..89aa07db78 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1739,7 +1739,11 @@ static void coroutine_fn v9fs_walk(void *opaque)
 
     trace_v9fs_walk(pdu->tag, pdu->id, fid, newfid, nwnames);
 
-    if (nwnames && nwnames <= P9_MAXWELEM) {
+    if (nwnames > P9_MAXWELEM) {
+        err = -EINVAL;
+        goto out_nofid;
+    }
+    if (nwnames) {
         wnames = g_new0(V9fsString, nwnames);
         qids   = g_new0(V9fsQID, nwnames);
         for (i = 0; i < nwnames; i++) {
@@ -1753,9 +1757,6 @@ static void coroutine_fn v9fs_walk(void *opaque)
             }
             offset += err;
         }
-    } else if (nwnames > P9_MAXWELEM) {
-        err = -EINVAL;
-        goto out_nofid;
     }
     fidp = get_fid(pdu, fid);
     if (fidp == NULL) {
-- 
2.20.1



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

* [PULL 0/8] 9p queue 2021-07-05
@ 2021-07-05 11:13 Christian Schoenebeck
  2021-07-05 11:13 ` [PULL 8/8] 9pfs: reduce latency of Twalk Christian Schoenebeck
                   ` (9 more replies)
  0 siblings, 10 replies; 12+ messages in thread
From: Christian Schoenebeck @ 2021-07-05 11:13 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell; +Cc: Greg Kurz

The following changes since commit 711c0418c8c1ce3a24346f058b001c4c5a2f0f81:

  Merge remote-tracking branch 'remotes/philmd/tags/mips-20210702' into staging (2021-07-04 14:04:12 +0100)

are available in the Git repository at:

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

for you to fetch changes up to 8d6cb100731c4d28535adbf2a3c2d1f29be3fef4:

  9pfs: reduce latency of Twalk (2021-07-05 13:03:16 +0200)

----------------------------------------------------------------
9pfs: misc patches

* Add link to 9p developer docs.

* Fix runtime check whether client supplied relative path is the export
  root.

* Performance optimization of Twalk requests.

* Code cleanup.

----------------------------------------------------------------
Christian Schoenebeck (8):
      9pfs: add link to 9p developer docs
      9pfs: simplify v9fs_walk()
      9pfs: fix not_same_qid()
      9pfs: capture root stat
      9pfs: drop fid_to_qid()
      9pfs: replace not_same_qid() by same_stat_id()
      9pfs: drop root_qid
      9pfs: reduce latency of Twalk

 MAINTAINERS                    |   1 +
 hw/9pfs/9p-local.c             |   5 ++
 hw/9pfs/9p-posix-acl.c         |   5 ++
 hw/9pfs/9p-proxy.c             |   5 ++
 hw/9pfs/9p-synth.c             |   5 ++
 hw/9pfs/9p-util.c              |   5 ++
 hw/9pfs/9p-xattr-user.c        |   5 ++
 hw/9pfs/9p-xattr.c             |   5 ++
 hw/9pfs/9p.c                   | 142 +++++++++++++++++++++++++++--------------
 hw/9pfs/9p.h                   |   2 +-
 hw/9pfs/codir.c                |   5 ++
 hw/9pfs/cofile.c               |   5 ++
 hw/9pfs/cofs.c                 |   5 ++
 hw/9pfs/coth.c                 |   5 ++
 hw/9pfs/coxattr.c              |   5 ++
 hw/9pfs/virtio-9p-device.c     |   5 ++
 hw/9pfs/xen-9p-backend.c       |   5 ++
 tests/qtest/libqos/virtio-9p.c |   5 ++
 tests/qtest/virtio-9p-test.c   |   5 ++
 19 files changed, 177 insertions(+), 48 deletions(-)


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

* [PULL 7/8] 9pfs: drop root_qid
  2021-07-05 11:13 [PULL 0/8] 9p queue 2021-07-05 Christian Schoenebeck
  2021-07-05 11:13 ` [PULL 8/8] 9pfs: reduce latency of Twalk Christian Schoenebeck
@ 2021-07-05 11:13 ` Christian Schoenebeck
  2021-07-05 11:13 ` [PULL 4/8] 9pfs: capture root stat Christian Schoenebeck
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Christian Schoenebeck @ 2021-07-05 11:13 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell; +Cc: Greg Kurz

There is no longer a user of root_qid, so drop it.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <6896dd161d3257db6b0513842a14f87ca191fdf6.1622821729.git.qemu_oss@crudebyte.com>
---
 hw/9pfs/9p.c | 1 -
 hw/9pfs/9p.h | 1 -
 2 files changed, 2 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 47b000d3a9..7be07f2d68 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1444,7 +1444,6 @@ static void coroutine_fn v9fs_attach(void *opaque)
     }
     err += offset;
 
-    memcpy(&s->root_qid, &qid, sizeof(qid));
     memcpy(&s->root_st, &stbuf, sizeof(stbuf));
     trace_v9fs_attach_return(pdu->tag, pdu->id,
                              qid.type, qid.version, qid.path);
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 6f0b4c78c0..1567b67841 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -355,7 +355,6 @@ struct V9fsState {
     int32_t root_fid;
     Error *migration_blocker;
     V9fsConf fsconf;
-    V9fsQID root_qid;
     struct stat root_st;
     dev_t dev_id;
     struct qht qpd_table;
-- 
2.20.1



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

* [PULL 1/8] 9pfs: add link to 9p developer docs
  2021-07-05 11:13 [PULL 0/8] 9p queue 2021-07-05 Christian Schoenebeck
                   ` (5 preceding siblings ...)
  2021-07-05 11:13 ` [PULL 3/8] 9pfs: fix not_same_qid() Christian Schoenebeck
@ 2021-07-05 11:13 ` Christian Schoenebeck
  2021-07-05 11:13 ` [PULL 6/8] 9pfs: replace not_same_qid() by same_stat_id() Christian Schoenebeck
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Christian Schoenebeck @ 2021-07-05 11:13 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell; +Cc: Greg Kurz

To lower the entry level for new developers, add a link to the 9p
developer docs (i.e. qemu wiki) to MAINTAINERS and to the beginning of
9p source files, that is to: https://wiki.qemu.org/Documentation/9p

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Acked-by: Greg Kurz <groug@kaod.org>
Message-Id: <E1leeDf-0008GZ-9q@lizzy.crudebyte.com>
---
 MAINTAINERS                    | 1 +
 hw/9pfs/9p-local.c             | 5 +++++
 hw/9pfs/9p-posix-acl.c         | 5 +++++
 hw/9pfs/9p-proxy.c             | 5 +++++
 hw/9pfs/9p-synth.c             | 5 +++++
 hw/9pfs/9p-util.c              | 5 +++++
 hw/9pfs/9p-xattr-user.c        | 5 +++++
 hw/9pfs/9p-xattr.c             | 5 +++++
 hw/9pfs/9p.c                   | 5 +++++
 hw/9pfs/codir.c                | 5 +++++
 hw/9pfs/cofile.c               | 5 +++++
 hw/9pfs/cofs.c                 | 5 +++++
 hw/9pfs/coth.c                 | 5 +++++
 hw/9pfs/coxattr.c              | 5 +++++
 hw/9pfs/virtio-9p-device.c     | 5 +++++
 hw/9pfs/xen-9p-backend.c       | 5 +++++
 tests/qtest/libqos/virtio-9p.c | 5 +++++
 tests/qtest/virtio-9p-test.c   | 5 +++++
 18 files changed, 86 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index cb8f3ea2c2..684142e12e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1881,6 +1881,7 @@ virtio-9p
 M: Greg Kurz <groug@kaod.org>
 M: Christian Schoenebeck <qemu_oss@crudebyte.com>
 S: Odd Fixes
+W: https://wiki.qemu.org/Documentation/9p
 F: hw/9pfs/
 X: hw/9pfs/xen-9p*
 F: fsdev/
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index af52c1daac..210d9e7705 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -10,6 +10,11 @@
  * the COPYING file in the top-level directory.
  */
 
+/*
+ * Not so fast! You might want to read the 9p developer docs first:
+ * https://wiki.qemu.org/Documentation/9p
+ */
+
 #include "qemu/osdep.h"
 #include "9p.h"
 #include "9p-local.h"
diff --git a/hw/9pfs/9p-posix-acl.c b/hw/9pfs/9p-posix-acl.c
index bbf89064f7..eadae270dd 100644
--- a/hw/9pfs/9p-posix-acl.c
+++ b/hw/9pfs/9p-posix-acl.c
@@ -11,6 +11,11 @@
  *
  */
 
+/*
+ * Not so fast! You might want to read the 9p developer docs first:
+ * https://wiki.qemu.org/Documentation/9p
+ */
+
 #include "qemu/osdep.h"
 #include "qemu/xattr.h"
 #include "9p.h"
diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
index 4aa4e0a3ba..09bd9f1464 100644
--- a/hw/9pfs/9p-proxy.c
+++ b/hw/9pfs/9p-proxy.c
@@ -10,6 +10,11 @@
  * the COPYING file in the top-level directory.
  */
 
+/*
+ * Not so fast! You might want to read the 9p developer docs first:
+ * https://wiki.qemu.org/Documentation/9p
+ */
+
 #include "qemu/osdep.h"
 #include <sys/socket.h>
 #include <sys/un.h>
diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index 473ef914b0..b38088e066 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -12,6 +12,11 @@
  *
  */
 
+/*
+ * Not so fast! You might want to read the 9p developer docs first:
+ * https://wiki.qemu.org/Documentation/9p
+ */
+
 #include "qemu/osdep.h"
 #include "9p.h"
 #include "fsdev/qemu-fsdev.h"
diff --git a/hw/9pfs/9p-util.c b/hw/9pfs/9p-util.c
index 614b7fc34d..3221d9b498 100644
--- a/hw/9pfs/9p-util.c
+++ b/hw/9pfs/9p-util.c
@@ -10,6 +10,11 @@
  * See the COPYING file in the top-level directory.
  */
 
+/*
+ * Not so fast! You might want to read the 9p developer docs first:
+ * https://wiki.qemu.org/Documentation/9p
+ */
+
 #include "qemu/osdep.h"
 #include "qemu/xattr.h"
 #include "9p-util.h"
diff --git a/hw/9pfs/9p-xattr-user.c b/hw/9pfs/9p-xattr-user.c
index 2c90817b75..f2ae9582e6 100644
--- a/hw/9pfs/9p-xattr-user.c
+++ b/hw/9pfs/9p-xattr-user.c
@@ -11,6 +11,11 @@
  *
  */
 
+/*
+ * Not so fast! You might want to read the 9p developer docs first:
+ * https://wiki.qemu.org/Documentation/9p
+ */
+
 #include "qemu/osdep.h"
 #include "9p.h"
 #include "fsdev/file-op-9p.h"
diff --git a/hw/9pfs/9p-xattr.c b/hw/9pfs/9p-xattr.c
index c696d8f846..9ae69dd8db 100644
--- a/hw/9pfs/9p-xattr.c
+++ b/hw/9pfs/9p-xattr.c
@@ -11,6 +11,11 @@
  *
  */
 
+/*
+ * Not so fast! You might want to read the 9p developer docs first:
+ * https://wiki.qemu.org/Documentation/9p
+ */
+
 #include "qemu/osdep.h"
 #include "9p.h"
 #include "fsdev/file-op-9p.h"
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 134806db52..0fa776af09 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -11,6 +11,11 @@
  *
  */
 
+/*
+ * Not so fast! You might want to read the 9p developer docs first:
+ * https://wiki.qemu.org/Documentation/9p
+ */
+
 #include "qemu/osdep.h"
 #include <glib/gprintf.h>
 #include "hw/virtio/virtio.h"
diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
index 1f70a58df5..032cce04c4 100644
--- a/hw/9pfs/codir.c
+++ b/hw/9pfs/codir.c
@@ -11,6 +11,11 @@
  *
  */
 
+/*
+ * Not so fast! You might want to read the 9p developer docs first:
+ * https://wiki.qemu.org/Documentation/9p
+ */
+
 #include "qemu/osdep.h"
 #include "fsdev/qemu-fsdev.h"
 #include "qemu/thread.h"
diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
index 83bb6c14e0..20f93a90e7 100644
--- a/hw/9pfs/cofile.c
+++ b/hw/9pfs/cofile.c
@@ -11,6 +11,11 @@
  *
  */
 
+/*
+ * Not so fast! You might want to read the 9p developer docs first:
+ * https://wiki.qemu.org/Documentation/9p
+ */
+
 #include "qemu/osdep.h"
 #include "fsdev/qemu-fsdev.h"
 #include "qemu/thread.h"
diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c
index 0b321b456e..9d0adc2e78 100644
--- a/hw/9pfs/cofs.c
+++ b/hw/9pfs/cofs.c
@@ -11,6 +11,11 @@
  *
  */
 
+/*
+ * Not so fast! You might want to read the 9p developer docs first:
+ * https://wiki.qemu.org/Documentation/9p
+ */
+
 #include "qemu/osdep.h"
 #include "fsdev/qemu-fsdev.h"
 #include "qemu/thread.h"
diff --git a/hw/9pfs/coth.c b/hw/9pfs/coth.c
index 9778f24b00..2802d41cce 100644
--- a/hw/9pfs/coth.c
+++ b/hw/9pfs/coth.c
@@ -12,6 +12,11 @@
  *
  */
 
+/*
+ * Not so fast! You might want to read the 9p developer docs first:
+ * https://wiki.qemu.org/Documentation/9p
+ */
+
 #include "qemu/osdep.h"
 #include "block/thread-pool.h"
 #include "qemu/coroutine.h"
diff --git a/hw/9pfs/coxattr.c b/hw/9pfs/coxattr.c
index 0e00ffaa0d..dbcd09e0fd 100644
--- a/hw/9pfs/coxattr.c
+++ b/hw/9pfs/coxattr.c
@@ -11,6 +11,11 @@
  *
  */
 
+/*
+ * Not so fast! You might want to read the 9p developer docs first:
+ * https://wiki.qemu.org/Documentation/9p
+ */
+
 #include "qemu/osdep.h"
 #include "fsdev/qemu-fsdev.h"
 #include "qemu/thread.h"
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 14371a78ef..54ee93b71f 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -11,6 +11,11 @@
  *
  */
 
+/*
+ * Not so fast! You might want to read the 9p developer docs first:
+ * https://wiki.qemu.org/Documentation/9p
+ */
+
 #include "qemu/osdep.h"
 #include "hw/virtio/virtio.h"
 #include "qemu/sockets.h"
diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
index a969fcc54c..65c4979c3c 100644
--- a/hw/9pfs/xen-9p-backend.c
+++ b/hw/9pfs/xen-9p-backend.c
@@ -8,6 +8,11 @@
  *
  */
 
+/*
+ * Not so fast! You might want to read the 9p developer docs first:
+ * https://wiki.qemu.org/Documentation/9p
+ */
+
 #include "qemu/osdep.h"
 
 #include "hw/9pfs/9p.h"
diff --git a/tests/qtest/libqos/virtio-9p.c b/tests/qtest/libqos/virtio-9p.c
index be91662c6f..b4e1143288 100644
--- a/tests/qtest/libqos/virtio-9p.c
+++ b/tests/qtest/libqos/virtio-9p.c
@@ -16,6 +16,11 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>
  */
 
+/*
+ * Not so fast! You might want to read the 9p developer docs first:
+ * https://wiki.qemu.org/Documentation/9p
+ */
+
 #include "qemu/osdep.h"
 #include "libqtest.h"
 #include "qemu/module.h"
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 92a498f249..41fed41de1 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -7,6 +7,11 @@
  * See the COPYING file in the top-level directory.
  */
 
+/*
+ * Not so fast! You might want to read the 9p developer docs first:
+ * https://wiki.qemu.org/Documentation/9p
+ */
+
 #include "qemu/osdep.h"
 #include "libqtest-single.h"
 #include "qemu/module.h"
-- 
2.20.1



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

* [PULL 4/8] 9pfs: capture root stat
  2021-07-05 11:13 [PULL 0/8] 9p queue 2021-07-05 Christian Schoenebeck
  2021-07-05 11:13 ` [PULL 8/8] 9pfs: reduce latency of Twalk Christian Schoenebeck
  2021-07-05 11:13 ` [PULL 7/8] 9pfs: drop root_qid Christian Schoenebeck
@ 2021-07-05 11:13 ` Christian Schoenebeck
  2021-07-05 11:13 ` [PULL 2/8] 9pfs: simplify v9fs_walk() Christian Schoenebeck
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Christian Schoenebeck @ 2021-07-05 11:13 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell; +Cc: Greg Kurz

We already capture the QID of the exported 9p root path, i.e. to
prevent client access outside the defined, exported filesystem's tree.
This is currently checked by comparing the root QID with another FID's
QID.

The problem with the latter is that resolving a QID of any given 9p path
can only be done on 9p server's main thread, that's because it might
mutate the server's state if inode remapping is enabled.

For that reason also capture the POSIX stat info of the root path for
being able to identify on any (e.g. worker) thread whether an
arbitrary given path is identical to the export root.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <eb07d6c2e9925788454cfe33d3802e4ffb23ea9a.1622821729.git.qemu_oss@crudebyte.com>
---
 hw/9pfs/9p.c | 10 +++++++++-
 hw/9pfs/9p.h |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index e10a02f71d..eb15ec2082 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1400,6 +1400,7 @@ static void coroutine_fn v9fs_attach(void *opaque)
     size_t offset = 7;
     V9fsQID qid;
     ssize_t err;
+    struct stat stbuf;
 
     v9fs_string_init(&uname);
     v9fs_string_init(&aname);
@@ -1422,7 +1423,13 @@ static void coroutine_fn v9fs_attach(void *opaque)
         clunk_fid(s, fid);
         goto out;
     }
-    err = fid_to_qid(pdu, fidp, &qid);
+    err = v9fs_co_lstat(pdu, &fidp->path, &stbuf);
+    if (err < 0) {
+        err = -EINVAL;
+        clunk_fid(s, fid);
+        goto out;
+    }
+    err = stat_to_qid(pdu, &stbuf, &qid);
     if (err < 0) {
         err = -EINVAL;
         clunk_fid(s, fid);
@@ -1455,6 +1462,7 @@ static void coroutine_fn v9fs_attach(void *opaque)
     err += offset;
 
     memcpy(&s->root_qid, &qid, sizeof(qid));
+    memcpy(&s->root_st, &stbuf, sizeof(stbuf));
     trace_v9fs_attach_return(pdu->tag, pdu->id,
                              qid.type, qid.version, qid.path);
 out:
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 00381591ff..6f0b4c78c0 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -356,6 +356,7 @@ struct V9fsState {
     Error *migration_blocker;
     V9fsConf fsconf;
     V9fsQID root_qid;
+    struct stat root_st;
     dev_t dev_id;
     struct qht qpd_table;
     struct qht qpp_table;
-- 
2.20.1



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

* Re: [PULL 0/8] 9p queue 2021-07-05
  2021-07-05 11:13 [PULL 0/8] 9p queue 2021-07-05 Christian Schoenebeck
                   ` (7 preceding siblings ...)
  2021-07-05 11:13 ` [PULL 6/8] 9pfs: replace not_same_qid() by same_stat_id() Christian Schoenebeck
@ 2021-07-05 13:57 ` Peter Maydell
  2021-07-05 15:34   ` Christian Schoenebeck
  2021-07-05 19:03 ` Peter Maydell
  9 siblings, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2021-07-05 13:57 UTC (permalink / raw)
  To: Christian Schoenebeck; +Cc: QEMU Developers, Greg Kurz

On Mon, 5 Jul 2021 at 12:24, Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
>
> The following changes since commit 711c0418c8c1ce3a24346f058b001c4c5a2f0f81:
>
>   Merge remote-tracking branch 'remotes/philmd/tags/mips-20210702' into staging (2021-07-04 14:04:12 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/cschoenebeck/qemu.git tags/pull-9p-20210705
>
> for you to fetch changes up to 8d6cb100731c4d28535adbf2a3c2d1f29be3fef4:
>
>   9pfs: reduce latency of Twalk (2021-07-05 13:03:16 +0200)
>
> ----------------------------------------------------------------
> 9pfs: misc patches
>
> * Add link to 9p developer docs.
>
> * Fix runtime check whether client supplied relative path is the export
>   root.
>
> * Performance optimization of Twalk requests.
>
> * Code cleanup.

GPG tells me the key you signed this with has expired. Can you point
me at a keyserver I can download an updated version of the key from,
please?

thanks
-- PMM


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

* Re: [PULL 0/8] 9p queue 2021-07-05
  2021-07-05 13:57 ` [PULL 0/8] 9p queue 2021-07-05 Peter Maydell
@ 2021-07-05 15:34   ` Christian Schoenebeck
  0 siblings, 0 replies; 12+ messages in thread
From: Christian Schoenebeck @ 2021-07-05 15:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Greg Kurz

On Montag, 5. Juli 2021 15:57:41 CEST Peter Maydell wrote:
> On Mon, 5 Jul 2021 at 12:24, Christian Schoenebeck
> 
> <qemu_oss@crudebyte.com> wrote:
> > The following changes since commit 711c0418c8c1ce3a24346f058b001c4c5a2f0f81:
> >   Merge remote-tracking branch 'remotes/philmd/tags/mips-20210702' into
> >   staging (2021-07-04 14:04:12 +0100)> 
> > are available in the Git repository at:
> >   https://github.com/cschoenebeck/qemu.git tags/pull-9p-20210705
> > 
> > for you to fetch changes up to 8d6cb100731c4d28535adbf2a3c2d1f29be3fef4:
> >   9pfs: reduce latency of Twalk (2021-07-05 13:03:16 +0200)
> > 
> > ----------------------------------------------------------------
> > 9pfs: misc patches
> > 
> > * Add link to 9p developer docs.
> > 
> > * Fix runtime check whether client supplied relative path is the export
> > 
> >   root.
> > 
> > * Performance optimization of Twalk requests.
> > 
> > * Code cleanup.
> 
> GPG tells me the key you signed this with has expired. Can you point
> me at a keyserver I can download an updated version of the key from,
> please?
> 
> thanks
> -- PMM

Hi Peter,

I must have done something wrong when I sent out the updated, prolonged key 
info couple weeks ago.

I just sent it out again to couple key servers now. They are all still the
same key IDs BTW, I just prolonged their expiration dates. Looks fine now:

https://keyserver.ubuntu.com/pks/lookup?search=qemu_oss%40crudebyte.com&op=vindex&fingerprint=on
http://pgp.mit.edu/pks/lookup?op=vindex&search=0x30DB47C3A012D5F4
https://github.com/cschoenebeck/qemu/releases/tag/pull-9p-20210705

sec#  rsa4096/30DB47C3A012D5F4 2020-05-28 [SC] [expires: 2024-05-16]
      ECAB1A4540141413BA38492630DB47C3A012D5F4
uid                 [ultimate] Christian Schoenebeck <qemu_oss@crudebyte.com>
ssb   rsa4096/C64F2382FC0F4C5E 2020-05-28 [E] [expires: 2022-11-08]
ssb   rsa4096/34C2B58765A47395 2020-05-28 [S] [expires: 2022-11-08]

Best regards,
Christian Schoenebeck




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

* Re: [PULL 0/8] 9p queue 2021-07-05
  2021-07-05 11:13 [PULL 0/8] 9p queue 2021-07-05 Christian Schoenebeck
                   ` (8 preceding siblings ...)
  2021-07-05 13:57 ` [PULL 0/8] 9p queue 2021-07-05 Peter Maydell
@ 2021-07-05 19:03 ` Peter Maydell
  9 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2021-07-05 19:03 UTC (permalink / raw)
  To: Christian Schoenebeck; +Cc: QEMU Developers, Greg Kurz

On Mon, 5 Jul 2021 at 12:24, Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
>
> The following changes since commit 711c0418c8c1ce3a24346f058b001c4c5a2f0f81:
>
>   Merge remote-tracking branch 'remotes/philmd/tags/mips-20210702' into staging (2021-07-04 14:04:12 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/cschoenebeck/qemu.git tags/pull-9p-20210705
>
> for you to fetch changes up to 8d6cb100731c4d28535adbf2a3c2d1f29be3fef4:
>
>   9pfs: reduce latency of Twalk (2021-07-05 13:03:16 +0200)
>
> ----------------------------------------------------------------
> 9pfs: misc patches
>
> * Add link to 9p developer docs.
>
> * Fix runtime check whether client supplied relative path is the export
>   root.
>
> * Performance optimization of Twalk requests.
>
> * Code cleanup.


Applied, thanks.

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

-- PMM


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

end of thread, other threads:[~2021-07-05 19:05 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-05 11:13 [PULL 0/8] 9p queue 2021-07-05 Christian Schoenebeck
2021-07-05 11:13 ` [PULL 8/8] 9pfs: reduce latency of Twalk Christian Schoenebeck
2021-07-05 11:13 ` [PULL 7/8] 9pfs: drop root_qid Christian Schoenebeck
2021-07-05 11:13 ` [PULL 4/8] 9pfs: capture root stat Christian Schoenebeck
2021-07-05 11:13 ` [PULL 2/8] 9pfs: simplify v9fs_walk() Christian Schoenebeck
2021-07-05 11:13 ` [PULL 5/8] 9pfs: drop fid_to_qid() Christian Schoenebeck
2021-07-05 11:13 ` [PULL 3/8] 9pfs: fix not_same_qid() Christian Schoenebeck
2021-07-05 11:13 ` [PULL 1/8] 9pfs: add link to 9p developer docs Christian Schoenebeck
2021-07-05 11:13 ` [PULL 6/8] 9pfs: replace not_same_qid() by same_stat_id() Christian Schoenebeck
2021-07-05 13:57 ` [PULL 0/8] 9p queue 2021-07-05 Peter Maydell
2021-07-05 15:34   ` Christian Schoenebeck
2021-07-05 19:03 ` Peter Maydell

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.