| Message ID | 20250321093145.17882-2-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | 9pfs: add some file operation hooks | expand |
Juergen Gross, le ven. 21 mars 2025 10:31:44 +0100, a ecrit: > Add a file operations fstat hook to the 9pfs frontend. > > Signed-off-by: Juergen Gross <jgross@suse.com> > Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> > --- > V2: > - or file access mode into st_mode (Jason Andryuk) > --- > 9pfront.c | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/9pfront.c b/9pfront.c > index 1741d600..7257a07e 100644 > --- a/9pfront.c > +++ b/9pfront.c > @@ -85,6 +85,8 @@ struct file_9pfs { > > #define P9_QID_SIZE 13 > > +#define QID_TYPE_DIR 0x80 /* Applies to qid[0]. */ > + > struct p9_header { > uint32_t size; > uint8_t cmd; > @@ -950,6 +952,32 @@ static int write_9pfs(struct file *file, const void *buf, size_t nbytes) > return ret; > } > > +static int fstat_9pfs(struct file *file, struct stat *buf) > +{ > + struct file_9pfs *f9pfs = file->filedata; > + struct p9_stat stat; > + int ret; > + > + ret = p9_stat(f9pfs->dev, f9pfs->fid, &stat); > + if ( ret ) > + { > + errno = EIO; > + return -1; > + } > + > + buf->st_mode = (stat.qid[0] == QID_TYPE_DIR) ? S_IFDIR : S_IFREG; > + buf->st_mode |= stat.mode & 0777; > + buf->st_atime = stat.atime; > + buf->st_mtime = stat.mtime; Should we perhaps also fill st_ctime? Leaving it at 0 could surprise other software layers. > + buf->st_size = stat.length; > + buf->st_uid = stat.n_uid; > + buf->st_gid = stat.n_gid; > + > + free_stat(&stat); > + > + return 0; > +} > + > static int close_9pfs(struct file *file) > { > struct file_9pfs *f9pfs = file->filedata; > @@ -1296,6 +1324,7 @@ static const struct file_ops ops_9pfs = { > .read = read_9pfs, > .write = write_9pfs, > .close = close_9pfs, > + .fstat = fstat_9pfs, > }; > > __attribute__((constructor)) > -- > 2.43.0 >
On 23.03.25 01:01, Samuel Thibault wrote: > Juergen Gross, le ven. 21 mars 2025 10:31:44 +0100, a ecrit: >> Add a file operations fstat hook to the 9pfs frontend. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> >> --- >> V2: >> - or file access mode into st_mode (Jason Andryuk) >> --- >> 9pfront.c | 29 +++++++++++++++++++++++++++++ >> 1 file changed, 29 insertions(+) >> >> diff --git a/9pfront.c b/9pfront.c >> index 1741d600..7257a07e 100644 >> --- a/9pfront.c >> +++ b/9pfront.c >> @@ -85,6 +85,8 @@ struct file_9pfs { >> >> #define P9_QID_SIZE 13 >> >> +#define QID_TYPE_DIR 0x80 /* Applies to qid[0]. */ >> + >> struct p9_header { >> uint32_t size; >> uint8_t cmd; >> @@ -950,6 +952,32 @@ static int write_9pfs(struct file *file, const void *buf, size_t nbytes) >> return ret; >> } >> >> +static int fstat_9pfs(struct file *file, struct stat *buf) >> +{ >> + struct file_9pfs *f9pfs = file->filedata; >> + struct p9_stat stat; >> + int ret; >> + >> + ret = p9_stat(f9pfs->dev, f9pfs->fid, &stat); >> + if ( ret ) >> + { >> + errno = EIO; >> + return -1; >> + } >> + >> + buf->st_mode = (stat.qid[0] == QID_TYPE_DIR) ? S_IFDIR : S_IFREG; >> + buf->st_mode |= stat.mode & 0777; >> + buf->st_atime = stat.atime; >> + buf->st_mtime = stat.mtime; > > Should we perhaps also fill st_ctime? Leaving it at 0 could surprise > other software layers. I can set it to the same value as st_mtime. I don't think there is a way to get the "real" ctime value via the 9p protocol. Juergen
Jürgen Groß, le dim. 23 mars 2025 15:57:16 +0100, a ecrit: > On 23.03.25 01:01, Samuel Thibault wrote: > > Juergen Gross, le ven. 21 mars 2025 10:31:44 +0100, a ecrit: > > > Add a file operations fstat hook to the 9pfs frontend. > > > > > > Signed-off-by: Juergen Gross <jgross@suse.com> > > > Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> > > > --- > > > V2: > > > - or file access mode into st_mode (Jason Andryuk) > > > --- > > > 9pfront.c | 29 +++++++++++++++++++++++++++++ > > > 1 file changed, 29 insertions(+) > > > > > > diff --git a/9pfront.c b/9pfront.c > > > index 1741d600..7257a07e 100644 > > > --- a/9pfront.c > > > +++ b/9pfront.c > > > @@ -85,6 +85,8 @@ struct file_9pfs { > > > #define P9_QID_SIZE 13 > > > +#define QID_TYPE_DIR 0x80 /* Applies to qid[0]. */ > > > + > > > struct p9_header { > > > uint32_t size; > > > uint8_t cmd; > > > @@ -950,6 +952,32 @@ static int write_9pfs(struct file *file, const void *buf, size_t nbytes) > > > return ret; > > > } > > > +static int fstat_9pfs(struct file *file, struct stat *buf) > > > +{ > > > + struct file_9pfs *f9pfs = file->filedata; > > > + struct p9_stat stat; > > > + int ret; > > > + > > > + ret = p9_stat(f9pfs->dev, f9pfs->fid, &stat); > > > + if ( ret ) > > > + { > > > + errno = EIO; > > > + return -1; > > > + } > > > + > > > + buf->st_mode = (stat.qid[0] == QID_TYPE_DIR) ? S_IFDIR : S_IFREG; > > > + buf->st_mode |= stat.mode & 0777; > > > + buf->st_atime = stat.atime; > > > + buf->st_mtime = stat.mtime; > > > > Should we perhaps also fill st_ctime? Leaving it at 0 could surprise > > other software layers. > > I can set it to the same value as st_mtime. That'd be preferrable, yes. With that, Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org> Thanks! Samuel
On 23.03.2025 15:57, Jürgen Groß wrote: > On 23.03.25 01:01, Samuel Thibault wrote: >> Juergen Gross, le ven. 21 mars 2025 10:31:44 +0100, a ecrit: >>> Add a file operations fstat hook to the 9pfs frontend. >>> >>> Signed-off-by: Juergen Gross <jgross@suse.com> >>> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> >>> --- >>> V2: >>> - or file access mode into st_mode (Jason Andryuk) >>> --- >>> 9pfront.c | 29 +++++++++++++++++++++++++++++ >>> 1 file changed, 29 insertions(+) >>> >>> diff --git a/9pfront.c b/9pfront.c >>> index 1741d600..7257a07e 100644 >>> --- a/9pfront.c >>> +++ b/9pfront.c >>> @@ -85,6 +85,8 @@ struct file_9pfs { >>> >>> #define P9_QID_SIZE 13 >>> >>> +#define QID_TYPE_DIR 0x80 /* Applies to qid[0]. */ >>> + >>> struct p9_header { >>> uint32_t size; >>> uint8_t cmd; >>> @@ -950,6 +952,32 @@ static int write_9pfs(struct file *file, const void *buf, size_t nbytes) >>> return ret; >>> } >>> >>> +static int fstat_9pfs(struct file *file, struct stat *buf) >>> +{ >>> + struct file_9pfs *f9pfs = file->filedata; >>> + struct p9_stat stat; >>> + int ret; >>> + >>> + ret = p9_stat(f9pfs->dev, f9pfs->fid, &stat); >>> + if ( ret ) >>> + { >>> + errno = EIO; >>> + return -1; >>> + } >>> + >>> + buf->st_mode = (stat.qid[0] == QID_TYPE_DIR) ? S_IFDIR : S_IFREG; >>> + buf->st_mode |= stat.mode & 0777; >>> + buf->st_atime = stat.atime; >>> + buf->st_mtime = stat.mtime; >> >> Should we perhaps also fill st_ctime? Leaving it at 0 could surprise >> other software layers. > > I can set it to the same value as st_mtime. Maybe the smaller of atime and mtime? Jan > I don't think there is a way > to get the "real" ctime value via the 9p protocol. > > > Juergen
Jan Beulich, le lun. 24 mars 2025 11:21:48 +0100, a ecrit: > On 23.03.2025 15:57, Jürgen Groß wrote: > > On 23.03.25 01:01, Samuel Thibault wrote: > >> Juergen Gross, le ven. 21 mars 2025 10:31:44 +0100, a ecrit: > >>> Add a file operations fstat hook to the 9pfs frontend. > >>> > >>> Signed-off-by: Juergen Gross <jgross@suse.com> > >>> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> > >>> --- > >>> V2: > >>> - or file access mode into st_mode (Jason Andryuk) > >>> --- > >>> 9pfront.c | 29 +++++++++++++++++++++++++++++ > >>> 1 file changed, 29 insertions(+) > >>> > >>> diff --git a/9pfront.c b/9pfront.c > >>> index 1741d600..7257a07e 100644 > >>> --- a/9pfront.c > >>> +++ b/9pfront.c > >>> @@ -85,6 +85,8 @@ struct file_9pfs { > >>> > >>> #define P9_QID_SIZE 13 > >>> > >>> +#define QID_TYPE_DIR 0x80 /* Applies to qid[0]. */ > >>> + > >>> struct p9_header { > >>> uint32_t size; > >>> uint8_t cmd; > >>> @@ -950,6 +952,32 @@ static int write_9pfs(struct file *file, const void *buf, size_t nbytes) > >>> return ret; > >>> } > >>> > >>> +static int fstat_9pfs(struct file *file, struct stat *buf) > >>> +{ > >>> + struct file_9pfs *f9pfs = file->filedata; > >>> + struct p9_stat stat; > >>> + int ret; > >>> + > >>> + ret = p9_stat(f9pfs->dev, f9pfs->fid, &stat); > >>> + if ( ret ) > >>> + { > >>> + errno = EIO; > >>> + return -1; > >>> + } > >>> + > >>> + buf->st_mode = (stat.qid[0] == QID_TYPE_DIR) ? S_IFDIR : S_IFREG; > >>> + buf->st_mode |= stat.mode & 0777; > >>> + buf->st_atime = stat.atime; > >>> + buf->st_mtime = stat.mtime; > >> > >> Should we perhaps also fill st_ctime? Leaving it at 0 could surprise > >> other software layers. > > > > I can set it to the same value as st_mtime. > > Maybe the smaller of atime and mtime? That'd be better, yes. Samuel
On 24.03.25 11:30, Samuel Thibault wrote: > Jan Beulich, le lun. 24 mars 2025 11:21:48 +0100, a ecrit: >> On 23.03.2025 15:57, Jürgen Groß wrote: >>> On 23.03.25 01:01, Samuel Thibault wrote: >>>> Juergen Gross, le ven. 21 mars 2025 10:31:44 +0100, a ecrit: >>>>> Add a file operations fstat hook to the 9pfs frontend. >>>>> >>>>> Signed-off-by: Juergen Gross <jgross@suse.com> >>>>> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> >>>>> --- >>>>> V2: >>>>> - or file access mode into st_mode (Jason Andryuk) >>>>> --- >>>>> 9pfront.c | 29 +++++++++++++++++++++++++++++ >>>>> 1 file changed, 29 insertions(+) >>>>> >>>>> diff --git a/9pfront.c b/9pfront.c >>>>> index 1741d600..7257a07e 100644 >>>>> --- a/9pfront.c >>>>> +++ b/9pfront.c >>>>> @@ -85,6 +85,8 @@ struct file_9pfs { >>>>> >>>>> #define P9_QID_SIZE 13 >>>>> >>>>> +#define QID_TYPE_DIR 0x80 /* Applies to qid[0]. */ >>>>> + >>>>> struct p9_header { >>>>> uint32_t size; >>>>> uint8_t cmd; >>>>> @@ -950,6 +952,32 @@ static int write_9pfs(struct file *file, const void *buf, size_t nbytes) >>>>> return ret; >>>>> } >>>>> >>>>> +static int fstat_9pfs(struct file *file, struct stat *buf) >>>>> +{ >>>>> + struct file_9pfs *f9pfs = file->filedata; >>>>> + struct p9_stat stat; >>>>> + int ret; >>>>> + >>>>> + ret = p9_stat(f9pfs->dev, f9pfs->fid, &stat); >>>>> + if ( ret ) >>>>> + { >>>>> + errno = EIO; >>>>> + return -1; >>>>> + } >>>>> + >>>>> + buf->st_mode = (stat.qid[0] == QID_TYPE_DIR) ? S_IFDIR : S_IFREG; >>>>> + buf->st_mode |= stat.mode & 0777; >>>>> + buf->st_atime = stat.atime; >>>>> + buf->st_mtime = stat.mtime; >>>> >>>> Should we perhaps also fill st_ctime? Leaving it at 0 could surprise >>>> other software layers. >>> >>> I can set it to the same value as st_mtime. >> >> Maybe the smaller of atime and mtime? > > That'd be better, yes. According to the references I could find ctime is changed whenever either file contents OR file status (uid, gid, permissions) are modified. So using the same value as mtime seems appropriate. Juergen
On 24.03.2025 12:18, Jürgen Groß wrote: > On 24.03.25 11:30, Samuel Thibault wrote: >> Jan Beulich, le lun. 24 mars 2025 11:21:48 +0100, a ecrit: >>> On 23.03.2025 15:57, Jürgen Groß wrote: >>>> On 23.03.25 01:01, Samuel Thibault wrote: >>>>> Juergen Gross, le ven. 21 mars 2025 10:31:44 +0100, a ecrit: >>>>>> Add a file operations fstat hook to the 9pfs frontend. >>>>>> >>>>>> Signed-off-by: Juergen Gross <jgross@suse.com> >>>>>> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> >>>>>> --- >>>>>> V2: >>>>>> - or file access mode into st_mode (Jason Andryuk) >>>>>> --- >>>>>> 9pfront.c | 29 +++++++++++++++++++++++++++++ >>>>>> 1 file changed, 29 insertions(+) >>>>>> >>>>>> diff --git a/9pfront.c b/9pfront.c >>>>>> index 1741d600..7257a07e 100644 >>>>>> --- a/9pfront.c >>>>>> +++ b/9pfront.c >>>>>> @@ -85,6 +85,8 @@ struct file_9pfs { >>>>>> >>>>>> #define P9_QID_SIZE 13 >>>>>> >>>>>> +#define QID_TYPE_DIR 0x80 /* Applies to qid[0]. */ >>>>>> + >>>>>> struct p9_header { >>>>>> uint32_t size; >>>>>> uint8_t cmd; >>>>>> @@ -950,6 +952,32 @@ static int write_9pfs(struct file *file, const void *buf, size_t nbytes) >>>>>> return ret; >>>>>> } >>>>>> >>>>>> +static int fstat_9pfs(struct file *file, struct stat *buf) >>>>>> +{ >>>>>> + struct file_9pfs *f9pfs = file->filedata; >>>>>> + struct p9_stat stat; >>>>>> + int ret; >>>>>> + >>>>>> + ret = p9_stat(f9pfs->dev, f9pfs->fid, &stat); >>>>>> + if ( ret ) >>>>>> + { >>>>>> + errno = EIO; >>>>>> + return -1; >>>>>> + } >>>>>> + >>>>>> + buf->st_mode = (stat.qid[0] == QID_TYPE_DIR) ? S_IFDIR : S_IFREG; >>>>>> + buf->st_mode |= stat.mode & 0777; >>>>>> + buf->st_atime = stat.atime; >>>>>> + buf->st_mtime = stat.mtime; >>>>> >>>>> Should we perhaps also fill st_ctime? Leaving it at 0 could surprise >>>>> other software layers. >>>> >>>> I can set it to the same value as st_mtime. >>> >>> Maybe the smaller of atime and mtime? >> >> That'd be better, yes. > > According to the references I could find ctime is changed whenever > either file contents OR file status (uid, gid, permissions) are > modified. So using the same value as mtime seems appropriate. Hmm, yes, one always learns something new. Having come from the DOS/Windows world originally, 'c' in the name to me only ever could stand for "create". When really, as you say, it's "change". I'm sorry for introducing confusion here. Jan
diff --git a/9pfront.c b/9pfront.c index 1741d600..7257a07e 100644 --- a/9pfront.c +++ b/9pfront.c @@ -85,6 +85,8 @@ struct file_9pfs { #define P9_QID_SIZE 13 +#define QID_TYPE_DIR 0x80 /* Applies to qid[0]. */ + struct p9_header { uint32_t size; uint8_t cmd; @@ -950,6 +952,32 @@ static int write_9pfs(struct file *file, const void *buf, size_t nbytes) return ret; } +static int fstat_9pfs(struct file *file, struct stat *buf) +{ + struct file_9pfs *f9pfs = file->filedata; + struct p9_stat stat; + int ret; + + ret = p9_stat(f9pfs->dev, f9pfs->fid, &stat); + if ( ret ) + { + errno = EIO; + return -1; + } + + buf->st_mode = (stat.qid[0] == QID_TYPE_DIR) ? S_IFDIR : S_IFREG; + buf->st_mode |= stat.mode & 0777; + buf->st_atime = stat.atime; + buf->st_mtime = stat.mtime; + buf->st_size = stat.length; + buf->st_uid = stat.n_uid; + buf->st_gid = stat.n_gid; + + free_stat(&stat); + + return 0; +} + static int close_9pfs(struct file *file) { struct file_9pfs *f9pfs = file->filedata; @@ -1296,6 +1324,7 @@ static const struct file_ops ops_9pfs = { .read = read_9pfs, .write = write_9pfs, .close = close_9pfs, + .fstat = fstat_9pfs, }; __attribute__((constructor))