| Message ID | f19d6f5fd2b569ebac797f18849710eb22c40984.1646850707.git.qemu_oss@crudebyte.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | 9pfs: fix 'Twalk' protocol violation | expand |
On Mittwoch, 9. März 2022 14:24:24 CET Christian Schoenebeck wrote: > Send Twalk request with nwname=0. In this case no QIDs should > be returned by 9p server; this is equivalent to walking to dot. > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > --- > tests/qtest/virtio-9p-test.c | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c > index 22bdd74bc1..6c00da03f4 100644 > --- a/tests/qtest/virtio-9p-test.c > +++ b/tests/qtest/virtio-9p-test.c > @@ -1002,6 +1002,27 @@ static void fs_walk_nonexistent(void *obj, void > *data, QGuestAllocator *t_alloc) do_walk_expect_error(v9p, "non-existent", > ENOENT); > } > > +static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) > +{ Or maybe calling this function fs_walk_clone_fid and the test case name "synth/walk/clone_fid" respectively instead? > + QVirtio9P *v9p = obj; > + alloc = t_alloc; > + v9fs_qid root_qid; > + g_autofree v9fs_qid *wqid = NULL; > + P9Req *req; > + > + do_version(v9p); > + req = v9fs_tattach(v9p, 0, getuid(), 0); > + v9fs_req_wait_for_reply(req, NULL); > + v9fs_rattach(req, &root_qid); > + > + req = v9fs_twalk(v9p, 0, 1, 0, NULL, 0); > + v9fs_req_wait_for_reply(req, NULL); > + v9fs_rwalk(req, NULL, &wqid); > + > + /* special case: no QID is returned if nwname=0 was sent */ > + g_assert(wqid == NULL); > +} > + > static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc) > { > QVirtio9P *v9p = obj; > @@ -1435,6 +1456,7 @@ static void register_virtio_9p_test(void) > qos_add_test("synth/walk/basic", "virtio-9p", fs_walk, &opts); > qos_add_test("synth/walk/no_slash", "virtio-9p", fs_walk_no_slash, > &opts); > + qos_add_test("synth/walk/none", "virtio-9p", fs_walk_none, &opts); > qos_add_test("synth/walk/dotdot_from_root", "virtio-9p", > fs_walk_dotdot, &opts); > qos_add_test("synth/walk/non_existent", "virtio-9p", fs_walk_nonexistent,
On Thu, 10 Mar 2022 09:57:25 +0100 Christian Schoenebeck <qemu_oss@crudebyte.com> wrote: > On Mittwoch, 9. März 2022 14:24:24 CET Christian Schoenebeck wrote: > > Send Twalk request with nwname=0. In this case no QIDs should > > be returned by 9p server; this is equivalent to walking to dot. > > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > > --- > > tests/qtest/virtio-9p-test.c | 22 ++++++++++++++++++++++ > > 1 file changed, 22 insertions(+) > > > > diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c > > index 22bdd74bc1..6c00da03f4 100644 > > --- a/tests/qtest/virtio-9p-test.c > > +++ b/tests/qtest/virtio-9p-test.c > > @@ -1002,6 +1002,27 @@ static void fs_walk_nonexistent(void *obj, void > > *data, QGuestAllocator *t_alloc) do_walk_expect_error(v9p, "non-existent", > > ENOENT); > > } > > > > +static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) > > +{ > > Or maybe calling this function fs_walk_clone_fid and the test case name > "synth/walk/clone_fid" respectively instead? > I agree that Twalk with nwname=0 does clone the fid in practice but the test doesn't explicitly check that. In its present form, I'd suggest a "no_names" wording but it is already fine as is, so: Reviewed-by: Greg Kurz <groug@kaod.org> > > + QVirtio9P *v9p = obj; > > + alloc = t_alloc; > > + v9fs_qid root_qid; > > + g_autofree v9fs_qid *wqid = NULL; > > + P9Req *req; > > + > > + do_version(v9p); > > + req = v9fs_tattach(v9p, 0, getuid(), 0); > > + v9fs_req_wait_for_reply(req, NULL); > > + v9fs_rattach(req, &root_qid); > > + > > + req = v9fs_twalk(v9p, 0, 1, 0, NULL, 0); > > + v9fs_req_wait_for_reply(req, NULL); > > + v9fs_rwalk(req, NULL, &wqid); > > + > > + /* special case: no QID is returned if nwname=0 was sent */ > > + g_assert(wqid == NULL); > > +} > > + > > static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc) > > { > > QVirtio9P *v9p = obj; > > @@ -1435,6 +1456,7 @@ static void register_virtio_9p_test(void) > > qos_add_test("synth/walk/basic", "virtio-9p", fs_walk, &opts); > > qos_add_test("synth/walk/no_slash", "virtio-9p", fs_walk_no_slash, > > &opts); > > + qos_add_test("synth/walk/none", "virtio-9p", fs_walk_none, &opts); > > qos_add_test("synth/walk/dotdot_from_root", "virtio-9p", > > fs_walk_dotdot, &opts); > > qos_add_test("synth/walk/non_existent", "virtio-9p", fs_walk_nonexistent, > >
On Freitag, 11. März 2022 12:41:32 CET Greg Kurz wrote: > On Thu, 10 Mar 2022 09:57:25 +0100 > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote: > > On Mittwoch, 9. März 2022 14:24:24 CET Christian Schoenebeck wrote: > > > Send Twalk request with nwname=0. In this case no QIDs should > > > be returned by 9p server; this is equivalent to walking to dot. > > > > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > > > --- > > > > > > tests/qtest/virtio-9p-test.c | 22 ++++++++++++++++++++++ > > > 1 file changed, 22 insertions(+) > > > > > > diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c > > > index 22bdd74bc1..6c00da03f4 100644 > > > --- a/tests/qtest/virtio-9p-test.c > > > +++ b/tests/qtest/virtio-9p-test.c > > > @@ -1002,6 +1002,27 @@ static void fs_walk_nonexistent(void *obj, void > > > *data, QGuestAllocator *t_alloc) do_walk_expect_error(v9p, > > > "non-existent", > > > ENOENT); > > > > > > } > > > > > > +static void fs_walk_none(void *obj, void *data, QGuestAllocator > > > *t_alloc) > > > +{ > > > > Or maybe calling this function fs_walk_clone_fid and the test case name > > "synth/walk/clone_fid" respectively instead? > > I agree that Twalk with nwname=0 does clone the fid in practice but > the test doesn't explicitly check that. In its present form, I'd > suggest a "no_names" wording but it is already fine as is, so: It actually does; not with this patch 2 yet, but with patch 3 (which compares QIDs). > Reviewed-by: Greg Kurz <groug@kaod.org> Thanks! Best regards, Christian Schoenebeck
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 22bdd74bc1..6c00da03f4 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -1002,6 +1002,27 @@ static void fs_walk_nonexistent(void *obj, void *data, QGuestAllocator *t_alloc) do_walk_expect_error(v9p, "non-existent", ENOENT); } +static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) +{ + QVirtio9P *v9p = obj; + alloc = t_alloc; + v9fs_qid root_qid; + g_autofree v9fs_qid *wqid = NULL; + P9Req *req; + + do_version(v9p); + req = v9fs_tattach(v9p, 0, getuid(), 0); + v9fs_req_wait_for_reply(req, NULL); + v9fs_rattach(req, &root_qid); + + req = v9fs_twalk(v9p, 0, 1, 0, NULL, 0); + v9fs_req_wait_for_reply(req, NULL); + v9fs_rwalk(req, NULL, &wqid); + + /* special case: no QID is returned if nwname=0 was sent */ + g_assert(wqid == NULL); +} + static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; @@ -1435,6 +1456,7 @@ static void register_virtio_9p_test(void) qos_add_test("synth/walk/basic", "virtio-9p", fs_walk, &opts); qos_add_test("synth/walk/no_slash", "virtio-9p", fs_walk_no_slash, &opts); + qos_add_test("synth/walk/none", "virtio-9p", fs_walk_none, &opts); qos_add_test("synth/walk/dotdot_from_root", "virtio-9p", fs_walk_dotdot, &opts); qos_add_test("synth/walk/non_existent", "virtio-9p", fs_walk_nonexistent,
Send Twalk request with nwname=0. In this case no QIDs should be returned by 9p server; this is equivalent to walking to dot. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> --- tests/qtest/virtio-9p-test.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)