qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kurz <groug@kaod.org>
To: qemu-devel@nongnu.org
Cc: Christian Schoenebeck <qemu_oss@crudebyte.com>,
	Greg Kurz <groug@kaod.org>
Subject: [PATCH 2/4] 9pfs: Don't use file descriptors in core code
Date: Mon, 10 Mar 2025 18:10:59 +0100	[thread overview]
Message-ID: <20250310171101.138380-3-groug@kaod.org> (raw)
In-Reply-To: <20250310171101.138380-1-groug@kaod.org>

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_handle() backend operation.

Make the existing local_fid_fd() helper more robust so that it
can cope with P9_FID_NONE or a null directory stream and reuse
it.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 fsdev/file-op-9p.h |  1 +
 hw/9pfs/9p-local.c | 12 +++++++++---
 hw/9pfs/9p-synth.c |  6 ++++++
 hw/9pfs/9p.c       |  9 ++++++---
 4 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index 4997677460e8..39fee185f4ce 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_handle)(int fid_type, V9fsFidOpenState *fs);
 };
 
 #endif
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index c4366c867988..03e5304ef888 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -768,11 +768,11 @@ out:
 
 static int local_fid_fd(int fid_type, V9fsFidOpenState *fs)
 {
-    int fd;
+    int fd = -1;
 
-    if (fid_type == P9_FID_DIR) {
+    if (fid_type == P9_FID_DIR && fs->dir.stream != NULL) {
         fd = dirfd(fs->dir.stream);
-    } else {
+    } else if (fid_type == P9_FID_FILE) {
         fd = fs->fd;
     }
 
@@ -1576,6 +1576,11 @@ static int local_parse_opts(QemuOpts *opts, FsDriverEntry *fse, Error **errp)
     return 0;
 }
 
+static bool local_has_valid_handle(int fid_type, V9fsFidOpenState *fs)
+{
+    return local_fid_fd(fid_type, fs) != -1;
+}
+
 FileOperations local_ops = {
     .parse_opts = local_parse_opts,
     .init  = local_init,
@@ -1613,4 +1618,5 @@ FileOperations local_ops = {
     .name_to_path = local_name_to_path,
     .renameat  = local_renameat,
     .unlinkat = local_unlinkat,
+    .has_valid_handle = local_has_valid_handle,
 };
diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index 2abaf3a2918a..fa0b187a1b80 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_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_handle = synth_has_valid_handle,
 };
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 7cad2bce6209..969fb2f8c494 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_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



  parent reply	other threads:[~2025-03-10 17:11 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-10 17:10 [PATCH 0/4] 9pfs: Fix ftruncate-after-unlink Greg Kurz
2025-03-10 17:10 ` [PATCH 1/4] 9pfs: local : Introduce local_fid_fd() helper Greg Kurz
2025-03-11 10:58   ` Christian Schoenebeck
2025-03-11 14:01     ` Greg Kurz
2025-03-10 17:10 ` Greg Kurz [this message]
2025-03-11 11:13   ` [PATCH 2/4] 9pfs: Don't use file descriptors in core code Christian Schoenebeck
2025-03-11 14:03     ` Greg Kurz
2025-03-11 14:13       ` Greg Kurz
2025-03-11 14:18       ` Christian Schoenebeck
2025-03-10 17:11 ` [PATCH 3/4] 9pfs: Introduce ftruncate file op Greg Kurz
2025-03-11 11:19   ` Christian Schoenebeck
2025-03-10 17:11 ` [PATCH 4/4] 9pfs: Introduce futimens " Greg Kurz
2025-03-11 11:25   ` Christian Schoenebeck
2025-03-11 10:51 ` [PATCH 0/4] 9pfs: Fix ftruncate-after-unlink Christian Schoenebeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250310171101.138380-3-groug@kaod.org \
    --to=groug@kaod.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu_oss@crudebyte.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).