On Thu, Jan 5, 2017 at 1:29 AM, J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote:
> On Sat, Dec 31, 2016 at 09:18:08PM +0800, Kinglong Mee wrote:
>> Commit fae5096ad217
>> "nfsd: assume writeable exportabled filesystems have f_sync"
>> have remove the checking of f_sync.
>>
>> Christoph Hellwig suggests,
>> "Warn and refuse the writable export."
>>
>> I think just covert to a readonly export for !fsync filesystem,
>> also, for a readonly filesystem is reasonable.
>
> Hmmm. It's not something we've done before. Off hand, I can't see why
> it would cause a problem, but I'm not convinced yet.
>
> Could you add to the changelog a description of the use case you gave
> Christoph in your defense of this idea?
Okay, I will give more description about the patch include that.
>
> Also:
>
>> Signed-off-by: Kinglong Mee <kinglongmee@xxxxxxxxx>
>> ---
>> fs/nfsd/export.c | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
>> index 43e109c..3ec3b6b 100644
>> --- a/fs/nfsd/export.c
>> +++ b/fs/nfsd/export.c
>> @@ -358,6 +358,18 @@ static int check_export(struct inode *inode, int *flags, unsigned char *uuid)
>> if (*flags & NFSEXP_V4ROOT)
>> *flags |= NFSEXP_READONLY;
>>
>> + /*
>> + * Convert to a readonly export for that,
>> + * 1. not supported fsync filesystem,
>> + * 2. readonly filesystem.
>> + */
>> + if ((!inode->i_fop->fsync || IS_RDONLY(inode))
>> + && !(*flags & NFSEXP_READONLY)) {
>> + dprintk("exp_export: Only support readonly export "
>> + "for fsync unsupported or readonly filesystem.\n");
>
> Something like this might be more helpful:
>
> "Filesystem %s: exporting read-only\n", IS_RDONLY(inode) ?
> "is read-only" : "has no fsync method"
>
> Also if we passed the dentry to check_export, could we do something
> like:
>
> "%s %s: exporting read-only\n", d_path(dentry,...), IS_RDONLY...
>
> here and in the other warnings?
A kstrdup from svc_export_parse() 's string path parsing is simplify,
also, I will show in the next patch.
thanks,
Kinglong Mee
>
> --b.
>
>> + *flags |= NFSEXP_READONLY;
>> + }
>> +
>> /* There are two requirements on a filesystem to be exportable.
>> * 1: We must be able to identify the filesystem from a number.
>> * either a device number (so FS_REQUIRES_DEV needed)
>> --
>> 2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
