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 <[email protected]> > > 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 <[email protected]> > 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); >
