* [PATCH v2 1/6] 9pfs: local : Introduce local_fid_fd() helper
2025-03-11 17:28 [PATCH v2 0/6] 9pfs: Fix ftruncate-after-unlink Greg Kurz
@ 2025-03-11 17:28 ` Greg Kurz
2025-03-12 14:00 ` Christian Schoenebeck
2025-03-11 17:28 ` [PATCH v2 2/6] 9pfs: Don't use file descriptors in core code Greg Kurz
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Greg Kurz @ 2025-03-11 17:28 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Christian Schoenebeck, Paolo Bonzini,
Laurent Vivier, Greg Kurz
Factor out duplicated code to a single helper. More users to come.
Signed-off-by: Greg Kurz <groug@kaod.org>
v2: - simplified local_fid_fd()
---
hw/9pfs/9p-local.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 928523afcc6c..99b9560a528b 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -766,16 +766,19 @@ out:
return err;
}
-static int local_fstat(FsContext *fs_ctx, int fid_type,
- V9fsFidOpenState *fs, struct stat *stbuf)
+static int local_fid_fd(int fid_type, V9fsFidOpenState *fs)
{
- int err, fd;
-
if (fid_type == P9_FID_DIR) {
- fd = dirfd(fs->dir.stream);
+ return dirfd(fs->dir.stream);
} else {
- fd = fs->fd;
+ return fs->fd;
}
+}
+
+static int local_fstat(FsContext *fs_ctx, int fid_type,
+ V9fsFidOpenState *fs, struct stat *stbuf)
+{
+ int err, fd = local_fid_fd(fid_type, fs);
err = fstat(fd, stbuf);
if (err) {
@@ -1167,13 +1170,7 @@ out:
static int local_fsync(FsContext *ctx, int fid_type,
V9fsFidOpenState *fs, int datasync)
{
- int fd;
-
- if (fid_type == P9_FID_DIR) {
- fd = dirfd(fs->dir.stream);
- } else {
- fd = fs->fd;
- }
+ int fd = local_fid_fd(fid_type, fs);
if (datasync) {
return qemu_fdatasync(fd);
--
2.48.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 1/6] 9pfs: local : Introduce local_fid_fd() helper
2025-03-11 17:28 ` [PATCH v2 1/6] 9pfs: local : Introduce local_fid_fd() helper Greg Kurz
@ 2025-03-12 14:00 ` Christian Schoenebeck
0 siblings, 0 replies; 14+ messages in thread
From: Christian Schoenebeck @ 2025-03-12 14:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Paolo Bonzini, Laurent Vivier, Greg Kurz
On Tuesday, March 11, 2025 6:28:04 PM CET Greg Kurz wrote:
> Factor out duplicated code to a single helper. More users to come.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
>
> v2: - simplified local_fid_fd()
> ---
Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> hw/9pfs/9p-local.c | 23 ++++++++++-------------
> 1 file changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index 928523afcc6c..99b9560a528b 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -766,16 +766,19 @@ out:
> return err;
> }
>
> -static int local_fstat(FsContext *fs_ctx, int fid_type,
> - V9fsFidOpenState *fs, struct stat *stbuf)
> +static int local_fid_fd(int fid_type, V9fsFidOpenState *fs)
> {
> - int err, fd;
> -
> if (fid_type == P9_FID_DIR) {
> - fd = dirfd(fs->dir.stream);
> + return dirfd(fs->dir.stream);
> } else {
> - fd = fs->fd;
> + return fs->fd;
> }
> +}
> +
> +static int local_fstat(FsContext *fs_ctx, int fid_type,
> + V9fsFidOpenState *fs, struct stat *stbuf)
> +{
> + int err, fd = local_fid_fd(fid_type, fs);
>
> err = fstat(fd, stbuf);
> if (err) {
> @@ -1167,13 +1170,7 @@ out:
> static int local_fsync(FsContext *ctx, int fid_type,
> V9fsFidOpenState *fs, int datasync)
> {
> - int fd;
> -
> - if (fid_type == P9_FID_DIR) {
> - fd = dirfd(fs->dir.stream);
> - } else {
> - fd = fs->fd;
> - }
> + int fd = local_fid_fd(fid_type, fs);
>
> if (datasync) {
> return qemu_fdatasync(fd);
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/6] 9pfs: Don't use file descriptors in core code
2025-03-11 17:28 [PATCH v2 0/6] 9pfs: Fix ftruncate-after-unlink Greg Kurz
2025-03-11 17:28 ` [PATCH v2 1/6] 9pfs: local : Introduce local_fid_fd() helper Greg Kurz
@ 2025-03-11 17:28 ` Greg Kurz
2025-03-12 14:02 ` Christian Schoenebeck
2025-03-11 17:28 ` [PATCH v2 3/6] 9pfs: Introduce ftruncate file op Greg Kurz
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Greg Kurz @ 2025-03-11 17:28 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Christian Schoenebeck, Paolo Bonzini,
Laurent Vivier, Greg Kurz
v9fs_getattr() currently peeks into V9fsFidOpenState to know if a fid
has a valid file descriptor or directory stream. Even though the fields
are accessible, this is an implementation detail of the local backend
that should not be manipulated directly by the server code.
Abstract that with a new has_valid_file_handle() backend operation.
Signed-off-by: Greg Kurz <groug@kaod.org>
v2: - rename to has_valid_file_handle()
- don't reuse local_fid_fd()
---
fsdev/file-op-9p.h | 1 +
hw/9pfs/9p-local.c | 8 ++++++++
hw/9pfs/9p-synth.c | 6 ++++++
hw/9pfs/9p.c | 9 ++++++---
4 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index 4997677460e8..b815cea44e85 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -164,6 +164,7 @@ struct FileOperations {
int (*renameat)(FsContext *ctx, V9fsPath *olddir, const char *old_name,
V9fsPath *newdir, const char *new_name);
int (*unlinkat)(FsContext *ctx, V9fsPath *dir, const char *name, int flags);
+ bool (*has_valid_file_handle)(int fid_type, V9fsFidOpenState *fs);
};
#endif
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 99b9560a528b..b16132299f2c 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -1572,6 +1572,13 @@ static int local_parse_opts(QemuOpts *opts, FsDriverEntry *fse, Error **errp)
return 0;
}
+static bool local_has_valid_file_handle(int fid_type, V9fsFidOpenState *fs)
+{
+ return
+ (fid_type == P9_FID_FILE && fs->fd != -1) ||
+ (fid_type == P9_FID_DIR && fs->dir.stream != NULL);
+}
+
FileOperations local_ops = {
.parse_opts = local_parse_opts,
.init = local_init,
@@ -1609,4 +1616,5 @@ FileOperations local_ops = {
.name_to_path = local_name_to_path,
.renameat = local_renameat,
.unlinkat = local_unlinkat,
+ .has_valid_file_handle = local_has_valid_file_handle,
};
diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index 2abaf3a2918a..be0492b400e1 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -615,6 +615,11 @@ static int synth_init(FsContext *ctx, Error **errp)
return 0;
}
+static bool synth_has_valid_file_handle(int fid_type, V9fsFidOpenState *fs)
+{
+ return false;
+}
+
FileOperations synth_ops = {
.init = synth_init,
.lstat = synth_lstat,
@@ -650,4 +655,5 @@ FileOperations synth_ops = {
.name_to_path = synth_name_to_path,
.renameat = synth_renameat,
.unlinkat = synth_unlinkat,
+ .has_valid_file_handle = synth_has_valid_file_handle,
};
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 7cad2bce6209..10363f1a1df8 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1574,6 +1574,11 @@ out_nofid:
pdu_complete(pdu, err);
}
+static bool fid_has_valid_handle(V9fsState *s, V9fsFidState *fidp)
+{
+ return s->ops->has_valid_file_handle(fidp->fid_type, &fidp->fs);
+}
+
static void coroutine_fn v9fs_getattr(void *opaque)
{
int32_t fid;
@@ -1596,9 +1601,7 @@ static void coroutine_fn v9fs_getattr(void *opaque)
retval = -ENOENT;
goto out_nofid;
}
- if ((fidp->fid_type == P9_FID_FILE && fidp->fs.fd != -1) ||
- (fidp->fid_type == P9_FID_DIR && fidp->fs.dir.stream))
- {
+ if (fid_has_valid_handle(pdu->s, fidp)) {
retval = v9fs_co_fstat(pdu, fidp, &stbuf);
} else {
retval = v9fs_co_lstat(pdu, &fidp->path, &stbuf);
--
2.48.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 2/6] 9pfs: Don't use file descriptors in core code
2025-03-11 17:28 ` [PATCH v2 2/6] 9pfs: Don't use file descriptors in core code Greg Kurz
@ 2025-03-12 14:02 ` Christian Schoenebeck
0 siblings, 0 replies; 14+ messages in thread
From: Christian Schoenebeck @ 2025-03-12 14:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Paolo Bonzini, Laurent Vivier, Greg Kurz
On Tuesday, March 11, 2025 6:28:05 PM CET Greg Kurz wrote:
> v9fs_getattr() currently peeks into V9fsFidOpenState to know if a fid
> has a valid file descriptor or directory stream. Even though the fields
> are accessible, this is an implementation detail of the local backend
> that should not be manipulated directly by the server code.
>
> Abstract that with a new has_valid_file_handle() backend operation.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
>
> v2: - rename to has_valid_file_handle()
> - don't reuse local_fid_fd()
> ---
> fsdev/file-op-9p.h | 1 +
> hw/9pfs/9p-local.c | 8 ++++++++
> hw/9pfs/9p-synth.c | 6 ++++++
> hw/9pfs/9p.c | 9 ++++++---
> 4 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
> index 4997677460e8..b815cea44e85 100644
> --- a/fsdev/file-op-9p.h
> +++ b/fsdev/file-op-9p.h
> @@ -164,6 +164,7 @@ struct FileOperations {
> int (*renameat)(FsContext *ctx, V9fsPath *olddir, const char *old_name,
> V9fsPath *newdir, const char *new_name);
> int (*unlinkat)(FsContext *ctx, V9fsPath *dir, const char *name, int flags);
> + bool (*has_valid_file_handle)(int fid_type, V9fsFidOpenState *fs);
> };
>
> #endif
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index 99b9560a528b..b16132299f2c 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -1572,6 +1572,13 @@ static int local_parse_opts(QemuOpts *opts, FsDriverEntry *fse, Error **errp)
> return 0;
> }
>
> +static bool local_has_valid_file_handle(int fid_type, V9fsFidOpenState *fs)
> +{
> + return
> + (fid_type == P9_FID_FILE && fs->fd != -1) ||
> + (fid_type == P9_FID_DIR && fs->dir.stream != NULL);
> +}
> +
> FileOperations local_ops = {
> .parse_opts = local_parse_opts,
> .init = local_init,
> @@ -1609,4 +1616,5 @@ FileOperations local_ops = {
> .name_to_path = local_name_to_path,
> .renameat = local_renameat,
> .unlinkat = local_unlinkat,
> + .has_valid_file_handle = local_has_valid_file_handle,
> };
> diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> index 2abaf3a2918a..be0492b400e1 100644
> --- a/hw/9pfs/9p-synth.c
> +++ b/hw/9pfs/9p-synth.c
> @@ -615,6 +615,11 @@ static int synth_init(FsContext *ctx, Error **errp)
> return 0;
> }
>
> +static bool synth_has_valid_file_handle(int fid_type, V9fsFidOpenState *fs)
> +{
> + return false;
> +}
> +
> FileOperations synth_ops = {
> .init = synth_init,
> .lstat = synth_lstat,
> @@ -650,4 +655,5 @@ FileOperations synth_ops = {
> .name_to_path = synth_name_to_path,
> .renameat = synth_renameat,
> .unlinkat = synth_unlinkat,
> + .has_valid_file_handle = synth_has_valid_file_handle,
> };
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 7cad2bce6209..10363f1a1df8 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1574,6 +1574,11 @@ out_nofid:
> pdu_complete(pdu, err);
> }
>
> +static bool fid_has_valid_handle(V9fsState *s, V9fsFidState *fidp)
> +{
> + return s->ops->has_valid_file_handle(fidp->fid_type, &fidp->fs);
> +}
> +
I would also rename that to fid_has_valid_file_handle(), but I can also do
this on my end.
Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> static void coroutine_fn v9fs_getattr(void *opaque)
> {
> int32_t fid;
> @@ -1596,9 +1601,7 @@ static void coroutine_fn v9fs_getattr(void *opaque)
> retval = -ENOENT;
> goto out_nofid;
> }
> - if ((fidp->fid_type == P9_FID_FILE && fidp->fs.fd != -1) ||
> - (fidp->fid_type == P9_FID_DIR && fidp->fs.dir.stream))
> - {
> + if (fid_has_valid_handle(pdu->s, fidp)) {
> retval = v9fs_co_fstat(pdu, fidp, &stbuf);
> } else {
> retval = v9fs_co_lstat(pdu, &fidp->path, &stbuf);
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 3/6] 9pfs: Introduce ftruncate file op
2025-03-11 17:28 [PATCH v2 0/6] 9pfs: Fix ftruncate-after-unlink Greg Kurz
2025-03-11 17:28 ` [PATCH v2 1/6] 9pfs: local : Introduce local_fid_fd() helper Greg Kurz
2025-03-11 17:28 ` [PATCH v2 2/6] 9pfs: Don't use file descriptors in core code Greg Kurz
@ 2025-03-11 17:28 ` Greg Kurz
2025-03-12 14:07 ` Christian Schoenebeck
2025-03-11 17:28 ` [PATCH v2 4/6] 9pfs: Introduce futimens " Greg Kurz
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Greg Kurz @ 2025-03-11 17:28 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Christian Schoenebeck, Paolo Bonzini,
Laurent Vivier, Greg Kurz
Add an ftruncate operation to the fs driver and use if when a fid has
a valid file descriptor. This is required to support more cases where
the client wants to do an action on an unlinked file which it still
has an open file decriptor for.
Only 9P2000.L was considered.
Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
v2: - moved v9fs_co_ftruncate() near v9fs_co_truncate() in coth.h
- similar change in file-op-9p.h
---
fsdev/file-op-9p.h | 2 ++
hw/9pfs/9p-local.c | 9 +++++++++
hw/9pfs/9p-synth.c | 8 ++++++++
hw/9pfs/9p.c | 6 +++++-
hw/9pfs/cofs.c | 18 ++++++++++++++++++
hw/9pfs/coth.h | 2 ++
6 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index b815cea44e85..26ba1438c0ed 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -152,6 +152,8 @@ struct FileOperations {
int (*fstat)(FsContext *, int, V9fsFidOpenState *, struct stat *);
int (*rename)(FsContext *, const char *, const char *);
int (*truncate)(FsContext *, V9fsPath *, off_t);
+ int (*ftruncate)(FsContext *ctx, int fid_type, V9fsFidOpenState *fs,
+ off_t size);
int (*fsync)(FsContext *, int, V9fsFidOpenState *, int);
int (*statfs)(FsContext *s, V9fsPath *path, struct statfs *stbuf);
ssize_t (*lgetxattr)(FsContext *, V9fsPath *,
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index b16132299f2c..0b33da8d2a46 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -1042,6 +1042,14 @@ static int local_truncate(FsContext *ctx, V9fsPath *fs_path, off_t size)
return ret;
}
+static int local_ftruncate(FsContext *ctx, int fid_type, V9fsFidOpenState *fs,
+ off_t size)
+{
+ int fd = local_fid_fd(fid_type, fs);
+
+ return ftruncate(fd, size);
+}
+
static int local_chown(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp)
{
char *dirpath = g_path_get_dirname(fs_path->data);
@@ -1617,4 +1625,5 @@ FileOperations local_ops = {
.renameat = local_renameat,
.unlinkat = local_unlinkat,
.has_valid_file_handle = local_has_valid_file_handle,
+ .ftruncate = local_ftruncate,
};
diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index be0492b400e1..3d28afc4d03d 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -356,6 +356,13 @@ static int synth_truncate(FsContext *ctx, V9fsPath *path, off_t offset)
return -1;
}
+static int synth_ftruncate(FsContext *ctx, int fid_type, V9fsFidOpenState *fs,
+ off_t size)
+{
+ errno = ENOSYS;
+ return -1;
+}
+
static int synth_chmod(FsContext *fs_ctx, V9fsPath *path, FsCred *credp)
{
errno = EPERM;
@@ -656,4 +663,5 @@ FileOperations synth_ops = {
.renameat = synth_renameat,
.unlinkat = synth_unlinkat,
.has_valid_file_handle = synth_has_valid_file_handle,
+ .ftruncate = synth_ftruncate,
};
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 10363f1a1df8..4616bd763012 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1733,7 +1733,11 @@ static void coroutine_fn v9fs_setattr(void *opaque)
}
}
if (v9iattr.valid & (P9_ATTR_SIZE)) {
- err = v9fs_co_truncate(pdu, &fidp->path, v9iattr.size);
+ if (fid_has_valid_handle(pdu->s, fidp)) {
+ err = v9fs_co_ftruncate(pdu, fidp, v9iattr.size);
+ } else {
+ err = v9fs_co_truncate(pdu, &fidp->path, v9iattr.size);
+ }
if (err < 0) {
goto out;
}
diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c
index 67e3ae5c5ccd..893466fb1a44 100644
--- a/hw/9pfs/cofs.c
+++ b/hw/9pfs/cofs.c
@@ -184,6 +184,24 @@ int coroutine_fn v9fs_co_truncate(V9fsPDU *pdu, V9fsPath *path, off_t size)
return err;
}
+int coroutine_fn v9fs_co_ftruncate(V9fsPDU *pdu, V9fsFidState *fidp, off_t size)
+{
+ int err;
+ V9fsState *s = pdu->s;
+
+ if (v9fs_request_cancelled(pdu)) {
+ return -EINTR;
+ }
+ v9fs_co_run_in_worker(
+ {
+ err = s->ops->ftruncate(&s->ctx, fidp->fid_type, &fidp->fs, size);
+ if (err < 0) {
+ err = -errno;
+ }
+ });
+ return err;
+}
+
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)
diff --git a/hw/9pfs/coth.h b/hw/9pfs/coth.h
index 2c54249b3577..62e922dc12e3 100644
--- a/hw/9pfs/coth.h
+++ b/hw/9pfs/coth.h
@@ -73,6 +73,8 @@ 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_ftruncate(V9fsPDU *pdu, V9fsFidState *fidp,
+ off_t size);
int coroutine_fn v9fs_co_llistxattr(V9fsPDU *, V9fsPath *, void *, size_t);
int coroutine_fn v9fs_co_lgetxattr(V9fsPDU *, V9fsPath *,
V9fsString *, void *, size_t);
--
2.48.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 3/6] 9pfs: Introduce ftruncate file op
2025-03-11 17:28 ` [PATCH v2 3/6] 9pfs: Introduce ftruncate file op Greg Kurz
@ 2025-03-12 14:07 ` Christian Schoenebeck
2025-03-12 14:21 ` Greg Kurz
0 siblings, 1 reply; 14+ messages in thread
From: Christian Schoenebeck @ 2025-03-12 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Paolo Bonzini, Laurent Vivier, Greg Kurz
On Tuesday, March 11, 2025 6:28:06 PM CET Greg Kurz wrote:
> Add an ftruncate operation to the fs driver and use if when a fid has
> a valid file descriptor. This is required to support more cases where
> the client wants to do an action on an unlinked file which it still
> has an open file decriptor for.
>
> Only 9P2000.L was considered.
>
> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> Signed-off-by: Greg Kurz <groug@kaod.org>
>
> v2: - moved v9fs_co_ftruncate() near v9fs_co_truncate() in coth.h
> - similar change in file-op-9p.h
> ---
> fsdev/file-op-9p.h | 2 ++
> hw/9pfs/9p-local.c | 9 +++++++++
> hw/9pfs/9p-synth.c | 8 ++++++++
> hw/9pfs/9p.c | 6 +++++-
> hw/9pfs/cofs.c | 18 ++++++++++++++++++
> hw/9pfs/coth.h | 2 ++
> 6 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
> index b815cea44e85..26ba1438c0ed 100644
> --- a/fsdev/file-op-9p.h
> +++ b/fsdev/file-op-9p.h
> @@ -152,6 +152,8 @@ struct FileOperations {
> int (*fstat)(FsContext *, int, V9fsFidOpenState *, struct stat *);
> int (*rename)(FsContext *, const char *, const char *);
> int (*truncate)(FsContext *, V9fsPath *, off_t);
> + int (*ftruncate)(FsContext *ctx, int fid_type, V9fsFidOpenState *fs,
> + off_t size);
> int (*fsync)(FsContext *, int, V9fsFidOpenState *, int);
> int (*statfs)(FsContext *s, V9fsPath *path, struct statfs *stbuf);
> ssize_t (*lgetxattr)(FsContext *, V9fsPath *,
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index b16132299f2c..0b33da8d2a46 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -1042,6 +1042,14 @@ static int local_truncate(FsContext *ctx, V9fsPath *fs_path, off_t size)
> return ret;
> }
>
> +static int local_ftruncate(FsContext *ctx, int fid_type, V9fsFidOpenState *fs,
> + off_t size)
> +{
> + int fd = local_fid_fd(fid_type, fs);
> +
> + return ftruncate(fd, size);
> +}
> +
> static int local_chown(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp)
> {
> char *dirpath = g_path_get_dirname(fs_path->data);
> @@ -1617,4 +1625,5 @@ FileOperations local_ops = {
> .renameat = local_renameat,
> .unlinkat = local_unlinkat,
> .has_valid_file_handle = local_has_valid_file_handle,
> + .ftruncate = local_ftruncate,
> };
> diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> index be0492b400e1..3d28afc4d03d 100644
> --- a/hw/9pfs/9p-synth.c
> +++ b/hw/9pfs/9p-synth.c
> @@ -356,6 +356,13 @@ static int synth_truncate(FsContext *ctx, V9fsPath *path, off_t offset)
> return -1;
> }
>
> +static int synth_ftruncate(FsContext *ctx, int fid_type, V9fsFidOpenState *fs,
> + off_t size)
> +{
> + errno = ENOSYS;
> + return -1;
> +}
> +
> static int synth_chmod(FsContext *fs_ctx, V9fsPath *path, FsCred *credp)
> {
> errno = EPERM;
> @@ -656,4 +663,5 @@ FileOperations synth_ops = {
> .renameat = synth_renameat,
> .unlinkat = synth_unlinkat,
> .has_valid_file_handle = synth_has_valid_file_handle,
> + .ftruncate = synth_ftruncate,
> };
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 10363f1a1df8..4616bd763012 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1733,7 +1733,11 @@ static void coroutine_fn v9fs_setattr(void *opaque)
> }
> }
> if (v9iattr.valid & (P9_ATTR_SIZE)) {
> - err = v9fs_co_truncate(pdu, &fidp->path, v9iattr.size);
> + if (fid_has_valid_handle(pdu->s, fidp)) {
> + err = v9fs_co_ftruncate(pdu, fidp, v9iattr.size);
> + } else {
> + err = v9fs_co_truncate(pdu, &fidp->path, v9iattr.size);
> + }
Like with previous patch, s/fid_has_valid_handle/fid_has_valid_file_handle/,
the rest is fine.
Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> if (err < 0) {
> goto out;
> }
> diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c
> index 67e3ae5c5ccd..893466fb1a44 100644
> --- a/hw/9pfs/cofs.c
> +++ b/hw/9pfs/cofs.c
> @@ -184,6 +184,24 @@ int coroutine_fn v9fs_co_truncate(V9fsPDU *pdu, V9fsPath *path, off_t size)
> return err;
> }
>
> +int coroutine_fn v9fs_co_ftruncate(V9fsPDU *pdu, V9fsFidState *fidp, off_t size)
> +{
> + int err;
> + V9fsState *s = pdu->s;
> +
> + if (v9fs_request_cancelled(pdu)) {
> + return -EINTR;
> + }
> + v9fs_co_run_in_worker(
> + {
> + err = s->ops->ftruncate(&s->ctx, fidp->fid_type, &fidp->fs, size);
> + if (err < 0) {
> + err = -errno;
> + }
> + });
> + return err;
> +}
> +
> 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)
> diff --git a/hw/9pfs/coth.h b/hw/9pfs/coth.h
> index 2c54249b3577..62e922dc12e3 100644
> --- a/hw/9pfs/coth.h
> +++ b/hw/9pfs/coth.h
> @@ -73,6 +73,8 @@ 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_ftruncate(V9fsPDU *pdu, V9fsFidState *fidp,
> + off_t size);
> int coroutine_fn v9fs_co_llistxattr(V9fsPDU *, V9fsPath *, void *, size_t);
> int coroutine_fn v9fs_co_lgetxattr(V9fsPDU *, V9fsPath *,
> V9fsString *, void *, size_t);
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 3/6] 9pfs: Introduce ftruncate file op
2025-03-12 14:07 ` Christian Schoenebeck
@ 2025-03-12 14:21 ` Greg Kurz
0 siblings, 0 replies; 14+ messages in thread
From: Greg Kurz @ 2025-03-12 14:21 UTC (permalink / raw)
To: Christian Schoenebeck
Cc: qemu-devel, Fabiano Rosas, Paolo Bonzini, Laurent Vivier
On Wed, 12 Mar 2025 15:07:20 +0100
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> On Tuesday, March 11, 2025 6:28:06 PM CET Greg Kurz wrote:
> > Add an ftruncate operation to the fs driver and use if when a fid has
> > a valid file descriptor. This is required to support more cases where
> > the client wants to do an action on an unlinked file which it still
> > has an open file decriptor for.
> >
> > Only 9P2000.L was considered.
> >
> > Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> >
> > v2: - moved v9fs_co_ftruncate() near v9fs_co_truncate() in coth.h
> > - similar change in file-op-9p.h
> > ---
> > fsdev/file-op-9p.h | 2 ++
> > hw/9pfs/9p-local.c | 9 +++++++++
> > hw/9pfs/9p-synth.c | 8 ++++++++
> > hw/9pfs/9p.c | 6 +++++-
> > hw/9pfs/cofs.c | 18 ++++++++++++++++++
> > hw/9pfs/coth.h | 2 ++
> > 6 files changed, 44 insertions(+), 1 deletion(-)
> >
> > diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
> > index b815cea44e85..26ba1438c0ed 100644
> > --- a/fsdev/file-op-9p.h
> > +++ b/fsdev/file-op-9p.h
> > @@ -152,6 +152,8 @@ struct FileOperations {
> > int (*fstat)(FsContext *, int, V9fsFidOpenState *, struct stat *);
> > int (*rename)(FsContext *, const char *, const char *);
> > int (*truncate)(FsContext *, V9fsPath *, off_t);
> > + int (*ftruncate)(FsContext *ctx, int fid_type, V9fsFidOpenState *fs,
> > + off_t size);
> > int (*fsync)(FsContext *, int, V9fsFidOpenState *, int);
> > int (*statfs)(FsContext *s, V9fsPath *path, struct statfs *stbuf);
> > ssize_t (*lgetxattr)(FsContext *, V9fsPath *,
> > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> > index b16132299f2c..0b33da8d2a46 100644
> > --- a/hw/9pfs/9p-local.c
> > +++ b/hw/9pfs/9p-local.c
> > @@ -1042,6 +1042,14 @@ static int local_truncate(FsContext *ctx, V9fsPath *fs_path, off_t size)
> > return ret;
> > }
> >
> > +static int local_ftruncate(FsContext *ctx, int fid_type, V9fsFidOpenState *fs,
> > + off_t size)
> > +{
> > + int fd = local_fid_fd(fid_type, fs);
> > +
> > + return ftruncate(fd, size);
> > +}
> > +
> > static int local_chown(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp)
> > {
> > char *dirpath = g_path_get_dirname(fs_path->data);
> > @@ -1617,4 +1625,5 @@ FileOperations local_ops = {
> > .renameat = local_renameat,
> > .unlinkat = local_unlinkat,
> > .has_valid_file_handle = local_has_valid_file_handle,
> > + .ftruncate = local_ftruncate,
> > };
> > diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> > index be0492b400e1..3d28afc4d03d 100644
> > --- a/hw/9pfs/9p-synth.c
> > +++ b/hw/9pfs/9p-synth.c
> > @@ -356,6 +356,13 @@ static int synth_truncate(FsContext *ctx, V9fsPath *path, off_t offset)
> > return -1;
> > }
> >
> > +static int synth_ftruncate(FsContext *ctx, int fid_type, V9fsFidOpenState *fs,
> > + off_t size)
> > +{
> > + errno = ENOSYS;
> > + return -1;
> > +}
> > +
> > static int synth_chmod(FsContext *fs_ctx, V9fsPath *path, FsCred *credp)
> > {
> > errno = EPERM;
> > @@ -656,4 +663,5 @@ FileOperations synth_ops = {
> > .renameat = synth_renameat,
> > .unlinkat = synth_unlinkat,
> > .has_valid_file_handle = synth_has_valid_file_handle,
> > + .ftruncate = synth_ftruncate,
> > };
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 10363f1a1df8..4616bd763012 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -1733,7 +1733,11 @@ static void coroutine_fn v9fs_setattr(void *opaque)
> > }
> > }
> > if (v9iattr.valid & (P9_ATTR_SIZE)) {
> > - err = v9fs_co_truncate(pdu, &fidp->path, v9iattr.size);
> > + if (fid_has_valid_handle(pdu->s, fidp)) {
> > + err = v9fs_co_ftruncate(pdu, fidp, v9iattr.size);
> > + } else {
> > + err = v9fs_co_truncate(pdu, &fidp->path, v9iattr.size);
> > + }
>
> Like with previous patch, s/fid_has_valid_handle/fid_has_valid_file_handle/,
> the rest is fine.
>
Oops... my bad. I'll fix that !
> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
>
> > if (err < 0) {
> > goto out;
> > }
> > diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c
> > index 67e3ae5c5ccd..893466fb1a44 100644
> > --- a/hw/9pfs/cofs.c
> > +++ b/hw/9pfs/cofs.c
> > @@ -184,6 +184,24 @@ int coroutine_fn v9fs_co_truncate(V9fsPDU *pdu, V9fsPath *path, off_t size)
> > return err;
> > }
> >
> > +int coroutine_fn v9fs_co_ftruncate(V9fsPDU *pdu, V9fsFidState *fidp, off_t size)
> > +{
> > + int err;
> > + V9fsState *s = pdu->s;
> > +
> > + if (v9fs_request_cancelled(pdu)) {
> > + return -EINTR;
> > + }
> > + v9fs_co_run_in_worker(
> > + {
> > + err = s->ops->ftruncate(&s->ctx, fidp->fid_type, &fidp->fs, size);
> > + if (err < 0) {
> > + err = -errno;
> > + }
> > + });
> > + return err;
> > +}
> > +
> > 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)
> > diff --git a/hw/9pfs/coth.h b/hw/9pfs/coth.h
> > index 2c54249b3577..62e922dc12e3 100644
> > --- a/hw/9pfs/coth.h
> > +++ b/hw/9pfs/coth.h
> > @@ -73,6 +73,8 @@ 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_ftruncate(V9fsPDU *pdu, V9fsFidState *fidp,
> > + off_t size);
> > int coroutine_fn v9fs_co_llistxattr(V9fsPDU *, V9fsPath *, void *, size_t);
> > int coroutine_fn v9fs_co_lgetxattr(V9fsPDU *, V9fsPath *,
> > V9fsString *, void *, size_t);
> >
>
>
--
Greg
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 4/6] 9pfs: Introduce futimens file op
2025-03-11 17:28 [PATCH v2 0/6] 9pfs: Fix ftruncate-after-unlink Greg Kurz
` (2 preceding siblings ...)
2025-03-11 17:28 ` [PATCH v2 3/6] 9pfs: Introduce ftruncate file op Greg Kurz
@ 2025-03-11 17:28 ` Greg Kurz
2025-03-11 17:28 ` [PATCH v2 5/6] tests/9p: add 'Tsetattr' request to test client Greg Kurz
2025-03-11 17:28 ` [PATCH v2 6/6] tests/9p: Test `Tsetattr` can truncate unlinked file Greg Kurz
5 siblings, 0 replies; 14+ messages in thread
From: Greg Kurz @ 2025-03-11 17:28 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Christian Schoenebeck, Paolo Bonzini,
Laurent Vivier, Greg Kurz
Add an futimens operation to the fs driver and use if when a fid has
a valid file descriptor. This is required to support more cases where
the client wants to do an action on an unlinked file which it still
has an open file decriptor for.
Only 9P2000.L was considered.
Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
v2: - moved v9fs_co_futimens() near v9fs_co_utimensat() in coth.h
- similar change in file-op-9p.h
---
fsdev/file-op-9p.h | 2 ++
hw/9pfs/9p-local.c | 9 +++++++++
hw/9pfs/9p-synth.c | 8 ++++++++
hw/9pfs/9p-util.h | 1 +
hw/9pfs/9p.c | 6 +++++-
hw/9pfs/cofs.c | 19 +++++++++++++++++++
hw/9pfs/coth.h | 2 ++
7 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index 26ba1438c0ed..b9dae8c84c23 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -129,6 +129,8 @@ struct FileOperations {
int (*chown)(FsContext *, V9fsPath *, FsCred *);
int (*mknod)(FsContext *, V9fsPath *, const char *, FsCred *);
int (*utimensat)(FsContext *, V9fsPath *, const struct timespec *);
+ int (*futimens)(FsContext *ctx, int fid_type, V9fsFidOpenState *fs,
+ const struct timespec *times);
int (*remove)(FsContext *, const char *);
int (*symlink)(FsContext *, const char *, V9fsPath *,
const char *, FsCred *);
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 0b33da8d2a46..31e216227cb9 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -1100,6 +1100,14 @@ out:
return ret;
}
+static int local_futimens(FsContext *s, int fid_type, V9fsFidOpenState *fs,
+ const struct timespec *times)
+{
+ int fd = local_fid_fd(fid_type, fs);
+
+ return qemu_futimens(fd, times);
+}
+
static int local_unlinkat_common(FsContext *ctx, int dirfd, const char *name,
int flags)
{
@@ -1626,4 +1634,5 @@ FileOperations local_ops = {
.unlinkat = local_unlinkat,
.has_valid_file_handle = local_has_valid_file_handle,
.ftruncate = local_ftruncate,
+ .futimens = local_futimens,
};
diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index 3d28afc4d03d..9cd188422421 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -424,6 +424,13 @@ static int synth_utimensat(FsContext *fs_ctx, V9fsPath *path,
return 0;
}
+static int synth_futimens(FsContext *fs_ctx, int fid_type, V9fsFidOpenState *fs,
+ const struct timespec *buf)
+{
+ errno = ENOSYS;
+ return -1;
+}
+
static int synth_remove(FsContext *ctx, const char *path)
{
errno = EPERM;
@@ -664,4 +671,5 @@ FileOperations synth_ops = {
.unlinkat = synth_unlinkat,
.has_valid_file_handle = synth_has_valid_file_handle,
.ftruncate = synth_ftruncate,
+ .futimens = synth_futimens,
};
diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index 7bc4ec8e85cc..a1924fe3f05a 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -103,6 +103,7 @@ static inline int errno_to_dotl(int err) {
#define qemu_renameat renameat
#define qemu_utimensat utimensat
#define qemu_unlinkat unlinkat
+#define qemu_futimens futimens
static inline void close_preserve_errno(int fd)
{
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 4616bd763012..501c5a4cdff2 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1708,7 +1708,11 @@ static void coroutine_fn v9fs_setattr(void *opaque)
} else {
times[1].tv_nsec = UTIME_OMIT;
}
- err = v9fs_co_utimensat(pdu, &fidp->path, times);
+ if (fid_has_valid_handle(pdu->s, fidp)) {
+ err = v9fs_co_futimens(pdu, fidp, times);
+ } else {
+ err = v9fs_co_utimensat(pdu, &fidp->path, times);
+ }
if (err < 0) {
goto out;
}
diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c
index 893466fb1a44..12fa8c9fe9cd 100644
--- a/hw/9pfs/cofs.c
+++ b/hw/9pfs/cofs.c
@@ -139,6 +139,25 @@ int coroutine_fn v9fs_co_utimensat(V9fsPDU *pdu, V9fsPath *path,
return err;
}
+int coroutine_fn v9fs_co_futimens(V9fsPDU *pdu, V9fsFidState *fidp,
+ struct timespec times[2])
+{
+ int err;
+ V9fsState *s = pdu->s;
+
+ if (v9fs_request_cancelled(pdu)) {
+ return -EINTR;
+ }
+ v9fs_co_run_in_worker(
+ {
+ err = s->ops->futimens(&s->ctx, fidp->fid_type, &fidp->fs, times);
+ if (err < 0) {
+ err = -errno;
+ }
+ });
+ return err;
+}
+
int coroutine_fn v9fs_co_chown(V9fsPDU *pdu, V9fsPath *path, uid_t uid,
gid_t gid)
{
diff --git a/hw/9pfs/coth.h b/hw/9pfs/coth.h
index 62e922dc12e3..7906fa7782d8 100644
--- a/hw/9pfs/coth.h
+++ b/hw/9pfs/coth.h
@@ -71,6 +71,8 @@ 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_futimens(V9fsPDU *pdu, V9fsFidState *fidp,
+ struct timespec times[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_ftruncate(V9fsPDU *pdu, V9fsFidState *fidp,
--
2.48.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 5/6] tests/9p: add 'Tsetattr' request to test client
2025-03-11 17:28 [PATCH v2 0/6] 9pfs: Fix ftruncate-after-unlink Greg Kurz
` (3 preceding siblings ...)
2025-03-11 17:28 ` [PATCH v2 4/6] 9pfs: Introduce futimens " Greg Kurz
@ 2025-03-11 17:28 ` Greg Kurz
2025-03-11 17:28 ` [PATCH v2 6/6] tests/9p: Test `Tsetattr` can truncate unlinked file Greg Kurz
5 siblings, 0 replies; 14+ messages in thread
From: Greg Kurz @ 2025-03-11 17:28 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Christian Schoenebeck, Paolo Bonzini,
Laurent Vivier, Greg Kurz
From: Christian Schoenebeck <qemu_oss@crudebyte.com>
Add and implement functions to 9pfs test client for sending a 9p2000.L
'Tsetattr' request and receiving its 'Rsetattr' response counterpart.
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
tests/qtest/libqos/virtio-9p-client.c | 49 +++++++++++++++++++++++++++
tests/qtest/libqos/virtio-9p-client.h | 34 +++++++++++++++++++
tests/qtest/virtio-9p-test.c | 1 +
3 files changed, 84 insertions(+)
diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c
index 98b77db51d77..6ab4501c6e1a 100644
--- a/tests/qtest/libqos/virtio-9p-client.c
+++ b/tests/qtest/libqos/virtio-9p-client.c
@@ -557,6 +557,55 @@ void v9fs_rgetattr(P9Req *req, v9fs_attr *attr)
v9fs_req_free(req);
}
+/*
+ * size[4] Tsetattr tag[2] fid[4] valid[4] mode[4] uid[4] gid[4] size[8]
+ * atime_sec[8] atime_nsec[8] mtime_sec[8] mtime_nsec[8]
+ */
+TSetAttrRes v9fs_tsetattr(TSetAttrOpt opt)
+{
+ P9Req *req;
+ uint32_t err;
+
+ g_assert(opt.client);
+
+ req = v9fs_req_init(
+ opt.client, 4/*fid*/ + 4/*valid*/ + 4/*mode*/ + 4/*uid*/ + 4/*gid*/ +
+ 8/*size*/ + 8/*atime_sec*/ + 8/*atime_nsec*/ + 8/*mtime_sec*/ +
+ 8/*mtime_nsec*/, P9_TSETATTR, opt.tag
+ );
+ v9fs_uint32_write(req, opt.fid);
+ v9fs_uint32_write(req, (uint32_t) opt.attr.valid);
+ v9fs_uint32_write(req, opt.attr.mode);
+ v9fs_uint32_write(req, opt.attr.uid);
+ v9fs_uint32_write(req, opt.attr.gid);
+ v9fs_uint64_write(req, opt.attr.size);
+ v9fs_uint64_write(req, opt.attr.atime_sec);
+ v9fs_uint64_write(req, opt.attr.atime_nsec);
+ v9fs_uint64_write(req, opt.attr.mtime_sec);
+ v9fs_uint64_write(req, opt.attr.mtime_nsec);
+ v9fs_req_send(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_rsetattr(req);
+ }
+ req = NULL; /* request was freed */
+ }
+
+ return (TSetAttrRes) { .req = req };
+}
+
+/* size[4] Rsetattr tag[2] */
+void v9fs_rsetattr(P9Req *req)
+{
+ v9fs_req_recv(req, P9_RSETATTR);
+ v9fs_req_free(req);
+}
+
/* size[4] Treaddir tag[2] fid[4] offset[8] count[4] */
TReadDirRes v9fs_treaddir(TReadDirOpt opt)
{
diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h
index 78228eb97d9e..e3221a310412 100644
--- a/tests/qtest/libqos/virtio-9p-client.h
+++ b/tests/qtest/libqos/virtio-9p-client.h
@@ -65,6 +65,16 @@ typedef struct v9fs_attr {
#define P9_GETATTR_BASIC 0x000007ffULL /* Mask for fields up to BLOCKS */
#define P9_GETATTR_ALL 0x00003fffULL /* Mask for ALL fields */
+#define P9_SETATTR_MODE 0x00000001UL
+#define P9_SETATTR_UID 0x00000002UL
+#define P9_SETATTR_GID 0x00000004UL
+#define P9_SETATTR_SIZE 0x00000008UL
+#define P9_SETATTR_ATIME 0x00000010UL
+#define P9_SETATTR_MTIME 0x00000020UL
+#define P9_SETATTR_CTIME 0x00000040UL
+#define P9_SETATTR_ATIME_SET 0x00000080UL
+#define P9_SETATTR_MTIME_SET 0x00000100UL
+
struct V9fsDirent {
v9fs_qid qid;
uint64_t offset;
@@ -182,6 +192,28 @@ typedef struct TGetAttrRes {
P9Req *req;
} TGetAttrRes;
+/* options for 'Tsetattr' 9p request */
+typedef struct TSetAttrOpt {
+ /* 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 modified (required) */
+ uint32_t fid;
+ /* new attribute values to be set by 9p server */
+ v9fs_attr attr;
+ /* only send Tsetattr 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;
+} TSetAttrOpt;
+
+/* result of 'Tsetattr' 9p request */
+typedef struct TSetAttrRes {
+ /* if requestOnly was set: request object for further processing */
+ P9Req *req;
+} TSetAttrRes;
+
/* options for 'Treaddir' 9p request */
typedef struct TReadDirOpt {
/* 9P client being used (mandatory) */
@@ -470,6 +502,8 @@ 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);
+TSetAttrRes v9fs_tsetattr(TSetAttrOpt opt);
+void v9fs_rsetattr(P9Req *req);
TReadDirRes v9fs_treaddir(TReadDirOpt);
void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t *nentries,
struct V9fsDirent **entries);
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index ab3a12c816d4..f515a9bb157b 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 tsetattr(...) v9fs_tsetattr((TSetAttrOpt) __VA_ARGS__)
#define treaddir(...) v9fs_treaddir((TReadDirOpt) __VA_ARGS__)
#define tlopen(...) v9fs_tlopen((TLOpenOpt) __VA_ARGS__)
#define twrite(...) v9fs_twrite((TWriteOpt) __VA_ARGS__)
--
2.48.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 6/6] tests/9p: Test `Tsetattr` can truncate unlinked file
2025-03-11 17:28 [PATCH v2 0/6] 9pfs: Fix ftruncate-after-unlink Greg Kurz
` (4 preceding siblings ...)
2025-03-11 17:28 ` [PATCH v2 5/6] tests/9p: add 'Tsetattr' request to test client Greg Kurz
@ 2025-03-11 17:28 ` Greg Kurz
2025-03-12 14:11 ` Christian Schoenebeck
5 siblings, 1 reply; 14+ messages in thread
From: Greg Kurz @ 2025-03-11 17:28 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Christian Schoenebeck, Paolo Bonzini,
Laurent Vivier, Greg Kurz
Enhance the `use-after-unlink` test with a new check for the
case where the client wants to alter the size of an unlinked
file for which it still has an active fid.
Suggested-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
tests/qtest/virtio-9p-test.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index f515a9bb157b..20c0d744fa56 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -736,6 +736,14 @@ static void fs_use_after_unlink(void *obj, void *data,
.data = buf
}).count;
g_assert_cmpint(count, ==, write_count);
+
+ /* truncate file to (arbitrarily chosen) size 2001 */
+ tsetattr({
+ .client = v9p, .fid = fid_file, .attr = (v9fs_attr) {
+ .valid = P9_SETATTR_SIZE,
+ .size = 2001
+ }
+ });
}
static void cleanup_9p_local_driver(void *data)
--
2.48.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 6/6] tests/9p: Test `Tsetattr` can truncate unlinked file
2025-03-11 17:28 ` [PATCH v2 6/6] tests/9p: Test `Tsetattr` can truncate unlinked file Greg Kurz
@ 2025-03-12 14:11 ` Christian Schoenebeck
2025-03-12 14:25 ` Greg Kurz
0 siblings, 1 reply; 14+ messages in thread
From: Christian Schoenebeck @ 2025-03-12 14:11 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Paolo Bonzini, Laurent Vivier, Greg Kurz
On Tuesday, March 11, 2025 6:28:09 PM CET Greg Kurz wrote:
> Enhance the `use-after-unlink` test with a new check for the
> case where the client wants to alter the size of an unlinked
> file for which it still has an active fid.
>
> Suggested-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> tests/qtest/virtio-9p-test.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> index f515a9bb157b..20c0d744fa56 100644
> --- a/tests/qtest/virtio-9p-test.c
> +++ b/tests/qtest/virtio-9p-test.c
> @@ -736,6 +736,14 @@ static void fs_use_after_unlink(void *obj, void *data,
> .data = buf
> }).count;
> g_assert_cmpint(count, ==, write_count);
> +
> + /* truncate file to (arbitrarily chosen) size 2001 */
> + tsetattr({
> + .client = v9p, .fid = fid_file, .attr = (v9fs_attr) {
> + .valid = P9_SETATTR_SIZE,
> + .size = 2001
> + }
> + });
> }
>
> static void cleanup_9p_local_driver(void *data)
>
Ah, I just meant the code snippet as a starting point, like I would have also
checked with a stat() call whether 9p server really did what it promised.
But OK, better some test coverage than nothing. :)
/Christian
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 6/6] tests/9p: Test `Tsetattr` can truncate unlinked file
2025-03-12 14:11 ` Christian Schoenebeck
@ 2025-03-12 14:25 ` Greg Kurz
2025-03-12 14:34 ` Christian Schoenebeck
0 siblings, 1 reply; 14+ messages in thread
From: Greg Kurz @ 2025-03-12 14:25 UTC (permalink / raw)
To: Christian Schoenebeck
Cc: qemu-devel, Fabiano Rosas, Paolo Bonzini, Laurent Vivier
On Wed, 12 Mar 2025 15:11:41 +0100
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> On Tuesday, March 11, 2025 6:28:09 PM CET Greg Kurz wrote:
> > Enhance the `use-after-unlink` test with a new check for the
> > case where the client wants to alter the size of an unlinked
> > file for which it still has an active fid.
> >
> > Suggested-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> > tests/qtest/virtio-9p-test.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> > index f515a9bb157b..20c0d744fa56 100644
> > --- a/tests/qtest/virtio-9p-test.c
> > +++ b/tests/qtest/virtio-9p-test.c
> > @@ -736,6 +736,14 @@ static void fs_use_after_unlink(void *obj, void *data,
> > .data = buf
> > }).count;
> > g_assert_cmpint(count, ==, write_count);
> > +
> > + /* truncate file to (arbitrarily chosen) size 2001 */
> > + tsetattr({
> > + .client = v9p, .fid = fid_file, .attr = (v9fs_attr) {
> > + .valid = P9_SETATTR_SIZE,
> > + .size = 2001
> > + }
> > + });
> > }
> >
> > static void cleanup_9p_local_driver(void *data)
> >
>
> Ah, I just meant the code snippet as a starting point, like I would have also
> checked with a stat() call whether 9p server really did what it promised.
>
> But OK, better some test coverage than nothing. :)
>
FWIW the server returns ENOENT if it doesn't have the fix which causes
the check to fail. I was assuming this would be enough but I'm fine with
adding an extra check if you want.
> /Christian
>
>
--
Greg
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 6/6] tests/9p: Test `Tsetattr` can truncate unlinked file
2025-03-12 14:25 ` Greg Kurz
@ 2025-03-12 14:34 ` Christian Schoenebeck
0 siblings, 0 replies; 14+ messages in thread
From: Christian Schoenebeck @ 2025-03-12 14:34 UTC (permalink / raw)
To: qemu-devel, Fabiano Rosas; +Cc: Paolo Bonzini, Laurent Vivier, Greg Kurz
On Wednesday, March 12, 2025 3:25:20 PM CET Greg Kurz wrote:
> On Wed, 12 Mar 2025 15:11:41 +0100
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
>
> > On Tuesday, March 11, 2025 6:28:09 PM CET Greg Kurz wrote:
> > > Enhance the `use-after-unlink` test with a new check for the
> > > case where the client wants to alter the size of an unlinked
> > > file for which it still has an active fid.
> > >
> > > Suggested-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > Signed-off-by: Greg Kurz <groug@kaod.org>
> > > ---
> > > tests/qtest/virtio-9p-test.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> > > index f515a9bb157b..20c0d744fa56 100644
> > > --- a/tests/qtest/virtio-9p-test.c
> > > +++ b/tests/qtest/virtio-9p-test.c
> > > @@ -736,6 +736,14 @@ static void fs_use_after_unlink(void *obj, void *data,
> > > .data = buf
> > > }).count;
> > > g_assert_cmpint(count, ==, write_count);
> > > +
> > > + /* truncate file to (arbitrarily chosen) size 2001 */
> > > + tsetattr({
> > > + .client = v9p, .fid = fid_file, .attr = (v9fs_attr) {
> > > + .valid = P9_SETATTR_SIZE,
> > > + .size = 2001
> > > + }
> > > + });
> > > }
> > >
> > > static void cleanup_9p_local_driver(void *data)
> > >
> >
> > Ah, I just meant the code snippet as a starting point, like I would have also
> > checked with a stat() call whether 9p server really did what it promised.
> >
> > But OK, better some test coverage than nothing. :)
> >
>
> FWIW the server returns ENOENT if it doesn't have the fix which causes
> the check to fail. I was assuming this would be enough but I'm fine with
> adding an extra check if you want.
Yeah, that's why I wasn't really anxious about it. If you have some cycles,
fine, I'll guess you can just copy & paste existing stat() code from another
test, otherwise deferred into future, NP.
Thanks!
/Christian
^ permalink raw reply [flat|nested] 14+ messages in thread