From f723f626627fda681327075105701695d7c630e5 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Tue, 27 Sep 2022 19:06:04 +0800 Subject: [PATCH 01/23] fsdev/virtfs-proxy-helper: Use g_mkdir() Use g_mkdir() to create a directory on all platforms. Signed-off-by: Bin Meng Reviewed-by: Christian Schoenebeck Message-Id: <20220927110632.1973965-27-bmeng.cn@gmail.com> Signed-off-by: Christian Schoenebeck --- fsdev/virtfs-proxy-helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c index 2dde27922f..5cafcd7703 100644 --- a/fsdev/virtfs-proxy-helper.c +++ b/fsdev/virtfs-proxy-helper.c @@ -10,6 +10,7 @@ */ #include "qemu/osdep.h" +#include #include #include #include @@ -639,7 +640,7 @@ static int do_create_others(int type, struct iovec *iovec) if (retval < 0) { goto err_out; } - retval = mkdir(path.data, mode); + retval = g_mkdir(path.data, mode); break; case T_SYMLINK: retval = proxy_unmarshal(iovec, offset, "ss", &oldpath, &path); From 684f912034395a4958600a3ccca972db5d31be94 Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Thu, 29 Sep 2022 13:41:06 +0200 Subject: [PATCH 02/23] tests/9p: split virtio-9p-test.c into tests and 9p client part This patch is pure refactoring, it does not change behaviour. virtio-9p-test.c grew to 1657 lines. Let's split this file up between actual 9p test cases vs. 9p test client, to make it easier to concentrate on the actual 9p tests. Move the 9p test client code to a new unit virtio-9p-client.c, which are basically all functions and types prefixed with v9fs_* already. Note that some client wrapper functions (do_*) are preserved in virtio-9p-test.c, simply because these wrapper functions are going to be wiped with subsequent patches anyway. As the global QGuestAllocator variable is moved to virtio-9p-client.c, add a new function v9fs_set_allocator() to be used by virtio-9p-test.c instead of fiddling with a global variable across units and libraries. Signed-off-by: Christian Schoenebeck Reviewed-by: Greg Kurz Message-Id: --- tests/qtest/libqos/meson.build | 1 + tests/qtest/libqos/virtio-9p-client.c | 684 +++++++++++++++++++++++ tests/qtest/libqos/virtio-9p-client.h | 138 +++++ tests/qtest/virtio-9p-test.c | 770 +------------------------- 4 files changed, 849 insertions(+), 744 deletions(-) create mode 100644 tests/qtest/libqos/virtio-9p-client.c create mode 100644 tests/qtest/libqos/virtio-9p-client.h diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build index a5b6d5197a..113c80b4e4 100644 --- a/tests/qtest/libqos/meson.build +++ b/tests/qtest/libqos/meson.build @@ -34,6 +34,7 @@ libqos_srcs = files( 'tpci200.c', 'virtio.c', 'virtio-9p.c', + 'virtio-9p-client.c', 'virtio-balloon.c', 'virtio-blk.c', 'vhost-user-blk.c', diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c new file mode 100644 index 0000000000..f5c35fd722 --- /dev/null +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -0,0 +1,684 @@ +/* + * 9P network client for VirtIO 9P test cases (based on QTest) + * + * Copyright (c) 2014 SUSE LINUX Products GmbH + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * 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 "virtio-9p-client.h" + +#define QVIRTIO_9P_TIMEOUT_US (10 * 1000 * 1000) +static QGuestAllocator *alloc; + +void v9fs_set_allocator(QGuestAllocator *t_alloc) +{ + alloc = t_alloc; +} + +void v9fs_memwrite(P9Req *req, const void *addr, size_t len) +{ + qtest_memwrite(req->qts, req->t_msg + req->t_off, addr, len); + req->t_off += len; +} + +void v9fs_memskip(P9Req *req, size_t len) +{ + req->r_off += len; +} + +void v9fs_memread(P9Req *req, void *addr, size_t len) +{ + qtest_memread(req->qts, req->r_msg + req->r_off, addr, len); + req->r_off += len; +} + +void v9fs_uint8_read(P9Req *req, uint8_t *val) +{ + v9fs_memread(req, val, 1); +} + +void v9fs_uint16_write(P9Req *req, uint16_t val) +{ + uint16_t le_val = cpu_to_le16(val); + + v9fs_memwrite(req, &le_val, 2); +} + +void v9fs_uint16_read(P9Req *req, uint16_t *val) +{ + v9fs_memread(req, val, 2); + le16_to_cpus(val); +} + +void v9fs_uint32_write(P9Req *req, uint32_t val) +{ + uint32_t le_val = cpu_to_le32(val); + + v9fs_memwrite(req, &le_val, 4); +} + +void v9fs_uint64_write(P9Req *req, uint64_t val) +{ + uint64_t le_val = cpu_to_le64(val); + + v9fs_memwrite(req, &le_val, 8); +} + +void v9fs_uint32_read(P9Req *req, uint32_t *val) +{ + v9fs_memread(req, val, 4); + le32_to_cpus(val); +} + +void v9fs_uint64_read(P9Req *req, uint64_t *val) +{ + v9fs_memread(req, val, 8); + le64_to_cpus(val); +} + +/* len[2] string[len] */ +uint16_t v9fs_string_size(const char *string) +{ + size_t len = strlen(string); + + g_assert_cmpint(len, <=, UINT16_MAX - 2); + + return 2 + len; +} + +void v9fs_string_write(P9Req *req, const char *string) +{ + int len = strlen(string); + + g_assert_cmpint(len, <=, UINT16_MAX); + + v9fs_uint16_write(req, (uint16_t) len); + v9fs_memwrite(req, string, len); +} + +void v9fs_string_read(P9Req *req, uint16_t *len, char **string) +{ + uint16_t local_len; + + v9fs_uint16_read(req, &local_len); + if (len) { + *len = local_len; + } + if (string) { + *string = g_malloc(local_len + 1); + v9fs_memread(req, *string, local_len); + (*string)[local_len] = 0; + } else { + v9fs_memskip(req, local_len); + } +} + +typedef struct { + uint32_t size; + uint8_t id; + uint16_t tag; +} QEMU_PACKED P9Hdr; + +P9Req *v9fs_req_init(QVirtio9P *v9p, uint32_t size, uint8_t id, + uint16_t tag) +{ + P9Req *req = g_new0(P9Req, 1); + uint32_t total_size = 7; /* 9P header has well-known size of 7 bytes */ + P9Hdr hdr = { + .id = id, + .tag = cpu_to_le16(tag) + }; + + g_assert_cmpint(total_size, <=, UINT32_MAX - size); + total_size += size; + hdr.size = cpu_to_le32(total_size); + + g_assert_cmpint(total_size, <=, P9_MAX_SIZE); + + req->qts = global_qtest; + req->v9p = v9p; + req->t_size = total_size; + req->t_msg = guest_alloc(alloc, req->t_size); + v9fs_memwrite(req, &hdr, 7); + req->tag = tag; + return req; +} + +void v9fs_req_send(P9Req *req) +{ + QVirtio9P *v9p = req->v9p; + + req->r_msg = guest_alloc(alloc, P9_MAX_SIZE); + req->free_head = qvirtqueue_add(req->qts, v9p->vq, req->t_msg, req->t_size, + false, true); + qvirtqueue_add(req->qts, v9p->vq, req->r_msg, P9_MAX_SIZE, true, false); + qvirtqueue_kick(req->qts, v9p->vdev, v9p->vq, req->free_head); + req->t_off = 0; +} + +static const char *rmessage_name(uint8_t id) +{ + return + id == P9_RLERROR ? "RLERROR" : + id == P9_RVERSION ? "RVERSION" : + id == P9_RATTACH ? "RATTACH" : + id == P9_RWALK ? "RWALK" : + id == P9_RLOPEN ? "RLOPEN" : + id == P9_RWRITE ? "RWRITE" : + id == P9_RMKDIR ? "RMKDIR" : + id == P9_RLCREATE ? "RLCREATE" : + id == P9_RSYMLINK ? "RSYMLINK" : + id == P9_RLINK ? "RLINK" : + id == P9_RUNLINKAT ? "RUNLINKAT" : + id == P9_RFLUSH ? "RFLUSH" : + id == P9_RREADDIR ? "READDIR" : + ""; +} + +void v9fs_req_wait_for_reply(P9Req *req, uint32_t *len) +{ + QVirtio9P *v9p = req->v9p; + + qvirtio_wait_used_elem(req->qts, v9p->vdev, v9p->vq, req->free_head, len, + QVIRTIO_9P_TIMEOUT_US); +} + +void v9fs_req_recv(P9Req *req, uint8_t id) +{ + P9Hdr hdr; + + v9fs_memread(req, &hdr, 7); + hdr.size = ldl_le_p(&hdr.size); + hdr.tag = lduw_le_p(&hdr.tag); + + g_assert_cmpint(hdr.size, >=, 7); + g_assert_cmpint(hdr.size, <=, P9_MAX_SIZE); + g_assert_cmpint(hdr.tag, ==, req->tag); + + if (hdr.id != id) { + g_printerr("Received response %d (%s) instead of %d (%s)\n", + hdr.id, rmessage_name(hdr.id), id, rmessage_name(id)); + + if (hdr.id == P9_RLERROR) { + uint32_t err; + v9fs_uint32_read(req, &err); + g_printerr("Rlerror has errno %d (%s)\n", err, strerror(err)); + } + } + g_assert_cmpint(hdr.id, ==, id); +} + +void v9fs_req_free(P9Req *req) +{ + guest_free(alloc, req->t_msg); + guest_free(alloc, req->r_msg); + g_free(req); +} + +/* size[4] Rlerror tag[2] ecode[4] */ +void v9fs_rlerror(P9Req *req, uint32_t *err) +{ + v9fs_req_recv(req, P9_RLERROR); + v9fs_uint32_read(req, err); + v9fs_req_free(req); +} + +/* size[4] Tversion tag[2] msize[4] version[s] */ +P9Req *v9fs_tversion(QVirtio9P *v9p, uint32_t msize, const char *version, + uint16_t tag) +{ + P9Req *req; + uint32_t body_size = 4; + uint16_t string_size = v9fs_string_size(version); + + g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); + body_size += string_size; + req = v9fs_req_init(v9p, body_size, P9_TVERSION, tag); + + v9fs_uint32_write(req, msize); + v9fs_string_write(req, version); + v9fs_req_send(req); + return req; +} + +/* size[4] Rversion tag[2] msize[4] version[s] */ +void v9fs_rversion(P9Req *req, uint16_t *len, char **version) +{ + uint32_t msize; + + v9fs_req_recv(req, P9_RVERSION); + v9fs_uint32_read(req, &msize); + + g_assert_cmpint(msize, ==, P9_MAX_SIZE); + + if (len || version) { + v9fs_string_read(req, len, version); + } + + v9fs_req_free(req); +} + +/* size[4] Tattach tag[2] fid[4] afid[4] uname[s] aname[s] n_uname[4] */ +P9Req *v9fs_tattach(QVirtio9P *v9p, uint32_t fid, uint32_t n_uname, + uint16_t tag) +{ + const char *uname = ""; /* ignored by QEMU */ + const char *aname = ""; /* ignored by QEMU */ + P9Req *req = v9fs_req_init(v9p, 4 + 4 + 2 + 2 + 4, P9_TATTACH, tag); + + v9fs_uint32_write(req, fid); + v9fs_uint32_write(req, P9_NOFID); + v9fs_string_write(req, uname); + v9fs_string_write(req, aname); + v9fs_uint32_write(req, n_uname); + v9fs_req_send(req); + return req; +} + +/* size[4] Rattach tag[2] qid[13] */ +void v9fs_rattach(P9Req *req, v9fs_qid *qid) +{ + v9fs_req_recv(req, P9_RATTACH); + if (qid) { + v9fs_memread(req, qid, 13); + } + v9fs_req_free(req); +} + +/* size[4] Twalk tag[2] fid[4] newfid[4] nwname[2] nwname*(wname[s]) */ +P9Req *v9fs_twalk(QVirtio9P *v9p, uint32_t fid, uint32_t newfid, + uint16_t nwname, char *const wnames[], uint16_t tag) +{ + P9Req *req; + int i; + uint32_t body_size = 4 + 4 + 2; + + for (i = 0; i < nwname; i++) { + uint16_t wname_size = v9fs_string_size(wnames[i]); + + g_assert_cmpint(body_size, <=, UINT32_MAX - wname_size); + body_size += wname_size; + } + req = v9fs_req_init(v9p, body_size, P9_TWALK, tag); + v9fs_uint32_write(req, fid); + v9fs_uint32_write(req, newfid); + v9fs_uint16_write(req, nwname); + for (i = 0; i < nwname; i++) { + v9fs_string_write(req, wnames[i]); + } + v9fs_req_send(req); + return req; +} + +/* size[4] Rwalk tag[2] nwqid[2] nwqid*(wqid[13]) */ +void v9fs_rwalk(P9Req *req, uint16_t *nwqid, v9fs_qid **wqid) +{ + uint16_t local_nwqid; + + v9fs_req_recv(req, P9_RWALK); + v9fs_uint16_read(req, &local_nwqid); + if (nwqid) { + *nwqid = local_nwqid; + } + if (wqid) { + *wqid = g_malloc(local_nwqid * 13); + v9fs_memread(req, *wqid, local_nwqid * 13); + } + v9fs_req_free(req); +} + +/* size[4] Tgetattr tag[2] fid[4] request_mask[8] */ +P9Req *v9fs_tgetattr(QVirtio9P *v9p, uint32_t fid, uint64_t request_mask, + uint16_t tag) +{ + P9Req *req; + + req = v9fs_req_init(v9p, 4 + 8, P9_TGETATTR, tag); + v9fs_uint32_write(req, fid); + v9fs_uint64_write(req, request_mask); + v9fs_req_send(req); + return req; +} + +/* + * size[4] Rgetattr tag[2] valid[8] qid[13] mode[4] uid[4] gid[4] nlink[8] + * rdev[8] size[8] blksize[8] blocks[8] + * atime_sec[8] atime_nsec[8] mtime_sec[8] mtime_nsec[8] + * ctime_sec[8] ctime_nsec[8] btime_sec[8] btime_nsec[8] + * gen[8] data_version[8] + */ +void v9fs_rgetattr(P9Req *req, v9fs_attr *attr) +{ + v9fs_req_recv(req, P9_RGETATTR); + + v9fs_uint64_read(req, &attr->valid); + v9fs_memread(req, &attr->qid, 13); + v9fs_uint32_read(req, &attr->mode); + v9fs_uint32_read(req, &attr->uid); + v9fs_uint32_read(req, &attr->gid); + v9fs_uint64_read(req, &attr->nlink); + v9fs_uint64_read(req, &attr->rdev); + v9fs_uint64_read(req, &attr->size); + v9fs_uint64_read(req, &attr->blksize); + v9fs_uint64_read(req, &attr->blocks); + v9fs_uint64_read(req, &attr->atime_sec); + v9fs_uint64_read(req, &attr->atime_nsec); + v9fs_uint64_read(req, &attr->mtime_sec); + v9fs_uint64_read(req, &attr->mtime_nsec); + v9fs_uint64_read(req, &attr->ctime_sec); + v9fs_uint64_read(req, &attr->ctime_nsec); + v9fs_uint64_read(req, &attr->btime_sec); + v9fs_uint64_read(req, &attr->btime_nsec); + v9fs_uint64_read(req, &attr->gen); + v9fs_uint64_read(req, &attr->data_version); + + v9fs_req_free(req); +} + +/* size[4] Treaddir tag[2] fid[4] offset[8] count[4] */ +P9Req *v9fs_treaddir(QVirtio9P *v9p, uint32_t fid, uint64_t offset, + uint32_t count, uint16_t tag) +{ + P9Req *req; + + req = v9fs_req_init(v9p, 4 + 8 + 4, P9_TREADDIR, tag); + v9fs_uint32_write(req, fid); + v9fs_uint64_write(req, offset); + v9fs_uint32_write(req, count); + v9fs_req_send(req); + return req; +} + +/* size[4] Rreaddir tag[2] count[4] data[count] */ +void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t *nentries, + struct V9fsDirent **entries) +{ + uint32_t local_count; + struct V9fsDirent *e = NULL; + uint16_t slen; + uint32_t n = 0; + + v9fs_req_recv(req, P9_RREADDIR); + v9fs_uint32_read(req, &local_count); + + if (count) { + *count = local_count; + } + + for (int32_t togo = (int32_t)local_count; + togo >= 13 + 8 + 1 + 2; + togo -= 13 + 8 + 1 + 2 + slen, ++n) + { + if (!e) { + e = g_new(struct V9fsDirent, 1); + if (entries) { + *entries = e; + } + } else { + e = e->next = g_new(struct V9fsDirent, 1); + } + e->next = NULL; + /* qid[13] offset[8] type[1] name[s] */ + v9fs_memread(req, &e->qid, 13); + v9fs_uint64_read(req, &e->offset); + v9fs_uint8_read(req, &e->type); + v9fs_string_read(req, &slen, &e->name); + } + + if (nentries) { + *nentries = n; + } + + v9fs_req_free(req); +} + +void v9fs_free_dirents(struct V9fsDirent *e) +{ + struct V9fsDirent *next = NULL; + + for (; e; e = next) { + next = e->next; + g_free(e->name); + g_free(e); + } +} + +/* size[4] Tlopen tag[2] fid[4] flags[4] */ +P9Req *v9fs_tlopen(QVirtio9P *v9p, uint32_t fid, uint32_t flags, + uint16_t tag) +{ + P9Req *req; + + req = v9fs_req_init(v9p, 4 + 4, P9_TLOPEN, tag); + v9fs_uint32_write(req, fid); + v9fs_uint32_write(req, flags); + v9fs_req_send(req); + return req; +} + +/* size[4] Rlopen tag[2] qid[13] iounit[4] */ +void v9fs_rlopen(P9Req *req, v9fs_qid *qid, uint32_t *iounit) +{ + v9fs_req_recv(req, P9_RLOPEN); + if (qid) { + v9fs_memread(req, qid, 13); + } else { + v9fs_memskip(req, 13); + } + if (iounit) { + v9fs_uint32_read(req, iounit); + } + v9fs_req_free(req); +} + +/* size[4] Twrite tag[2] fid[4] offset[8] count[4] data[count] */ +P9Req *v9fs_twrite(QVirtio9P *v9p, uint32_t fid, uint64_t offset, + uint32_t count, const void *data, uint16_t tag) +{ + P9Req *req; + uint32_t body_size = 4 + 8 + 4; + + g_assert_cmpint(body_size, <=, UINT32_MAX - count); + body_size += count; + req = v9fs_req_init(v9p, body_size, P9_TWRITE, tag); + v9fs_uint32_write(req, fid); + v9fs_uint64_write(req, offset); + v9fs_uint32_write(req, count); + v9fs_memwrite(req, data, count); + v9fs_req_send(req); + return req; +} + +/* size[4] Rwrite tag[2] count[4] */ +void v9fs_rwrite(P9Req *req, uint32_t *count) +{ + v9fs_req_recv(req, P9_RWRITE); + if (count) { + v9fs_uint32_read(req, count); + } + v9fs_req_free(req); +} + +/* size[4] Tflush tag[2] oldtag[2] */ +P9Req *v9fs_tflush(QVirtio9P *v9p, uint16_t oldtag, uint16_t tag) +{ + P9Req *req; + + req = v9fs_req_init(v9p, 2, P9_TFLUSH, tag); + v9fs_uint32_write(req, oldtag); + v9fs_req_send(req); + return req; +} + +/* size[4] Rflush tag[2] */ +void v9fs_rflush(P9Req *req) +{ + v9fs_req_recv(req, P9_RFLUSH); + v9fs_req_free(req); +} + +/* size[4] Tmkdir tag[2] dfid[4] name[s] mode[4] gid[4] */ +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] */ +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); +} + +/* size[4] Tlcreate tag[2] fid[4] name[s] flags[4] mode[4] gid[4] */ +P9Req *v9fs_tlcreate(QVirtio9P *v9p, uint32_t fid, const char *name, + uint32_t flags, uint32_t mode, uint32_t gid, + uint16_t tag) +{ + P9Req *req; + + uint32_t body_size = 4 + 4 + 4 + 4; + uint16_t string_size = v9fs_string_size(name); + + g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); + body_size += string_size; + + req = v9fs_req_init(v9p, body_size, P9_TLCREATE, tag); + v9fs_uint32_write(req, fid); + v9fs_string_write(req, name); + v9fs_uint32_write(req, flags); + v9fs_uint32_write(req, mode); + v9fs_uint32_write(req, gid); + v9fs_req_send(req); + return req; +} + +/* size[4] Rlcreate tag[2] qid[13] iounit[4] */ +void v9fs_rlcreate(P9Req *req, v9fs_qid *qid, uint32_t *iounit) +{ + v9fs_req_recv(req, P9_RLCREATE); + if (qid) { + v9fs_memread(req, qid, 13); + } else { + v9fs_memskip(req, 13); + } + if (iounit) { + v9fs_uint32_read(req, iounit); + } + v9fs_req_free(req); +} + +/* size[4] Tsymlink tag[2] fid[4] name[s] symtgt[s] gid[4] */ +P9Req *v9fs_tsymlink(QVirtio9P *v9p, uint32_t fid, const char *name, + const char *symtgt, uint32_t gid, uint16_t tag) +{ + P9Req *req; + + uint32_t body_size = 4 + 4; + uint16_t string_size = v9fs_string_size(name) + v9fs_string_size(symtgt); + + g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); + body_size += string_size; + + req = v9fs_req_init(v9p, body_size, P9_TSYMLINK, tag); + v9fs_uint32_write(req, fid); + v9fs_string_write(req, name); + v9fs_string_write(req, symtgt); + v9fs_uint32_write(req, gid); + v9fs_req_send(req); + return req; +} + +/* size[4] Rsymlink tag[2] qid[13] */ +void v9fs_rsymlink(P9Req *req, v9fs_qid *qid) +{ + v9fs_req_recv(req, P9_RSYMLINK); + if (qid) { + v9fs_memread(req, qid, 13); + } else { + v9fs_memskip(req, 13); + } + v9fs_req_free(req); +} + +/* size[4] Tlink tag[2] dfid[4] fid[4] name[s] */ +P9Req *v9fs_tlink(QVirtio9P *v9p, uint32_t dfid, uint32_t fid, + const char *name, uint16_t tag) +{ + P9Req *req; + + uint32_t body_size = 4 + 4; + uint16_t string_size = v9fs_string_size(name); + + g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); + body_size += string_size; + + req = v9fs_req_init(v9p, body_size, P9_TLINK, tag); + v9fs_uint32_write(req, dfid); + v9fs_uint32_write(req, fid); + v9fs_string_write(req, name); + v9fs_req_send(req); + return req; +} + +/* size[4] Rlink tag[2] */ +void v9fs_rlink(P9Req *req) +{ + v9fs_req_recv(req, P9_RLINK); + v9fs_req_free(req); +} + +/* size[4] Tunlinkat tag[2] dirfd[4] name[s] flags[4] */ +P9Req *v9fs_tunlinkat(QVirtio9P *v9p, uint32_t dirfd, const char *name, + uint32_t flags, uint16_t tag) +{ + P9Req *req; + + uint32_t body_size = 4 + 4; + uint16_t string_size = v9fs_string_size(name); + + g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); + body_size += string_size; + + req = v9fs_req_init(v9p, body_size, P9_TUNLINKAT, tag); + v9fs_uint32_write(req, dirfd); + v9fs_string_write(req, name); + v9fs_uint32_write(req, flags); + v9fs_req_send(req); + return req; +} + +/* size[4] Runlinkat tag[2] */ +void v9fs_runlinkat(P9Req *req) +{ + v9fs_req_recv(req, P9_RUNLINKAT); + v9fs_req_free(req); +} diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h new file mode 100644 index 0000000000..c502d12a66 --- /dev/null +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -0,0 +1,138 @@ +/* + * 9P network client for VirtIO 9P test cases (based on QTest) + * + * Copyright (c) 2014 SUSE LINUX Products GmbH + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * 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 + */ + +#ifndef TESTS_LIBQOS_VIRTIO_9P_CLIENT_H +#define TESTS_LIBQOS_VIRTIO_9P_CLIENT_H + +#include "hw/9pfs/9p.h" +#include "hw/9pfs/9p-synth.h" +#include "virtio-9p.h" +#include "qgraph.h" +#include "tests/qtest/libqtest-single.h" + +#define P9_MAX_SIZE 4096 /* Max size of a T-message or R-message */ + +typedef struct { + QTestState *qts; + QVirtio9P *v9p; + uint16_t tag; + uint64_t t_msg; + uint32_t t_size; + uint64_t r_msg; + /* No r_size, it is hardcoded to P9_MAX_SIZE */ + size_t t_off; + size_t r_off; + uint32_t free_head; +} P9Req; + +/* type[1] version[4] path[8] */ +typedef char v9fs_qid[13]; + +typedef struct v9fs_attr { + uint64_t valid; + v9fs_qid qid; + uint32_t mode; + uint32_t uid; + uint32_t gid; + uint64_t nlink; + uint64_t rdev; + uint64_t size; + uint64_t blksize; + uint64_t blocks; + uint64_t atime_sec; + uint64_t atime_nsec; + uint64_t mtime_sec; + uint64_t mtime_nsec; + uint64_t ctime_sec; + uint64_t ctime_nsec; + uint64_t btime_sec; + uint64_t btime_nsec; + uint64_t gen; + uint64_t data_version; +} v9fs_attr; + +#define P9_GETATTR_BASIC 0x000007ffULL /* Mask for fields up to BLOCKS */ + +struct V9fsDirent { + v9fs_qid qid; + uint64_t offset; + uint8_t type; + char *name; + struct V9fsDirent *next; +}; + +void v9fs_set_allocator(QGuestAllocator *t_alloc); +void v9fs_memwrite(P9Req *req, const void *addr, size_t len); +void v9fs_memskip(P9Req *req, size_t len); +void v9fs_memread(P9Req *req, void *addr, size_t len); +void v9fs_uint8_read(P9Req *req, uint8_t *val); +void v9fs_uint16_write(P9Req *req, uint16_t val); +void v9fs_uint16_read(P9Req *req, uint16_t *val); +void v9fs_uint32_write(P9Req *req, uint32_t val); +void v9fs_uint64_write(P9Req *req, uint64_t val); +void v9fs_uint32_read(P9Req *req, uint32_t *val); +void v9fs_uint64_read(P9Req *req, uint64_t *val); +uint16_t v9fs_string_size(const char *string); +void v9fs_string_write(P9Req *req, const char *string); +void v9fs_string_read(P9Req *req, uint16_t *len, char **string); +P9Req *v9fs_req_init(QVirtio9P *v9p, uint32_t size, uint8_t id, + uint16_t tag); +void v9fs_req_send(P9Req *req); +void v9fs_req_wait_for_reply(P9Req *req, uint32_t *len); +void v9fs_req_recv(P9Req *req, uint8_t id); +void v9fs_req_free(P9Req *req); +void v9fs_rlerror(P9Req *req, uint32_t *err); +P9Req *v9fs_tversion(QVirtio9P *v9p, uint32_t msize, const char *version, + uint16_t tag); +void v9fs_rversion(P9Req *req, uint16_t *len, char **version); +P9Req *v9fs_tattach(QVirtio9P *v9p, uint32_t fid, uint32_t n_uname, + uint16_t tag); +void v9fs_rattach(P9Req *req, v9fs_qid *qid); +P9Req *v9fs_twalk(QVirtio9P *v9p, uint32_t fid, uint32_t newfid, + uint16_t nwname, char *const wnames[], uint16_t tag); +void v9fs_rwalk(P9Req *req, uint16_t *nwqid, v9fs_qid **wqid); +P9Req *v9fs_tgetattr(QVirtio9P *v9p, uint32_t fid, uint64_t request_mask, + uint16_t tag); +void v9fs_rgetattr(P9Req *req, v9fs_attr *attr); +P9Req *v9fs_treaddir(QVirtio9P *v9p, uint32_t fid, uint64_t offset, + uint32_t count, uint16_t tag); +void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t *nentries, + struct V9fsDirent **entries); +void v9fs_free_dirents(struct V9fsDirent *e); +P9Req *v9fs_tlopen(QVirtio9P *v9p, uint32_t fid, uint32_t flags, + uint16_t tag); +void v9fs_rlopen(P9Req *req, v9fs_qid *qid, uint32_t *iounit); +P9Req *v9fs_twrite(QVirtio9P *v9p, uint32_t fid, uint64_t offset, + uint32_t count, const void *data, uint16_t tag); +void v9fs_rwrite(P9Req *req, uint32_t *count); +P9Req *v9fs_tflush(QVirtio9P *v9p, uint16_t oldtag, uint16_t tag); +void v9fs_rflush(P9Req *req); +P9Req *v9fs_tmkdir(QVirtio9P *v9p, uint32_t dfid, const char *name, + uint32_t mode, uint32_t gid, uint16_t tag); +void v9fs_rmkdir(P9Req *req, v9fs_qid *qid); +P9Req *v9fs_tlcreate(QVirtio9P *v9p, uint32_t fid, const char *name, + uint32_t flags, uint32_t mode, uint32_t gid, + uint16_t tag); +void v9fs_rlcreate(P9Req *req, v9fs_qid *qid, uint32_t *iounit); +P9Req *v9fs_tsymlink(QVirtio9P *v9p, uint32_t fid, const char *name, + const char *symtgt, uint32_t gid, uint16_t tag); +void v9fs_rsymlink(P9Req *req, v9fs_qid *qid); +P9Req *v9fs_tlink(QVirtio9P *v9p, uint32_t dfid, uint32_t fid, + const char *name, uint16_t tag); +void v9fs_rlink(P9Req *req); +P9Req *v9fs_tunlinkat(QVirtio9P *v9p, uint32_t dirfd, const char *name, + uint32_t flags, uint16_t tag); +void v9fs_runlinkat(P9Req *req); + +#endif diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 25305a4cf7..498c32e21b 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -13,15 +13,8 @@ */ #include "qemu/osdep.h" -#include "libqtest-single.h" #include "qemu/module.h" -#include "hw/9pfs/9p.h" -#include "hw/9pfs/9p-synth.h" -#include "libqos/virtio-9p.h" -#include "libqos/qgraph.h" - -#define QVIRTIO_9P_TIMEOUT_US (10 * 1000 * 1000) -static QGuestAllocator *alloc; +#include "libqos/virtio-9p-client.h" /* * Used to auto generate new fids. Start with arbitrary high value to avoid @@ -82,7 +75,7 @@ static void split_free(char ***out) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); size_t tag_len = qvirtio_config_readw(v9p->vdev, 0); g_autofree char *tag = NULL; int i; @@ -96,565 +89,12 @@ static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) g_assert_cmpmem(tag, tag_len, MOUNT_TAG, tag_len); } -#define P9_MAX_SIZE 4096 /* Max size of a T-message or R-message */ - -typedef struct { - QTestState *qts; - QVirtio9P *v9p; - uint16_t tag; - uint64_t t_msg; - uint32_t t_size; - uint64_t r_msg; - /* No r_size, it is hardcoded to P9_MAX_SIZE */ - size_t t_off; - size_t r_off; - uint32_t free_head; -} P9Req; - -static void v9fs_memwrite(P9Req *req, const void *addr, size_t len) -{ - qtest_memwrite(req->qts, req->t_msg + req->t_off, addr, len); - req->t_off += len; -} - -static void v9fs_memskip(P9Req *req, size_t len) -{ - req->r_off += len; -} - -static void v9fs_memread(P9Req *req, void *addr, size_t len) -{ - qtest_memread(req->qts, req->r_msg + req->r_off, addr, len); - req->r_off += len; -} - -static void v9fs_uint8_read(P9Req *req, uint8_t *val) -{ - v9fs_memread(req, val, 1); -} - -static void v9fs_uint16_write(P9Req *req, uint16_t val) -{ - uint16_t le_val = cpu_to_le16(val); - - v9fs_memwrite(req, &le_val, 2); -} - -static void v9fs_uint16_read(P9Req *req, uint16_t *val) -{ - v9fs_memread(req, val, 2); - le16_to_cpus(val); -} - -static void v9fs_uint32_write(P9Req *req, uint32_t val) -{ - uint32_t le_val = cpu_to_le32(val); - - v9fs_memwrite(req, &le_val, 4); -} - -static void v9fs_uint64_write(P9Req *req, uint64_t val) -{ - uint64_t le_val = cpu_to_le64(val); - - v9fs_memwrite(req, &le_val, 8); -} - -static void v9fs_uint32_read(P9Req *req, uint32_t *val) -{ - v9fs_memread(req, val, 4); - le32_to_cpus(val); -} - -static void v9fs_uint64_read(P9Req *req, uint64_t *val) -{ - v9fs_memread(req, val, 8); - le64_to_cpus(val); -} - -/* len[2] string[len] */ -static uint16_t v9fs_string_size(const char *string) -{ - size_t len = strlen(string); - - g_assert_cmpint(len, <=, UINT16_MAX - 2); - - return 2 + len; -} - -static void v9fs_string_write(P9Req *req, const char *string) -{ - int len = strlen(string); - - g_assert_cmpint(len, <=, UINT16_MAX); - - v9fs_uint16_write(req, (uint16_t) len); - v9fs_memwrite(req, string, len); -} - -static void v9fs_string_read(P9Req *req, uint16_t *len, char **string) -{ - uint16_t local_len; - - v9fs_uint16_read(req, &local_len); - if (len) { - *len = local_len; - } - if (string) { - *string = g_malloc(local_len + 1); - v9fs_memread(req, *string, local_len); - (*string)[local_len] = 0; - } else { - v9fs_memskip(req, local_len); - } -} - - typedef struct { - uint32_t size; - uint8_t id; - uint16_t tag; -} QEMU_PACKED P9Hdr; - -static P9Req *v9fs_req_init(QVirtio9P *v9p, uint32_t size, uint8_t id, - uint16_t tag) -{ - P9Req *req = g_new0(P9Req, 1); - uint32_t total_size = 7; /* 9P header has well-known size of 7 bytes */ - P9Hdr hdr = { - .id = id, - .tag = cpu_to_le16(tag) - }; - - g_assert_cmpint(total_size, <=, UINT32_MAX - size); - total_size += size; - hdr.size = cpu_to_le32(total_size); - - g_assert_cmpint(total_size, <=, P9_MAX_SIZE); - - req->qts = global_qtest; - req->v9p = v9p; - req->t_size = total_size; - req->t_msg = guest_alloc(alloc, req->t_size); - v9fs_memwrite(req, &hdr, 7); - req->tag = tag; - return req; -} - -static void v9fs_req_send(P9Req *req) -{ - QVirtio9P *v9p = req->v9p; - - req->r_msg = guest_alloc(alloc, P9_MAX_SIZE); - req->free_head = qvirtqueue_add(req->qts, v9p->vq, req->t_msg, req->t_size, - false, true); - qvirtqueue_add(req->qts, v9p->vq, req->r_msg, P9_MAX_SIZE, true, false); - qvirtqueue_kick(req->qts, v9p->vdev, v9p->vq, req->free_head); - req->t_off = 0; -} - -static const char *rmessage_name(uint8_t id) -{ - return - id == P9_RLERROR ? "RLERROR" : - id == P9_RVERSION ? "RVERSION" : - id == P9_RATTACH ? "RATTACH" : - id == P9_RWALK ? "RWALK" : - id == P9_RLOPEN ? "RLOPEN" : - id == P9_RWRITE ? "RWRITE" : - id == P9_RMKDIR ? "RMKDIR" : - id == P9_RLCREATE ? "RLCREATE" : - id == P9_RSYMLINK ? "RSYMLINK" : - id == P9_RLINK ? "RLINK" : - id == P9_RUNLINKAT ? "RUNLINKAT" : - id == P9_RFLUSH ? "RFLUSH" : - id == P9_RREADDIR ? "READDIR" : - ""; -} - -static void v9fs_req_wait_for_reply(P9Req *req, uint32_t *len) -{ - QVirtio9P *v9p = req->v9p; - - qvirtio_wait_used_elem(req->qts, v9p->vdev, v9p->vq, req->free_head, len, - QVIRTIO_9P_TIMEOUT_US); -} - -static void v9fs_req_recv(P9Req *req, uint8_t id) -{ - P9Hdr hdr; - - v9fs_memread(req, &hdr, 7); - hdr.size = ldl_le_p(&hdr.size); - hdr.tag = lduw_le_p(&hdr.tag); - - g_assert_cmpint(hdr.size, >=, 7); - g_assert_cmpint(hdr.size, <=, P9_MAX_SIZE); - g_assert_cmpint(hdr.tag, ==, req->tag); - - if (hdr.id != id) { - g_printerr("Received response %d (%s) instead of %d (%s)\n", - hdr.id, rmessage_name(hdr.id), id, rmessage_name(id)); - - if (hdr.id == P9_RLERROR) { - uint32_t err; - v9fs_uint32_read(req, &err); - g_printerr("Rlerror has errno %d (%s)\n", err, strerror(err)); - } - } - g_assert_cmpint(hdr.id, ==, id); -} - -static void v9fs_req_free(P9Req *req) -{ - guest_free(alloc, req->t_msg); - guest_free(alloc, req->r_msg); - g_free(req); -} - -/* size[4] Rlerror tag[2] ecode[4] */ -static void v9fs_rlerror(P9Req *req, uint32_t *err) -{ - v9fs_req_recv(req, P9_RLERROR); - v9fs_uint32_read(req, err); - v9fs_req_free(req); -} - -/* size[4] Tversion tag[2] msize[4] version[s] */ -static P9Req *v9fs_tversion(QVirtio9P *v9p, uint32_t msize, const char *version, - uint16_t tag) -{ - P9Req *req; - uint32_t body_size = 4; - uint16_t string_size = v9fs_string_size(version); - - g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); - body_size += string_size; - req = v9fs_req_init(v9p, body_size, P9_TVERSION, tag); - - v9fs_uint32_write(req, msize); - v9fs_string_write(req, version); - v9fs_req_send(req); - return req; -} - -/* size[4] Rversion tag[2] msize[4] version[s] */ -static void v9fs_rversion(P9Req *req, uint16_t *len, char **version) -{ - uint32_t msize; - - v9fs_req_recv(req, P9_RVERSION); - v9fs_uint32_read(req, &msize); - - g_assert_cmpint(msize, ==, P9_MAX_SIZE); - - if (len || version) { - v9fs_string_read(req, len, version); - } - - v9fs_req_free(req); -} - -/* size[4] Tattach tag[2] fid[4] afid[4] uname[s] aname[s] n_uname[4] */ -static P9Req *v9fs_tattach(QVirtio9P *v9p, uint32_t fid, uint32_t n_uname, - uint16_t tag) -{ - const char *uname = ""; /* ignored by QEMU */ - const char *aname = ""; /* ignored by QEMU */ - P9Req *req = v9fs_req_init(v9p, 4 + 4 + 2 + 2 + 4, P9_TATTACH, tag); - - v9fs_uint32_write(req, fid); - v9fs_uint32_write(req, P9_NOFID); - v9fs_string_write(req, uname); - v9fs_string_write(req, aname); - v9fs_uint32_write(req, n_uname); - v9fs_req_send(req); - return req; -} - -/* type[1] version[4] path[8] */ -typedef char v9fs_qid[13]; - static inline bool is_same_qid(v9fs_qid a, v9fs_qid b) { /* don't compare QID version for checking for file ID equalness */ return a[0] == b[0] && memcmp(&a[5], &b[5], 8) == 0; } -/* size[4] Rattach tag[2] qid[13] */ -static void v9fs_rattach(P9Req *req, v9fs_qid *qid) -{ - v9fs_req_recv(req, P9_RATTACH); - if (qid) { - v9fs_memread(req, qid, 13); - } - v9fs_req_free(req); -} - -/* size[4] Twalk tag[2] fid[4] newfid[4] nwname[2] nwname*(wname[s]) */ -static P9Req *v9fs_twalk(QVirtio9P *v9p, uint32_t fid, uint32_t newfid, - uint16_t nwname, char *const wnames[], uint16_t tag) -{ - P9Req *req; - int i; - uint32_t body_size = 4 + 4 + 2; - - for (i = 0; i < nwname; i++) { - uint16_t wname_size = v9fs_string_size(wnames[i]); - - g_assert_cmpint(body_size, <=, UINT32_MAX - wname_size); - body_size += wname_size; - } - req = v9fs_req_init(v9p, body_size, P9_TWALK, tag); - v9fs_uint32_write(req, fid); - v9fs_uint32_write(req, newfid); - v9fs_uint16_write(req, nwname); - for (i = 0; i < nwname; i++) { - v9fs_string_write(req, wnames[i]); - } - v9fs_req_send(req); - return req; -} - -/* size[4] Rwalk tag[2] nwqid[2] nwqid*(wqid[13]) */ -static void v9fs_rwalk(P9Req *req, uint16_t *nwqid, v9fs_qid **wqid) -{ - uint16_t local_nwqid; - - v9fs_req_recv(req, P9_RWALK); - v9fs_uint16_read(req, &local_nwqid); - if (nwqid) { - *nwqid = local_nwqid; - } - if (wqid) { - *wqid = g_malloc(local_nwqid * 13); - v9fs_memread(req, *wqid, local_nwqid * 13); - } - v9fs_req_free(req); -} - -/* size[4] Tgetattr tag[2] fid[4] request_mask[8] */ -static P9Req *v9fs_tgetattr(QVirtio9P *v9p, uint32_t fid, uint64_t request_mask, - uint16_t tag) -{ - P9Req *req; - - req = v9fs_req_init(v9p, 4 + 8, P9_TGETATTR, tag); - v9fs_uint32_write(req, fid); - v9fs_uint64_write(req, request_mask); - v9fs_req_send(req); - return req; -} - -typedef struct v9fs_attr { - uint64_t valid; - v9fs_qid qid; - uint32_t mode; - uint32_t uid; - uint32_t gid; - uint64_t nlink; - uint64_t rdev; - uint64_t size; - uint64_t blksize; - uint64_t blocks; - uint64_t atime_sec; - uint64_t atime_nsec; - uint64_t mtime_sec; - uint64_t mtime_nsec; - uint64_t ctime_sec; - uint64_t ctime_nsec; - uint64_t btime_sec; - uint64_t btime_nsec; - uint64_t gen; - uint64_t data_version; -} v9fs_attr; - -#define P9_GETATTR_BASIC 0x000007ffULL /* Mask for fields up to BLOCKS */ - -/* - * size[4] Rgetattr tag[2] valid[8] qid[13] mode[4] uid[4] gid[4] nlink[8] - * rdev[8] size[8] blksize[8] blocks[8] - * atime_sec[8] atime_nsec[8] mtime_sec[8] mtime_nsec[8] - * ctime_sec[8] ctime_nsec[8] btime_sec[8] btime_nsec[8] - * gen[8] data_version[8] - */ -static void v9fs_rgetattr(P9Req *req, v9fs_attr *attr) -{ - v9fs_req_recv(req, P9_RGETATTR); - - v9fs_uint64_read(req, &attr->valid); - v9fs_memread(req, &attr->qid, 13); - v9fs_uint32_read(req, &attr->mode); - v9fs_uint32_read(req, &attr->uid); - v9fs_uint32_read(req, &attr->gid); - v9fs_uint64_read(req, &attr->nlink); - v9fs_uint64_read(req, &attr->rdev); - v9fs_uint64_read(req, &attr->size); - v9fs_uint64_read(req, &attr->blksize); - v9fs_uint64_read(req, &attr->blocks); - v9fs_uint64_read(req, &attr->atime_sec); - v9fs_uint64_read(req, &attr->atime_nsec); - v9fs_uint64_read(req, &attr->mtime_sec); - v9fs_uint64_read(req, &attr->mtime_nsec); - v9fs_uint64_read(req, &attr->ctime_sec); - v9fs_uint64_read(req, &attr->ctime_nsec); - v9fs_uint64_read(req, &attr->btime_sec); - v9fs_uint64_read(req, &attr->btime_nsec); - v9fs_uint64_read(req, &attr->gen); - v9fs_uint64_read(req, &attr->data_version); - - v9fs_req_free(req); -} - -/* size[4] Treaddir tag[2] fid[4] offset[8] count[4] */ -static P9Req *v9fs_treaddir(QVirtio9P *v9p, uint32_t fid, uint64_t offset, - uint32_t count, uint16_t tag) -{ - P9Req *req; - - req = v9fs_req_init(v9p, 4 + 8 + 4, P9_TREADDIR, tag); - v9fs_uint32_write(req, fid); - v9fs_uint64_write(req, offset); - v9fs_uint32_write(req, count); - v9fs_req_send(req); - return req; -} - -struct V9fsDirent { - v9fs_qid qid; - uint64_t offset; - uint8_t type; - char *name; - struct V9fsDirent *next; -}; - -/* size[4] Rreaddir tag[2] count[4] data[count] */ -static void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t *nentries, - struct V9fsDirent **entries) -{ - uint32_t local_count; - struct V9fsDirent *e = NULL; - uint16_t slen; - uint32_t n = 0; - - v9fs_req_recv(req, P9_RREADDIR); - v9fs_uint32_read(req, &local_count); - - if (count) { - *count = local_count; - } - - for (int32_t togo = (int32_t)local_count; - togo >= 13 + 8 + 1 + 2; - togo -= 13 + 8 + 1 + 2 + slen, ++n) - { - if (!e) { - e = g_new(struct V9fsDirent, 1); - if (entries) { - *entries = e; - } - } else { - e = e->next = g_new(struct V9fsDirent, 1); - } - e->next = NULL; - /* qid[13] offset[8] type[1] name[s] */ - v9fs_memread(req, &e->qid, 13); - v9fs_uint64_read(req, &e->offset); - v9fs_uint8_read(req, &e->type); - v9fs_string_read(req, &slen, &e->name); - } - - if (nentries) { - *nentries = n; - } - - v9fs_req_free(req); -} - -static void v9fs_free_dirents(struct V9fsDirent *e) -{ - struct V9fsDirent *next = NULL; - - for (; e; e = next) { - next = e->next; - g_free(e->name); - g_free(e); - } -} - -/* size[4] Tlopen tag[2] fid[4] flags[4] */ -static P9Req *v9fs_tlopen(QVirtio9P *v9p, uint32_t fid, uint32_t flags, - uint16_t tag) -{ - P9Req *req; - - req = v9fs_req_init(v9p, 4 + 4, P9_TLOPEN, tag); - v9fs_uint32_write(req, fid); - v9fs_uint32_write(req, flags); - v9fs_req_send(req); - return req; -} - -/* size[4] Rlopen tag[2] qid[13] iounit[4] */ -static void v9fs_rlopen(P9Req *req, v9fs_qid *qid, uint32_t *iounit) -{ - v9fs_req_recv(req, P9_RLOPEN); - if (qid) { - v9fs_memread(req, qid, 13); - } else { - v9fs_memskip(req, 13); - } - if (iounit) { - v9fs_uint32_read(req, iounit); - } - v9fs_req_free(req); -} - -/* size[4] Twrite tag[2] fid[4] offset[8] count[4] data[count] */ -static P9Req *v9fs_twrite(QVirtio9P *v9p, uint32_t fid, uint64_t offset, - uint32_t count, const void *data, uint16_t tag) -{ - P9Req *req; - uint32_t body_size = 4 + 8 + 4; - - g_assert_cmpint(body_size, <=, UINT32_MAX - count); - body_size += count; - req = v9fs_req_init(v9p, body_size, P9_TWRITE, tag); - v9fs_uint32_write(req, fid); - v9fs_uint64_write(req, offset); - v9fs_uint32_write(req, count); - v9fs_memwrite(req, data, count); - v9fs_req_send(req); - return req; -} - -/* size[4] Rwrite tag[2] count[4] */ -static void v9fs_rwrite(P9Req *req, uint32_t *count) -{ - v9fs_req_recv(req, P9_RWRITE); - if (count) { - v9fs_uint32_read(req, count); - } - v9fs_req_free(req); -} - -/* size[4] Tflush tag[2] oldtag[2] */ -static P9Req *v9fs_tflush(QVirtio9P *v9p, uint16_t oldtag, uint16_t tag) -{ - P9Req *req; - - req = v9fs_req_init(v9p, 2, P9_TFLUSH, tag); - v9fs_uint32_write(req, oldtag); - v9fs_req_send(req); - return req; -} - -/* size[4] Rflush tag[2] */ -static void v9fs_rflush(P9Req *req) -{ - v9fs_req_recv(req, P9_RFLUSH); - v9fs_req_free(req); -} - static void do_version(QVirtio9P *v9p) { const char *version = "9P2000.L"; @@ -717,7 +157,7 @@ static void do_walk_expect_error(QVirtio9P *v9p, const char *path, uint32_t err) static void fs_version(void *obj, void *data, QGuestAllocator *t_alloc) { - alloc = t_alloc; + v9fs_set_allocator(t_alloc); do_version(obj); } @@ -738,14 +178,14 @@ static void do_attach(QVirtio9P *v9p) static void fs_attach(void *obj, void *data, QGuestAllocator *t_alloc) { - alloc = t_alloc; + v9fs_set_allocator(t_alloc); do_attach(obj); } static void fs_walk(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); char *wnames[P9_MAXWELEM]; uint16_t nwqid; g_autofree v9fs_qid *wqid = NULL; @@ -778,169 +218,11 @@ 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); -} - -/* size[4] Tlcreate tag[2] fid[4] name[s] flags[4] mode[4] gid[4] */ -static P9Req *v9fs_tlcreate(QVirtio9P *v9p, uint32_t fid, const char *name, - uint32_t flags, uint32_t mode, uint32_t gid, - uint16_t tag) -{ - P9Req *req; - - uint32_t body_size = 4 + 4 + 4 + 4; - uint16_t string_size = v9fs_string_size(name); - - g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); - body_size += string_size; - - req = v9fs_req_init(v9p, body_size, P9_TLCREATE, tag); - v9fs_uint32_write(req, fid); - v9fs_string_write(req, name); - v9fs_uint32_write(req, flags); - v9fs_uint32_write(req, mode); - v9fs_uint32_write(req, gid); - v9fs_req_send(req); - return req; -} - -/* size[4] Rlcreate tag[2] qid[13] iounit[4] */ -static void v9fs_rlcreate(P9Req *req, v9fs_qid *qid, uint32_t *iounit) -{ - v9fs_req_recv(req, P9_RLCREATE); - if (qid) { - v9fs_memread(req, qid, 13); - } else { - v9fs_memskip(req, 13); - } - if (iounit) { - v9fs_uint32_read(req, iounit); - } - v9fs_req_free(req); -} - -/* size[4] Tsymlink tag[2] fid[4] name[s] symtgt[s] gid[4] */ -static P9Req *v9fs_tsymlink(QVirtio9P *v9p, uint32_t fid, const char *name, - const char *symtgt, uint32_t gid, uint16_t tag) -{ - P9Req *req; - - uint32_t body_size = 4 + 4; - uint16_t string_size = v9fs_string_size(name) + v9fs_string_size(symtgt); - - g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); - body_size += string_size; - - req = v9fs_req_init(v9p, body_size, P9_TSYMLINK, tag); - v9fs_uint32_write(req, fid); - v9fs_string_write(req, name); - v9fs_string_write(req, symtgt); - v9fs_uint32_write(req, gid); - v9fs_req_send(req); - return req; -} - -/* size[4] Rsymlink tag[2] qid[13] */ -static void v9fs_rsymlink(P9Req *req, v9fs_qid *qid) -{ - v9fs_req_recv(req, P9_RSYMLINK); - if (qid) { - v9fs_memread(req, qid, 13); - } else { - v9fs_memskip(req, 13); - } - v9fs_req_free(req); -} - -/* size[4] Tlink tag[2] dfid[4] fid[4] name[s] */ -static P9Req *v9fs_tlink(QVirtio9P *v9p, uint32_t dfid, uint32_t fid, - const char *name, uint16_t tag) -{ - P9Req *req; - - uint32_t body_size = 4 + 4; - uint16_t string_size = v9fs_string_size(name); - - g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); - body_size += string_size; - - req = v9fs_req_init(v9p, body_size, P9_TLINK, tag); - v9fs_uint32_write(req, dfid); - v9fs_uint32_write(req, fid); - v9fs_string_write(req, name); - v9fs_req_send(req); - return req; -} - -/* size[4] Rlink tag[2] */ -static void v9fs_rlink(P9Req *req) -{ - v9fs_req_recv(req, P9_RLINK); - v9fs_req_free(req); -} - -/* size[4] Tunlinkat tag[2] dirfd[4] name[s] flags[4] */ -static P9Req *v9fs_tunlinkat(QVirtio9P *v9p, uint32_t dirfd, const char *name, - uint32_t flags, uint16_t tag) -{ - P9Req *req; - - uint32_t body_size = 4 + 4; - uint16_t string_size = v9fs_string_size(name); - - g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); - body_size += string_size; - - req = v9fs_req_init(v9p, body_size, P9_TUNLINKAT, tag); - v9fs_uint32_write(req, dirfd); - v9fs_string_write(req, name); - v9fs_uint32_write(req, flags); - v9fs_req_send(req); - return req; -} - -/* size[4] Runlinkat tag[2] */ -static void v9fs_runlinkat(P9Req *req) -{ - v9fs_req_recv(req, P9_RUNLINKAT); - v9fs_req_free(req); -} - /* basic readdir test where reply fits into a single response message */ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_READDIR_DIR) }; uint16_t nqid; v9fs_qid qid; @@ -1073,7 +355,7 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count) static void fs_walk_no_slash(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); char *const wnames[] = { g_strdup(" /") }; P9Req *req; uint32_t err; @@ -1091,7 +373,7 @@ static void fs_walk_no_slash(void *obj, void *data, QGuestAllocator *t_alloc) static void fs_walk_nonexistent(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); do_attach(v9p); /* @@ -1105,7 +387,7 @@ static void fs_walk_2nd_nonexistent(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); v9fs_qid root_qid; uint16_t nwqid; uint32_t fid, err; @@ -1137,7 +419,7 @@ static void fs_walk_2nd_nonexistent(void *obj, void *data, static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); v9fs_qid root_qid; g_autofree v9fs_qid *wqid = NULL; P9Req *req; @@ -1165,7 +447,7 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); char *const wnames[] = { g_strdup("..") }; v9fs_qid root_qid; g_autofree v9fs_qid *wqid = NULL; @@ -1188,7 +470,7 @@ static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc) static void fs_lopen(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_LOPEN_FILE) }; P9Req *req; @@ -1207,7 +489,7 @@ static void fs_lopen(void *obj, void *data, QGuestAllocator *t_alloc) static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); static const uint32_t write_count = P9_MAX_SIZE / 2; char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_WRITE_FILE) }; g_autofree char *buf = g_malloc0(write_count); @@ -1234,7 +516,7 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc) static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_FLUSH_FILE) }; P9Req *req, *flush_req; uint32_t reply_len; @@ -1271,7 +553,7 @@ static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc) static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_FLUSH_FILE) }; P9Req *req, *flush_req; uint32_t count; @@ -1383,21 +665,21 @@ static void do_unlinkat(QVirtio9P *v9p, const char *atpath, const char *rpath, static void fs_readdir_split_128(void *obj, void *data, QGuestAllocator *t_alloc) { - alloc = t_alloc; + v9fs_set_allocator(t_alloc); do_readdir_split(obj, 128); } static void fs_readdir_split_256(void *obj, void *data, QGuestAllocator *t_alloc) { - alloc = t_alloc; + v9fs_set_allocator(t_alloc); do_readdir_split(obj, 256); } static void fs_readdir_split_512(void *obj, void *data, QGuestAllocator *t_alloc) { - alloc = t_alloc; + v9fs_set_allocator(t_alloc); do_readdir_split(obj, 512); } @@ -1407,7 +689,7 @@ static void fs_readdir_split_512(void *obj, void *data, static void fs_create_dir(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); struct stat st; g_autofree char *root_path = virtio_9p_test_path(""); g_autofree char *new_dir = virtio_9p_test_path("01"); @@ -1426,7 +708,7 @@ static void fs_create_dir(void *obj, void *data, QGuestAllocator *t_alloc) static void fs_unlinkat_dir(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); struct stat st; g_autofree char *root_path = virtio_9p_test_path(""); g_autofree char *new_dir = virtio_9p_test_path("02"); @@ -1449,7 +731,7 @@ static void fs_unlinkat_dir(void *obj, void *data, QGuestAllocator *t_alloc) static void fs_create_file(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); struct stat st; g_autofree char *new_file = virtio_9p_test_path("03/1st_file"); @@ -1466,7 +748,7 @@ static void fs_create_file(void *obj, void *data, QGuestAllocator *t_alloc) static void fs_unlinkat_file(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); struct stat st; g_autofree char *new_file = virtio_9p_test_path("04/doa_file"); @@ -1487,7 +769,7 @@ static void fs_unlinkat_file(void *obj, void *data, QGuestAllocator *t_alloc) static void fs_symlink_file(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); struct stat st; g_autofree char *real_file = virtio_9p_test_path("05/real_file"); g_autofree char *symlink_file = virtio_9p_test_path("05/symlink_file"); @@ -1508,7 +790,7 @@ static void fs_unlinkat_symlink(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); struct stat st; g_autofree char *real_file = virtio_9p_test_path("06/real_file"); g_autofree char *symlink_file = virtio_9p_test_path("06/symlink_file"); @@ -1530,7 +812,7 @@ static void fs_unlinkat_symlink(void *obj, void *data, static void fs_hardlink_file(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); struct stat st_real, st_link; g_autofree char *real_file = virtio_9p_test_path("07/real_file"); g_autofree char *hardlink_file = virtio_9p_test_path("07/hardlink_file"); @@ -1555,7 +837,7 @@ static void fs_unlinkat_hardlink(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; - alloc = t_alloc; + v9fs_set_allocator(t_alloc); struct stat st_real, st_link; g_autofree char *real_file = virtio_9p_test_path("08/real_file"); g_autofree char *hardlink_file = virtio_9p_test_path("08/hardlink_file"); From f5265c8f917ea8c71a30e549b7e3017c1038db63 Mon Sep 17 00:00:00 2001 From: Linus Heckemann Date: Tue, 4 Oct 2022 12:41:21 +0200 Subject: [PATCH 03/23] 9pfs: use GHashTable for fid table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous implementation would iterate over the fid table for lookup operations, resulting in an operation with O(n) complexity on the number of open files and poor cache locality -- for every open, stat, read, write, etc operation. This change uses a hashtable for this instead, significantly improving the performance of the 9p filesystem. The runtime of NixOS's simple installer test, which copies ~122k files totalling ~1.8GiB from 9p, decreased by a factor of about 10. Signed-off-by: Linus Heckemann Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Greg Kurz [CS: - Retain BUG_ON(f->clunked) in get_fid(). - Add TODO comment in clunk_fid(). ] Message-Id: <20221004104121.713689-1-git@sphalerite.org> [CS: - Drop unnecessary goto and out: label. ] Signed-off-by: Christian Schoenebeck --- hw/9pfs/9p.c | 196 +++++++++++++++++++++++++++++---------------------- hw/9pfs/9p.h | 2 +- 2 files changed, 113 insertions(+), 85 deletions(-) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index aebadeaa03..9bf13133e5 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -256,7 +256,8 @@ static size_t v9fs_string_size(V9fsString *str) } /* - * returns 0 if fid got re-opened, 1 if not, < 0 on error */ + * returns 0 if fid got re-opened, 1 if not, < 0 on error + */ static int coroutine_fn v9fs_reopen_fid(V9fsPDU *pdu, V9fsFidState *f) { int err = 1; @@ -282,33 +283,32 @@ static V9fsFidState *coroutine_fn get_fid(V9fsPDU *pdu, int32_t fid) V9fsFidState *f; V9fsState *s = pdu->s; - QSIMPLEQ_FOREACH(f, &s->fid_list, next) { + f = g_hash_table_lookup(s->fids, GINT_TO_POINTER(fid)); + if (f) { BUG_ON(f->clunked); - if (f->fid == fid) { - /* - * Update the fid ref upfront so that - * we don't get reclaimed when we yield - * in open later. - */ - f->ref++; - /* - * check whether we need to reopen the - * file. We might have closed the fd - * while trying to free up some file - * descriptors. - */ - err = v9fs_reopen_fid(pdu, f); - if (err < 0) { - f->ref--; - return NULL; - } - /* - * Mark the fid as referenced so that the LRU - * reclaim won't close the file descriptor - */ - f->flags |= FID_REFERENCED; - return f; + /* + * Update the fid ref upfront so that + * we don't get reclaimed when we yield + * in open later. + */ + f->ref++; + /* + * check whether we need to reopen the + * file. We might have closed the fd + * while trying to free up some file + * descriptors. + */ + err = v9fs_reopen_fid(pdu, f); + if (err < 0) { + f->ref--; + return NULL; } + /* + * Mark the fid as referenced so that the LRU + * reclaim won't close the file descriptor + */ + f->flags |= FID_REFERENCED; + return f; } return NULL; } @@ -317,12 +317,11 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid) { V9fsFidState *f; - QSIMPLEQ_FOREACH(f, &s->fid_list, next) { + f = g_hash_table_lookup(s->fids, GINT_TO_POINTER(fid)); + if (f) { /* If fid is already there return NULL */ BUG_ON(f->clunked); - if (f->fid == fid) { - return NULL; - } + return NULL; } f = g_new0(V9fsFidState, 1); f->fid = fid; @@ -333,7 +332,7 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid) * reclaim won't close the file descriptor */ f->flags |= FID_REFERENCED; - QSIMPLEQ_INSERT_TAIL(&s->fid_list, f, next); + g_hash_table_insert(s->fids, GINT_TO_POINTER(fid), f); v9fs_readdir_init(s->proto_version, &f->fs.dir); v9fs_readdir_init(s->proto_version, &f->fs_reclaim.dir); @@ -424,12 +423,12 @@ static V9fsFidState *clunk_fid(V9fsState *s, int32_t fid) { V9fsFidState *fidp; - QSIMPLEQ_FOREACH(fidp, &s->fid_list, next) { - if (fidp->fid == fid) { - QSIMPLEQ_REMOVE(&s->fid_list, fidp, V9fsFidState, next); - fidp->clunked = true; - return fidp; - } + /* TODO: Use g_hash_table_steal_extended() instead? */ + fidp = g_hash_table_lookup(s->fids, GINT_TO_POINTER(fid)); + if (fidp) { + g_hash_table_remove(s->fids, GINT_TO_POINTER(fid)); + fidp->clunked = true; + return fidp; } return NULL; } @@ -439,10 +438,15 @@ void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu) int reclaim_count = 0; V9fsState *s = pdu->s; V9fsFidState *f; + GHashTableIter iter; + gpointer fid; + + g_hash_table_iter_init(&iter, s->fids); + QSLIST_HEAD(, V9fsFidState) reclaim_list = QSLIST_HEAD_INITIALIZER(reclaim_list); - QSIMPLEQ_FOREACH(f, &s->fid_list, next) { + while (g_hash_table_iter_next(&iter, &fid, (gpointer *) &f)) { /* * Unlink fids cannot be reclaimed. Check * for them and skip them. Also skip fids @@ -514,72 +518,85 @@ void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu) } } +/* + * This is used when a path is removed from the directory tree. Any + * fids that still reference it must not be closed from then on, since + * they cannot be reopened. + */ static int coroutine_fn v9fs_mark_fids_unreclaim(V9fsPDU *pdu, V9fsPath *path) { - int err; + int err = 0; V9fsState *s = pdu->s; - V9fsFidState *fidp, *fidp_next; + V9fsFidState *fidp; + gpointer fid; + GHashTableIter iter; + /* + * The most common case is probably that we have exactly one + * fid for the given path, so preallocate exactly one. + */ + g_autoptr(GArray) to_reopen = g_array_sized_new(FALSE, FALSE, + sizeof(V9fsFidState *), 1); + gint i; - fidp = QSIMPLEQ_FIRST(&s->fid_list); - if (!fidp) { - return 0; - } + g_hash_table_iter_init(&iter, s->fids); /* - * v9fs_reopen_fid() can yield : a reference on the fid must be held - * to ensure its pointer remains valid and we can safely pass it to - * QSIMPLEQ_NEXT(). The corresponding put_fid() can also yield so - * we must keep a reference on the next fid as well. So the logic here - * is to get a reference on a fid and only put it back during the next - * iteration after we could get a reference on the next fid. Start with - * the first one. + * We iterate over the fid table looking for the entries we need + * to reopen, and store them in to_reopen. This is because + * v9fs_reopen_fid() and put_fid() yield. This allows the fid table + * to be modified in the meantime, invalidating our iterator. */ - for (fidp->ref++; fidp; fidp = fidp_next) { + while (g_hash_table_iter_next(&iter, &fid, (gpointer *) &fidp)) { if (fidp->path.size == path->size && !memcmp(fidp->path.data, path->data, path->size)) { - /* Mark the fid non reclaimable. */ - fidp->flags |= FID_NON_RECLAIMABLE; - - /* reopen the file/dir if already closed */ - err = v9fs_reopen_fid(pdu, fidp); - if (err < 0) { - put_fid(pdu, fidp); - return err; - } - } - - fidp_next = QSIMPLEQ_NEXT(fidp, next); - - if (fidp_next) { /* - * Ensure the next fid survives a potential clunk request during - * put_fid() below and v9fs_reopen_fid() in the next iteration. + * Ensure the fid survives a potential clunk request during + * v9fs_reopen_fid or put_fid. */ - fidp_next->ref++; + fidp->ref++; + fidp->flags |= FID_NON_RECLAIMABLE; + g_array_append_val(to_reopen, fidp); } - - /* We're done with this fid */ - put_fid(pdu, fidp); } - return 0; + for (i = 0; i < to_reopen->len; i++) { + fidp = g_array_index(to_reopen, V9fsFidState*, i); + /* reopen the file/dir if already closed */ + err = v9fs_reopen_fid(pdu, fidp); + if (err < 0) { + break; + } + } + + for (i = 0; i < to_reopen->len; i++) { + put_fid(pdu, g_array_index(to_reopen, V9fsFidState*, i)); + } + return err; } static void coroutine_fn virtfs_reset(V9fsPDU *pdu) { V9fsState *s = pdu->s; V9fsFidState *fidp; + GList *freeing; + /* + * Get a list of all the values (fid states) in the table, which + * we then... + */ + g_autoptr(GList) fids = g_hash_table_get_values(s->fids); - /* Free all fids */ - while (!QSIMPLEQ_EMPTY(&s->fid_list)) { - /* Get fid */ - fidp = QSIMPLEQ_FIRST(&s->fid_list); + /* ... remove from the table, taking over ownership. */ + g_hash_table_steal_all(s->fids); + + /* + * This allows us to release our references to them asynchronously without + * iterating over the hash table and risking iterator invalidation + * through concurrent modifications. + */ + for (freeing = fids; freeing; freeing = freeing->next) { + fidp = freeing->data; fidp->ref++; - - /* Clunk fid */ - QSIMPLEQ_REMOVE(&s->fid_list, fidp, V9fsFidState, next); fidp->clunked = true; - put_fid(pdu, fidp); } } @@ -3205,6 +3222,8 @@ static int coroutine_fn v9fs_complete_rename(V9fsPDU *pdu, V9fsFidState *fidp, V9fsFidState *tfidp; V9fsState *s = pdu->s; V9fsFidState *dirfidp = NULL; + GHashTableIter iter; + gpointer fid; v9fs_path_init(&new_path); if (newdirfid != -1) { @@ -3238,11 +3257,13 @@ static int coroutine_fn v9fs_complete_rename(V9fsPDU *pdu, V9fsFidState *fidp, if (err < 0) { goto out; } + /* * Fixup fid's pointing to the old name to * start pointing to the new name */ - QSIMPLEQ_FOREACH(tfidp, &s->fid_list, next) { + g_hash_table_iter_init(&iter, s->fids); + while (g_hash_table_iter_next(&iter, &fid, (gpointer *) &tfidp)) { if (v9fs_path_is_ancestor(&fidp->path, &tfidp->path)) { /* replace the name */ v9fs_fix_path(&tfidp->path, &new_path, strlen(fidp->path.data)); @@ -3320,6 +3341,8 @@ static int coroutine_fn v9fs_fix_fid_paths(V9fsPDU *pdu, V9fsPath *olddir, V9fsPath oldpath, newpath; V9fsState *s = pdu->s; int err; + GHashTableIter iter; + gpointer fid; v9fs_path_init(&oldpath); v9fs_path_init(&newpath); @@ -3336,7 +3359,8 @@ static int coroutine_fn v9fs_fix_fid_paths(V9fsPDU *pdu, V9fsPath *olddir, * Fixup fid's pointing to the old name to * start pointing to the new name */ - QSIMPLEQ_FOREACH(tfidp, &s->fid_list, next) { + g_hash_table_iter_init(&iter, s->fids); + while (g_hash_table_iter_next(&iter, &fid, (gpointer *) &tfidp)) { if (v9fs_path_is_ancestor(&oldpath, &tfidp->path)) { /* replace the name */ v9fs_fix_path(&tfidp->path, &newpath, strlen(oldpath.data)); @@ -4226,7 +4250,7 @@ int v9fs_device_realize_common(V9fsState *s, const V9fsTransport *t, s->ctx.fmode = fse->fmode; s->ctx.dmode = fse->dmode; - QSIMPLEQ_INIT(&s->fid_list); + s->fids = g_hash_table_new(NULL, NULL); qemu_co_rwlock_init(&s->rename_lock); if (s->ops->init(&s->ctx, errp) < 0) { @@ -4286,6 +4310,10 @@ void v9fs_device_unrealize_common(V9fsState *s) if (s->ctx.fst) { fsdev_throttle_cleanup(s->ctx.fst); } + if (s->fids) { + g_hash_table_destroy(s->fids); + s->fids = NULL; + } g_free(s->tag); qp_table_destroy(&s->qpd_table); qp_table_destroy(&s->qpp_table); diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h index a523ac34a9..2fce4140d1 100644 --- a/hw/9pfs/9p.h +++ b/hw/9pfs/9p.h @@ -339,7 +339,7 @@ typedef struct { struct V9fsState { QLIST_HEAD(, V9fsPDU) free_list; QLIST_HEAD(, V9fsPDU) active_list; - QSIMPLEQ_HEAD(, V9fsFidState) fid_list; + GHashTable *fids; FileOperations *ops; FsContext ctx; char *tag; From 569f3b63ad5f65d0f86724766fedfffffe0feda3 Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:53:23 +0200 Subject: [PATCH 04/23] tests/9p: merge *walk*() functions Introduce declarative function calls. There are currently 4 different functions for sending a 9p 'Twalk' request: v9fs_twalk(), do_walk(), do_walk_rqids() and do_walk_expect_error(). They are all doing the same thing, just in a slightly different way and with slightly different function arguments. Merge those 4 functions into a single function by using a struct for function call arguments and use designated initializers when calling this function to turn usage into a declarative approach, which is better readable and easier to maintain. Also move private functions genfid(), split() and split_free() from virtio-9p-test.c to virtio-9p-client.c. Based-on: Signed-off-by: Christian Schoenebeck Message-Id: <607969dbfbc63c1be008df9131133711b046e979.1664917004.git.qemu_oss@crudebyte.com> --- tests/qtest/libqos/virtio-9p-client.c | 114 ++++++++++++++-- tests/qtest/libqos/virtio-9p-client.h | 37 ++++- tests/qtest/virtio-9p-test.c | 187 +++++++++----------------- 3 files changed, 198 insertions(+), 140 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index f5c35fd722..a95bbad9c8 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -23,6 +23,65 @@ void v9fs_set_allocator(QGuestAllocator *t_alloc) alloc = t_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; + if (!*out) { + return; + } + for (i = 0; (*out)[i]; ++i) { + g_free((*out)[i]); + } + g_free(*out); + *out = NULL; +} + void v9fs_memwrite(P9Req *req, const void *addr, size_t len) { qtest_memwrite(req->qts, req->t_msg + req->t_off, addr, len); @@ -294,28 +353,61 @@ void v9fs_rattach(P9Req *req, v9fs_qid *qid) } /* size[4] Twalk tag[2] fid[4] newfid[4] nwname[2] nwname*(wname[s]) */ -P9Req *v9fs_twalk(QVirtio9P *v9p, uint32_t fid, uint32_t newfid, - uint16_t nwname, char *const wnames[], uint16_t tag) +TWalkRes v9fs_twalk(TWalkOpt opt) { P9Req *req; int i; uint32_t body_size = 4 + 4 + 2; + uint32_t err; + char **wnames = NULL; - for (i = 0; i < nwname; i++) { - uint16_t wname_size = v9fs_string_size(wnames[i]); + g_assert(opt.client); + /* expecting either high- or low-level path, both not both */ + g_assert(!opt.path || !(opt.nwname || opt.wnames)); + /* expecting either Rwalk or Rlerror, but obviously not both */ + g_assert(!opt.expectErr || !(opt.rwalk.nwqid || opt.rwalk.wqid)); + + if (!opt.newfid) { + opt.newfid = genfid(); + } + + if (opt.path) { + opt.nwname = split(opt.path, "/", &wnames); + opt.wnames = wnames; + } + + for (i = 0; i < opt.nwname; i++) { + uint16_t wname_size = v9fs_string_size(opt.wnames[i]); g_assert_cmpint(body_size, <=, UINT32_MAX - wname_size); body_size += wname_size; } - req = v9fs_req_init(v9p, body_size, P9_TWALK, tag); - v9fs_uint32_write(req, fid); - v9fs_uint32_write(req, newfid); - v9fs_uint16_write(req, nwname); - for (i = 0; i < nwname; i++) { - v9fs_string_write(req, wnames[i]); + req = v9fs_req_init(opt.client, body_size, P9_TWALK, opt.tag); + v9fs_uint32_write(req, opt.fid); + v9fs_uint32_write(req, opt.newfid); + v9fs_uint16_write(req, opt.nwname); + for (i = 0; i < opt.nwname; i++) { + v9fs_string_write(req, opt.wnames[i]); } v9fs_req_send(req); - return req; + + if (!opt.requestOnly) { + v9fs_req_wait_for_reply(req, NULL); + if (opt.expectErr) { + v9fs_rlerror(req, &err); + g_assert_cmpint(err, ==, opt.expectErr); + } else { + v9fs_rwalk(req, opt.rwalk.nwqid, opt.rwalk.wqid); + } + req = NULL; /* request was freed */ + } + + split_free(&wnames); + + return (TWalkRes) { + .newfid = opt.newfid, + .req = req, + }; } /* size[4] Rwalk tag[2] nwqid[2] nwqid*(wqid[13]) */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index c502d12a66..8c6abbb173 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -72,6 +72,40 @@ struct V9fsDirent { struct V9fsDirent *next; }; +/* options for 'Twalk' 9p request */ +typedef struct TWalkOpt { + /* 9P client being used (mandatory) */ + QVirtio9P *client; + /* user supplied tag number being returned with response (optional) */ + uint16_t tag; + /* file ID of directory from where walk should start (optional) */ + uint32_t fid; + /* file ID for target directory being walked to (optional) */ + uint32_t newfid; + /* low level variant of path to walk to (optional) */ + uint16_t nwname; + char **wnames; + /* high level variant of path to walk to (optional) */ + const char *path; + /* data being received from 9p server as 'Rwalk' response (optional) */ + struct { + uint16_t *nwqid; + v9fs_qid **wqid; + } rwalk; + /* only send Twalk request but not wait for a reply? (optional) */ + bool requestOnly; + /* do we expect an Rlerror response, if yes which error code? (optional) */ + uint32_t expectErr; +} TWalkOpt; + +/* result of 'Twalk' 9p request */ +typedef struct TWalkRes { + /* file ID of target directory been walked to */ + uint32_t newfid; + /* if requestOnly was set: request object for further processing */ + P9Req *req; +} TWalkRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -99,8 +133,7 @@ void v9fs_rversion(P9Req *req, uint16_t *len, char **version); P9Req *v9fs_tattach(QVirtio9P *v9p, uint32_t fid, uint32_t n_uname, uint16_t tag); void v9fs_rattach(P9Req *req, v9fs_qid *qid); -P9Req *v9fs_twalk(QVirtio9P *v9p, uint32_t fid, uint32_t newfid, - uint16_t nwname, char *const wnames[], uint16_t tag); +TWalkRes v9fs_twalk(TWalkOpt opt); void v9fs_rwalk(P9Req *req, uint16_t *nwqid, v9fs_qid **wqid); P9Req *v9fs_tgetattr(QVirtio9P *v9p, uint32_t fid, uint64_t request_mask, uint16_t tag); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 498c32e21b..cf5d6146ad 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -16,61 +16,7 @@ #include "qemu/module.h" #include "libqos/virtio-9p-client.h" -/* - * 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; -} +#define twalk(...) v9fs_twalk((TWalkOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -109,52 +55,6 @@ static void do_version(QVirtio9P *v9p) g_assert_cmpmem(server_version, server_len, version, strlen(version)); } -/* - * utility function: walk to requested dir and return fid for that dir and - * the QIDs of server response - */ -static uint32_t do_walk_rqids(QVirtio9P *v9p, const char *path, uint16_t *nwqid, - v9fs_qid **wqid) -{ - char **wnames; - P9Req *req; - const uint32_t fid = genfid(); - - int nwnames = split(path, "/", &wnames); - - req = v9fs_twalk(v9p, 0, fid, nwnames, wnames, 0); - v9fs_req_wait_for_reply(req, NULL); - v9fs_rwalk(req, nwqid, wqid); - - split_free(&wnames); - return fid; -} - -/* utility function: walk to requested dir and return fid for that dir */ -static uint32_t do_walk(QVirtio9P *v9p, const char *path) -{ - return do_walk_rqids(v9p, path, NULL, NULL); -} - -/* utility function: walk to requested dir and expect passed error response */ -static void do_walk_expect_error(QVirtio9P *v9p, const char *path, uint32_t err) -{ - char **wnames; - P9Req *req; - uint32_t _err; - 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_rlerror(req, &_err); - - g_assert_cmpint(_err, ==, err); - - split_free(&wnames); -} - static void fs_version(void *obj, void *data, QGuestAllocator *t_alloc) { v9fs_set_allocator(t_alloc); @@ -197,7 +97,10 @@ static void fs_walk(void *obj, void *data, QGuestAllocator *t_alloc) } do_attach(v9p); - req = v9fs_twalk(v9p, 0, 1, P9_MAXWELEM, wnames, 0); + req = twalk({ + .client = v9p, .fid = 0, .newfid = 1, + .nwname = P9_MAXWELEM, .wnames = wnames, .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rwalk(req, &nwqid, &wqid); @@ -223,7 +126,7 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; v9fs_set_allocator(t_alloc); - char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_READDIR_DIR) }; + char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_READDIR_DIR) }; uint16_t nqid; v9fs_qid qid; uint32_t count, nentries; @@ -231,7 +134,10 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc) P9Req *req; do_attach(v9p); - req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0); + req = twalk({ + .client = v9p, .fid = 0, .newfid = 1, + .nwname = 1, .wnames = wnames, .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rwalk(req, &nqid, NULL); g_assert_cmpint(nqid, ==, 1); @@ -275,7 +181,7 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc) /* readdir test where overall request is split over several messages */ static void do_readdir_split(QVirtio9P *v9p, uint32_t count) { - char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_READDIR_DIR) }; + char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_READDIR_DIR) }; uint16_t nqid; v9fs_qid qid; uint32_t nentries, npartialentries; @@ -292,7 +198,10 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count) nentries = 0; tail = NULL; - req = v9fs_twalk(v9p, 0, fid, 1, wnames, 0); + req = twalk({ + .client = v9p, .fid = 0, .newfid = fid, + .nwname = 1, .wnames = wnames, .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rwalk(req, &nqid, NULL); g_assert_cmpint(nqid, ==, 1); @@ -356,12 +265,15 @@ static void fs_walk_no_slash(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; v9fs_set_allocator(t_alloc); - char *const wnames[] = { g_strdup(" /") }; + char *wnames[] = { g_strdup(" /") }; P9Req *req; uint32_t err; do_attach(v9p); - req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0); + req = twalk({ + .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, + .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rlerror(req, &err); @@ -380,7 +292,7 @@ static void fs_walk_nonexistent(void *obj, void *data, QGuestAllocator *t_alloc) * The 9p2000 protocol spec says: "If the first element cannot be walked * for any reason, Rerror is returned." */ - do_walk_expect_error(v9p, "non-existent", ENOENT); + twalk({ .client = v9p, .path = "non-existent", .expectErr = ENOENT }); } static void fs_walk_2nd_nonexistent(void *obj, void *data, @@ -398,7 +310,10 @@ static void fs_walk_2nd_nonexistent(void *obj, void *data, ); do_attach_rqid(v9p, &root_qid); - fid = do_walk_rqids(v9p, path, &nwqid, &wqid); + fid = twalk({ + .client = v9p, .path = path, + .rwalk.nwqid = &nwqid, .rwalk.wqid = &wqid + }).newfid; /* * The 9p2000 protocol spec says: "nwqid is therefore either nwname or the * index of the first elementwise walk that failed." @@ -430,7 +345,10 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) v9fs_req_wait_for_reply(req, NULL); v9fs_rattach(req, &root_qid); - req = v9fs_twalk(v9p, 0, 1, 0, NULL, 0); + req = twalk({ + .client = v9p, .fid = 0, .newfid = 1, .nwname = 0, .wnames = NULL, + .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rwalk(req, NULL, &wqid); @@ -448,7 +366,7 @@ static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; v9fs_set_allocator(t_alloc); - char *const wnames[] = { g_strdup("..") }; + char *wnames[] = { g_strdup("..") }; v9fs_qid root_qid; g_autofree v9fs_qid *wqid = NULL; P9Req *req; @@ -458,7 +376,10 @@ static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc) v9fs_req_wait_for_reply(req, NULL); v9fs_rattach(req, &root_qid); - req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0); + req = twalk({ + .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, + .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rwalk(req, NULL, &wqid); /* We now we'll get one qid */ @@ -471,11 +392,14 @@ static void fs_lopen(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; v9fs_set_allocator(t_alloc); - char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_LOPEN_FILE) }; + char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_LOPEN_FILE) }; P9Req *req; do_attach(v9p); - req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0); + req = twalk({ + .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, + .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rwalk(req, NULL, NULL); @@ -491,13 +415,16 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc) QVirtio9P *v9p = obj; v9fs_set_allocator(t_alloc); static const uint32_t write_count = P9_MAX_SIZE / 2; - char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_WRITE_FILE) }; + char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_WRITE_FILE) }; g_autofree char *buf = g_malloc0(write_count); uint32_t count; P9Req *req; do_attach(v9p); - req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0); + req = twalk({ + .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, + .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rwalk(req, NULL, NULL); @@ -517,13 +444,16 @@ static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; v9fs_set_allocator(t_alloc); - char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_FLUSH_FILE) }; + char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_FLUSH_FILE) }; P9Req *req, *flush_req; uint32_t reply_len; uint8_t should_block; do_attach(v9p); - req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0); + req = twalk({ + .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, + .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rwalk(req, NULL, NULL); @@ -554,13 +484,16 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; v9fs_set_allocator(t_alloc); - char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_FLUSH_FILE) }; + char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_FLUSH_FILE) }; P9Req *req, *flush_req; uint32_t count; uint8_t should_block; do_attach(v9p); - req = v9fs_twalk(v9p, 0, 1, 1, wnames, 0); + req = twalk({ + .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, + .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rwalk(req, NULL, NULL); @@ -593,7 +526,7 @@ static void do_mkdir(QVirtio9P *v9p, const char *path, const char *cname) uint32_t fid; P9Req *req; - fid = do_walk(v9p, path); + fid = twalk({ .client = v9p, .path = path }).newfid; req = v9fs_tmkdir(v9p, fid, name, 0750, 0, 0); v9fs_req_wait_for_reply(req, NULL); @@ -608,7 +541,7 @@ static uint32_t do_lcreate(QVirtio9P *v9p, const char *path, uint32_t fid; P9Req *req; - fid = do_walk(v9p, path); + fid = twalk({ .client = v9p, .path = path }).newfid; req = v9fs_tlcreate(v9p, fid, name, 0, 0750, 0, 0); v9fs_req_wait_for_reply(req, NULL); @@ -626,7 +559,7 @@ static void do_symlink(QVirtio9P *v9p, const char *path, const char *clink, uint32_t fid; P9Req *req; - fid = do_walk(v9p, path); + fid = twalk({ .client = v9p, .path = path }).newfid; req = v9fs_tsymlink(v9p, fid, name, dst, 0, 0); v9fs_req_wait_for_reply(req, NULL); @@ -640,8 +573,8 @@ static void do_hardlink(QVirtio9P *v9p, const char *path, const char *clink, uint32_t dfid, fid; P9Req *req; - dfid = do_walk(v9p, path); - fid = do_walk(v9p, to); + dfid = twalk({ .client = v9p, .path = path }).newfid; + fid = twalk({ .client = v9p, .path = to }).newfid; req = v9fs_tlink(v9p, dfid, fid, clink, 0); v9fs_req_wait_for_reply(req, NULL); @@ -655,7 +588,7 @@ static void do_unlinkat(QVirtio9P *v9p, const char *atpath, const char *rpath, uint32_t fid; P9Req *req; - fid = do_walk(v9p, atpath); + fid = twalk({ .client = v9p, .path = atpath }).newfid; req = v9fs_tunlinkat(v9p, fid, name, flags, 0); v9fs_req_wait_for_reply(req, NULL); From 3f3e9232207fc4f0e5cb5cf63f11ed9449efeefa Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:53:29 +0200 Subject: [PATCH 05/23] tests/9p: simplify callers of twalk() Now as twalk() is using a declarative approach, simplify the code of callers of this function. Signed-off-by: Christian Schoenebeck Message-Id: <8b9d3c656ad43b6c953d6bdacd8d9f4c8e599b2a.1664917004.git.qemu_oss@crudebyte.com> --- tests/qtest/virtio-9p-test.c | 92 +++++++++++++----------------------- 1 file changed, 32 insertions(+), 60 deletions(-) diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index cf5d6146ad..3c326451b1 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -90,19 +90,17 @@ static void fs_walk(void *obj, void *data, QGuestAllocator *t_alloc) uint16_t nwqid; g_autofree v9fs_qid *wqid = NULL; int i; - P9Req *req; for (i = 0; i < P9_MAXWELEM; i++) { wnames[i] = g_strdup_printf(QTEST_V9FS_SYNTH_WALK_FILE, i); } do_attach(v9p); - req = twalk({ + twalk({ .client = v9p, .fid = 0, .newfid = 1, - .nwname = P9_MAXWELEM, .wnames = wnames, .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rwalk(req, &nwqid, &wqid); + .nwname = P9_MAXWELEM, .wnames = wnames, + .rwalk = { .nwqid = &nwqid, .wqid = &wqid } + }); g_assert_cmpint(nwqid, ==, P9_MAXWELEM); @@ -134,12 +132,10 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc) P9Req *req; do_attach(v9p); - req = twalk({ + twalk({ .client = v9p, .fid = 0, .newfid = 1, - .nwname = 1, .wnames = wnames, .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rwalk(req, &nqid, NULL); + .nwname = 1, .wnames = wnames, .rwalk.nwqid = &nqid + }); g_assert_cmpint(nqid, ==, 1); req = v9fs_tlopen(v9p, 1, O_DIRECTORY, 0); @@ -198,12 +194,10 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count) nentries = 0; tail = NULL; - req = twalk({ + twalk({ .client = v9p, .fid = 0, .newfid = fid, - .nwname = 1, .wnames = wnames, .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rwalk(req, &nqid, NULL); + .nwname = 1, .wnames = wnames, .rwalk.nwqid = &nqid + }); g_assert_cmpint(nqid, ==, 1); req = v9fs_tlopen(v9p, fid, O_DIRECTORY, 0); @@ -266,18 +260,12 @@ static void fs_walk_no_slash(void *obj, void *data, QGuestAllocator *t_alloc) QVirtio9P *v9p = obj; v9fs_set_allocator(t_alloc); char *wnames[] = { g_strdup(" /") }; - P9Req *req; - uint32_t err; do_attach(v9p); - req = twalk({ + twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, - .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rlerror(req, &err); - - g_assert_cmpint(err, ==, ENOENT); + .expectErr = ENOENT + }); g_free(wnames[0]); } @@ -312,7 +300,7 @@ static void fs_walk_2nd_nonexistent(void *obj, void *data, do_attach_rqid(v9p, &root_qid); fid = twalk({ .client = v9p, .path = path, - .rwalk.nwqid = &nwqid, .rwalk.wqid = &wqid + .rwalk = { .nwqid = &nwqid, .wqid = &wqid } }).newfid; /* * The 9p2000 protocol spec says: "nwqid is therefore either nwname or the @@ -345,12 +333,10 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) v9fs_req_wait_for_reply(req, NULL); v9fs_rattach(req, &root_qid); - req = twalk({ + twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = 0, .wnames = NULL, - .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rwalk(req, NULL, &wqid); + .rwalk.wqid = &wqid + }); /* special case: no QID is returned if nwname=0 was sent */ g_assert(wqid == NULL); @@ -376,12 +362,10 @@ static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc) v9fs_req_wait_for_reply(req, NULL); v9fs_rattach(req, &root_qid); - req = twalk({ + twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, - .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rwalk(req, NULL, &wqid); /* We now we'll get one qid */ + .rwalk.wqid = &wqid /* We now we'll get one qid */ + }); g_assert_cmpmem(&root_qid, 13, wqid[0], 13); @@ -396,12 +380,9 @@ static void fs_lopen(void *obj, void *data, QGuestAllocator *t_alloc) P9Req *req; do_attach(v9p); - req = twalk({ - .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, - .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rwalk(req, NULL, NULL); + twalk({ + .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames + }); req = v9fs_tlopen(v9p, 1, O_WRONLY, 0); v9fs_req_wait_for_reply(req, NULL); @@ -421,12 +402,9 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc) P9Req *req; do_attach(v9p); - req = twalk({ - .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, - .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rwalk(req, NULL, NULL); + twalk({ + .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames + }); req = v9fs_tlopen(v9p, 1, O_WRONLY, 0); v9fs_req_wait_for_reply(req, NULL); @@ -450,12 +428,9 @@ static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc) uint8_t should_block; do_attach(v9p); - req = twalk({ - .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, - .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rwalk(req, NULL, NULL); + twalk({ + .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames + }); req = v9fs_tlopen(v9p, 1, O_WRONLY, 0); v9fs_req_wait_for_reply(req, NULL); @@ -490,12 +465,9 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc) uint8_t should_block; do_attach(v9p); - req = twalk({ - .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, - .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rwalk(req, NULL, NULL); + twalk({ + .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames + }); req = v9fs_tlopen(v9p, 1, O_WRONLY, 0); v9fs_req_wait_for_reply(req, NULL); From bee8fda2f9841cc8d76fbe60f65f9aeb29fce1c2 Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:53:33 +0200 Subject: [PATCH 06/23] tests/9p: merge v9fs_tversion() and do_version() As with previous patches, unify functions v9fs_tversion() and do_version() into a single function v9fs_tversion() by using a declarative function arguments approach. Signed-off-by: Christian Schoenebeck Message-Id: <2d253491aaffd267ec295f056dda47456692cd0c.1664917004.git.qemu_oss@crudebyte.com> --- tests/qtest/libqos/virtio-9p-client.c | 47 +++++++++++++++++++++++---- tests/qtest/libqos/virtio-9p-client.h | 25 ++++++++++++-- tests/qtest/virtio-9p-test.c | 23 +++---------- 3 files changed, 68 insertions(+), 27 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index a95bbad9c8..e8364f8d64 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -291,21 +291,54 @@ void v9fs_rlerror(P9Req *req, uint32_t *err) } /* size[4] Tversion tag[2] msize[4] version[s] */ -P9Req *v9fs_tversion(QVirtio9P *v9p, uint32_t msize, const char *version, - uint16_t tag) +TVersionRes v9fs_tversion(TVersionOpt opt) { P9Req *req; + uint32_t err; uint32_t body_size = 4; - uint16_t string_size = v9fs_string_size(version); + uint16_t string_size; + uint16_t server_len; + g_autofree char *server_version = NULL; + g_assert(opt.client); + + if (!opt.msize) { + opt.msize = P9_MAX_SIZE; + } + + if (!opt.tag) { + opt.tag = P9_NOTAG; + } + + if (!opt.version) { + opt.version = "9P2000.L"; + } + + string_size = v9fs_string_size(opt.version); g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); body_size += string_size; - req = v9fs_req_init(v9p, body_size, P9_TVERSION, tag); + req = v9fs_req_init(opt.client, body_size, P9_TVERSION, opt.tag); - v9fs_uint32_write(req, msize); - v9fs_string_write(req, version); + v9fs_uint32_write(req, opt.msize); + v9fs_string_write(req, opt.version); v9fs_req_send(req); - return req; + + if (!opt.requestOnly) { + v9fs_req_wait_for_reply(req, NULL); + if (opt.expectErr) { + v9fs_rlerror(req, &err); + g_assert_cmpint(err, ==, opt.expectErr); + } else { + v9fs_rversion(req, &server_len, &server_version); + g_assert_cmpmem(server_version, server_len, + opt.version, strlen(opt.version)); + } + req = NULL; /* request was freed */ + } + + return (TVersionRes) { + .req = req, + }; } /* size[4] Rversion tag[2] msize[4] version[s] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index 8c6abbb173..fcde849b5d 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -106,6 +106,28 @@ typedef struct TWalkRes { P9Req *req; } TWalkRes; +/* options for 'Tversion' 9p request */ +typedef struct TVersionOpt { + /* 9P client being used (mandatory) */ + QVirtio9P *client; + /* user supplied tag number being returned with response (optional) */ + uint16_t tag; + /* maximum message size that can be handled by client (optional) */ + uint32_t msize; + /* protocol version (optional) */ + const char *version; + /* only send Tversion request but not wait for a reply? (optional) */ + bool requestOnly; + /* do we expect an Rlerror response, if yes which error code? (optional) */ + uint32_t expectErr; +} TVersionOpt; + +/* result of 'Tversion' 9p request */ +typedef struct TVersionRes { + /* if requestOnly was set: request object for further processing */ + P9Req *req; +} TVersionRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -127,8 +149,7 @@ void v9fs_req_wait_for_reply(P9Req *req, uint32_t *len); void v9fs_req_recv(P9Req *req, uint8_t id); void v9fs_req_free(P9Req *req); void v9fs_rlerror(P9Req *req, uint32_t *err); -P9Req *v9fs_tversion(QVirtio9P *v9p, uint32_t msize, const char *version, - uint16_t tag); +TVersionRes v9fs_tversion(TVersionOpt); void v9fs_rversion(P9Req *req, uint16_t *len, char **version); P9Req *v9fs_tattach(QVirtio9P *v9p, uint32_t fid, uint32_t n_uname, uint16_t tag); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 3c326451b1..f2907c8026 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -17,6 +17,7 @@ #include "libqos/virtio-9p-client.h" #define twalk(...) v9fs_twalk((TWalkOpt) __VA_ARGS__) +#define tversion(...) v9fs_tversion((TVersionOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -41,31 +42,17 @@ static inline bool is_same_qid(v9fs_qid a, v9fs_qid b) return a[0] == b[0] && memcmp(&a[5], &b[5], 8) == 0; } -static void do_version(QVirtio9P *v9p) -{ - const char *version = "9P2000.L"; - uint16_t server_len; - g_autofree char *server_version = NULL; - P9Req *req; - - req = v9fs_tversion(v9p, P9_MAX_SIZE, version, P9_NOTAG); - v9fs_req_wait_for_reply(req, NULL); - v9fs_rversion(req, &server_len, &server_version); - - g_assert_cmpmem(server_version, server_len, version, strlen(version)); -} - static void fs_version(void *obj, void *data, QGuestAllocator *t_alloc) { v9fs_set_allocator(t_alloc); - do_version(obj); + tversion({ .client = obj }); } static void do_attach_rqid(QVirtio9P *v9p, v9fs_qid *qid) { P9Req *req; - do_version(v9p); + tversion({ .client = v9p }); req = v9fs_tattach(v9p, 0, getuid(), 0); v9fs_req_wait_for_reply(req, NULL); v9fs_rattach(req, qid); @@ -328,7 +315,7 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) P9Req *req; struct v9fs_attr attr; - do_version(v9p); + tversion({ .client = v9p }); req = v9fs_tattach(v9p, 0, getuid(), 0); v9fs_req_wait_for_reply(req, NULL); v9fs_rattach(req, &root_qid); @@ -357,7 +344,7 @@ static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc) g_autofree v9fs_qid *wqid = NULL; P9Req *req; - do_version(v9p); + tversion({ .client = v9p }); req = v9fs_tattach(v9p, 0, getuid(), 0); v9fs_req_wait_for_reply(req, NULL); v9fs_rattach(req, &root_qid); From 74a160aba921a2ca37b026dbfcdd03386bc05e85 Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:53:36 +0200 Subject: [PATCH 07/23] tests/9p: merge v9fs_tattach(), do_attach(), do_attach_rqid() As with previous patches, unify those 3 functions into a single function v9fs_tattach() by using a declarative function arguments approach. Signed-off-by: Christian Schoenebeck Message-Id: --- tests/qtest/libqos/virtio-9p-client.c | 40 ++++++++++++++--- tests/qtest/libqos/virtio-9p-client.h | 30 ++++++++++++- tests/qtest/virtio-9p-test.c | 62 +++++++++++---------------- 3 files changed, 88 insertions(+), 44 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index e8364f8d64..5e6bd6120c 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -359,20 +359,48 @@ void v9fs_rversion(P9Req *req, uint16_t *len, char **version) } /* size[4] Tattach tag[2] fid[4] afid[4] uname[s] aname[s] n_uname[4] */ -P9Req *v9fs_tattach(QVirtio9P *v9p, uint32_t fid, uint32_t n_uname, - uint16_t tag) +TAttachRes v9fs_tattach(TAttachOpt opt) { + uint32_t err; const char *uname = ""; /* ignored by QEMU */ const char *aname = ""; /* ignored by QEMU */ - P9Req *req = v9fs_req_init(v9p, 4 + 4 + 2 + 2 + 4, P9_TATTACH, tag); - v9fs_uint32_write(req, fid); + g_assert(opt.client); + /* expecting either Rattach or Rlerror, but obviously not both */ + g_assert(!opt.expectErr || !opt.rattach.qid); + + if (!opt.requestOnly) { + v9fs_tversion((TVersionOpt) { .client = opt.client }); + } + + if (!opt.n_uname) { + opt.n_uname = getuid(); + } + + P9Req *req = v9fs_req_init(opt.client, 4 + 4 + 2 + 2 + 4, P9_TATTACH, + opt.tag); + + v9fs_uint32_write(req, opt.fid); v9fs_uint32_write(req, P9_NOFID); v9fs_string_write(req, uname); v9fs_string_write(req, aname); - v9fs_uint32_write(req, n_uname); + v9fs_uint32_write(req, opt.n_uname); v9fs_req_send(req); - return req; + + if (!opt.requestOnly) { + v9fs_req_wait_for_reply(req, NULL); + if (opt.expectErr) { + v9fs_rlerror(req, &err); + g_assert_cmpint(err, ==, opt.expectErr); + } else { + v9fs_rattach(req, opt.rattach.qid); + } + req = NULL; /* request was freed */ + } + + return (TAttachRes) { + .req = req, + }; } /* size[4] Rattach tag[2] qid[13] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index fcde849b5d..64b97b229b 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -128,6 +128,33 @@ typedef struct TVersionRes { P9Req *req; } TVersionRes; +/* options for 'Tattach' 9p request */ +typedef struct TAttachOpt { + /* 9P client being used (mandatory) */ + QVirtio9P *client; + /* user supplied tag number being returned with response (optional) */ + uint16_t tag; + /* file ID to be associated with root of file tree (optional) */ + uint32_t fid; + /* numerical uid of user being introduced to server (optional) */ + uint32_t n_uname; + /* data being received from 9p server as 'Rattach' response (optional) */ + struct { + /* server's idea of the root of the file tree */ + v9fs_qid *qid; + } rattach; + /* only send Tattach request but not wait for a reply? (optional) */ + bool requestOnly; + /* do we expect an Rlerror response, if yes which error code? (optional) */ + uint32_t expectErr; +} TAttachOpt; + +/* result of 'Tattach' 9p request */ +typedef struct TAttachRes { + /* if requestOnly was set: request object for further processing */ + P9Req *req; +} TAttachRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -151,8 +178,7 @@ void v9fs_req_free(P9Req *req); void v9fs_rlerror(P9Req *req, uint32_t *err); TVersionRes v9fs_tversion(TVersionOpt); void v9fs_rversion(P9Req *req, uint16_t *len, char **version); -P9Req *v9fs_tattach(QVirtio9P *v9p, uint32_t fid, uint32_t n_uname, - uint16_t tag); +TAttachRes v9fs_tattach(TAttachOpt); void v9fs_rattach(P9Req *req, v9fs_qid *qid); TWalkRes v9fs_twalk(TWalkOpt opt); void v9fs_rwalk(P9Req *req, uint16_t *nwqid, v9fs_qid **wqid); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index f2907c8026..271c42f6f9 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -18,6 +18,7 @@ #define twalk(...) v9fs_twalk((TWalkOpt) __VA_ARGS__) #define tversion(...) v9fs_tversion((TVersionOpt) __VA_ARGS__) +#define tattach(...) v9fs_tattach((TAttachOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -48,25 +49,10 @@ static void fs_version(void *obj, void *data, QGuestAllocator *t_alloc) tversion({ .client = obj }); } -static void do_attach_rqid(QVirtio9P *v9p, v9fs_qid *qid) -{ - P9Req *req; - - tversion({ .client = v9p }); - req = v9fs_tattach(v9p, 0, getuid(), 0); - v9fs_req_wait_for_reply(req, NULL); - v9fs_rattach(req, qid); -} - -static void do_attach(QVirtio9P *v9p) -{ - do_attach_rqid(v9p, NULL); -} - static void fs_attach(void *obj, void *data, QGuestAllocator *t_alloc) { v9fs_set_allocator(t_alloc); - do_attach(obj); + tattach({ .client = obj }); } static void fs_walk(void *obj, void *data, QGuestAllocator *t_alloc) @@ -82,7 +68,7 @@ static void fs_walk(void *obj, void *data, QGuestAllocator *t_alloc) wnames[i] = g_strdup_printf(QTEST_V9FS_SYNTH_WALK_FILE, i); } - do_attach(v9p); + tattach({ .client = v9p }); twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = P9_MAXWELEM, .wnames = wnames, @@ -118,7 +104,7 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc) struct V9fsDirent *entries = NULL; P9Req *req; - do_attach(v9p); + tattach({ .client = v9p }); twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, .rwalk.nwqid = &nqid @@ -173,7 +159,7 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count) int fid; uint64_t offset; - do_attach(v9p); + tattach({ .client = v9p }); fid = 1; offset = 0; @@ -248,7 +234,7 @@ static void fs_walk_no_slash(void *obj, void *data, QGuestAllocator *t_alloc) v9fs_set_allocator(t_alloc); char *wnames[] = { g_strdup(" /") }; - do_attach(v9p); + tattach({ .client = v9p }); twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, .expectErr = ENOENT @@ -262,7 +248,7 @@ static void fs_walk_nonexistent(void *obj, void *data, QGuestAllocator *t_alloc) QVirtio9P *v9p = obj; v9fs_set_allocator(t_alloc); - do_attach(v9p); + tattach({ .client = v9p }); /* * The 9p2000 protocol spec says: "If the first element cannot be walked * for any reason, Rerror is returned." @@ -284,7 +270,7 @@ static void fs_walk_2nd_nonexistent(void *obj, void *data, QTEST_V9FS_SYNTH_WALK_FILE "/non-existent", 0 ); - do_attach_rqid(v9p, &root_qid); + tattach({ .client = v9p, .rattach.qid = &root_qid }); fid = twalk({ .client = v9p, .path = path, .rwalk = { .nwqid = &nwqid, .wqid = &wqid } @@ -316,7 +302,9 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) struct v9fs_attr attr; tversion({ .client = v9p }); - req = v9fs_tattach(v9p, 0, getuid(), 0); + req = tattach({ + .client = v9p, .fid = 0, .n_uname = getuid(), .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rattach(req, &root_qid); @@ -345,7 +333,9 @@ static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc) P9Req *req; tversion({ .client = v9p }); - req = v9fs_tattach(v9p, 0, getuid(), 0); + req = tattach((TAttachOpt) { + .client = v9p, .fid = 0, .n_uname = getuid(), .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rattach(req, &root_qid); @@ -366,7 +356,7 @@ static void fs_lopen(void *obj, void *data, QGuestAllocator *t_alloc) char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_LOPEN_FILE) }; P9Req *req; - do_attach(v9p); + tattach({ .client = v9p }); twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames }); @@ -388,7 +378,7 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc) uint32_t count; P9Req *req; - do_attach(v9p); + tattach({ .client = v9p }); twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames }); @@ -414,7 +404,7 @@ static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc) uint32_t reply_len; uint8_t should_block; - do_attach(v9p); + tattach({ .client = v9p }); twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames }); @@ -451,7 +441,7 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc) uint32_t count; uint8_t should_block; - do_attach(v9p); + tattach({ .client = v9p }); twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames }); @@ -588,7 +578,7 @@ static void fs_create_dir(void *obj, void *data, QGuestAllocator *t_alloc) g_assert(root_path != NULL); - do_attach(v9p); + tattach({ .client = v9p }); do_mkdir(v9p, "/", "01"); /* check if created directory really exists now ... */ @@ -607,7 +597,7 @@ static void fs_unlinkat_dir(void *obj, void *data, QGuestAllocator *t_alloc) g_assert(root_path != NULL); - do_attach(v9p); + tattach({ .client = v9p }); do_mkdir(v9p, "/", "02"); /* check if created directory really exists now ... */ @@ -627,7 +617,7 @@ static void fs_create_file(void *obj, void *data, QGuestAllocator *t_alloc) struct stat st; g_autofree char *new_file = virtio_9p_test_path("03/1st_file"); - do_attach(v9p); + tattach({ .client = v9p }); do_mkdir(v9p, "/", "03"); do_lcreate(v9p, "03", "1st_file"); @@ -644,7 +634,7 @@ static void fs_unlinkat_file(void *obj, void *data, QGuestAllocator *t_alloc) struct stat st; g_autofree char *new_file = virtio_9p_test_path("04/doa_file"); - do_attach(v9p); + tattach({ .client = v9p }); do_mkdir(v9p, "/", "04"); do_lcreate(v9p, "04", "doa_file"); @@ -666,7 +656,7 @@ static void fs_symlink_file(void *obj, void *data, QGuestAllocator *t_alloc) g_autofree char *real_file = virtio_9p_test_path("05/real_file"); g_autofree char *symlink_file = virtio_9p_test_path("05/symlink_file"); - do_attach(v9p); + tattach({ .client = v9p }); do_mkdir(v9p, "/", "05"); do_lcreate(v9p, "05", "real_file"); g_assert(stat(real_file, &st) == 0); @@ -687,7 +677,7 @@ static void fs_unlinkat_symlink(void *obj, void *data, g_autofree char *real_file = virtio_9p_test_path("06/real_file"); g_autofree char *symlink_file = virtio_9p_test_path("06/symlink_file"); - do_attach(v9p); + tattach({ .client = v9p }); do_mkdir(v9p, "/", "06"); do_lcreate(v9p, "06", "real_file"); g_assert(stat(real_file, &st) == 0); @@ -709,7 +699,7 @@ static void fs_hardlink_file(void *obj, void *data, QGuestAllocator *t_alloc) g_autofree char *real_file = virtio_9p_test_path("07/real_file"); g_autofree char *hardlink_file = virtio_9p_test_path("07/hardlink_file"); - do_attach(v9p); + tattach({ .client = v9p }); do_mkdir(v9p, "/", "07"); do_lcreate(v9p, "07", "real_file"); g_assert(stat(real_file, &st_real) == 0); @@ -734,7 +724,7 @@ static void fs_unlinkat_hardlink(void *obj, void *data, g_autofree char *real_file = virtio_9p_test_path("08/real_file"); g_autofree char *hardlink_file = virtio_9p_test_path("08/hardlink_file"); - do_attach(v9p); + tattach({ .client = v9p }); do_mkdir(v9p, "/", "08"); do_lcreate(v9p, "08", "real_file"); g_assert(stat(real_file, &st_real) == 0); From 1125ddf66f47dc4986d97948253890fdb3c0a6d6 Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:53:38 +0200 Subject: [PATCH 08/23] tests/9p: simplify callers of tattach() Now as tattach() is using a declarative approach, simplify the code of callers of this function. Signed-off-by: Christian Schoenebeck Message-Id: <9b50e5b89a0072e84a9191d18c19a53546a28bba.1664917004.git.qemu_oss@crudebyte.com> --- tests/qtest/virtio-9p-test.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 271c42f6f9..46bb189b81 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -302,11 +302,10 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) struct v9fs_attr attr; tversion({ .client = v9p }); - req = tattach({ - .client = v9p, .fid = 0, .n_uname = getuid(), .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rattach(req, &root_qid); + tattach({ + .client = v9p, .fid = 0, .n_uname = getuid(), + .rattach.qid = &root_qid + }); twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = 0, .wnames = NULL, @@ -330,14 +329,12 @@ static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc) char *wnames[] = { g_strdup("..") }; v9fs_qid root_qid; g_autofree v9fs_qid *wqid = NULL; - P9Req *req; tversion({ .client = v9p }); - req = tattach((TAttachOpt) { - .client = v9p, .fid = 0, .n_uname = getuid(), .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rattach(req, &root_qid); + tattach({ + .client = v9p, .fid = 0, .n_uname = getuid(), + .rattach.qid = &root_qid + }); twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, From 2af5be47b9ba264f31f5594e587207cd854e01cc Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:53:41 +0200 Subject: [PATCH 09/23] tests/9p: convert v9fs_tgetattr() to declarative arguments Use declarative function arguments for function v9fs_tgetattr(). Signed-off-by: Christian Schoenebeck Message-Id: --- tests/qtest/libqos/virtio-9p-client.c | 32 ++++++++++++++++++++++----- tests/qtest/libqos/virtio-9p-client.h | 30 +++++++++++++++++++++++-- tests/qtest/virtio-9p-test.c | 11 +++++++-- 3 files changed, 63 insertions(+), 10 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index 5e6bd6120c..29916a23b5 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -489,16 +489,36 @@ void v9fs_rwalk(P9Req *req, uint16_t *nwqid, v9fs_qid **wqid) } /* size[4] Tgetattr tag[2] fid[4] request_mask[8] */ -P9Req *v9fs_tgetattr(QVirtio9P *v9p, uint32_t fid, uint64_t request_mask, - uint16_t tag) +TGetAttrRes v9fs_tgetattr(TGetAttrOpt opt) { P9Req *req; + uint32_t err; - req = v9fs_req_init(v9p, 4 + 8, P9_TGETATTR, tag); - v9fs_uint32_write(req, fid); - v9fs_uint64_write(req, request_mask); + g_assert(opt.client); + /* expecting either Rgetattr or Rlerror, but obviously not both */ + g_assert(!opt.expectErr || !opt.rgetattr.attr); + + if (!opt.request_mask) { + opt.request_mask = P9_GETATTR_ALL; + } + + req = v9fs_req_init(opt.client, 4 + 8, P9_TGETATTR, opt.tag); + v9fs_uint32_write(req, opt.fid); + v9fs_uint64_write(req, opt.request_mask); v9fs_req_send(req); - return req; + + if (!opt.requestOnly) { + v9fs_req_wait_for_reply(req, NULL); + if (opt.expectErr) { + v9fs_rlerror(req, &err); + g_assert_cmpint(err, ==, opt.expectErr); + } else { + v9fs_rgetattr(req, opt.rgetattr.attr); + } + req = NULL; /* request was freed */ + } + + return (TGetAttrRes) { .req = req }; } /* diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index 64b97b229b..f7b1bfc79a 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -63,6 +63,7 @@ typedef struct v9fs_attr { } v9fs_attr; #define P9_GETATTR_BASIC 0x000007ffULL /* Mask for fields up to BLOCKS */ +#define P9_GETATTR_ALL 0x00003fffULL /* Mask for ALL fields */ struct V9fsDirent { v9fs_qid qid; @@ -155,6 +156,32 @@ typedef struct TAttachRes { P9Req *req; } TAttachRes; +/* options for 'Tgetattr' 9p request */ +typedef struct TGetAttrOpt { + /* 9P client being used (mandatory) */ + QVirtio9P *client; + /* user supplied tag number being returned with response (optional) */ + uint16_t tag; + /* file ID of file/dir whose attributes shall be retrieved (required) */ + uint32_t fid; + /* bitmask indicating attribute fields to be retrieved (optional) */ + uint64_t request_mask; + /* data being received from 9p server as 'Rgetattr' response (optional) */ + struct { + v9fs_attr *attr; + } rgetattr; + /* only send Tgetattr request but not wait for a reply? (optional) */ + bool requestOnly; + /* do we expect an Rlerror response, if yes which error code? (optional) */ + uint32_t expectErr; +} TGetAttrOpt; + +/* result of 'Tgetattr' 9p request */ +typedef struct TGetAttrRes { + /* if requestOnly was set: request object for further processing */ + P9Req *req; +} TGetAttrRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -182,8 +209,7 @@ TAttachRes v9fs_tattach(TAttachOpt); void v9fs_rattach(P9Req *req, v9fs_qid *qid); TWalkRes v9fs_twalk(TWalkOpt opt); void v9fs_rwalk(P9Req *req, uint16_t *nwqid, v9fs_qid **wqid); -P9Req *v9fs_tgetattr(QVirtio9P *v9p, uint32_t fid, uint64_t request_mask, - uint16_t tag); +TGetAttrRes v9fs_tgetattr(TGetAttrOpt); void v9fs_rgetattr(P9Req *req, v9fs_attr *attr); P9Req *v9fs_treaddir(QVirtio9P *v9p, uint32_t fid, uint64_t offset, uint32_t count, uint16_t tag); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 46bb189b81..9c1219db33 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -19,6 +19,7 @@ #define twalk(...) v9fs_twalk((TWalkOpt) __VA_ARGS__) #define tversion(...) v9fs_tversion((TVersionOpt) __VA_ARGS__) #define tattach(...) v9fs_tattach((TAttachOpt) __VA_ARGS__) +#define tgetattr(...) v9fs_tgetattr((TGetAttrOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -285,7 +286,10 @@ static void fs_walk_2nd_nonexistent(void *obj, void *data, g_assert(wqid && wqid[0] && !is_same_qid(root_qid, wqid[0])); /* expect fid being unaffected by walk above */ - req = v9fs_tgetattr(v9p, fid, P9_GETATTR_BASIC, 0); + req = tgetattr({ + .client = v9p, .fid = fid, .request_mask = P9_GETATTR_BASIC, + .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rlerror(req, &err); @@ -315,7 +319,10 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) /* special case: no QID is returned if nwname=0 was sent */ g_assert(wqid == NULL); - req = v9fs_tgetattr(v9p, 1, P9_GETATTR_BASIC, 0); + req = tgetattr({ + .client = v9p, .fid = 1, .request_mask = P9_GETATTR_BASIC, + .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rgetattr(req, &attr); From 28c736709b82c8f47edf3cb18b9fb601fdab9151 Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:53:44 +0200 Subject: [PATCH 10/23] tests/9p: simplify callers of tgetattr() Now as tgetattr() is using a declarative approach, simplify the code of callers of this function. Signed-off-by: Christian Schoenebeck Message-Id: <60c6a083f320b86f3172951445df7bbc895932e2.1664917004.git.qemu_oss@crudebyte.com> --- tests/qtest/virtio-9p-test.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 9c1219db33..ae1220d0cb 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -264,8 +264,7 @@ static void fs_walk_2nd_nonexistent(void *obj, void *data, v9fs_set_allocator(t_alloc); v9fs_qid root_qid; uint16_t nwqid; - uint32_t fid, err; - P9Req *req; + uint32_t fid; g_autofree v9fs_qid *wqid = NULL; g_autofree char *path = g_strdup_printf( QTEST_V9FS_SYNTH_WALK_FILE "/non-existent", 0 @@ -286,14 +285,10 @@ static void fs_walk_2nd_nonexistent(void *obj, void *data, g_assert(wqid && wqid[0] && !is_same_qid(root_qid, wqid[0])); /* expect fid being unaffected by walk above */ - req = tgetattr({ + tgetattr({ .client = v9p, .fid = fid, .request_mask = P9_GETATTR_BASIC, - .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rlerror(req, &err); - - g_assert_cmpint(err, ==, ENOENT); + .expectErr = ENOENT + }); } static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) @@ -302,7 +297,6 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) v9fs_set_allocator(t_alloc); v9fs_qid root_qid; g_autofree v9fs_qid *wqid = NULL; - P9Req *req; struct v9fs_attr attr; tversion({ .client = v9p }); @@ -319,12 +313,10 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) /* special case: no QID is returned if nwname=0 was sent */ g_assert(wqid == NULL); - req = tgetattr({ + tgetattr({ .client = v9p, .fid = 1, .request_mask = P9_GETATTR_BASIC, - .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rgetattr(req, &attr); + .rgetattr.attr = &attr + }); g_assert(is_same_qid(root_qid, attr.qid)); } From 1ebacc40ca925626cf601543326066434c7c1a7f Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:53:47 +0200 Subject: [PATCH 11/23] tests/9p: convert v9fs_treaddir() to declarative arguments Use declarative function arguments for function v9fs_treaddir(). Signed-off-by: Christian Schoenebeck Message-Id: --- tests/qtest/libqos/virtio-9p-client.c | 32 ++++++++++++++++++++------ tests/qtest/libqos/virtio-9p-client.h | 33 +++++++++++++++++++++++++-- tests/qtest/virtio-9p-test.c | 11 +++++++-- 3 files changed, 65 insertions(+), 11 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index 29916a23b5..047c8993b6 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -557,17 +557,35 @@ void v9fs_rgetattr(P9Req *req, v9fs_attr *attr) } /* size[4] Treaddir tag[2] fid[4] offset[8] count[4] */ -P9Req *v9fs_treaddir(QVirtio9P *v9p, uint32_t fid, uint64_t offset, - uint32_t count, uint16_t tag) +TReadDirRes v9fs_treaddir(TReadDirOpt opt) { P9Req *req; + uint32_t err; - req = v9fs_req_init(v9p, 4 + 8 + 4, P9_TREADDIR, tag); - v9fs_uint32_write(req, fid); - v9fs_uint64_write(req, offset); - v9fs_uint32_write(req, count); + g_assert(opt.client); + /* expecting either Rreaddir or Rlerror, but obviously not both */ + g_assert(!opt.expectErr || !(opt.rreaddir.count || + opt.rreaddir.nentries || opt.rreaddir.entries)); + + req = v9fs_req_init(opt.client, 4 + 8 + 4, P9_TREADDIR, opt.tag); + v9fs_uint32_write(req, opt.fid); + v9fs_uint64_write(req, opt.offset); + v9fs_uint32_write(req, opt.count); v9fs_req_send(req); - return req; + + if (!opt.requestOnly) { + v9fs_req_wait_for_reply(req, NULL); + if (opt.expectErr) { + v9fs_rlerror(req, &err); + g_assert_cmpint(err, ==, opt.expectErr); + } else { + v9fs_rreaddir(req, opt.rreaddir.count, opt.rreaddir.nentries, + opt.rreaddir.entries); + } + req = NULL; /* request was freed */ + } + + return (TReadDirRes) { .req = req }; } /* size[4] Rreaddir tag[2] count[4] data[count] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index f7b1bfc79a..2bf649085f 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -182,6 +182,36 @@ typedef struct TGetAttrRes { P9Req *req; } TGetAttrRes; +/* options for 'Treaddir' 9p request */ +typedef struct TReadDirOpt { + /* 9P client being used (mandatory) */ + QVirtio9P *client; + /* user supplied tag number being returned with response (optional) */ + uint16_t tag; + /* file ID of directory whose entries shall be retrieved (required) */ + uint32_t fid; + /* offset in entries stream, i.e. for multiple requests (optional) */ + uint64_t offset; + /* maximum bytes to be returned by server (required) */ + uint32_t count; + /* data being received from 9p server as 'Rreaddir' response (optional) */ + struct { + uint32_t *count; + uint32_t *nentries; + struct V9fsDirent **entries; + } rreaddir; + /* only send Treaddir request but not wait for a reply? (optional) */ + bool requestOnly; + /* do we expect an Rlerror response, if yes which error code? (optional) */ + uint32_t expectErr; +} TReadDirOpt; + +/* result of 'Treaddir' 9p request */ +typedef struct TReadDirRes { + /* if requestOnly was set: request object for further processing */ + P9Req *req; +} TReadDirRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -211,8 +241,7 @@ TWalkRes v9fs_twalk(TWalkOpt opt); void v9fs_rwalk(P9Req *req, uint16_t *nwqid, v9fs_qid **wqid); TGetAttrRes v9fs_tgetattr(TGetAttrOpt); void v9fs_rgetattr(P9Req *req, v9fs_attr *attr); -P9Req *v9fs_treaddir(QVirtio9P *v9p, uint32_t fid, uint64_t offset, - uint32_t count, uint16_t tag); +TReadDirRes v9fs_treaddir(TReadDirOpt); void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t *nentries, struct V9fsDirent **entries); void v9fs_free_dirents(struct V9fsDirent *e); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index ae1220d0cb..e5c174c218 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -20,6 +20,7 @@ #define tversion(...) v9fs_tversion((TVersionOpt) __VA_ARGS__) #define tattach(...) v9fs_tattach((TAttachOpt) __VA_ARGS__) #define tgetattr(...) v9fs_tgetattr((TGetAttrOpt) __VA_ARGS__) +#define treaddir(...) v9fs_treaddir((TReadDirOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -119,7 +120,10 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc) /* * submit count = msize - 11, because 11 is the header size of Rreaddir */ - req = v9fs_treaddir(v9p, 1, 0, P9_MAX_SIZE - 11, 0); + req = treaddir({ + .client = v9p, .fid = 1, .offset = 0, .count = P9_MAX_SIZE - 11, + .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rreaddir(req, &count, &nentries, &entries); @@ -186,7 +190,10 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count) npartialentries = 0; partialentries = NULL; - req = v9fs_treaddir(v9p, fid, offset, count, 0); + req = treaddir({ + .client = v9p, .fid = fid, .offset = offset, .count = count, + .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rreaddir(req, &count, &npartialentries, &partialentries); if (npartialentries > 0 && partialentries) { From a9a53769318503c4d8884e642e00603ea6885cb1 Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:53:49 +0200 Subject: [PATCH 12/23] tests/9p: simplify callers of treaddir() Now as treaddir() is using a declarative approach, simplify the code of callers of this function. Signed-off-by: Christian Schoenebeck Message-Id: <7cec6f2c7011a481806c34908893b7282702a7a6.1664917004.git.qemu_oss@crudebyte.com> --- tests/qtest/virtio-9p-test.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index e5c174c218..99e24fce0b 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -120,12 +120,12 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc) /* * submit count = msize - 11, because 11 is the header size of Rreaddir */ - req = treaddir({ + treaddir({ .client = v9p, .fid = 1, .offset = 0, .count = P9_MAX_SIZE - 11, - .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rreaddir(req, &count, &nentries, &entries); + .rreaddir = { + .count = &count, .nentries = &nentries, .entries = &entries + } + }); /* * Assuming msize (P9_MAX_SIZE) is large enough so we can retrieve all @@ -190,12 +190,13 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count) npartialentries = 0; partialentries = NULL; - req = treaddir({ + treaddir({ .client = v9p, .fid = fid, .offset = offset, .count = count, - .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rreaddir(req, &count, &npartialentries, &partialentries); + .rreaddir = { + .count = &count, .nentries = &npartialentries, + .entries = &partialentries + } + }); if (npartialentries > 0 && partialentries) { if (!entries) { entries = partialentries; From 3878ce4cc2f1cb9e802076c827834c34d3788b78 Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:53:52 +0200 Subject: [PATCH 13/23] tests/9p: convert v9fs_tlopen() to declarative arguments Use declarative function arguments for function v9fs_tlopen(). Signed-off-by: Christian Schoenebeck Message-Id: <765ab515353c56f88f0a163631f626a44e9565d6.1664917004.git.qemu_oss@crudebyte.com> --- tests/qtest/libqos/virtio-9p-client.c | 28 +++++++++++++++++++------ tests/qtest/libqos/virtio-9p-client.h | 30 +++++++++++++++++++++++++-- tests/qtest/virtio-9p-test.c | 25 ++++++++++++++++------ 3 files changed, 69 insertions(+), 14 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index 047c8993b6..15fde54d63 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -643,16 +643,32 @@ void v9fs_free_dirents(struct V9fsDirent *e) } /* size[4] Tlopen tag[2] fid[4] flags[4] */ -P9Req *v9fs_tlopen(QVirtio9P *v9p, uint32_t fid, uint32_t flags, - uint16_t tag) +TLOpenRes v9fs_tlopen(TLOpenOpt opt) { P9Req *req; + uint32_t err; - req = v9fs_req_init(v9p, 4 + 4, P9_TLOPEN, tag); - v9fs_uint32_write(req, fid); - v9fs_uint32_write(req, flags); + g_assert(opt.client); + /* expecting either Rlopen or Rlerror, but obviously not both */ + g_assert(!opt.expectErr || !(opt.rlopen.qid || opt.rlopen.iounit)); + + req = v9fs_req_init(opt.client, 4 + 4, P9_TLOPEN, opt.tag); + v9fs_uint32_write(req, opt.fid); + v9fs_uint32_write(req, opt.flags); v9fs_req_send(req); - return req; + + if (!opt.requestOnly) { + v9fs_req_wait_for_reply(req, NULL); + if (opt.expectErr) { + v9fs_rlerror(req, &err); + g_assert_cmpint(err, ==, opt.expectErr); + } else { + v9fs_rlopen(req, opt.rlopen.qid, opt.rlopen.iounit); + } + req = NULL; /* request was freed */ + } + + return (TLOpenRes) { .req = req }; } /* size[4] Rlopen tag[2] qid[13] iounit[4] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index 2bf649085f..3b70aef51e 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -212,6 +212,33 @@ typedef struct TReadDirRes { P9Req *req; } TReadDirRes; +/* options for 'Tlopen' 9p request */ +typedef struct TLOpenOpt { + /* 9P client being used (mandatory) */ + QVirtio9P *client; + /* user supplied tag number being returned with response (optional) */ + uint16_t tag; + /* file ID of file / directory to be opened (required) */ + uint32_t fid; + /* Linux open(2) flags such as O_RDONLY, O_RDWR, O_WRONLY (optional) */ + uint32_t flags; + /* data being received from 9p server as 'Rlopen' response (optional) */ + struct { + v9fs_qid *qid; + uint32_t *iounit; + } rlopen; + /* only send Tlopen request but not wait for a reply? (optional) */ + bool requestOnly; + /* do we expect an Rlerror response, if yes which error code? (optional) */ + uint32_t expectErr; +} TLOpenOpt; + +/* result of 'Tlopen' 9p request */ +typedef struct TLOpenRes { + /* if requestOnly was set: request object for further processing */ + P9Req *req; +} TLOpenRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -245,8 +272,7 @@ TReadDirRes v9fs_treaddir(TReadDirOpt); void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t *nentries, struct V9fsDirent **entries); void v9fs_free_dirents(struct V9fsDirent *e); -P9Req *v9fs_tlopen(QVirtio9P *v9p, uint32_t fid, uint32_t flags, - uint16_t tag); +TLOpenRes v9fs_tlopen(TLOpenOpt); void v9fs_rlopen(P9Req *req, v9fs_qid *qid, uint32_t *iounit); P9Req *v9fs_twrite(QVirtio9P *v9p, uint32_t fid, uint64_t offset, uint32_t count, const void *data, uint16_t tag); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 99e24fce0b..0455c3a094 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -21,6 +21,7 @@ #define tattach(...) v9fs_tattach((TAttachOpt) __VA_ARGS__) #define tgetattr(...) v9fs_tgetattr((TGetAttrOpt) __VA_ARGS__) #define treaddir(...) v9fs_treaddir((TReadDirOpt) __VA_ARGS__) +#define tlopen(...) v9fs_tlopen((TLOpenOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -113,7 +114,9 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc) }); g_assert_cmpint(nqid, ==, 1); - req = v9fs_tlopen(v9p, 1, O_DIRECTORY, 0); + req = tlopen({ + .client = v9p, .fid = 1, .flags = O_DIRECTORY, .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rlopen(req, &qid, NULL); @@ -178,7 +181,9 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count) }); g_assert_cmpint(nqid, ==, 1); - req = v9fs_tlopen(v9p, fid, O_DIRECTORY, 0); + req = tlopen({ + .client = v9p, .fid = fid, .flags = O_DIRECTORY, .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rlopen(req, &qid, NULL); @@ -365,7 +370,9 @@ static void fs_lopen(void *obj, void *data, QGuestAllocator *t_alloc) .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames }); - req = v9fs_tlopen(v9p, 1, O_WRONLY, 0); + req = tlopen({ + .client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rlopen(req, NULL, NULL); @@ -387,7 +394,9 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc) .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames }); - req = v9fs_tlopen(v9p, 1, O_WRONLY, 0); + req = tlopen({ + .client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rlopen(req, NULL, NULL); @@ -413,7 +422,9 @@ static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc) .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames }); - req = v9fs_tlopen(v9p, 1, O_WRONLY, 0); + req = tlopen({ + .client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rlopen(req, NULL, NULL); @@ -450,7 +461,9 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc) .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames }); - req = v9fs_tlopen(v9p, 1, O_WRONLY, 0); + req = tlopen({ + .client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rlopen(req, NULL, NULL); From 0e4c4ff02aecaa3cea3e583d07509378b7783307 Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:53:55 +0200 Subject: [PATCH 14/23] tests/9p: simplify callers of tlopen() Now as tlopen() is using a declarative approach, simplify the code of callers of this function. Signed-off-by: Christian Schoenebeck Message-Id: --- tests/qtest/virtio-9p-test.c | 43 +++++++++--------------------------- 1 file changed, 10 insertions(+), 33 deletions(-) diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 0455c3a094..60a030b877 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -105,7 +105,6 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc) v9fs_qid qid; uint32_t count, nentries; struct V9fsDirent *entries = NULL; - P9Req *req; tattach({ .client = v9p }); twalk({ @@ -114,11 +113,9 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc) }); g_assert_cmpint(nqid, ==, 1); - req = tlopen({ - .client = v9p, .fid = 1, .flags = O_DIRECTORY, .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rlopen(req, &qid, NULL); + tlopen({ + .client = v9p, .fid = 1, .flags = O_DIRECTORY, .rlopen.qid = &qid + }); /* * submit count = msize - 11, because 11 is the header size of Rreaddir @@ -163,7 +160,6 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count) v9fs_qid qid; uint32_t nentries, npartialentries; struct V9fsDirent *entries, *tail, *partialentries; - P9Req *req; int fid; uint64_t offset; @@ -181,11 +177,9 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count) }); g_assert_cmpint(nqid, ==, 1); - req = tlopen({ - .client = v9p, .fid = fid, .flags = O_DIRECTORY, .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rlopen(req, &qid, NULL); + tlopen({ + .client = v9p, .fid = fid, .flags = O_DIRECTORY, .rlopen.qid = &qid + }); /* * send as many Treaddir requests as required to get all directory @@ -363,18 +357,13 @@ static void fs_lopen(void *obj, void *data, QGuestAllocator *t_alloc) QVirtio9P *v9p = obj; v9fs_set_allocator(t_alloc); char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_LOPEN_FILE) }; - P9Req *req; tattach({ .client = v9p }); twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames }); - req = tlopen({ - .client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rlopen(req, NULL, NULL); + tlopen({ .client = v9p, .fid = 1, .flags = O_WRONLY }); g_free(wnames[0]); } @@ -394,11 +383,7 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc) .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames }); - req = tlopen({ - .client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rlopen(req, NULL, NULL); + tlopen({ .client = v9p, .fid = 1, .flags = O_WRONLY }); req = v9fs_twrite(v9p, 1, 0, write_count, buf, 0); v9fs_req_wait_for_reply(req, NULL); @@ -422,11 +407,7 @@ static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc) .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames }); - req = tlopen({ - .client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rlopen(req, NULL, NULL); + tlopen({ .client = v9p, .fid = 1, .flags = O_WRONLY }); /* This will cause the 9p server to try to write data to the backend, * until the write request gets cancelled. @@ -461,11 +442,7 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc) .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames }); - req = tlopen({ - .client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rlopen(req, NULL, NULL); + tlopen({ .client = v9p, .fid = 1, .flags = O_WRONLY }); /* This will cause the write request to complete right away, before it * could be actually cancelled. From ac9e4e6185f0f5090d18dce2dd3f60d9660be496 Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:53:58 +0200 Subject: [PATCH 15/23] tests/9p: convert v9fs_twrite() to declarative arguments Use declarative function arguments for function v9fs_twrite(). Signed-off-by: Christian Schoenebeck Message-Id: --- tests/qtest/libqos/virtio-9p-client.c | 38 ++++++++++++++++++++------- tests/qtest/libqos/virtio-9p-client.h | 31 ++++++++++++++++++++-- tests/qtest/virtio-9p-test.c | 18 ++++++++++--- 3 files changed, 72 insertions(+), 15 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index 15fde54d63..9ae347fad5 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -687,21 +687,39 @@ void v9fs_rlopen(P9Req *req, v9fs_qid *qid, uint32_t *iounit) } /* size[4] Twrite tag[2] fid[4] offset[8] count[4] data[count] */ -P9Req *v9fs_twrite(QVirtio9P *v9p, uint32_t fid, uint64_t offset, - uint32_t count, const void *data, uint16_t tag) +TWriteRes v9fs_twrite(TWriteOpt opt) { P9Req *req; + uint32_t err; uint32_t body_size = 4 + 8 + 4; + uint32_t written = 0; - g_assert_cmpint(body_size, <=, UINT32_MAX - count); - body_size += count; - req = v9fs_req_init(v9p, body_size, P9_TWRITE, tag); - v9fs_uint32_write(req, fid); - v9fs_uint64_write(req, offset); - v9fs_uint32_write(req, count); - v9fs_memwrite(req, data, count); + g_assert(opt.client); + + g_assert_cmpint(body_size, <=, UINT32_MAX - opt.count); + body_size += opt.count; + req = v9fs_req_init(opt.client, body_size, P9_TWRITE, opt.tag); + v9fs_uint32_write(req, opt.fid); + v9fs_uint64_write(req, opt.offset); + v9fs_uint32_write(req, opt.count); + v9fs_memwrite(req, opt.data, opt.count); v9fs_req_send(req); - return req; + + if (!opt.requestOnly) { + v9fs_req_wait_for_reply(req, NULL); + if (opt.expectErr) { + v9fs_rlerror(req, &err); + g_assert_cmpint(err, ==, opt.expectErr); + } else { + v9fs_rwrite(req, &written); + } + req = NULL; /* request was freed */ + } + + return (TWriteRes) { + .req = req, + .count = written + }; } /* size[4] Rwrite tag[2] count[4] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index 3b70aef51e..dda371c054 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -239,6 +239,34 @@ typedef struct TLOpenRes { P9Req *req; } TLOpenRes; +/* options for 'Twrite' 9p request */ +typedef struct TWriteOpt { + /* 9P client being used (mandatory) */ + QVirtio9P *client; + /* user supplied tag number being returned with response (optional) */ + uint16_t tag; + /* file ID of file to write to (required) */ + uint32_t fid; + /* start position of write from beginning of file (optional) */ + uint64_t offset; + /* how many bytes to write */ + uint32_t count; + /* data to be written */ + const void *data; + /* only send Twrite request but not wait for a reply? (optional) */ + bool requestOnly; + /* do we expect an Rlerror response, if yes which error code? (optional) */ + uint32_t expectErr; +} TWriteOpt; + +/* result of 'Twrite' 9p request */ +typedef struct TWriteRes { + /* if requestOnly was set: request object for further processing */ + P9Req *req; + /* amount of bytes written */ + uint32_t count; +} TWriteRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -274,8 +302,7 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t *nentries, void v9fs_free_dirents(struct V9fsDirent *e); TLOpenRes v9fs_tlopen(TLOpenOpt); void v9fs_rlopen(P9Req *req, v9fs_qid *qid, uint32_t *iounit); -P9Req *v9fs_twrite(QVirtio9P *v9p, uint32_t fid, uint64_t offset, - uint32_t count, const void *data, uint16_t tag); +TWriteRes v9fs_twrite(TWriteOpt); void v9fs_rwrite(P9Req *req, uint32_t *count); P9Req *v9fs_tflush(QVirtio9P *v9p, uint16_t oldtag, uint16_t tag); void v9fs_rflush(P9Req *req); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 60a030b877..a5b9284acb 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -22,6 +22,7 @@ #define tgetattr(...) v9fs_tgetattr((TGetAttrOpt) __VA_ARGS__) #define treaddir(...) v9fs_treaddir((TReadDirOpt) __VA_ARGS__) #define tlopen(...) v9fs_tlopen((TLOpenOpt) __VA_ARGS__) +#define twrite(...) v9fs_twrite((TWriteOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -385,7 +386,10 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc) tlopen({ .client = v9p, .fid = 1, .flags = O_WRONLY }); - req = v9fs_twrite(v9p, 1, 0, write_count, buf, 0); + req = twrite({ + .client = v9p, .fid = 1, .offset = 0, .count = write_count, + .data = buf, .requestOnly = true + }).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rwrite(req, &count); g_assert_cmpint(count, ==, write_count); @@ -413,7 +417,11 @@ static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc) * until the write request gets cancelled. */ should_block = 1; - req = v9fs_twrite(v9p, 1, 0, sizeof(should_block), &should_block, 0); + req = twrite({ + .client = v9p, .fid = 1, .offset = 0, + .count = sizeof(should_block), .data = &should_block, + .requestOnly = true + }).req; flush_req = v9fs_tflush(v9p, req->tag, 1); @@ -448,7 +456,11 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc) * could be actually cancelled. */ should_block = 0; - req = v9fs_twrite(v9p, 1, 0, sizeof(should_block), &should_block, 0); + req = twrite({ + .client = v9p, .fid = 1, .offset = 0, + .count = sizeof(should_block), .data = &should_block, + .requestOnly = true + }).req; flush_req = v9fs_tflush(v9p, req->tag, 1); From bb286ff8e85dcc222c93c9f3a164034561d1f585 Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:54:00 +0200 Subject: [PATCH 16/23] tests/9p: simplify callers of twrite() Now as twrite() is using a declarative approach, simplify the code of callers of this function. Signed-off-by: Christian Schoenebeck Message-Id: <7f280ec6a1f9d8afed46567a796562c4dc28afa9.1664917004.git.qemu_oss@crudebyte.com> --- tests/qtest/virtio-9p-test.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index a5b9284acb..5ad7bebec7 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -377,7 +377,6 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc) char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_WRITE_FILE) }; g_autofree char *buf = g_malloc0(write_count); uint32_t count; - P9Req *req; tattach({ .client = v9p }); twalk({ @@ -386,12 +385,10 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc) tlopen({ .client = v9p, .fid = 1, .flags = O_WRONLY }); - req = twrite({ + count = twrite({ .client = v9p, .fid = 1, .offset = 0, .count = write_count, - .data = buf, .requestOnly = true - }).req; - v9fs_req_wait_for_reply(req, NULL); - v9fs_rwrite(req, &count); + .data = buf + }).count; g_assert_cmpint(count, ==, write_count); g_free(wnames[0]); From d89146fd16775aa734b1e67b8fdee0301ad80cf5 Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:54:03 +0200 Subject: [PATCH 17/23] tests/9p: convert v9fs_tflush() to declarative arguments Use declarative function arguments for function v9fs_tflush(). Signed-off-by: Christian Schoenebeck Message-Id: <91b7b154298c500d100b05137146c2905c3acdec.1664917004.git.qemu_oss@crudebyte.com> --- tests/qtest/libqos/virtio-9p-client.c | 23 +++++++++++++++++++---- tests/qtest/libqos/virtio-9p-client.h | 22 +++++++++++++++++++++- tests/qtest/virtio-9p-test.c | 9 +++++++-- 3 files changed, 47 insertions(+), 7 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index 9ae347fad5..3be0ffc7da 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -733,14 +733,29 @@ void v9fs_rwrite(P9Req *req, uint32_t *count) } /* size[4] Tflush tag[2] oldtag[2] */ -P9Req *v9fs_tflush(QVirtio9P *v9p, uint16_t oldtag, uint16_t tag) +TFlushRes v9fs_tflush(TFlushOpt opt) { P9Req *req; + uint32_t err; - req = v9fs_req_init(v9p, 2, P9_TFLUSH, tag); - v9fs_uint32_write(req, oldtag); + g_assert(opt.client); + + req = v9fs_req_init(opt.client, 2, P9_TFLUSH, opt.tag); + v9fs_uint32_write(req, opt.oldtag); v9fs_req_send(req); - return req; + + if (!opt.requestOnly) { + v9fs_req_wait_for_reply(req, NULL); + if (opt.expectErr) { + v9fs_rlerror(req, &err); + g_assert_cmpint(err, ==, opt.expectErr); + } else { + v9fs_rflush(req); + } + req = NULL; /* request was freed */ + } + + return (TFlushRes) { .req = req }; } /* size[4] Rflush tag[2] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index dda371c054..b22b54c720 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -267,6 +267,26 @@ typedef struct TWriteRes { uint32_t count; } TWriteRes; +/* options for 'Tflush' 9p request */ +typedef struct TFlushOpt { + /* 9P client being used (mandatory) */ + QVirtio9P *client; + /* user supplied tag number being returned with response (optional) */ + uint16_t tag; + /* message to flush (required) */ + uint16_t oldtag; + /* only send Tflush request but not wait for a reply? (optional) */ + bool requestOnly; + /* do we expect an Rlerror response, if yes which error code? (optional) */ + uint32_t expectErr; +} TFlushOpt; + +/* result of 'Tflush' 9p request */ +typedef struct TFlushRes { + /* if requestOnly was set: request object for further processing */ + P9Req *req; +} TFlushRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -304,7 +324,7 @@ TLOpenRes v9fs_tlopen(TLOpenOpt); void v9fs_rlopen(P9Req *req, v9fs_qid *qid, uint32_t *iounit); TWriteRes v9fs_twrite(TWriteOpt); void v9fs_rwrite(P9Req *req, uint32_t *count); -P9Req *v9fs_tflush(QVirtio9P *v9p, uint16_t oldtag, uint16_t tag); +TFlushRes v9fs_tflush(TFlushOpt); void v9fs_rflush(P9Req *req); P9Req *v9fs_tmkdir(QVirtio9P *v9p, uint32_t dfid, const char *name, uint32_t mode, uint32_t gid, uint16_t tag); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 5ad7bebec7..5544998bac 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -23,6 +23,7 @@ #define treaddir(...) v9fs_treaddir((TReadDirOpt) __VA_ARGS__) #define tlopen(...) v9fs_tlopen((TLOpenOpt) __VA_ARGS__) #define twrite(...) v9fs_twrite((TWriteOpt) __VA_ARGS__) +#define tflush(...) v9fs_tflush((TFlushOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -420,7 +421,9 @@ static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc) .requestOnly = true }).req; - flush_req = v9fs_tflush(v9p, req->tag, 1); + flush_req = tflush({ + .client = v9p, .oldtag = req->tag, .tag = 1, .requestOnly = true + }).req; /* The write request is supposed to be flushed: the server should just * mark the write request as used and reply to the flush request. @@ -459,7 +462,9 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc) .requestOnly = true }).req; - flush_req = v9fs_tflush(v9p, req->tag, 1); + flush_req = tflush({ + .client = v9p, .oldtag = req->tag, .tag = 1, .requestOnly = true + }).req; /* The write request is supposed to complete. The server should * reply to the write request and the flush request. From e11680102af6a9f42ab198a77015696820a1993e Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:54:06 +0200 Subject: [PATCH 18/23] tests/9p: merge v9fs_tmkdir() and do_mkdir() As with previous patches, unify those 2 functions into a single function v9fs_tmkdir() by using a declarative function arguments approach. Signed-off-by: Christian Schoenebeck Message-Id: --- tests/qtest/libqos/virtio-9p-client.c | 42 ++++++++++++++++++++++----- tests/qtest/libqos/virtio-9p-client.h | 36 +++++++++++++++++++++-- tests/qtest/virtio-9p-test.c | 30 ++++++------------- 3 files changed, 78 insertions(+), 30 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index 3be0ffc7da..c374ba2048 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -766,10 +766,26 @@ void v9fs_rflush(P9Req *req) } /* size[4] Tmkdir tag[2] dfid[4] name[s] mode[4] gid[4] */ -P9Req *v9fs_tmkdir(QVirtio9P *v9p, uint32_t dfid, const char *name, - uint32_t mode, uint32_t gid, uint16_t tag) +TMkdirRes v9fs_tmkdir(TMkdirOpt opt) { P9Req *req; + uint32_t err; + g_autofree char *name = g_strdup(opt.name); + + g_assert(opt.client); + /* expecting either hi-level atPath or low-level dfid, but not both */ + g_assert(!opt.atPath || !opt.dfid); + /* expecting either Rmkdir or Rlerror, but obviously not both */ + g_assert(!opt.expectErr || !opt.rmkdir.qid); + + if (opt.atPath) { + opt.dfid = v9fs_twalk((TWalkOpt) { .client = opt.client, + .path = opt.atPath }).newfid; + } + + if (!opt.mode) { + opt.mode = 0750; + } uint32_t body_size = 4 + 4 + 4; uint16_t string_size = v9fs_string_size(name); @@ -777,13 +793,25 @@ P9Req *v9fs_tmkdir(QVirtio9P *v9p, uint32_t dfid, const char *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); + req = v9fs_req_init(opt.client, body_size, P9_TMKDIR, opt.tag); + v9fs_uint32_write(req, opt.dfid); v9fs_string_write(req, name); - v9fs_uint32_write(req, mode); - v9fs_uint32_write(req, gid); + v9fs_uint32_write(req, opt.mode); + v9fs_uint32_write(req, opt.gid); v9fs_req_send(req); - return req; + + if (!opt.requestOnly) { + v9fs_req_wait_for_reply(req, NULL); + if (opt.expectErr) { + v9fs_rlerror(req, &err); + g_assert_cmpint(err, ==, opt.expectErr); + } else { + v9fs_rmkdir(req, opt.rmkdir.qid); + } + req = NULL; /* request was freed */ + } + + return (TMkdirRes) { .req = req }; } /* size[4] Rmkdir tag[2] qid[13] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index b22b54c720..ae44f95a4d 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -287,6 +287,39 @@ typedef struct TFlushRes { P9Req *req; } TFlushRes; +/* options for 'Tmkdir' 9p request */ +typedef struct TMkdirOpt { + /* 9P client being used (mandatory) */ + QVirtio9P *client; + /* user supplied tag number being returned with response (optional) */ + uint16_t tag; + /* low level variant of directory where new one shall be created */ + uint32_t dfid; + /* high-level variant of directory where new one shall be created */ + const char *atPath; + /* New directory's name (required) */ + const char *name; + /* Linux mkdir(2) mode bits (optional) */ + uint32_t mode; + /* effective group ID of caller */ + uint32_t gid; + /* data being received from 9p server as 'Rmkdir' response (optional) */ + struct { + /* QID of newly created directory */ + v9fs_qid *qid; + } rmkdir; + /* only send Tmkdir request but not wait for a reply? (optional) */ + bool requestOnly; + /* do we expect an Rlerror response, if yes which error code? (optional) */ + uint32_t expectErr; +} TMkdirOpt; + +/* result of 'TMkdir' 9p request */ +typedef struct TMkdirRes { + /* if requestOnly was set: request object for further processing */ + P9Req *req; +} TMkdirRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -326,8 +359,7 @@ TWriteRes v9fs_twrite(TWriteOpt); void v9fs_rwrite(P9Req *req, uint32_t *count); TFlushRes v9fs_tflush(TFlushOpt); void v9fs_rflush(P9Req *req); -P9Req *v9fs_tmkdir(QVirtio9P *v9p, uint32_t dfid, const char *name, - uint32_t mode, uint32_t gid, uint16_t tag); +TMkdirRes v9fs_tmkdir(TMkdirOpt); void v9fs_rmkdir(P9Req *req, v9fs_qid *qid); P9Req *v9fs_tlcreate(QVirtio9P *v9p, uint32_t fid, const char *name, uint32_t flags, uint32_t mode, uint32_t gid, diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 5544998bac..6d75afee87 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -24,6 +24,7 @@ #define tlopen(...) v9fs_tlopen((TLOpenOpt) __VA_ARGS__) #define twrite(...) v9fs_twrite((TWriteOpt) __VA_ARGS__) #define tflush(...) v9fs_tflush((TFlushOpt) __VA_ARGS__) +#define tmkdir(...) v9fs_tmkdir((TMkdirOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -477,19 +478,6 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc) g_free(wnames[0]); } -static void do_mkdir(QVirtio9P *v9p, const char *path, const char *cname) -{ - g_autofree char *name = g_strdup(cname); - uint32_t fid; - P9Req *req; - - fid = twalk({ .client = v9p, .path = path }).newfid; - - req = v9fs_tmkdir(v9p, fid, name, 0750, 0, 0); - v9fs_req_wait_for_reply(req, NULL); - v9fs_rmkdir(req, NULL); -} - /* create a regular file with Tlcreate and return file's fid */ static uint32_t do_lcreate(QVirtio9P *v9p, const char *path, const char *cname) @@ -587,7 +575,7 @@ static void fs_create_dir(void *obj, void *data, QGuestAllocator *t_alloc) g_assert(root_path != NULL); tattach({ .client = v9p }); - do_mkdir(v9p, "/", "01"); + tmkdir({ .client = v9p, .atPath = "/", .name = "01" }); /* check if created directory really exists now ... */ g_assert(stat(new_dir, &st) == 0); @@ -606,7 +594,7 @@ static void fs_unlinkat_dir(void *obj, void *data, QGuestAllocator *t_alloc) g_assert(root_path != NULL); tattach({ .client = v9p }); - do_mkdir(v9p, "/", "02"); + tmkdir({ .client = v9p, .atPath = "/", .name = "02" }); /* check if created directory really exists now ... */ g_assert(stat(new_dir, &st) == 0); @@ -626,7 +614,7 @@ static void fs_create_file(void *obj, void *data, QGuestAllocator *t_alloc) g_autofree char *new_file = virtio_9p_test_path("03/1st_file"); tattach({ .client = v9p }); - do_mkdir(v9p, "/", "03"); + tmkdir({ .client = v9p, .atPath = "/", .name = "03" }); do_lcreate(v9p, "03", "1st_file"); /* check if created file exists now ... */ @@ -643,7 +631,7 @@ static void fs_unlinkat_file(void *obj, void *data, QGuestAllocator *t_alloc) g_autofree char *new_file = virtio_9p_test_path("04/doa_file"); tattach({ .client = v9p }); - do_mkdir(v9p, "/", "04"); + tmkdir({ .client = v9p, .atPath = "/", .name = "04" }); do_lcreate(v9p, "04", "doa_file"); /* check if created file exists now ... */ @@ -665,7 +653,7 @@ static void fs_symlink_file(void *obj, void *data, QGuestAllocator *t_alloc) g_autofree char *symlink_file = virtio_9p_test_path("05/symlink_file"); tattach({ .client = v9p }); - do_mkdir(v9p, "/", "05"); + tmkdir({ .client = v9p, .atPath = "/", .name = "05" }); do_lcreate(v9p, "05", "real_file"); g_assert(stat(real_file, &st) == 0); g_assert((st.st_mode & S_IFMT) == S_IFREG); @@ -686,7 +674,7 @@ static void fs_unlinkat_symlink(void *obj, void *data, g_autofree char *symlink_file = virtio_9p_test_path("06/symlink_file"); tattach({ .client = v9p }); - do_mkdir(v9p, "/", "06"); + tmkdir({ .client = v9p, .atPath = "/", .name = "06" }); do_lcreate(v9p, "06", "real_file"); g_assert(stat(real_file, &st) == 0); g_assert((st.st_mode & S_IFMT) == S_IFREG); @@ -708,7 +696,7 @@ static void fs_hardlink_file(void *obj, void *data, QGuestAllocator *t_alloc) g_autofree char *hardlink_file = virtio_9p_test_path("07/hardlink_file"); tattach({ .client = v9p }); - do_mkdir(v9p, "/", "07"); + tmkdir({ .client = v9p, .atPath = "/", .name = "07" }); do_lcreate(v9p, "07", "real_file"); g_assert(stat(real_file, &st_real) == 0); g_assert((st_real.st_mode & S_IFMT) == S_IFREG); @@ -733,7 +721,7 @@ static void fs_unlinkat_hardlink(void *obj, void *data, g_autofree char *hardlink_file = virtio_9p_test_path("08/hardlink_file"); tattach({ .client = v9p }); - do_mkdir(v9p, "/", "08"); + tmkdir({ .client = v9p, .atPath = "/", .name = "08" }); do_lcreate(v9p, "08", "real_file"); g_assert(stat(real_file, &st_real) == 0); g_assert((st_real.st_mode & S_IFMT) == S_IFREG); From bd4660d49ac64886fb27649138e52d81be161fee Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:54:09 +0200 Subject: [PATCH 19/23] tests/9p: merge v9fs_tlcreate() and do_lcreate() As with previous patches, unify those 2 functions into a single function v9fs_tlcreate() by using a declarative function arguments approach. Signed-off-by: Christian Schoenebeck Message-Id: <4c01b2caa5f5b54a2020fc92701deadd2abf0571.1664917004.git.qemu_oss@crudebyte.com> --- tests/qtest/libqos/virtio-9p-client.c | 45 +++++++++++++++++++++------ tests/qtest/libqos/virtio-9p-client.h | 39 +++++++++++++++++++++-- tests/qtest/virtio-9p-test.c | 30 +++++------------- 3 files changed, 79 insertions(+), 35 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index c374ba2048..5c805a133c 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -827,11 +827,26 @@ void v9fs_rmkdir(P9Req *req, v9fs_qid *qid) } /* size[4] Tlcreate tag[2] fid[4] name[s] flags[4] mode[4] gid[4] */ -P9Req *v9fs_tlcreate(QVirtio9P *v9p, uint32_t fid, const char *name, - uint32_t flags, uint32_t mode, uint32_t gid, - uint16_t tag) +TlcreateRes v9fs_tlcreate(TlcreateOpt opt) { P9Req *req; + uint32_t err; + g_autofree char *name = g_strdup(opt.name); + + g_assert(opt.client); + /* expecting either hi-level atPath or low-level fid, but not both */ + g_assert(!opt.atPath || !opt.fid); + /* expecting either Rlcreate or Rlerror, but obviously not both */ + g_assert(!opt.expectErr || !(opt.rlcreate.qid || opt.rlcreate.iounit)); + + if (opt.atPath) { + opt.fid = v9fs_twalk((TWalkOpt) { .client = opt.client, + .path = opt.atPath }).newfid; + } + + if (!opt.mode) { + opt.mode = 0750; + } uint32_t body_size = 4 + 4 + 4 + 4; uint16_t string_size = v9fs_string_size(name); @@ -839,14 +854,26 @@ P9Req *v9fs_tlcreate(QVirtio9P *v9p, uint32_t fid, const char *name, g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); body_size += string_size; - req = v9fs_req_init(v9p, body_size, P9_TLCREATE, tag); - v9fs_uint32_write(req, fid); + req = v9fs_req_init(opt.client, body_size, P9_TLCREATE, opt.tag); + v9fs_uint32_write(req, opt.fid); v9fs_string_write(req, name); - v9fs_uint32_write(req, flags); - v9fs_uint32_write(req, mode); - v9fs_uint32_write(req, gid); + v9fs_uint32_write(req, opt.flags); + v9fs_uint32_write(req, opt.mode); + v9fs_uint32_write(req, opt.gid); v9fs_req_send(req); - return req; + + if (!opt.requestOnly) { + v9fs_req_wait_for_reply(req, NULL); + if (opt.expectErr) { + v9fs_rlerror(req, &err); + g_assert_cmpint(err, ==, opt.expectErr); + } else { + v9fs_rlcreate(req, opt.rlcreate.qid, opt.rlcreate.iounit); + } + req = NULL; /* request was freed */ + } + + return (TlcreateRes) { .req = req }; } /* size[4] Rlcreate tag[2] qid[13] iounit[4] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index ae44f95a4d..8916b1c7aa 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -320,6 +320,41 @@ typedef struct TMkdirRes { P9Req *req; } TMkdirRes; +/* options for 'Tlcreate' 9p request */ +typedef struct TlcreateOpt { + /* 9P client being used (mandatory) */ + QVirtio9P *client; + /* user supplied tag number being returned with response (optional) */ + uint16_t tag; + /* low-level variant of directory where new file shall be created */ + uint32_t fid; + /* high-level variant of directory where new file shall be created */ + const char *atPath; + /* name of new file (required) */ + const char *name; + /* Linux kernel intent bits */ + uint32_t flags; + /* Linux create(2) mode bits */ + uint32_t mode; + /* effective group ID of caller */ + uint32_t gid; + /* data being received from 9p server as 'Rlcreate' response (optional) */ + struct { + v9fs_qid *qid; + uint32_t *iounit; + } rlcreate; + /* only send Tlcreate request but not wait for a reply? (optional) */ + bool requestOnly; + /* do we expect an Rlerror response, if yes which error code? (optional) */ + uint32_t expectErr; +} TlcreateOpt; + +/* result of 'Tlcreate' 9p request */ +typedef struct TlcreateRes { + /* if requestOnly was set: request object for further processing */ + P9Req *req; +} TlcreateRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -361,9 +396,7 @@ TFlushRes v9fs_tflush(TFlushOpt); void v9fs_rflush(P9Req *req); TMkdirRes v9fs_tmkdir(TMkdirOpt); void v9fs_rmkdir(P9Req *req, v9fs_qid *qid); -P9Req *v9fs_tlcreate(QVirtio9P *v9p, uint32_t fid, const char *name, - uint32_t flags, uint32_t mode, uint32_t gid, - uint16_t tag); +TlcreateRes v9fs_tlcreate(TlcreateOpt); void v9fs_rlcreate(P9Req *req, v9fs_qid *qid, uint32_t *iounit); P9Req *v9fs_tsymlink(QVirtio9P *v9p, uint32_t fid, const char *name, const char *symtgt, uint32_t gid, uint16_t tag); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 6d75afee87..d13b27bd2e 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -25,6 +25,7 @@ #define twrite(...) v9fs_twrite((TWriteOpt) __VA_ARGS__) #define tflush(...) v9fs_tflush((TFlushOpt) __VA_ARGS__) #define tmkdir(...) v9fs_tmkdir((TMkdirOpt) __VA_ARGS__) +#define tlcreate(...) v9fs_tlcreate((TlcreateOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -478,23 +479,6 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc) g_free(wnames[0]); } -/* create a regular file with Tlcreate and return file's fid */ -static uint32_t do_lcreate(QVirtio9P *v9p, const char *path, - const char *cname) -{ - g_autofree char *name = g_strdup(cname); - uint32_t fid; - P9Req *req; - - fid = twalk({ .client = v9p, .path = path }).newfid; - - req = v9fs_tlcreate(v9p, fid, name, 0, 0750, 0, 0); - v9fs_req_wait_for_reply(req, NULL); - v9fs_rlcreate(req, NULL, NULL); - - return fid; -} - /* create symlink named @a clink in directory @a path pointing to @a to */ static void do_symlink(QVirtio9P *v9p, const char *path, const char *clink, const char *to) @@ -615,7 +599,7 @@ static void fs_create_file(void *obj, void *data, QGuestAllocator *t_alloc) tattach({ .client = v9p }); tmkdir({ .client = v9p, .atPath = "/", .name = "03" }); - do_lcreate(v9p, "03", "1st_file"); + tlcreate({ .client = v9p, .atPath = "03", .name = "1st_file" }); /* check if created file exists now ... */ g_assert(stat(new_file, &st) == 0); @@ -632,7 +616,7 @@ static void fs_unlinkat_file(void *obj, void *data, QGuestAllocator *t_alloc) tattach({ .client = v9p }); tmkdir({ .client = v9p, .atPath = "/", .name = "04" }); - do_lcreate(v9p, "04", "doa_file"); + tlcreate({ .client = v9p, .atPath = "04", .name = "doa_file" }); /* check if created file exists now ... */ g_assert(stat(new_file, &st) == 0); @@ -654,7 +638,7 @@ static void fs_symlink_file(void *obj, void *data, QGuestAllocator *t_alloc) tattach({ .client = v9p }); tmkdir({ .client = v9p, .atPath = "/", .name = "05" }); - do_lcreate(v9p, "05", "real_file"); + tlcreate({ .client = v9p, .atPath = "05", .name = "real_file" }); g_assert(stat(real_file, &st) == 0); g_assert((st.st_mode & S_IFMT) == S_IFREG); @@ -675,7 +659,7 @@ static void fs_unlinkat_symlink(void *obj, void *data, tattach({ .client = v9p }); tmkdir({ .client = v9p, .atPath = "/", .name = "06" }); - do_lcreate(v9p, "06", "real_file"); + tlcreate({ .client = v9p, .atPath = "06", .name = "real_file" }); g_assert(stat(real_file, &st) == 0); g_assert((st.st_mode & S_IFMT) == S_IFREG); @@ -697,7 +681,7 @@ static void fs_hardlink_file(void *obj, void *data, QGuestAllocator *t_alloc) tattach({ .client = v9p }); tmkdir({ .client = v9p, .atPath = "/", .name = "07" }); - do_lcreate(v9p, "07", "real_file"); + tlcreate({ .client = v9p, .atPath = "07", .name = "real_file" }); g_assert(stat(real_file, &st_real) == 0); g_assert((st_real.st_mode & S_IFMT) == S_IFREG); @@ -722,7 +706,7 @@ static void fs_unlinkat_hardlink(void *obj, void *data, tattach({ .client = v9p }); tmkdir({ .client = v9p, .atPath = "/", .name = "08" }); - do_lcreate(v9p, "08", "real_file"); + tlcreate({ .client = v9p, .atPath = "08", .name = "real_file" }); g_assert(stat(real_file, &st_real) == 0); g_assert((st_real.st_mode & S_IFMT) == S_IFREG); From 9beabfa52cb17fad0e8211031f301f12281d65f8 Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:54:11 +0200 Subject: [PATCH 20/23] tests/9p: merge v9fs_tsymlink() and do_symlink() As with previous patches, unify those 2 functions into a single function v9fs_tsymlink() by using a declarative function arguments approach. Signed-off-by: Christian Schoenebeck Message-Id: <563f3ad04fe596ce0ae1e2654d1d08237f18c830.1664917004.git.qemu_oss@crudebyte.com> --- tests/qtest/libqos/virtio-9p-client.c | 37 ++++++++++++++++++++++----- tests/qtest/libqos/virtio-9p-client.h | 35 +++++++++++++++++++++++-- tests/qtest/virtio-9p-test.c | 27 +++++++------------ 3 files changed, 73 insertions(+), 26 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index 5c805a133c..89eaf50355 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -892,10 +892,23 @@ void v9fs_rlcreate(P9Req *req, v9fs_qid *qid, uint32_t *iounit) } /* size[4] Tsymlink tag[2] fid[4] name[s] symtgt[s] gid[4] */ -P9Req *v9fs_tsymlink(QVirtio9P *v9p, uint32_t fid, const char *name, - const char *symtgt, uint32_t gid, uint16_t tag) +TsymlinkRes v9fs_tsymlink(TsymlinkOpt opt) { P9Req *req; + uint32_t err; + g_autofree char *name = g_strdup(opt.name); + g_autofree char *symtgt = g_strdup(opt.symtgt); + + g_assert(opt.client); + /* expecting either hi-level atPath or low-level fid, but not both */ + g_assert(!opt.atPath || !opt.fid); + /* expecting either Rsymlink or Rlerror, but obviously not both */ + g_assert(!opt.expectErr || !opt.rsymlink.qid); + + if (opt.atPath) { + opt.fid = v9fs_twalk((TWalkOpt) { .client = opt.client, + .path = opt.atPath }).newfid; + } uint32_t body_size = 4 + 4; uint16_t string_size = v9fs_string_size(name) + v9fs_string_size(symtgt); @@ -903,13 +916,25 @@ P9Req *v9fs_tsymlink(QVirtio9P *v9p, uint32_t fid, const char *name, g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); body_size += string_size; - req = v9fs_req_init(v9p, body_size, P9_TSYMLINK, tag); - v9fs_uint32_write(req, fid); + req = v9fs_req_init(opt.client, body_size, P9_TSYMLINK, opt.tag); + v9fs_uint32_write(req, opt.fid); v9fs_string_write(req, name); v9fs_string_write(req, symtgt); - v9fs_uint32_write(req, gid); + v9fs_uint32_write(req, opt.gid); v9fs_req_send(req); - return req; + + if (!opt.requestOnly) { + v9fs_req_wait_for_reply(req, NULL); + if (opt.expectErr) { + v9fs_rlerror(req, &err); + g_assert_cmpint(err, ==, opt.expectErr); + } else { + v9fs_rsymlink(req, opt.rsymlink.qid); + } + req = NULL; /* request was freed */ + } + + return (TsymlinkRes) { .req = req }; } /* size[4] Rsymlink tag[2] qid[13] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index 8916b1c7aa..b905a54966 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -355,6 +355,38 @@ typedef struct TlcreateRes { P9Req *req; } TlcreateRes; +/* options for 'Tsymlink' 9p request */ +typedef struct TsymlinkOpt { + /* 9P client being used (mandatory) */ + QVirtio9P *client; + /* user supplied tag number being returned with response (optional) */ + uint16_t tag; + /* low-level variant of directory where symlink shall be created */ + uint32_t fid; + /* high-level variant of directory where symlink shall be created */ + const char *atPath; + /* name of symlink (required) */ + const char *name; + /* where symlink will point to (required) */ + const char *symtgt; + /* effective group ID of caller */ + uint32_t gid; + /* data being received from 9p server as 'Rsymlink' response (optional) */ + struct { + v9fs_qid *qid; + } rsymlink; + /* only send Tsymlink request but not wait for a reply? (optional) */ + bool requestOnly; + /* do we expect an Rlerror response, if yes which error code? (optional) */ + uint32_t expectErr; +} TsymlinkOpt; + +/* result of 'Tsymlink' 9p request */ +typedef struct TsymlinkRes { + /* if requestOnly was set: request object for further processing */ + P9Req *req; +} TsymlinkRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -398,8 +430,7 @@ TMkdirRes v9fs_tmkdir(TMkdirOpt); void v9fs_rmkdir(P9Req *req, v9fs_qid *qid); TlcreateRes v9fs_tlcreate(TlcreateOpt); void v9fs_rlcreate(P9Req *req, v9fs_qid *qid, uint32_t *iounit); -P9Req *v9fs_tsymlink(QVirtio9P *v9p, uint32_t fid, const char *name, - const char *symtgt, uint32_t gid, uint16_t tag); +TsymlinkRes v9fs_tsymlink(TsymlinkOpt); void v9fs_rsymlink(P9Req *req, v9fs_qid *qid); P9Req *v9fs_tlink(QVirtio9P *v9p, uint32_t dfid, uint32_t fid, const char *name, uint16_t tag); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index d13b27bd2e..c7213d6caf 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -26,6 +26,7 @@ #define tflush(...) v9fs_tflush((TFlushOpt) __VA_ARGS__) #define tmkdir(...) v9fs_tmkdir((TMkdirOpt) __VA_ARGS__) #define tlcreate(...) v9fs_tlcreate((TlcreateOpt) __VA_ARGS__) +#define tsymlink(...) v9fs_tsymlink((TsymlinkOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -479,22 +480,6 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc) g_free(wnames[0]); } -/* create symlink named @a clink in directory @a path pointing to @a to */ -static void do_symlink(QVirtio9P *v9p, const char *path, const char *clink, - const char *to) -{ - g_autofree char *name = g_strdup(clink); - g_autofree char *dst = g_strdup(to); - uint32_t fid; - P9Req *req; - - fid = twalk({ .client = v9p, .path = path }).newfid; - - req = v9fs_tsymlink(v9p, fid, name, dst, 0, 0); - v9fs_req_wait_for_reply(req, NULL); - v9fs_rsymlink(req, NULL); -} - /* create a hard link named @a clink in directory @a path pointing to @a to */ static void do_hardlink(QVirtio9P *v9p, const char *path, const char *clink, const char *to) @@ -642,7 +627,10 @@ static void fs_symlink_file(void *obj, void *data, QGuestAllocator *t_alloc) g_assert(stat(real_file, &st) == 0); g_assert((st.st_mode & S_IFMT) == S_IFREG); - do_symlink(v9p, "05", "symlink_file", "real_file"); + tsymlink({ + .client = v9p, .atPath = "05", .name = "symlink_file", + .symtgt = "real_file" + }); /* check if created link exists now */ g_assert(stat(symlink_file, &st) == 0); @@ -663,7 +651,10 @@ static void fs_unlinkat_symlink(void *obj, void *data, g_assert(stat(real_file, &st) == 0); g_assert((st.st_mode & S_IFMT) == S_IFREG); - do_symlink(v9p, "06", "symlink_file", "real_file"); + tsymlink({ + .client = v9p, .atPath = "06", .name = "symlink_file", + .symtgt = "real_file" + }); g_assert(stat(symlink_file, &st) == 0); do_unlinkat(v9p, "06", "symlink_file", 0); From d41a9462ea15f0e3ef789d496032536c33098275 Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:54:14 +0200 Subject: [PATCH 21/23] tests/9p: merge v9fs_tlink() and do_hardlink() As with previous patches, unify those 2 functions into a single function v9fs_tlink() by using a declarative function arguments approach. Signed-off-by: Christian Schoenebeck Message-Id: --- tests/qtest/libqos/virtio-9p-client.c | 43 ++++++++++++++++++++++----- tests/qtest/libqos/virtio-9p-client.h | 31 +++++++++++++++++-- tests/qtest/virtio-9p-test.c | 26 ++++++---------- 3 files changed, 73 insertions(+), 27 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index 89eaf50355..a2770719b9 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -950,23 +950,50 @@ void v9fs_rsymlink(P9Req *req, v9fs_qid *qid) } /* size[4] Tlink tag[2] dfid[4] fid[4] name[s] */ -P9Req *v9fs_tlink(QVirtio9P *v9p, uint32_t dfid, uint32_t fid, - const char *name, uint16_t tag) +TlinkRes v9fs_tlink(TlinkOpt opt) { P9Req *req; + uint32_t err; + + g_assert(opt.client); + /* expecting either hi-level atPath or low-level dfid, but not both */ + g_assert(!opt.atPath || !opt.dfid); + /* expecting either hi-level toPath or low-level fid, but not both */ + g_assert(!opt.toPath || !opt.fid); + + if (opt.atPath) { + opt.dfid = v9fs_twalk((TWalkOpt) { .client = opt.client, + .path = opt.atPath }).newfid; + } + if (opt.toPath) { + opt.fid = v9fs_twalk((TWalkOpt) { .client = opt.client, + .path = opt.toPath }).newfid; + } uint32_t body_size = 4 + 4; - uint16_t string_size = v9fs_string_size(name); + uint16_t string_size = v9fs_string_size(opt.name); g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); body_size += string_size; - req = v9fs_req_init(v9p, body_size, P9_TLINK, tag); - v9fs_uint32_write(req, dfid); - v9fs_uint32_write(req, fid); - v9fs_string_write(req, name); + req = v9fs_req_init(opt.client, body_size, P9_TLINK, opt.tag); + v9fs_uint32_write(req, opt.dfid); + v9fs_uint32_write(req, opt.fid); + v9fs_string_write(req, opt.name); v9fs_req_send(req); - return req; + + if (!opt.requestOnly) { + v9fs_req_wait_for_reply(req, NULL); + if (opt.expectErr) { + v9fs_rlerror(req, &err); + g_assert_cmpint(err, ==, opt.expectErr); + } else { + v9fs_rlink(req); + } + req = NULL; /* request was freed */ + } + + return (TlinkRes) { .req = req }; } /* size[4] Rlink tag[2] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index b905a54966..49ffd0fc51 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -387,6 +387,34 @@ typedef struct TsymlinkRes { P9Req *req; } TsymlinkRes; +/* options for 'Tlink' 9p request */ +typedef struct TlinkOpt { + /* 9P client being used (mandatory) */ + QVirtio9P *client; + /* user supplied tag number being returned with response (optional) */ + uint16_t tag; + /* low-level variant of directory where hard link shall be created */ + uint32_t dfid; + /* high-level variant of directory where hard link shall be created */ + const char *atPath; + /* low-level variant of target referenced by new hard link */ + uint32_t fid; + /* high-level variant of target referenced by new hard link */ + const char *toPath; + /* name of hard link (required) */ + const char *name; + /* only send Tlink request but not wait for a reply? (optional) */ + bool requestOnly; + /* do we expect an Rlerror response, if yes which error code? (optional) */ + uint32_t expectErr; +} TlinkOpt; + +/* result of 'Tlink' 9p request */ +typedef struct TlinkRes { + /* if requestOnly was set: request object for further processing */ + P9Req *req; +} TlinkRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -432,8 +460,7 @@ TlcreateRes v9fs_tlcreate(TlcreateOpt); void v9fs_rlcreate(P9Req *req, v9fs_qid *qid, uint32_t *iounit); TsymlinkRes v9fs_tsymlink(TsymlinkOpt); void v9fs_rsymlink(P9Req *req, v9fs_qid *qid); -P9Req *v9fs_tlink(QVirtio9P *v9p, uint32_t dfid, uint32_t fid, - const char *name, uint16_t tag); +TlinkRes v9fs_tlink(TlinkOpt); void v9fs_rlink(P9Req *req); P9Req *v9fs_tunlinkat(QVirtio9P *v9p, uint32_t dirfd, const char *name, uint32_t flags, uint16_t tag); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index c7213d6caf..185eaf8b1e 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -27,6 +27,7 @@ #define tmkdir(...) v9fs_tmkdir((TMkdirOpt) __VA_ARGS__) #define tlcreate(...) v9fs_tlcreate((TlcreateOpt) __VA_ARGS__) #define tsymlink(...) v9fs_tsymlink((TsymlinkOpt) __VA_ARGS__) +#define tlink(...) v9fs_tlink((TlinkOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -480,21 +481,6 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc) g_free(wnames[0]); } -/* create a hard link named @a clink in directory @a path pointing to @a to */ -static void do_hardlink(QVirtio9P *v9p, const char *path, const char *clink, - const char *to) -{ - uint32_t dfid, fid; - P9Req *req; - - dfid = twalk({ .client = v9p, .path = path }).newfid; - fid = twalk({ .client = v9p, .path = to }).newfid; - - req = v9fs_tlink(v9p, dfid, fid, clink, 0); - v9fs_req_wait_for_reply(req, NULL); - v9fs_rlink(req); -} - static void do_unlinkat(QVirtio9P *v9p, const char *atpath, const char *rpath, uint32_t flags) { @@ -676,7 +662,10 @@ static void fs_hardlink_file(void *obj, void *data, QGuestAllocator *t_alloc) g_assert(stat(real_file, &st_real) == 0); g_assert((st_real.st_mode & S_IFMT) == S_IFREG); - do_hardlink(v9p, "07", "hardlink_file", "07/real_file"); + tlink({ + .client = v9p, .atPath = "07", .name = "hardlink_file", + .toPath = "07/real_file" + }); /* check if link exists now ... */ g_assert(stat(hardlink_file, &st_link) == 0); @@ -701,7 +690,10 @@ static void fs_unlinkat_hardlink(void *obj, void *data, g_assert(stat(real_file, &st_real) == 0); g_assert((st_real.st_mode & S_IFMT) == S_IFREG); - do_hardlink(v9p, "08", "hardlink_file", "08/real_file"); + tlink({ + .client = v9p, .atPath = "08", .name = "hardlink_file", + .toPath = "08/real_file" + }); g_assert(stat(hardlink_file, &st_link) == 0); do_unlinkat(v9p, "08", "hardlink_file", 0); From 43e0d9fb358b01539d0de9494208e80ff84b5456 Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:54:16 +0200 Subject: [PATCH 22/23] tests/9p: merge v9fs_tunlinkat() and do_unlinkat() As with previous patches, unify those 2 functions into a single function v9fs_tunlinkat() by using a declarative function arguments approach. Signed-off-by: Christian Schoenebeck Message-Id: <1dea593edd464908d92501933c068388c01f1744.1664917004.git.qemu_oss@crudebyte.com> --- tests/qtest/libqos/virtio-9p-client.c | 37 +++++++++++++++++++++------ tests/qtest/libqos/virtio-9p-client.h | 29 +++++++++++++++++++-- tests/qtest/virtio-9p-test.c | 26 ++++++------------- 3 files changed, 64 insertions(+), 28 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index a2770719b9..e017e030ec 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -1004,23 +1004,44 @@ void v9fs_rlink(P9Req *req) } /* size[4] Tunlinkat tag[2] dirfd[4] name[s] flags[4] */ -P9Req *v9fs_tunlinkat(QVirtio9P *v9p, uint32_t dirfd, const char *name, - uint32_t flags, uint16_t tag) +TunlinkatRes v9fs_tunlinkat(TunlinkatOpt opt) { P9Req *req; + uint32_t err; + + g_assert(opt.client); + /* expecting either hi-level atPath or low-level dirfd, but not both */ + g_assert(!opt.atPath || !opt.dirfd); + + if (opt.atPath) { + opt.dirfd = v9fs_twalk((TWalkOpt) { .client = opt.client, + .path = opt.atPath }).newfid; + } uint32_t body_size = 4 + 4; - uint16_t string_size = v9fs_string_size(name); + uint16_t string_size = v9fs_string_size(opt.name); g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); body_size += string_size; - req = v9fs_req_init(v9p, body_size, P9_TUNLINKAT, tag); - v9fs_uint32_write(req, dirfd); - v9fs_string_write(req, name); - v9fs_uint32_write(req, flags); + req = v9fs_req_init(opt.client, body_size, P9_TUNLINKAT, opt.tag); + v9fs_uint32_write(req, opt.dirfd); + v9fs_string_write(req, opt.name); + v9fs_uint32_write(req, opt.flags); v9fs_req_send(req); - return req; + + if (!opt.requestOnly) { + v9fs_req_wait_for_reply(req, NULL); + if (opt.expectErr) { + v9fs_rlerror(req, &err); + g_assert_cmpint(err, ==, opt.expectErr); + } else { + v9fs_runlinkat(req); + } + req = NULL; /* request was freed */ + } + + return (TunlinkatRes) { .req = req }; } /* size[4] Runlinkat tag[2] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index 49ffd0fc51..78228eb97d 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -415,6 +415,32 @@ typedef struct TlinkRes { P9Req *req; } TlinkRes; +/* options for 'Tunlinkat' 9p request */ +typedef struct TunlinkatOpt { + /* 9P client being used (mandatory) */ + QVirtio9P *client; + /* user supplied tag number being returned with response (optional) */ + uint16_t tag; + /* low-level variant of directory where name shall be unlinked */ + uint32_t dirfd; + /* high-level variant of directory where name shall be unlinked */ + const char *atPath; + /* name of directory entry to be unlinked (required) */ + const char *name; + /* Linux unlinkat(2) flags */ + uint32_t flags; + /* only send Tunlinkat request but not wait for a reply? (optional) */ + bool requestOnly; + /* do we expect an Rlerror response, if yes which error code? (optional) */ + uint32_t expectErr; +} TunlinkatOpt; + +/* result of 'Tunlinkat' 9p request */ +typedef struct TunlinkatRes { + /* if requestOnly was set: request object for further processing */ + P9Req *req; +} TunlinkatRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -462,8 +488,7 @@ TsymlinkRes v9fs_tsymlink(TsymlinkOpt); void v9fs_rsymlink(P9Req *req, v9fs_qid *qid); TlinkRes v9fs_tlink(TlinkOpt); void v9fs_rlink(P9Req *req); -P9Req *v9fs_tunlinkat(QVirtio9P *v9p, uint32_t dirfd, const char *name, - uint32_t flags, uint16_t tag); +TunlinkatRes v9fs_tunlinkat(TunlinkatOpt); void v9fs_runlinkat(P9Req *req); #endif diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 185eaf8b1e..65e69491e5 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -28,6 +28,7 @@ #define tlcreate(...) v9fs_tlcreate((TlcreateOpt) __VA_ARGS__) #define tsymlink(...) v9fs_tsymlink((TsymlinkOpt) __VA_ARGS__) #define tlink(...) v9fs_tlink((TlinkOpt) __VA_ARGS__) +#define tunlinkat(...) v9fs_tunlinkat((TunlinkatOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -481,20 +482,6 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc) g_free(wnames[0]); } -static void do_unlinkat(QVirtio9P *v9p, const char *atpath, const char *rpath, - uint32_t flags) -{ - g_autofree char *name = g_strdup(rpath); - uint32_t fid; - P9Req *req; - - fid = twalk({ .client = v9p, .path = atpath }).newfid; - - req = v9fs_tunlinkat(v9p, fid, name, flags, 0); - v9fs_req_wait_for_reply(req, NULL); - v9fs_runlinkat(req); -} - static void fs_readdir_split_128(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -556,7 +543,10 @@ static void fs_unlinkat_dir(void *obj, void *data, QGuestAllocator *t_alloc) /* ... and is actually a directory */ g_assert((st.st_mode & S_IFMT) == S_IFDIR); - do_unlinkat(v9p, "/", "02", P9_DOTL_AT_REMOVEDIR); + tunlinkat({ + .client = v9p, .atPath = "/", .name = "02", + .flags = P9_DOTL_AT_REMOVEDIR + }); /* directory should be gone now */ g_assert(stat(new_dir, &st) != 0); } @@ -594,7 +584,7 @@ static void fs_unlinkat_file(void *obj, void *data, QGuestAllocator *t_alloc) /* ... and is a regular file */ g_assert((st.st_mode & S_IFMT) == S_IFREG); - do_unlinkat(v9p, "04", "doa_file", 0); + tunlinkat({ .client = v9p, .atPath = "04", .name = "doa_file" }); /* file should be gone now */ g_assert(stat(new_file, &st) != 0); } @@ -643,7 +633,7 @@ static void fs_unlinkat_symlink(void *obj, void *data, }); g_assert(stat(symlink_file, &st) == 0); - do_unlinkat(v9p, "06", "symlink_file", 0); + tunlinkat({ .client = v9p, .atPath = "06", .name = "symlink_file" }); /* symlink should be gone now */ g_assert(stat(symlink_file, &st) != 0); } @@ -696,7 +686,7 @@ static void fs_unlinkat_hardlink(void *obj, void *data, }); g_assert(stat(hardlink_file, &st_link) == 0); - do_unlinkat(v9p, "08", "hardlink_file", 0); + tunlinkat({ .client = v9p, .atPath = "08", .name = "hardlink_file" }); /* symlink should be gone now */ g_assert(stat(hardlink_file, &st_link) != 0); /* and old file should still exist */ From 3ce77865bf813f313cf79c00fd951bfc95a50165 Mon Sep 17 00:00:00 2001 From: Christian Schoenebeck Date: Tue, 4 Oct 2022 22:54:30 +0200 Subject: [PATCH 23/23] tests/9p: remove unnecessary g_strdup() calls This is a leftover from before the recent function merge and refactoring patches: As these functions do not return control to the caller in between, it is not necessary to duplicate strings passed to them. Signed-off-by: Christian Schoenebeck Message-Id: <0f80141cde3904ed0591354059da49d1d60bcdbc.1664917004.git.qemu_oss@crudebyte.com> --- tests/qtest/libqos/virtio-9p-client.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index e017e030ec..e4a368e036 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -770,7 +770,6 @@ TMkdirRes v9fs_tmkdir(TMkdirOpt opt) { P9Req *req; uint32_t err; - g_autofree char *name = g_strdup(opt.name); g_assert(opt.client); /* expecting either hi-level atPath or low-level dfid, but not both */ @@ -788,14 +787,14 @@ TMkdirRes v9fs_tmkdir(TMkdirOpt opt) } uint32_t body_size = 4 + 4 + 4; - uint16_t string_size = v9fs_string_size(name); + uint16_t string_size = v9fs_string_size(opt.name); g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); body_size += string_size; req = v9fs_req_init(opt.client, body_size, P9_TMKDIR, opt.tag); v9fs_uint32_write(req, opt.dfid); - v9fs_string_write(req, name); + v9fs_string_write(req, opt.name); v9fs_uint32_write(req, opt.mode); v9fs_uint32_write(req, opt.gid); v9fs_req_send(req); @@ -831,7 +830,6 @@ TlcreateRes v9fs_tlcreate(TlcreateOpt opt) { P9Req *req; uint32_t err; - g_autofree char *name = g_strdup(opt.name); g_assert(opt.client); /* expecting either hi-level atPath or low-level fid, but not both */ @@ -849,14 +847,14 @@ TlcreateRes v9fs_tlcreate(TlcreateOpt opt) } uint32_t body_size = 4 + 4 + 4 + 4; - uint16_t string_size = v9fs_string_size(name); + uint16_t string_size = v9fs_string_size(opt.name); g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); body_size += string_size; req = v9fs_req_init(opt.client, body_size, P9_TLCREATE, opt.tag); v9fs_uint32_write(req, opt.fid); - v9fs_string_write(req, name); + v9fs_string_write(req, opt.name); v9fs_uint32_write(req, opt.flags); v9fs_uint32_write(req, opt.mode); v9fs_uint32_write(req, opt.gid); @@ -896,8 +894,6 @@ TsymlinkRes v9fs_tsymlink(TsymlinkOpt opt) { P9Req *req; uint32_t err; - g_autofree char *name = g_strdup(opt.name); - g_autofree char *symtgt = g_strdup(opt.symtgt); g_assert(opt.client); /* expecting either hi-level atPath or low-level fid, but not both */ @@ -911,15 +907,16 @@ TsymlinkRes v9fs_tsymlink(TsymlinkOpt opt) } uint32_t body_size = 4 + 4; - uint16_t string_size = v9fs_string_size(name) + v9fs_string_size(symtgt); + uint16_t string_size = v9fs_string_size(opt.name) + + v9fs_string_size(opt.symtgt); g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); body_size += string_size; req = v9fs_req_init(opt.client, body_size, P9_TSYMLINK, opt.tag); v9fs_uint32_write(req, opt.fid); - v9fs_string_write(req, name); - v9fs_string_write(req, symtgt); + v9fs_string_write(req, opt.name); + v9fs_string_write(req, opt.symtgt); v9fs_uint32_write(req, opt.gid); v9fs_req_send(req);