* Re: [PATCH] hw/9pfs: add support ORCLOSE flag to create and open request
2026-07-17 6:43 [PATCH] hw/9pfs: add support ORCLOSE flag to create and open request Kyohei Kadota via qemu development
@ 2026-07-21 15:03 ` Christian Schoenebeck
0 siblings, 0 replies; 2+ messages in thread
From: Christian Schoenebeck @ 2026-07-21 15:03 UTC (permalink / raw)
To: Kyohei Kadota; +Cc: qemu-devel, Greg Kurz
On Friday, 17 July 2026 08:43:26 CEST Kyohei Kadota wrote:
> 9P, and its successor 9P2000, allows ORCLOSE flag to both open- and
> create-request.
> When 9pclient requests close(fid) operation to the 9pserver, and if
> the fid has ORCLOSE flag,
> 9pserver deletes the file, which is pointed of the fid, from
> 9pserver's filesystem.
>
> However, the 9P specification does not allow any directories to be
> specified ORCLOSE flag.
>
> Signed-off-by: KADOTA, Kyohei <lufia@lufia.org>
As this is not a trivial change, rather the contrary, I think this new feature
deserves being covered by test cases:
https://wiki.qemu.org/Documentation/9p#Test_Cases
Tests should be separate patch(es).
> ---
> hw/9pfs/9p.c | 17 +++++++++++++++++
> hw/9pfs/9p.h | 1 +
> 2 files changed, 18 insertions(+)
>
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 3119f01117..6fff57cc47 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -416,6 +416,9 @@ static int coroutine_fn free_fid(V9fsPDU *pdu,
> V9fsFidState *fidp)
> if (fidp->fs.fd != -1) {
> retval = v9fs_co_close(pdu, &fidp->fs);
> }
> + if (retval >= 0 && fidp->orclose) {
> + retval = v9fs_co_remove(pdu, &fidp->path);
> + }
> } else if (fidp->fid_type == P9_FID_DIR) {
> if (fidp->fs.dir.stream != NULL) {
> retval = v9fs_co_closedir(pdu, &fidp->fs);
1. That would cause a silent ignore (neither removed, no error sent to client)
when creating special files with Orclose, like symlink, hardlink, device,
pipe, socket, as these end up as fid_type P9_FID_NONE, which are not covered
by this change.
2. According to POSIX, close() errors should in general be ignored, with one
exception: if errno == EBADF. You find this pattern at other locations of
*close*() calls in this code base already.
> @@ -2164,6 +2167,10 @@ static void coroutine_fn v9fs_open(void *opaque)
> goto out;
> }
> if (S_ISDIR(stbuf.st_mode)) {
> + if (mode & Orclose) {
> + err = -EINVAL;
> + goto out;
> + }
> err = v9fs_co_opendir(pdu, fidp);
> if (err < 0) {
> goto out;
> @@ -2193,6 +2200,9 @@ static void coroutine_fn v9fs_open(void *opaque)
> }
> fidp->fid_type = P9_FID_FILE;
> fidp->open_flags = flags;
> + if (mode & Orclose) {
> + fidp->orclose = true;
> + }
> if (flags & O_EXCL) {
> /*
> * We let the host file system do O_EXCL check
> @@ -2959,6 +2969,10 @@ static void coroutine_fn v9fs_create(void *opaque)
> goto out;
> }
> if (perm & P9_STAT_MODE_DIR) {
> + if (mode & Orclose) {
> + err = -EINVAL;
> + goto out;
> + }
> err = v9fs_co_mkdir(pdu, fidp, &name, perm & 0777,
> fidp->uid, -1, &stbuf);
> if (err < 0) {
> @@ -3083,6 +3097,9 @@ static void coroutine_fn v9fs_create(void *opaque)
> }
> fidp->fid_type = P9_FID_FILE;
> fidp->open_flags = omode_to_uflags(mode);
> + if (mode & Orclose) {
> + fidp->orclose = true;
> + }
> if (fidp->open_flags & O_EXCL) {
> /*
> * We let the host file system do O_EXCL check
That's just the handler for legacy 9p2000.u protocol version (Tcreate
request). What's missing is also a similar change for v9fs_lcreate() which is
the corresponding 9p2000.L protocol version handler (Tlcreate request).
> diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> index 1a309664f6..16730cbd93 100644
> --- a/hw/9pfs/9p.h
> +++ b/hw/9pfs/9p.h
> @@ -276,6 +276,7 @@ struct V9fsFidState {
> V9fsFidOpenState fs;
> V9fsFidOpenState fs_reclaim;
> int flags;
> + bool orclose;
> int open_flags;
> uid_t uid;
> int ref;
V9fsFidState is typically allocated in a huge amount. Therefore I am not keen
to make this struct (unnecessarily) larger. In this case you could simply use
the already existing 'flags' field and just add a define for the new flag.
And last but not least: how should all of this behave with reclaiming file
descriptors? That's happening when 9p server is getting under pressure and
running against host's max. amount of open file descriptors limit. In this
case 9p server must temporarily close older file descriptors for allowing
opening new descriptors, and the old ones are then automatically re-opened if
those old FIDs are re-used by client later on.
Maybe it is sufficient to call v9fs_mark_fids_unreclaim() just right before
your added v9fs_co_remove() call, but not absolutely sure.
/Christian
^ permalink raw reply [flat|nested] 2+ messages in thread