Re: [MINI-OS PATCH v2 1/2] 9pfs: add fstat file operation hook

[Jan Beulich <jbeulich@xxxxxxxx>]

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@xxxxxxxx>
>>>>>> Reviewed-by: Jason Andryuk <jason.andryuk@xxxxxxx>
>>>>>> ---
>>>>>> 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