From ba42ebb863ab7d40adc79298422ed9596df8f73a Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Mon, 17 Oct 2016 14:13:58 +0200 Subject: [PATCH 01/12] 9pfs: allocate space for guest originated empty strings If a guest sends an empty string paramater to any 9P operation, the current code unmarshals it into a V9fsString equal to { .size = 0, .data = NULL }. This is unfortunate because it can cause NULL pointer dereference to happen at various locations in the 9pfs code. And we don't want to check str->data everywhere we pass it to strcmp() or any other function which expects a dereferenceable pointer. This patch enforces the allocation of genuine C empty strings instead, so callers don't have to bother. Out of all v9fs_iov_vunmarshal() users, only v9fs_xattrwalk() checks if the returned string is empty. It now uses v9fs_string_size() since name.data cannot be NULL anymore. Signed-off-by: Li Qiang [groug, rewritten title and changelog, fix empty string check in v9fs_xattrwalk()] Signed-off-by: Greg Kurz --- fsdev/9p-iov-marshal.c | 2 +- hw/9pfs/9p.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fsdev/9p-iov-marshal.c b/fsdev/9p-iov-marshal.c index 663cad5429..1d16f8df4b 100644 --- a/fsdev/9p-iov-marshal.c +++ b/fsdev/9p-iov-marshal.c @@ -125,7 +125,7 @@ ssize_t v9fs_iov_vunmarshal(struct iovec *out_sg, int out_num, size_t offset, str->data = g_malloc(str->size + 1); copied = v9fs_unpack(str->data, out_sg, out_num, offset, str->size); - if (copied > 0) { + if (copied >= 0) { str->data[str->size] = 0; } else { v9fs_string_free(str); diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index 119ee58496..39a7e1d52d 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -3174,7 +3174,7 @@ static void v9fs_xattrwalk(void *opaque) goto out; } v9fs_path_copy(&xattr_fidp->path, &file_fidp->path); - if (name.data == NULL) { + if (!v9fs_string_size(&name)) { /* * listxattr request. Get the size first */ From e95c9a493a5a8d6f969e86c9f19f80ffe6587e19 Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Mon, 17 Oct 2016 14:13:58 +0200 Subject: [PATCH 02/12] 9pfs: fix potential host memory leak in v9fs_read In 9pfs read dispatch function, it doesn't free two QEMUIOVector object thus causing potential memory leak. This patch avoid this. Signed-off-by: Li Qiang Signed-off-by: Greg Kurz --- hw/9pfs/9p.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index 39a7e1d52d..ff94a6272c 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -1826,14 +1826,15 @@ static void v9fs_read(void *opaque) if (len < 0) { /* IO error return the error */ err = len; - goto out; + goto out_free_iovec; } } while (count < max_count && len > 0); err = pdu_marshal(pdu, offset, "d", count); if (err < 0) { - goto out; + goto out_free_iovec; } err += offset + count; +out_free_iovec: qemu_iovec_destroy(&qiov); qemu_iovec_destroy(&qiov_full); } else if (fidp->fid_type == P9_FID_XATTR) { From bc70a5925f1928623b1fcc033f772daa0d0d271f Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 17 Oct 2016 14:13:58 +0200 Subject: [PATCH 03/12] 9pfs: fsdev: drop useless extern annotation for functions Signed-off-by: Greg Kurz --- fsdev/9p-marshal.h | 6 +-- hw/9pfs/9p-synth.h | 10 ++--- hw/9pfs/9p.h | 18 ++++----- hw/9pfs/coth.h | 94 ++++++++++++++++++++++----------------------- hw/9pfs/virtio-9p.h | 2 +- 5 files changed, 65 insertions(+), 65 deletions(-) diff --git a/fsdev/9p-marshal.h b/fsdev/9p-marshal.h index 77f7fef326..c8823d878f 100644 --- a/fsdev/9p-marshal.h +++ b/fsdev/9p-marshal.h @@ -76,8 +76,8 @@ static inline void v9fs_string_init(V9fsString *str) str->data = NULL; str->size = 0; } -extern void v9fs_string_free(V9fsString *str); -extern void v9fs_string_sprintf(V9fsString *str, const char *fmt, ...); -extern void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs); +void v9fs_string_free(V9fsString *str); +void v9fs_string_sprintf(V9fsString *str, const char *fmt, ...); +void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs); #endif diff --git a/hw/9pfs/9p-synth.h b/hw/9pfs/9p-synth.h index 6bcb44ace2..49c2fc7b27 100644 --- a/hw/9pfs/9p-synth.h +++ b/hw/9pfs/9p-synth.h @@ -43,10 +43,10 @@ typedef struct V9fsSynthOpenState { struct dirent dent; } V9fsSynthOpenState; -extern int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int mode, - const char *name, V9fsSynthNode **result); -extern int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int mode, - const char *name, v9fs_synth_read read, - v9fs_synth_write write, void *arg); +int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int mode, + const char *name, V9fsSynthNode **result); +int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int mode, + const char *name, v9fs_synth_read read, + v9fs_synth_write write, void *arg); #endif diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h index d539d2ebe9..5225b4f120 100644 --- a/hw/9pfs/9p.h +++ b/hw/9pfs/9p.h @@ -324,15 +324,15 @@ static inline uint8_t v9fs_request_cancelled(V9fsPDU *pdu) return pdu->cancelled; } -extern void v9fs_reclaim_fd(V9fsPDU *pdu); -extern void v9fs_path_init(V9fsPath *path); -extern void v9fs_path_free(V9fsPath *path); -extern void v9fs_path_sprintf(V9fsPath *path, const char *fmt, ...); -extern void v9fs_path_copy(V9fsPath *lhs, V9fsPath *rhs); -extern int v9fs_name_to_path(V9fsState *s, V9fsPath *dirpath, - const char *name, V9fsPath *path); -extern int v9fs_device_realize_common(V9fsState *s, Error **errp); -extern void v9fs_device_unrealize_common(V9fsState *s, Error **errp); +void v9fs_reclaim_fd(V9fsPDU *pdu); +void v9fs_path_init(V9fsPath *path); +void v9fs_path_free(V9fsPath *path); +void v9fs_path_sprintf(V9fsPath *path, const char *fmt, ...); +void v9fs_path_copy(V9fsPath *lhs, V9fsPath *rhs); +int v9fs_name_to_path(V9fsState *s, V9fsPath *dirpath, + const char *name, V9fsPath *path); +int v9fs_device_realize_common(V9fsState *s, Error **errp); +void v9fs_device_unrealize_common(V9fsState *s, Error **errp); ssize_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...); ssize_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...); diff --git a/hw/9pfs/coth.h b/hw/9pfs/coth.h index 3c7424e423..af6db5e84e 100644 --- a/hw/9pfs/coth.h +++ b/hw/9pfs/coth.h @@ -47,52 +47,52 @@ qemu_coroutine_yield(); \ } while (0) -extern void co_run_in_worker_bh(void *); -extern int v9fs_co_readlink(V9fsPDU *, V9fsPath *, V9fsString *); -extern int v9fs_co_readdir(V9fsPDU *, V9fsFidState *, struct dirent **); -extern off_t v9fs_co_telldir(V9fsPDU *, V9fsFidState *); -extern void v9fs_co_seekdir(V9fsPDU *, V9fsFidState *, off_t); -extern void v9fs_co_rewinddir(V9fsPDU *, V9fsFidState *); -extern int v9fs_co_statfs(V9fsPDU *, V9fsPath *, struct statfs *); -extern int v9fs_co_lstat(V9fsPDU *, V9fsPath *, struct stat *); -extern int v9fs_co_chmod(V9fsPDU *, V9fsPath *, mode_t); -extern int v9fs_co_utimensat(V9fsPDU *, V9fsPath *, struct timespec [2]); -extern int v9fs_co_chown(V9fsPDU *, V9fsPath *, uid_t, gid_t); -extern int v9fs_co_truncate(V9fsPDU *, V9fsPath *, off_t); -extern int v9fs_co_llistxattr(V9fsPDU *, V9fsPath *, void *, size_t); -extern int v9fs_co_lgetxattr(V9fsPDU *, V9fsPath *, - V9fsString *, void *, size_t); -extern int v9fs_co_mknod(V9fsPDU *, V9fsFidState *, V9fsString *, uid_t, - gid_t, dev_t, mode_t, struct stat *); -extern int v9fs_co_mkdir(V9fsPDU *, V9fsFidState *, V9fsString *, - mode_t, uid_t, gid_t, struct stat *); -extern int v9fs_co_remove(V9fsPDU *, V9fsPath *); -extern int v9fs_co_rename(V9fsPDU *, V9fsPath *, V9fsPath *); -extern int v9fs_co_unlinkat(V9fsPDU *, V9fsPath *, V9fsString *, int flags); -extern int v9fs_co_renameat(V9fsPDU *, V9fsPath *, V9fsString *, - V9fsPath *, V9fsString *); -extern int v9fs_co_fstat(V9fsPDU *, V9fsFidState *, struct stat *); -extern int v9fs_co_opendir(V9fsPDU *, V9fsFidState *); -extern int v9fs_co_open(V9fsPDU *, V9fsFidState *, int); -extern int v9fs_co_open2(V9fsPDU *, V9fsFidState *, V9fsString *, - gid_t, int, int, struct stat *); -extern int v9fs_co_lsetxattr(V9fsPDU *, V9fsPath *, V9fsString *, - void *, size_t, int); -extern int v9fs_co_lremovexattr(V9fsPDU *, V9fsPath *, V9fsString *); -extern int v9fs_co_closedir(V9fsPDU *, V9fsFidOpenState *); -extern int v9fs_co_close(V9fsPDU *, V9fsFidOpenState *); -extern int v9fs_co_fsync(V9fsPDU *, V9fsFidState *, int); -extern int v9fs_co_symlink(V9fsPDU *, V9fsFidState *, V9fsString *, - const char *, gid_t, struct stat *); -extern int v9fs_co_link(V9fsPDU *, V9fsFidState *, - V9fsFidState *, V9fsString *); -extern int v9fs_co_pwritev(V9fsPDU *, V9fsFidState *, - struct iovec *, int, int64_t); -extern int v9fs_co_preadv(V9fsPDU *, V9fsFidState *, - struct iovec *, int, int64_t); -extern int v9fs_co_name_to_path(V9fsPDU *, V9fsPath *, - const char *, V9fsPath *); -extern int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t, - V9fsStatDotl *v9stat); +void co_run_in_worker_bh(void *); +int v9fs_co_readlink(V9fsPDU *, V9fsPath *, V9fsString *); +int v9fs_co_readdir(V9fsPDU *, V9fsFidState *, struct dirent **); +off_t v9fs_co_telldir(V9fsPDU *, V9fsFidState *); +void v9fs_co_seekdir(V9fsPDU *, V9fsFidState *, off_t); +void v9fs_co_rewinddir(V9fsPDU *, V9fsFidState *); +int v9fs_co_statfs(V9fsPDU *, V9fsPath *, struct statfs *); +int v9fs_co_lstat(V9fsPDU *, V9fsPath *, struct stat *); +int v9fs_co_chmod(V9fsPDU *, V9fsPath *, mode_t); +int v9fs_co_utimensat(V9fsPDU *, V9fsPath *, struct timespec [2]); +int v9fs_co_chown(V9fsPDU *, V9fsPath *, uid_t, gid_t); +int v9fs_co_truncate(V9fsPDU *, V9fsPath *, off_t); +int v9fs_co_llistxattr(V9fsPDU *, V9fsPath *, void *, size_t); +int v9fs_co_lgetxattr(V9fsPDU *, V9fsPath *, + V9fsString *, void *, size_t); +int v9fs_co_mknod(V9fsPDU *, V9fsFidState *, V9fsString *, uid_t, + gid_t, dev_t, mode_t, struct stat *); +int v9fs_co_mkdir(V9fsPDU *, V9fsFidState *, V9fsString *, + mode_t, uid_t, gid_t, struct stat *); +int v9fs_co_remove(V9fsPDU *, V9fsPath *); +int v9fs_co_rename(V9fsPDU *, V9fsPath *, V9fsPath *); +int v9fs_co_unlinkat(V9fsPDU *, V9fsPath *, V9fsString *, int flags); +int v9fs_co_renameat(V9fsPDU *, V9fsPath *, V9fsString *, + V9fsPath *, V9fsString *); +int v9fs_co_fstat(V9fsPDU *, V9fsFidState *, struct stat *); +int v9fs_co_opendir(V9fsPDU *, V9fsFidState *); +int v9fs_co_open(V9fsPDU *, V9fsFidState *, int); +int v9fs_co_open2(V9fsPDU *, V9fsFidState *, V9fsString *, + gid_t, int, int, struct stat *); +int v9fs_co_lsetxattr(V9fsPDU *, V9fsPath *, V9fsString *, + void *, size_t, int); +int v9fs_co_lremovexattr(V9fsPDU *, V9fsPath *, V9fsString *); +int v9fs_co_closedir(V9fsPDU *, V9fsFidOpenState *); +int v9fs_co_close(V9fsPDU *, V9fsFidOpenState *); +int v9fs_co_fsync(V9fsPDU *, V9fsFidState *, int); +int v9fs_co_symlink(V9fsPDU *, V9fsFidState *, V9fsString *, + const char *, gid_t, struct stat *); +int v9fs_co_link(V9fsPDU *, V9fsFidState *, + V9fsFidState *, V9fsString *); +int v9fs_co_pwritev(V9fsPDU *, V9fsFidState *, + struct iovec *, int, int64_t); +int v9fs_co_preadv(V9fsPDU *, V9fsFidState *, + struct iovec *, int, int64_t); +int v9fs_co_name_to_path(V9fsPDU *, V9fsPath *, + const char *, V9fsPath *); +int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t, + V9fsStatDotl *v9stat); #endif diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h index 7586b792d6..25c47c7cb6 100644 --- a/hw/9pfs/virtio-9p.h +++ b/hw/9pfs/virtio-9p.h @@ -15,7 +15,7 @@ typedef struct V9fsVirtioState V9fsState state; } V9fsVirtioState; -extern void virtio_9p_push_and_notify(V9fsPDU *pdu); +void virtio_9p_push_and_notify(V9fsPDU *pdu); ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, va_list ap); From 5bdade66211c8023d8e81c535f4944cbf830b25a Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 17 Oct 2016 14:13:58 +0200 Subject: [PATCH 04/12] 9pfs: use coroutine_fn annotation in hw/9pfs/co*.[ch] All these functions use the v9fs_co_run_in_worker() macro, and thus always call qemu_coroutine_self() and qemu_coroutine_yield(). Let's mark them to make it obvious they execute in coroutine context. Signed-off-by: Greg Kurz --- hw/9pfs/codir.c | 17 +++++---- hw/9pfs/cofile.c | 32 ++++++++-------- hw/9pfs/cofs.c | 43 +++++++++++++--------- hw/9pfs/coth.h | 93 ++++++++++++++++++++++++----------------------- hw/9pfs/coxattr.c | 19 +++++----- 5 files changed, 109 insertions(+), 95 deletions(-) diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c index d91f9ad6eb..7cd6fce1ad 100644 --- a/hw/9pfs/codir.c +++ b/hw/9pfs/codir.c @@ -17,7 +17,8 @@ #include "qemu/coroutine.h" #include "coth.h" -int v9fs_co_readdir(V9fsPDU *pdu, V9fsFidState *fidp, struct dirent **dent) +int coroutine_fn v9fs_co_readdir(V9fsPDU *pdu, V9fsFidState *fidp, + struct dirent **dent) { int err; V9fsState *s = pdu->s; @@ -59,7 +60,8 @@ off_t v9fs_co_telldir(V9fsPDU *pdu, V9fsFidState *fidp) return err; } -void v9fs_co_seekdir(V9fsPDU *pdu, V9fsFidState *fidp, off_t offset) +void coroutine_fn v9fs_co_seekdir(V9fsPDU *pdu, V9fsFidState *fidp, + off_t offset) { V9fsState *s = pdu->s; if (v9fs_request_cancelled(pdu)) { @@ -71,7 +73,7 @@ void v9fs_co_seekdir(V9fsPDU *pdu, V9fsFidState *fidp, off_t offset) }); } -void v9fs_co_rewinddir(V9fsPDU *pdu, V9fsFidState *fidp) +void coroutine_fn v9fs_co_rewinddir(V9fsPDU *pdu, V9fsFidState *fidp) { V9fsState *s = pdu->s; if (v9fs_request_cancelled(pdu)) { @@ -83,8 +85,9 @@ void v9fs_co_rewinddir(V9fsPDU *pdu, V9fsFidState *fidp) }); } -int v9fs_co_mkdir(V9fsPDU *pdu, V9fsFidState *fidp, V9fsString *name, - mode_t mode, uid_t uid, gid_t gid, struct stat *stbuf) +int coroutine_fn v9fs_co_mkdir(V9fsPDU *pdu, V9fsFidState *fidp, + V9fsString *name, mode_t mode, uid_t uid, + gid_t gid, struct stat *stbuf) { int err; FsCred cred; @@ -120,7 +123,7 @@ int v9fs_co_mkdir(V9fsPDU *pdu, V9fsFidState *fidp, V9fsString *name, return err; } -int v9fs_co_opendir(V9fsPDU *pdu, V9fsFidState *fidp) +int coroutine_fn v9fs_co_opendir(V9fsPDU *pdu, V9fsFidState *fidp) { int err; V9fsState *s = pdu->s; @@ -148,7 +151,7 @@ int v9fs_co_opendir(V9fsPDU *pdu, V9fsFidState *fidp) return err; } -int v9fs_co_closedir(V9fsPDU *pdu, V9fsFidOpenState *fs) +int coroutine_fn v9fs_co_closedir(V9fsPDU *pdu, V9fsFidOpenState *fs) { int err; V9fsState *s = pdu->s; diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c index 10343c0a93..120e267108 100644 --- a/hw/9pfs/cofile.c +++ b/hw/9pfs/cofile.c @@ -17,8 +17,8 @@ #include "qemu/coroutine.h" #include "coth.h" -int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t st_mode, - V9fsStatDotl *v9stat) +int coroutine_fn v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t st_mode, + V9fsStatDotl *v9stat) { int err = 0; V9fsState *s = pdu->s; @@ -41,7 +41,7 @@ int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t st_mode, return err; } -int v9fs_co_lstat(V9fsPDU *pdu, V9fsPath *path, struct stat *stbuf) +int coroutine_fn v9fs_co_lstat(V9fsPDU *pdu, V9fsPath *path, struct stat *stbuf) { int err; V9fsState *s = pdu->s; @@ -61,7 +61,8 @@ int v9fs_co_lstat(V9fsPDU *pdu, V9fsPath *path, struct stat *stbuf) return err; } -int v9fs_co_fstat(V9fsPDU *pdu, V9fsFidState *fidp, struct stat *stbuf) +int coroutine_fn v9fs_co_fstat(V9fsPDU *pdu, V9fsFidState *fidp, + struct stat *stbuf) { int err; V9fsState *s = pdu->s; @@ -93,7 +94,7 @@ int v9fs_co_fstat(V9fsPDU *pdu, V9fsFidState *fidp, struct stat *stbuf) return err; } -int v9fs_co_open(V9fsPDU *pdu, V9fsFidState *fidp, int flags) +int coroutine_fn v9fs_co_open(V9fsPDU *pdu, V9fsFidState *fidp, int flags) { int err; V9fsState *s = pdu->s; @@ -121,8 +122,9 @@ int v9fs_co_open(V9fsPDU *pdu, V9fsFidState *fidp, int flags) return err; } -int v9fs_co_open2(V9fsPDU *pdu, V9fsFidState *fidp, V9fsString *name, gid_t gid, - int flags, int mode, struct stat *stbuf) +int coroutine_fn v9fs_co_open2(V9fsPDU *pdu, V9fsFidState *fidp, + V9fsString *name, gid_t gid, int flags, int mode, + struct stat *stbuf) { int err; FsCred cred; @@ -175,7 +177,7 @@ int v9fs_co_open2(V9fsPDU *pdu, V9fsFidState *fidp, V9fsString *name, gid_t gid, return err; } -int v9fs_co_close(V9fsPDU *pdu, V9fsFidOpenState *fs) +int coroutine_fn v9fs_co_close(V9fsPDU *pdu, V9fsFidOpenState *fs) { int err; V9fsState *s = pdu->s; @@ -196,7 +198,7 @@ int v9fs_co_close(V9fsPDU *pdu, V9fsFidOpenState *fs) return err; } -int v9fs_co_fsync(V9fsPDU *pdu, V9fsFidState *fidp, int datasync) +int coroutine_fn v9fs_co_fsync(V9fsPDU *pdu, V9fsFidState *fidp, int datasync) { int err; V9fsState *s = pdu->s; @@ -214,8 +216,8 @@ int v9fs_co_fsync(V9fsPDU *pdu, V9fsFidState *fidp, int datasync) return err; } -int v9fs_co_link(V9fsPDU *pdu, V9fsFidState *oldfid, - V9fsFidState *newdirfid, V9fsString *name) +int coroutine_fn v9fs_co_link(V9fsPDU *pdu, V9fsFidState *oldfid, + V9fsFidState *newdirfid, V9fsString *name) { int err; V9fsState *s = pdu->s; @@ -236,8 +238,8 @@ int v9fs_co_link(V9fsPDU *pdu, V9fsFidState *oldfid, return err; } -int v9fs_co_pwritev(V9fsPDU *pdu, V9fsFidState *fidp, - struct iovec *iov, int iovcnt, int64_t offset) +int coroutine_fn v9fs_co_pwritev(V9fsPDU *pdu, V9fsFidState *fidp, + struct iovec *iov, int iovcnt, int64_t offset) { int err; V9fsState *s = pdu->s; @@ -255,8 +257,8 @@ int v9fs_co_pwritev(V9fsPDU *pdu, V9fsFidState *fidp, return err; } -int v9fs_co_preadv(V9fsPDU *pdu, V9fsFidState *fidp, - struct iovec *iov, int iovcnt, int64_t offset) +int coroutine_fn v9fs_co_preadv(V9fsPDU *pdu, V9fsFidState *fidp, + struct iovec *iov, int iovcnt, int64_t offset) { int err; V9fsState *s = pdu->s; diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c index 70f584fcbd..c62103221d 100644 --- a/hw/9pfs/cofs.c +++ b/hw/9pfs/cofs.c @@ -49,7 +49,7 @@ static ssize_t __readlink(V9fsState *s, V9fsPath *path, V9fsString *buf) return len; } -int v9fs_co_readlink(V9fsPDU *pdu, V9fsPath *path, V9fsString *buf) +int coroutine_fn v9fs_co_readlink(V9fsPDU *pdu, V9fsPath *path, V9fsString *buf) { int err; V9fsState *s = pdu->s; @@ -69,7 +69,8 @@ int v9fs_co_readlink(V9fsPDU *pdu, V9fsPath *path, V9fsString *buf) return err; } -int v9fs_co_statfs(V9fsPDU *pdu, V9fsPath *path, struct statfs *stbuf) +int coroutine_fn v9fs_co_statfs(V9fsPDU *pdu, V9fsPath *path, + struct statfs *stbuf) { int err; V9fsState *s = pdu->s; @@ -89,7 +90,7 @@ int v9fs_co_statfs(V9fsPDU *pdu, V9fsPath *path, struct statfs *stbuf) return err; } -int v9fs_co_chmod(V9fsPDU *pdu, V9fsPath *path, mode_t mode) +int coroutine_fn v9fs_co_chmod(V9fsPDU *pdu, V9fsPath *path, mode_t mode) { int err; FsCred cred; @@ -112,8 +113,8 @@ int v9fs_co_chmod(V9fsPDU *pdu, V9fsPath *path, mode_t mode) return err; } -int v9fs_co_utimensat(V9fsPDU *pdu, V9fsPath *path, - struct timespec times[2]) +int coroutine_fn v9fs_co_utimensat(V9fsPDU *pdu, V9fsPath *path, + struct timespec times[2]) { int err; V9fsState *s = pdu->s; @@ -133,7 +134,8 @@ int v9fs_co_utimensat(V9fsPDU *pdu, V9fsPath *path, return err; } -int v9fs_co_chown(V9fsPDU *pdu, V9fsPath *path, uid_t uid, gid_t gid) +int coroutine_fn v9fs_co_chown(V9fsPDU *pdu, V9fsPath *path, uid_t uid, + gid_t gid) { int err; FsCred cred; @@ -157,7 +159,7 @@ int v9fs_co_chown(V9fsPDU *pdu, V9fsPath *path, uid_t uid, gid_t gid) return err; } -int v9fs_co_truncate(V9fsPDU *pdu, V9fsPath *path, off_t size) +int coroutine_fn v9fs_co_truncate(V9fsPDU *pdu, V9fsPath *path, off_t size) { int err; V9fsState *s = pdu->s; @@ -177,8 +179,9 @@ int v9fs_co_truncate(V9fsPDU *pdu, V9fsPath *path, off_t size) return err; } -int v9fs_co_mknod(V9fsPDU *pdu, V9fsFidState *fidp, V9fsString *name, uid_t uid, - gid_t gid, dev_t dev, mode_t mode, struct stat *stbuf) +int coroutine_fn v9fs_co_mknod(V9fsPDU *pdu, V9fsFidState *fidp, + V9fsString *name, uid_t uid, gid_t gid, + dev_t dev, mode_t mode, struct stat *stbuf) { int err; V9fsPath path; @@ -216,7 +219,7 @@ int v9fs_co_mknod(V9fsPDU *pdu, V9fsFidState *fidp, V9fsString *name, uid_t uid, } /* Only works with path name based fid */ -int v9fs_co_remove(V9fsPDU *pdu, V9fsPath *path) +int coroutine_fn v9fs_co_remove(V9fsPDU *pdu, V9fsPath *path) { int err; V9fsState *s = pdu->s; @@ -236,7 +239,8 @@ int v9fs_co_remove(V9fsPDU *pdu, V9fsPath *path) return err; } -int v9fs_co_unlinkat(V9fsPDU *pdu, V9fsPath *path, V9fsString *name, int flags) +int coroutine_fn v9fs_co_unlinkat(V9fsPDU *pdu, V9fsPath *path, + V9fsString *name, int flags) { int err; V9fsState *s = pdu->s; @@ -257,7 +261,8 @@ int v9fs_co_unlinkat(V9fsPDU *pdu, V9fsPath *path, V9fsString *name, int flags) } /* Only work with path name based fid */ -int v9fs_co_rename(V9fsPDU *pdu, V9fsPath *oldpath, V9fsPath *newpath) +int coroutine_fn v9fs_co_rename(V9fsPDU *pdu, V9fsPath *oldpath, + V9fsPath *newpath) { int err; V9fsState *s = pdu->s; @@ -275,8 +280,9 @@ int v9fs_co_rename(V9fsPDU *pdu, V9fsPath *oldpath, V9fsPath *newpath) return err; } -int v9fs_co_renameat(V9fsPDU *pdu, V9fsPath *olddirpath, V9fsString *oldname, - V9fsPath *newdirpath, V9fsString *newname) +int coroutine_fn v9fs_co_renameat(V9fsPDU *pdu, V9fsPath *olddirpath, + V9fsString *oldname, V9fsPath *newdirpath, + V9fsString *newname) { int err; V9fsState *s = pdu->s; @@ -295,8 +301,9 @@ int v9fs_co_renameat(V9fsPDU *pdu, V9fsPath *olddirpath, V9fsString *oldname, return err; } -int v9fs_co_symlink(V9fsPDU *pdu, V9fsFidState *dfidp, V9fsString *name, - const char *oldpath, gid_t gid, struct stat *stbuf) +int coroutine_fn v9fs_co_symlink(V9fsPDU *pdu, V9fsFidState *dfidp, + V9fsString *name, const char *oldpath, + gid_t gid, struct stat *stbuf) { int err; FsCred cred; @@ -337,8 +344,8 @@ int v9fs_co_symlink(V9fsPDU *pdu, V9fsFidState *dfidp, V9fsString *name, * For path name based fid we don't block. So we can * directly call the fs driver ops. */ -int v9fs_co_name_to_path(V9fsPDU *pdu, V9fsPath *dirpath, - const char *name, V9fsPath *path) +int coroutine_fn v9fs_co_name_to_path(V9fsPDU *pdu, V9fsPath *dirpath, + const char *name, V9fsPath *path) { int err; V9fsState *s = pdu->s; diff --git a/hw/9pfs/coth.h b/hw/9pfs/coth.h index af6db5e84e..19e4d9287e 100644 --- a/hw/9pfs/coth.h +++ b/hw/9pfs/coth.h @@ -48,51 +48,52 @@ } while (0) void co_run_in_worker_bh(void *); -int v9fs_co_readlink(V9fsPDU *, V9fsPath *, V9fsString *); -int v9fs_co_readdir(V9fsPDU *, V9fsFidState *, struct dirent **); -off_t v9fs_co_telldir(V9fsPDU *, V9fsFidState *); -void v9fs_co_seekdir(V9fsPDU *, V9fsFidState *, off_t); -void v9fs_co_rewinddir(V9fsPDU *, V9fsFidState *); -int v9fs_co_statfs(V9fsPDU *, V9fsPath *, struct statfs *); -int v9fs_co_lstat(V9fsPDU *, V9fsPath *, struct stat *); -int v9fs_co_chmod(V9fsPDU *, V9fsPath *, mode_t); -int v9fs_co_utimensat(V9fsPDU *, V9fsPath *, struct timespec [2]); -int v9fs_co_chown(V9fsPDU *, V9fsPath *, uid_t, gid_t); -int v9fs_co_truncate(V9fsPDU *, V9fsPath *, off_t); -int v9fs_co_llistxattr(V9fsPDU *, V9fsPath *, void *, size_t); -int v9fs_co_lgetxattr(V9fsPDU *, V9fsPath *, - V9fsString *, void *, size_t); -int v9fs_co_mknod(V9fsPDU *, V9fsFidState *, V9fsString *, uid_t, - gid_t, dev_t, mode_t, struct stat *); -int v9fs_co_mkdir(V9fsPDU *, V9fsFidState *, V9fsString *, - mode_t, uid_t, gid_t, struct stat *); -int v9fs_co_remove(V9fsPDU *, V9fsPath *); -int v9fs_co_rename(V9fsPDU *, V9fsPath *, V9fsPath *); -int v9fs_co_unlinkat(V9fsPDU *, V9fsPath *, V9fsString *, int flags); -int v9fs_co_renameat(V9fsPDU *, V9fsPath *, V9fsString *, - V9fsPath *, V9fsString *); -int v9fs_co_fstat(V9fsPDU *, V9fsFidState *, struct stat *); -int v9fs_co_opendir(V9fsPDU *, V9fsFidState *); -int v9fs_co_open(V9fsPDU *, V9fsFidState *, int); -int v9fs_co_open2(V9fsPDU *, V9fsFidState *, V9fsString *, - gid_t, int, int, struct stat *); -int v9fs_co_lsetxattr(V9fsPDU *, V9fsPath *, V9fsString *, - void *, size_t, int); -int v9fs_co_lremovexattr(V9fsPDU *, V9fsPath *, V9fsString *); -int v9fs_co_closedir(V9fsPDU *, V9fsFidOpenState *); -int v9fs_co_close(V9fsPDU *, V9fsFidOpenState *); -int v9fs_co_fsync(V9fsPDU *, V9fsFidState *, int); -int v9fs_co_symlink(V9fsPDU *, V9fsFidState *, V9fsString *, - const char *, gid_t, struct stat *); -int v9fs_co_link(V9fsPDU *, V9fsFidState *, - V9fsFidState *, V9fsString *); -int v9fs_co_pwritev(V9fsPDU *, V9fsFidState *, - struct iovec *, int, int64_t); -int v9fs_co_preadv(V9fsPDU *, V9fsFidState *, - struct iovec *, int, int64_t); -int v9fs_co_name_to_path(V9fsPDU *, V9fsPath *, - const char *, V9fsPath *); -int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t, - V9fsStatDotl *v9stat); +int coroutine_fn v9fs_co_readlink(V9fsPDU *, V9fsPath *, V9fsString *); +int coroutine_fn v9fs_co_readdir(V9fsPDU *, V9fsFidState *, struct dirent **); +off_t coroutine_fn v9fs_co_telldir(V9fsPDU *, V9fsFidState *); +void coroutine_fn v9fs_co_seekdir(V9fsPDU *, V9fsFidState *, off_t); +void coroutine_fn v9fs_co_rewinddir(V9fsPDU *, V9fsFidState *); +int coroutine_fn v9fs_co_statfs(V9fsPDU *, V9fsPath *, struct statfs *); +int coroutine_fn v9fs_co_lstat(V9fsPDU *, V9fsPath *, struct stat *); +int coroutine_fn v9fs_co_chmod(V9fsPDU *, V9fsPath *, mode_t); +int coroutine_fn v9fs_co_utimensat(V9fsPDU *, V9fsPath *, struct timespec [2]); +int coroutine_fn v9fs_co_chown(V9fsPDU *, V9fsPath *, uid_t, gid_t); +int coroutine_fn v9fs_co_truncate(V9fsPDU *, V9fsPath *, off_t); +int coroutine_fn v9fs_co_llistxattr(V9fsPDU *, V9fsPath *, void *, size_t); +int coroutine_fn v9fs_co_lgetxattr(V9fsPDU *, V9fsPath *, + V9fsString *, void *, size_t); +int coroutine_fn v9fs_co_mknod(V9fsPDU *, V9fsFidState *, V9fsString *, uid_t, + gid_t, dev_t, mode_t, struct stat *); +int coroutine_fn v9fs_co_mkdir(V9fsPDU *, V9fsFidState *, V9fsString *, + mode_t, uid_t, gid_t, struct stat *); +int coroutine_fn v9fs_co_remove(V9fsPDU *, V9fsPath *); +int coroutine_fn v9fs_co_rename(V9fsPDU *, V9fsPath *, V9fsPath *); +int coroutine_fn v9fs_co_unlinkat(V9fsPDU *, V9fsPath *, V9fsString *, + int flags); +int coroutine_fn v9fs_co_renameat(V9fsPDU *, V9fsPath *, V9fsString *, + V9fsPath *, V9fsString *); +int coroutine_fn v9fs_co_fstat(V9fsPDU *, V9fsFidState *, struct stat *); +int coroutine_fn v9fs_co_opendir(V9fsPDU *, V9fsFidState *); +int coroutine_fn v9fs_co_open(V9fsPDU *, V9fsFidState *, int); +int coroutine_fn v9fs_co_open2(V9fsPDU *, V9fsFidState *, V9fsString *, + gid_t, int, int, struct stat *); +int coroutine_fn v9fs_co_lsetxattr(V9fsPDU *, V9fsPath *, V9fsString *, + void *, size_t, int); +int coroutine_fn v9fs_co_lremovexattr(V9fsPDU *, V9fsPath *, V9fsString *); +int coroutine_fn v9fs_co_closedir(V9fsPDU *, V9fsFidOpenState *); +int coroutine_fn v9fs_co_close(V9fsPDU *, V9fsFidOpenState *); +int coroutine_fn v9fs_co_fsync(V9fsPDU *, V9fsFidState *, int); +int coroutine_fn v9fs_co_symlink(V9fsPDU *, V9fsFidState *, V9fsString *, + const char *, gid_t, struct stat *); +int coroutine_fn v9fs_co_link(V9fsPDU *, V9fsFidState *, + V9fsFidState *, V9fsString *); +int coroutine_fn v9fs_co_pwritev(V9fsPDU *, V9fsFidState *, + struct iovec *, int, int64_t); +int coroutine_fn v9fs_co_preadv(V9fsPDU *, V9fsFidState *, + struct iovec *, int, int64_t); +int coroutine_fn v9fs_co_name_to_path(V9fsPDU *, V9fsPath *, + const char *, V9fsPath *); +int coroutine_fn v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t, + V9fsStatDotl *v9stat); #endif diff --git a/hw/9pfs/coxattr.c b/hw/9pfs/coxattr.c index 133c4ead37..154392eade 100644 --- a/hw/9pfs/coxattr.c +++ b/hw/9pfs/coxattr.c @@ -17,7 +17,8 @@ #include "qemu/coroutine.h" #include "coth.h" -int v9fs_co_llistxattr(V9fsPDU *pdu, V9fsPath *path, void *value, size_t size) +int coroutine_fn v9fs_co_llistxattr(V9fsPDU *pdu, V9fsPath *path, void *value, + size_t size) { int err; V9fsState *s = pdu->s; @@ -37,9 +38,9 @@ int v9fs_co_llistxattr(V9fsPDU *pdu, V9fsPath *path, void *value, size_t size) return err; } -int v9fs_co_lgetxattr(V9fsPDU *pdu, V9fsPath *path, - V9fsString *xattr_name, - void *value, size_t size) +int coroutine_fn v9fs_co_lgetxattr(V9fsPDU *pdu, V9fsPath *path, + V9fsString *xattr_name, void *value, + size_t size) { int err; V9fsState *s = pdu->s; @@ -61,9 +62,9 @@ int v9fs_co_lgetxattr(V9fsPDU *pdu, V9fsPath *path, return err; } -int v9fs_co_lsetxattr(V9fsPDU *pdu, V9fsPath *path, - V9fsString *xattr_name, void *value, - size_t size, int flags) +int coroutine_fn v9fs_co_lsetxattr(V9fsPDU *pdu, V9fsPath *path, + V9fsString *xattr_name, void *value, + size_t size, int flags) { int err; V9fsState *s = pdu->s; @@ -85,8 +86,8 @@ int v9fs_co_lsetxattr(V9fsPDU *pdu, V9fsPath *path, return err; } -int v9fs_co_lremovexattr(V9fsPDU *pdu, V9fsPath *path, - V9fsString *xattr_name) +int coroutine_fn v9fs_co_lremovexattr(V9fsPDU *pdu, V9fsPath *path, + V9fsString *xattr_name) { int err; V9fsState *s = pdu->s; From 8440e22ec1a5deabc4fcf5c4826d5c73ddc15765 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 17 Oct 2016 14:13:58 +0200 Subject: [PATCH 05/12] 9pfs: use coroutine_fn annotation in hw/9pfs/9p.[ch] All these functions either call the v9fs_co_* functions which have the coroutine_fn annotation, or pdu_complete() which calls qemu_co_queue_next(). Let's mark them to make it obvious they execute in coroutine context. Signed-off-by: Greg Kurz --- hw/9pfs/9p.c | 117 +++++++++++++++++++++++++++------------------------ hw/9pfs/9p.h | 2 +- 2 files changed, 62 insertions(+), 57 deletions(-) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index ff94a6272c..f2ab1dfab2 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -236,7 +236,7 @@ static size_t v9fs_string_size(V9fsString *str) /* * returns 0 if fid got re-opened, 1 if not, < 0 on error */ -static int v9fs_reopen_fid(V9fsPDU *pdu, V9fsFidState *f) +static int coroutine_fn v9fs_reopen_fid(V9fsPDU *pdu, V9fsFidState *f) { int err = 1; if (f->fid_type == P9_FID_FILE) { @@ -255,7 +255,7 @@ static int v9fs_reopen_fid(V9fsPDU *pdu, V9fsFidState *f) return err; } -static V9fsFidState *get_fid(V9fsPDU *pdu, int32_t fid) +static V9fsFidState *coroutine_fn get_fid(V9fsPDU *pdu, int32_t fid) { int err; V9fsFidState *f; @@ -321,7 +321,7 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid) return f; } -static int v9fs_xattr_fid_clunk(V9fsPDU *pdu, V9fsFidState *fidp) +static int coroutine_fn v9fs_xattr_fid_clunk(V9fsPDU *pdu, V9fsFidState *fidp) { int retval = 0; @@ -353,7 +353,7 @@ free_value: return retval; } -static int free_fid(V9fsPDU *pdu, V9fsFidState *fidp) +static int coroutine_fn free_fid(V9fsPDU *pdu, V9fsFidState *fidp) { int retval = 0; @@ -374,7 +374,7 @@ static int free_fid(V9fsPDU *pdu, V9fsFidState *fidp) return retval; } -static int put_fid(V9fsPDU *pdu, V9fsFidState *fidp) +static int coroutine_fn put_fid(V9fsPDU *pdu, V9fsFidState *fidp) { BUG_ON(!fidp->ref); fidp->ref--; @@ -418,7 +418,7 @@ static V9fsFidState *clunk_fid(V9fsState *s, int32_t fid) return fidp; } -void v9fs_reclaim_fd(V9fsPDU *pdu) +void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu) { int reclaim_count = 0; V9fsState *s = pdu->s; @@ -499,7 +499,7 @@ void v9fs_reclaim_fd(V9fsPDU *pdu) } } -static int v9fs_mark_fids_unreclaim(V9fsPDU *pdu, V9fsPath *path) +static int coroutine_fn v9fs_mark_fids_unreclaim(V9fsPDU *pdu, V9fsPath *path) { int err; V9fsState *s = pdu->s; @@ -532,7 +532,7 @@ static int v9fs_mark_fids_unreclaim(V9fsPDU *pdu, V9fsPath *path) return 0; } -static void virtfs_reset(V9fsPDU *pdu) +static void coroutine_fn virtfs_reset(V9fsPDU *pdu) { V9fsState *s = pdu->s; V9fsFidState *fidp = NULL; @@ -598,7 +598,8 @@ static void stat_to_qid(const struct stat *stbuf, V9fsQID *qidp) } } -static int fid_to_qid(V9fsPDU *pdu, V9fsFidState *fidp, V9fsQID *qidp) +static int coroutine_fn fid_to_qid(V9fsPDU *pdu, V9fsFidState *fidp, + V9fsQID *qidp) { struct stat stbuf; int err; @@ -643,7 +644,7 @@ void pdu_free(V9fsPDU *pdu) * because we always expect to have enough space to encode * error details */ -static void pdu_complete(V9fsPDU *pdu, ssize_t len) +static void coroutine_fn pdu_complete(V9fsPDU *pdu, ssize_t len) { int8_t id = pdu->id + 1; /* Response */ V9fsState *s = pdu->s; @@ -810,9 +811,9 @@ static uint32_t stat_to_v9mode(const struct stat *stbuf) return mode; } -static int stat_to_v9stat(V9fsPDU *pdu, V9fsPath *name, - const struct stat *stbuf, - V9fsStat *v9stat) +static int coroutine_fn stat_to_v9stat(V9fsPDU *pdu, V9fsPath *name, + const struct stat *stbuf, + V9fsStat *v9stat) { int err; const char *str; @@ -941,7 +942,7 @@ static inline bool is_ro_export(FsContext *ctx) return ctx->export_flags & V9FS_RDONLY; } -static void v9fs_version(void *opaque) +static void coroutine_fn v9fs_version(void *opaque) { ssize_t err; V9fsPDU *pdu = opaque; @@ -979,7 +980,7 @@ out: v9fs_string_free(&version); } -static void v9fs_attach(void *opaque) +static void coroutine_fn v9fs_attach(void *opaque) { V9fsPDU *pdu = opaque; V9fsState *s = pdu->s; @@ -1045,7 +1046,7 @@ out_nofid: v9fs_string_free(&aname); } -static void v9fs_stat(void *opaque) +static void coroutine_fn v9fs_stat(void *opaque) { int32_t fid; V9fsStat v9stat; @@ -1089,7 +1090,7 @@ out_nofid: pdu_complete(pdu, err); } -static void v9fs_getattr(void *opaque) +static void coroutine_fn v9fs_getattr(void *opaque) { int32_t fid; size_t offset = 7; @@ -1165,7 +1166,7 @@ out_nofid: #define P9_ATTR_MASK 127 -static void v9fs_setattr(void *opaque) +static void coroutine_fn v9fs_setattr(void *opaque) { int err = 0; int32_t fid; @@ -1283,7 +1284,7 @@ static bool not_same_qid(const V9fsQID *qid1, const V9fsQID *qid2) qid1->path != qid2->path; } -static void v9fs_walk(void *opaque) +static void coroutine_fn v9fs_walk(void *opaque) { int name_idx; V9fsQID *qids = NULL; @@ -1397,7 +1398,7 @@ out_nofid: } } -static int32_t get_iounit(V9fsPDU *pdu, V9fsPath *path) +static int32_t coroutine_fn get_iounit(V9fsPDU *pdu, V9fsPath *path) { struct statfs stbuf; int32_t iounit = 0; @@ -1417,7 +1418,7 @@ static int32_t get_iounit(V9fsPDU *pdu, V9fsPath *path) return iounit; } -static void v9fs_open(void *opaque) +static void coroutine_fn v9fs_open(void *opaque) { int flags; int32_t fid; @@ -1507,7 +1508,7 @@ out_nofid: pdu_complete(pdu, err); } -static void v9fs_lcreate(void *opaque) +static void coroutine_fn v9fs_lcreate(void *opaque) { int32_t dfid, flags, mode; gid_t gid; @@ -1604,7 +1605,7 @@ out_nofid: pdu_complete(pdu, err); } -static void v9fs_clunk(void *opaque) +static void coroutine_fn v9fs_clunk(void *opaque) { int err; int32_t fid; @@ -1673,8 +1674,9 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp, return offset; } -static int v9fs_do_readdir_with_stat(V9fsPDU *pdu, - V9fsFidState *fidp, uint32_t max_count) +static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU *pdu, + V9fsFidState *fidp, + uint32_t max_count) { V9fsPath path; V9fsStat v9stat; @@ -1764,7 +1766,7 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu, qemu_iovec_concat(qiov, &elem, skip, size); } -static void v9fs_read(void *opaque) +static void coroutine_fn v9fs_read(void *opaque) { int32_t fid; uint64_t off; @@ -1858,8 +1860,8 @@ static size_t v9fs_readdir_data_size(V9fsString *name) return 24 + v9fs_string_size(name); } -static int v9fs_do_readdir(V9fsPDU *pdu, - V9fsFidState *fidp, int32_t max_count) +static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, V9fsFidState *fidp, + int32_t max_count) { size_t size; V9fsQID qid; @@ -1928,7 +1930,7 @@ static int v9fs_do_readdir(V9fsPDU *pdu, return count; } -static void v9fs_readdir(void *opaque) +static void coroutine_fn v9fs_readdir(void *opaque) { int32_t fid; V9fsFidState *fidp; @@ -2024,7 +2026,7 @@ out: return err; } -static void v9fs_write(void *opaque) +static void coroutine_fn v9fs_write(void *opaque) { ssize_t err; int32_t fid; @@ -2107,7 +2109,7 @@ out_nofid: pdu_complete(pdu, err); } -static void v9fs_create(void *opaque) +static void coroutine_fn v9fs_create(void *opaque) { int32_t fid; int err = 0; @@ -2287,7 +2289,7 @@ out_nofid: v9fs_path_free(&path); } -static void v9fs_symlink(void *opaque) +static void coroutine_fn v9fs_symlink(void *opaque) { V9fsPDU *pdu = opaque; V9fsString name; @@ -2376,7 +2378,7 @@ static void v9fs_flush(void *opaque) pdu_complete(pdu, 7); } -static void v9fs_link(void *opaque) +static void coroutine_fn v9fs_link(void *opaque) { V9fsPDU *pdu = opaque; int32_t dfid, oldfid; @@ -2425,7 +2427,7 @@ out_nofid: } /* Only works with path name based fid */ -static void v9fs_remove(void *opaque) +static void coroutine_fn v9fs_remove(void *opaque) { int32_t fid; int err = 0; @@ -2469,7 +2471,7 @@ out_nofid: pdu_complete(pdu, err); } -static void v9fs_unlinkat(void *opaque) +static void coroutine_fn v9fs_unlinkat(void *opaque) { int err = 0; V9fsString name; @@ -2532,8 +2534,9 @@ out_nofid: /* Only works with path name based fid */ -static int v9fs_complete_rename(V9fsPDU *pdu, V9fsFidState *fidp, - int32_t newdirfid, V9fsString *name) +static int coroutine_fn v9fs_complete_rename(V9fsPDU *pdu, V9fsFidState *fidp, + int32_t newdirfid, + V9fsString *name) { char *end; int err = 0; @@ -2590,7 +2593,7 @@ out_nofid: } /* Only works with path name based fid */ -static void v9fs_rename(void *opaque) +static void coroutine_fn v9fs_rename(void *opaque) { int32_t fid; ssize_t err = 0; @@ -2641,9 +2644,10 @@ out_nofid: v9fs_string_free(&name); } -static void v9fs_fix_fid_paths(V9fsPDU *pdu, V9fsPath *olddir, - V9fsString *old_name, V9fsPath *newdir, - V9fsString *new_name) +static void coroutine_fn v9fs_fix_fid_paths(V9fsPDU *pdu, V9fsPath *olddir, + V9fsString *old_name, + V9fsPath *newdir, + V9fsString *new_name) { V9fsFidState *tfidp; V9fsPath oldpath, newpath; @@ -2669,9 +2673,10 @@ static void v9fs_fix_fid_paths(V9fsPDU *pdu, V9fsPath *olddir, v9fs_path_free(&newpath); } -static int v9fs_complete_renameat(V9fsPDU *pdu, int32_t olddirfid, - V9fsString *old_name, int32_t newdirfid, - V9fsString *new_name) +static int coroutine_fn v9fs_complete_renameat(V9fsPDU *pdu, int32_t olddirfid, + V9fsString *old_name, + int32_t newdirfid, + V9fsString *new_name) { int err = 0; V9fsState *s = pdu->s; @@ -2712,7 +2717,7 @@ out: return err; } -static void v9fs_renameat(void *opaque) +static void coroutine_fn v9fs_renameat(void *opaque) { ssize_t err = 0; size_t offset = 7; @@ -2754,7 +2759,7 @@ out_err: v9fs_string_free(&new_name); } -static void v9fs_wstat(void *opaque) +static void coroutine_fn v9fs_wstat(void *opaque) { int32_t fid; int err = 0; @@ -2893,7 +2898,7 @@ static int v9fs_fill_statfs(V9fsState *s, V9fsPDU *pdu, struct statfs *stbuf) fsid_val, f_namelen); } -static void v9fs_statfs(void *opaque) +static void coroutine_fn v9fs_statfs(void *opaque) { int32_t fid; ssize_t retval = 0; @@ -2927,7 +2932,7 @@ out_nofid: pdu_complete(pdu, retval); } -static void v9fs_mknod(void *opaque) +static void coroutine_fn v9fs_mknod(void *opaque) { int mode; @@ -2993,7 +2998,7 @@ out_nofid: * do any thing in * qemu 9p server side lock code path. * So when a TLOCK request comes, always return success */ -static void v9fs_lock(void *opaque) +static void coroutine_fn v9fs_lock(void *opaque) { int8_t status; V9fsFlock flock; @@ -3046,7 +3051,7 @@ out_nofid: * When a TGETLOCK request comes, always return success because all lock * handling is done by client's VFS layer. */ -static void v9fs_getlock(void *opaque) +static void coroutine_fn v9fs_getlock(void *opaque) { size_t offset = 7; struct stat stbuf; @@ -3091,7 +3096,7 @@ out_nofid: v9fs_string_free(&glock.client_id); } -static void v9fs_mkdir(void *opaque) +static void coroutine_fn v9fs_mkdir(void *opaque) { V9fsPDU *pdu = opaque; size_t offset = 7; @@ -3145,7 +3150,7 @@ out_nofid: v9fs_string_free(&name); } -static void v9fs_xattrwalk(void *opaque) +static void coroutine_fn v9fs_xattrwalk(void *opaque) { int64_t size; V9fsString name; @@ -3251,7 +3256,7 @@ out_nofid: v9fs_string_free(&name); } -static void v9fs_xattrcreate(void *opaque) +static void coroutine_fn v9fs_xattrcreate(void *opaque) { int flags; int32_t fid; @@ -3291,7 +3296,7 @@ out_nofid: v9fs_string_free(&name); } -static void v9fs_readlink(void *opaque) +static void coroutine_fn v9fs_readlink(void *opaque) { V9fsPDU *pdu = opaque; size_t offset = 7; @@ -3367,13 +3372,13 @@ static CoroutineEntry *pdu_co_handlers[] = { [P9_TREMOVE] = v9fs_remove, }; -static void v9fs_op_not_supp(void *opaque) +static void coroutine_fn v9fs_op_not_supp(void *opaque) { V9fsPDU *pdu = opaque; pdu_complete(pdu, -EOPNOTSUPP); } -static void v9fs_fs_ro(void *opaque) +static void coroutine_fn v9fs_fs_ro(void *opaque) { V9fsPDU *pdu = opaque; pdu_complete(pdu, -EROFS); diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h index 5225b4f120..c4df66d1c4 100644 --- a/hw/9pfs/9p.h +++ b/hw/9pfs/9p.h @@ -324,7 +324,7 @@ static inline uint8_t v9fs_request_cancelled(V9fsPDU *pdu) return pdu->cancelled; } -void v9fs_reclaim_fd(V9fsPDU *pdu); +void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu); void v9fs_path_init(V9fsPath *path); void v9fs_path_free(V9fsPath *path); void v9fs_path_sprintf(V9fsPath *path, const char *fmt, ...); From 6868a420c519d74926ea814d48f6ce9beda35b98 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 17 Oct 2016 14:13:58 +0200 Subject: [PATCH 06/12] 9pfs: drop useless check in pdu_free() Out of the three users of pdu_free(), none ever passes a NULL pointer to this function. Signed-off-by: Greg Kurz --- hw/9pfs/9p.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index f2ab1dfab2..df8aa726c9 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -626,16 +626,14 @@ V9fsPDU *pdu_alloc(V9fsState *s) void pdu_free(V9fsPDU *pdu) { - if (pdu) { - V9fsState *s = pdu->s; - /* - * Cancelled pdu are added back to the freelist - * by flush request . - */ - if (!pdu->cancelled) { - QLIST_REMOVE(pdu, next); - QLIST_INSERT_HEAD(&s->free_list, pdu, next); - } + V9fsState *s = pdu->s; + /* + * Cancelled pdu are added back to the freelist + * by flush request . + */ + if (!pdu->cancelled) { + QLIST_REMOVE(pdu, next); + QLIST_INSERT_HEAD(&s->free_list, pdu, next); } } From f74e27bf0f07425aba6cb812aa7f5aa98bb68542 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 17 Oct 2016 14:13:58 +0200 Subject: [PATCH 07/12] 9pfs: only free completed request if not flushed If a PDU has a flush request pending, the current code calls pdu_free() twice: 1) pdu_complete()->pdu_free() with pdu->cancelled set, which does nothing 2) v9fs_flush()->pdu_free() with pdu->cancelled cleared, which moves the PDU back to the free list. This works but it complexifies the logic of pdu_free(). With this patch, pdu_complete() only calls pdu_free() if no flush request is pending, i.e. qemu_co_queue_next() returns false. Since pdu_free() is now supposed to be called with pdu->cancelled cleared, the check in pdu_free() is dropped and replaced by an assertion. Signed-off-by: Greg Kurz --- hw/9pfs/9p.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index df8aa726c9..f0dc2ce589 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -627,14 +627,10 @@ V9fsPDU *pdu_alloc(V9fsState *s) void pdu_free(V9fsPDU *pdu) { V9fsState *s = pdu->s; - /* - * Cancelled pdu are added back to the freelist - * by flush request . - */ - if (!pdu->cancelled) { - QLIST_REMOVE(pdu, next); - QLIST_INSERT_HEAD(&s->free_list, pdu, next); - } + + g_assert(!pdu->cancelled); + QLIST_REMOVE(pdu, next); + QLIST_INSERT_HEAD(&s->free_list, pdu, next); } /* @@ -679,9 +675,9 @@ static void coroutine_fn pdu_complete(V9fsPDU *pdu, ssize_t len) pdu_push_and_notify(pdu); /* Now wakeup anybody waiting in flush for this request */ - qemu_co_queue_next(&pdu->complete); - - pdu_free(pdu); + if (!qemu_co_queue_next(&pdu->complete)) { + pdu_free(pdu); + } } static mode_t v9mode_to_mode(uint32_t mode, V9fsString *extension) From 0e44a0fd3f28cccb8963fdfc05c53c546b3f46b6 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 17 Oct 2016 14:13:58 +0200 Subject: [PATCH 08/12] virtio-9p: add reset handler Virtio devices should implement the VirtIODevice->reset() function to perform necessary cleanup actions and to bring the device to a quiescent state. In the case of the virtio-9p device, this means: - emptying the list of active PDUs (i.e. draining all in-flight I/O) - freeing all fids (i.e. close open file descriptors and free memory) That's what this patch does. The reset handler first waits for all active PDUs to complete. Since completion happens in the QEMU global aio context, we just have to loop around aio_poll() until the active list is empty. The freeing part involves some actions to be performed on the backend, like closing file descriptors or flushing extended attributes to the underlying filesystem. The virtfs_reset() function already does the job: it calls free_fid() for all open fids not involved in an ongoing I/O operation. We are sure this is the case since we have drained the PDU active list. The current code implements all backend accesses with coroutines, but we want to stay synchronous on the reset path. We can either change the current code to be able to run when not in coroutine context, or create a coroutine context and wait for virtfs_reset() to complete. This patch goes for the latter because it results in simpler code. Note that we also need to create a dummy PDU because it is also an API to pass the FsContext pointer to all backend callbacks. Signed-off-by: Greg Kurz Reviewed-by: Michael S. Tsirkin Reviewed-by: Stefan Hajnoczi --- hw/9pfs/9p.c | 30 ++++++++++++++++++++++++++++++ hw/9pfs/9p.h | 1 + hw/9pfs/virtio-9p-device.c | 8 ++++++++ 3 files changed, 39 insertions(+) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index f0dc2ce589..26aa7d5648 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -3522,6 +3522,36 @@ void v9fs_device_unrealize_common(V9fsState *s, Error **errp) g_free(s->tag); } +typedef struct VirtfsCoResetData { + V9fsPDU pdu; + bool done; +} VirtfsCoResetData; + +static void coroutine_fn virtfs_co_reset(void *opaque) +{ + VirtfsCoResetData *data = opaque; + + virtfs_reset(&data->pdu); + data->done = true; +} + +void v9fs_reset(V9fsState *s) +{ + VirtfsCoResetData data = { .pdu = { .s = s }, .done = false }; + Coroutine *co; + + while (!QLIST_EMPTY(&s->active_list)) { + aio_poll(qemu_get_aio_context(), true); + } + + co = qemu_coroutine_create(virtfs_co_reset, &data); + qemu_coroutine_enter(co); + + while (!data.done) { + aio_poll(qemu_get_aio_context(), true); + } +} + static void __attribute__((__constructor__)) v9fs_set_fd_limit(void) { struct rlimit rlim; diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h index c4df66d1c4..2523a445f8 100644 --- a/hw/9pfs/9p.h +++ b/hw/9pfs/9p.h @@ -339,5 +339,6 @@ ssize_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...); V9fsPDU *pdu_alloc(V9fsState *s); void pdu_free(V9fsPDU *pdu); void pdu_submit(V9fsPDU *pdu); +void v9fs_reset(V9fsState *s); #endif diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c index e98dd0c4c0..1782e4a227 100644 --- a/hw/9pfs/virtio-9p-device.c +++ b/hw/9pfs/virtio-9p-device.c @@ -141,6 +141,13 @@ static void virtio_9p_device_unrealize(DeviceState *dev, Error **errp) v9fs_device_unrealize_common(s, errp); } +static void virtio_9p_reset(VirtIODevice *vdev) +{ + V9fsVirtioState *v = (V9fsVirtioState *)vdev; + + v9fs_reset(&v->state); +} + ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, va_list ap) { @@ -207,6 +214,7 @@ static void virtio_9p_class_init(ObjectClass *klass, void *data) vdc->unrealize = virtio_9p_device_unrealize; vdc->get_features = virtio_9p_get_features; vdc->get_config = virtio_9p_get_config; + vdc->reset = virtio_9p_reset; } static const TypeInfo virtio_device_info = { From eb687602853b4ae656e9236ee4222609f3a6887d Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Mon, 17 Oct 2016 14:13:58 +0200 Subject: [PATCH 09/12] 9pfs: fix information leak in xattr read 9pfs uses g_malloc() to allocate the xattr memory space, if the guest reads this memory before writing to it, this will leak host heap memory to the guest. This patch avoid this. Signed-off-by: Li Qiang Reviewed-by: Greg Kurz Signed-off-by: Greg Kurz --- hw/9pfs/9p.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index 26aa7d5648..bf23b011a8 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -3282,7 +3282,7 @@ static void coroutine_fn v9fs_xattrcreate(void *opaque) xattr_fidp->fs.xattr.flags = flags; v9fs_string_init(&xattr_fidp->fs.xattr.name); v9fs_string_copy(&xattr_fidp->fs.xattr.name, &name); - xattr_fidp->fs.xattr.value = g_malloc(size); + xattr_fidp->fs.xattr.value = g_malloc0(size); err = offset; put_fid(pdu, file_fidp); out_nofid: From ff55e94d23ae94c8628b0115320157c763eb3e06 Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Mon, 17 Oct 2016 14:13:58 +0200 Subject: [PATCH 10/12] 9pfs: fix memory leak in v9fs_xattrcreate The 'fs.xattr.value' field in V9fsFidState object doesn't consider the situation that this field has been allocated previously. Every time, it will be allocated directly. This leads to a host memory leak issue if the client sends another Txattrcreate message with the same fid number before the fid from the previous time got clunked. Signed-off-by: Li Qiang Reviewed-by: Greg Kurz [groug, updated the changelog to indicate how the leak can occur] Signed-off-by: Greg Kurz --- hw/9pfs/9p.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index bf23b011a8..66135cf121 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -3282,6 +3282,7 @@ static void coroutine_fn v9fs_xattrcreate(void *opaque) xattr_fidp->fs.xattr.flags = flags; v9fs_string_init(&xattr_fidp->fs.xattr.name); v9fs_string_copy(&xattr_fidp->fs.xattr.name, &name); + g_free(xattr_fidp->fs.xattr.value); xattr_fidp->fs.xattr.value = g_malloc0(size); err = offset; put_fid(pdu, file_fidp); From 4c1586787ff43c9acd18a56c12d720e3e6be9f7c Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Mon, 17 Oct 2016 14:13:58 +0200 Subject: [PATCH 11/12] 9pfs: fix memory leak in v9fs_link The v9fs_link() function keeps a reference on the source fid object. This causes a memory leak since the reference never goes down to 0. This patch fixes the issue. Signed-off-by: Li Qiang Reviewed-by: Greg Kurz [groug, rephrased the changelog] Signed-off-by: Greg Kurz --- hw/9pfs/9p.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index 66135cf121..d43a552234 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -2413,6 +2413,7 @@ static void coroutine_fn v9fs_link(void *opaque) if (!err) { err = offset; } + put_fid(pdu, oldfidp); out: put_fid(pdu, dfidp); out_nofid: From fdfcc9aeea1492f4b819a24c94dfb678145b1bf9 Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Mon, 17 Oct 2016 14:13:58 +0200 Subject: [PATCH 12/12] 9pfs: fix memory leak in v9fs_write If an error occurs when marshalling the transfer length to the guest, the v9fs_write() function doesn't free an IO vector, thus leading to a memory leak. This patch fixes the issue. Signed-off-by: Li Qiang Reviewed-by: Greg Kurz [groug, rephrased the changelog] Signed-off-by: Greg Kurz --- hw/9pfs/9p.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index d43a552234..e88cf257a2 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -2090,7 +2090,7 @@ static void coroutine_fn v9fs_write(void *opaque) offset = 7; err = pdu_marshal(pdu, offset, "d", total); if (err < 0) { - goto out; + goto out_qiov; } err += offset; trace_v9fs_write_return(pdu->tag, pdu->id, total, err);