diff --git a/MAINTAINERS b/MAINTAINERS index 0cf37fce7b..c6859cbb0e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2254,8 +2254,8 @@ F: include/system/balloon.h F: tests/qtest/virtio-balloon-test.c virtio-9p -M: Greg Kurz M: Christian Schoenebeck +R: Greg Kurz S: Maintained W: https://wiki.qemu.org/Documentation/9p F: hw/9pfs/ @@ -2263,7 +2263,6 @@ X: hw/9pfs/xen-9p* F: fsdev/ F: tests/qtest/virtio-9p-test.c F: tests/qtest/libqos/virtio-9p* -T: git https://gitlab.com/gkurz/qemu.git 9p-next T: git https://github.com/cschoenebeck/qemu.git 9p.next virtio-blk diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c index 1b1f3b9ec8..928523afcc 100644 --- a/hw/9pfs/9p-local.c +++ b/hw/9pfs/9p-local.c @@ -1538,6 +1538,9 @@ static int local_parse_opts(QemuOpts *opts, FsDriverEntry *fse, Error **errp) "[remap|forbid|warn]\n"); return -1; } + } else { + fse->export_flags &= ~V9FS_FORBID_MULTIDEVS; + fse->export_flags |= V9FS_REMAP_INODES; } if (!path) { diff --git a/hw/9pfs/9p-util-generic.c b/hw/9pfs/9p-util-generic.c new file mode 100644 index 0000000000..4c1e9c887d --- /dev/null +++ b/hw/9pfs/9p-util-generic.c @@ -0,0 +1,50 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include "qemu/osdep.h" +#include "9p-util.h" +#include + +char *qemu_open_flags_tostr(int flags) +{ + int acc = flags & O_ACCMODE; + return g_strconcat( + (acc == O_WRONLY) ? "WRONLY" : (acc == O_RDONLY) ? "RDONLY" : "RDWR", + (flags & O_CREAT) ? "|CREAT" : "", + (flags & O_EXCL) ? "|EXCL" : "", + (flags & O_NOCTTY) ? "|NOCTTY" : "", + (flags & O_TRUNC) ? "|TRUNC" : "", + (flags & O_APPEND) ? "|APPEND" : "", + (flags & O_NONBLOCK) ? "|NONBLOCK" : "", + (flags & O_DSYNC) ? "|DSYNC" : "", + #ifdef O_DIRECT + (flags & O_DIRECT) ? "|DIRECT" : "", + #endif + (flags & O_LARGEFILE) ? "|LARGEFILE" : "", + (flags & O_DIRECTORY) ? "|DIRECTORY" : "", + (flags & O_NOFOLLOW) ? "|NOFOLLOW" : "", + #ifdef O_NOATIME + (flags & O_NOATIME) ? "|NOATIME" : "", + #endif + #ifdef O_CLOEXEC + (flags & O_CLOEXEC) ? "|CLOEXEC" : "", + #endif + #ifdef __O_SYNC + (flags & __O_SYNC) ? "|SYNC" : "", + #else + ((flags & O_SYNC) == O_SYNC) ? "|SYNC" : "", + #endif + #ifdef O_PATH + (flags & O_PATH) ? "|PATH" : "", + #endif + #ifdef __O_TMPFILE + (flags & __O_TMPFILE) ? "|TMPFILE" : "", + #elif defined(O_TMPFILE) + ((flags & O_TMPFILE) == O_TMPFILE) ? "|TMPFILE" : "", + #endif + /* O_NDELAY is usually just an alias of O_NONBLOCK */ + #if defined(O_NDELAY) && O_NDELAY != O_NONBLOCK + (flags & O_NDELAY) ? "|NDELAY" : "", + #endif + NULL /* always last (required NULL termination) */ + ); +} diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h index 95ee4da9bd..7bc4ec8e85 100644 --- a/hw/9pfs/9p-util.h +++ b/hw/9pfs/9p-util.h @@ -267,4 +267,10 @@ int pthread_fchdir_np(int fd) __attribute__((weak_import)); #endif int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev); +/* + * Returns a newly allocated string presentation of open() flags, intended + * for debugging (tracing) purposes only. + */ +char *qemu_open_flags_tostr(int flags); + #endif diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index 578517739a..7cad2bce62 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -1774,6 +1774,21 @@ static bool same_stat_id(const struct stat *a, const struct stat *b) return a->st_dev == b->st_dev && a->st_ino == b->st_ino; } +/* + * Returns a (newly allocated) comma-separated string presentation of the + * passed array for logging (tracing) purpose for trace event "v9fs_walk". + * + * It is caller's responsibility to free the returned string. + */ +static char *trace_v9fs_walk_wnames(V9fsString *wnames, size_t nwnames) +{ + g_autofree char **arr = g_malloc0_n(nwnames + 1, sizeof(char *)); + for (size_t i = 0; i < nwnames; ++i) { + arr[i] = wnames[i].data; + } + return g_strjoinv(", ", arr); +} + static void coroutine_fn v9fs_walk(void *opaque) { int name_idx, nwalked; @@ -1787,6 +1802,7 @@ static void coroutine_fn v9fs_walk(void *opaque) size_t offset = 7; int32_t fid, newfid; P9ARRAY_REF(V9fsString) wnames = NULL; + g_autofree char *trace_wnames = NULL; V9fsFidState *fidp; V9fsFidState *newfidp = NULL; V9fsPDU *pdu = opaque; @@ -1800,11 +1816,9 @@ static void coroutine_fn v9fs_walk(void *opaque) } offset += err; - trace_v9fs_walk(pdu->tag, pdu->id, fid, newfid, nwnames); - if (nwnames > P9_MAXWELEM) { err = -EINVAL; - goto out_nofid; + goto out_nofid_nownames; } if (nwnames) { P9ARRAY_NEW(V9fsString, wnames, nwnames); @@ -1814,15 +1828,23 @@ static void coroutine_fn v9fs_walk(void *opaque) for (i = 0; i < nwnames; i++) { err = pdu_unmarshal(pdu, offset, "s", &wnames[i]); if (err < 0) { - goto out_nofid; + goto out_nofid_nownames; } if (name_is_illegal(wnames[i].data)) { err = -ENOENT; - goto out_nofid; + goto out_nofid_nownames; } offset += err; } + if (trace_event_get_state_backends(TRACE_V9FS_WALK)) { + trace_wnames = trace_v9fs_walk_wnames(wnames, nwnames); + trace_v9fs_walk(pdu->tag, pdu->id, fid, newfid, nwnames, + trace_wnames); + } + } else { + trace_v9fs_walk(pdu->tag, pdu->id, fid, newfid, nwnames, ""); } + fidp = get_fid(pdu, fid); if (fidp == NULL) { err = -ENOENT; @@ -1957,7 +1979,11 @@ out: } v9fs_path_free(&dpath); v9fs_path_free(&path); + goto out_pdu_complete; +out_nofid_nownames: + trace_v9fs_walk(pdu->tag, pdu->id, fid, newfid, nwnames, ""); out_nofid: +out_pdu_complete: pdu_complete(pdu, err); } @@ -1982,6 +2008,7 @@ static void coroutine_fn v9fs_open(void *opaque) V9fsFidState *fidp; V9fsPDU *pdu = opaque; V9fsState *s = pdu->s; + g_autofree char *trace_oflags = NULL; if (s->proto_version == V9FS_PROTO_2000L) { err = pdu_unmarshal(pdu, offset, "dd", &fid, &mode); @@ -1993,7 +2020,13 @@ static void coroutine_fn v9fs_open(void *opaque) if (err < 0) { goto out_nofid; } - trace_v9fs_open(pdu->tag, pdu->id, fid, mode); + if (trace_event_get_state_backends(TRACE_V9FS_OPEN)) { + trace_oflags = qemu_open_flags_tostr( + (s->proto_version == V9FS_PROTO_2000L) ? + dotl_to_open_flags(mode) : omode_to_uflags(mode) + ); + trace_v9fs_open(pdu->tag, pdu->id, fid, mode, trace_oflags); + } fidp = get_fid(pdu, fid); if (fidp == NULL) { diff --git a/hw/9pfs/meson.build b/hw/9pfs/meson.build index eceffdb81e..d35d4f44ff 100644 --- a/hw/9pfs/meson.build +++ b/hw/9pfs/meson.build @@ -3,6 +3,7 @@ fs_ss.add(files( '9p-local.c', '9p-posix-acl.c', '9p-synth.c', + '9p-util-generic.c', '9p-xattr-user.c', '9p-xattr.c', '9p.c', diff --git a/hw/9pfs/trace-events b/hw/9pfs/trace-events index a12e55c165..0e0fc37261 100644 --- a/hw/9pfs/trace-events +++ b/hw/9pfs/trace-events @@ -11,9 +11,9 @@ v9fs_stat(uint16_t tag, uint8_t id, int32_t fid) "tag %d id %d fid %d" v9fs_stat_return(uint16_t tag, uint8_t id, int32_t mode, int32_t atime, int32_t mtime, int64_t length) "tag %d id %d stat={mode %d atime %d mtime %d length %"PRId64"}" v9fs_getattr(uint16_t tag, uint8_t id, int32_t fid, uint64_t request_mask) "tag %d id %d fid %d request_mask %"PRIu64 v9fs_getattr_return(uint16_t tag, uint8_t id, uint64_t result_mask, uint32_t mode, uint32_t uid, uint32_t gid) "tag %d id %d getattr={result_mask %"PRId64" mode %u uid %u gid %u}" -v9fs_walk(uint16_t tag, uint8_t id, int32_t fid, int32_t newfid, uint16_t nwnames) "tag %d id %d fid %d newfid %d nwnames %d" +v9fs_walk(uint16_t tag, uint8_t id, int32_t fid, int32_t newfid, uint16_t nwnames, const char* wnames) "tag=%d id=%d fid=%d newfid=%d nwnames=%d wnames={%s}" v9fs_walk_return(uint16_t tag, uint8_t id, uint16_t nwnames, void* qids) "tag %d id %d nwnames %d qids %p" -v9fs_open(uint16_t tag, uint8_t id, int32_t fid, int32_t mode) "tag %d id %d fid %d mode %d" +v9fs_open(uint16_t tag, uint8_t id, int32_t fid, int32_t mode, const char* oflags) "tag=%d id=%d fid=%d mode=%d(%s)" v9fs_open_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t version, uint64_t path, int iounit) "tag %u id %u qid={type %u version %u path %"PRIu64"} iounit %d" v9fs_lcreate(uint16_t tag, uint8_t id, int32_t dfid, int32_t flags, int32_t mode, uint32_t gid) "tag %d id %d dfid %d flags %d mode %d gid %u" v9fs_lcreate_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t version, uint64_t path, int32_t iounit) "tag %u id %u qid={type %u version %u path %"PRIu64"} iounit %d" diff --git a/qemu-options.hx b/qemu-options.hx index ec0090dfe2..1b26ad53bd 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1951,32 +1951,37 @@ SRST Specifies the tag name to be used by the guest to mount this export point. - ``multidevs=multidevs`` - Specifies how to deal with multiple devices being shared with a - 9p export. Supported behaviours are either "remap", "forbid" or - "warn". The latter is the default behaviour on which virtfs 9p - expects only one device to be shared with the same export, and - if more than one device is shared and accessed via the same 9p - export then only a warning message is logged (once) by qemu on - host side. In order to avoid file ID collisions on guest you - should either create a separate virtfs export for each device to - be shared with guests (recommended way) or you might use "remap" - instead which allows you to share multiple devices with only one - export instead, which is achieved by remapping the original - inode numbers from host to guest in a way that would prevent - such collisions. Remapping inodes in such use cases is required + ``multidevs=remap|forbid|warn`` + Specifies how to deal with multiple devices being shared with + the same 9p export in order to avoid file ID collisions on guest. + Supported behaviours are either "remap" (default), "forbid" or + "warn". + + ``remap`` : assumes the possibility that more than one device is + shared with the same 9p export. Therefore inode numbers from host + are remapped for guest in a way that would prevent file ID + collisions on guest. Remapping inodes in such cases is required because the original device IDs from host are never passed and exposed on guest. Instead all files of an export shared with - virtfs always share the same device id on guest. So two files + virtfs always share the same device ID on guest. So two files with identical inode numbers but from actually different devices on host would otherwise cause a file ID collision and hence - potential misbehaviours on guest. "forbid" on the other hand - assumes like "warn" that only one device is shared by the same - export, however it will not only log a warning message but also - deny access to additional devices on guest. Note though that - "forbid" does currently not block all possible file access - operations (e.g. readdir() would still return entries from other - devices). + potential severe misbehaviours on guest. + + ``warn`` : virtfs 9p expects only one device to be shared with + the same export. If however more than one device is shared and + accessed via the same 9p export then only a warning message is + logged (once) by qemu on host side. No further action is performed + in this case that would prevent file ID collisions on guest. This + could thus lead to severe misbehaviours in this case like wrong + files being accessed and data corruption on the exported tree. + + ``forbid`` : assumes like "warn" that only one device is shared + by the same 9p export, however it will not only log a warning + message but also deny access to additional devices on guest. Note + though that "forbid" does currently not block all possible file + access operations (e.g. readdir() would still return entries from + other devices). ERST DEF("iscsi", HAS_ARG, QEMU_OPTION_iscsi,