| Message ID | a7debcd84dafac8b0d0f67da6b4e410ea346bffb.1605007036.git.naohiro.aota@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: zoned block device support | expand |
On 10/11/20 7:26 pm, Naohiro Aota wrote: > NODATACOW implies overwriting the file data on a device, which is > impossible in sequential required zones. Disable NODATACOW globally with > mount option and per-file NODATACOW attribute by masking FS_NOCOW_FL. > > Reviewed-by: Josef Bacik <josef@toxicpanda.com> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com> Looks good. Reviewed-by: Anand Jain <anand.jain@oracle.com> A nit below. > --- > fs/btrfs/ioctl.c | 13 +++++++++++++ > fs/btrfs/zoned.c | 5 +++++ > 2 files changed, 18 insertions(+) > > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c > index ab408a23ba32..d13b522e7bb2 100644 > --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -193,6 +193,15 @@ static int check_fsflags(unsigned int old_flags, unsigned int flags) > return 0; > } > > +static int check_fsflags_compatible(struct btrfs_fs_info *fs_info, > + unsigned int flags) > +{ > + if (btrfs_is_zoned(fs_info) && (flags & FS_NOCOW_FL)) > + return -EPERM; nit: Should it be -EINVAL instead? I am not sure. May be David can fix while integrating. Thanks. > + > + return 0; > +} > + > static int btrfs_ioctl_setflags(struct file *file, void __user *arg) > { > struct inode *inode = file_inode(file); > @@ -230,6 +239,10 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg) > if (ret) > goto out_unlock; > > + ret = check_fsflags_compatible(fs_info, fsflags); > + if (ret) > + goto out_unlock; > + > binode_flags = binode->flags; > if (fsflags & FS_SYNC_FL) > binode_flags |= BTRFS_INODE_SYNC; > diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c > index d6b8165e2c91..bd153932606e 100644 > --- a/fs/btrfs/zoned.c > +++ b/fs/btrfs/zoned.c > @@ -290,5 +290,10 @@ int btrfs_check_mountopts_zoned(struct btrfs_fs_info *info) > return -EINVAL; > } > > + if (btrfs_test_opt(info, NODATACOW)) { > + btrfs_err(info, "zoned: NODATACOW not supported"); > + return -EINVAL; > + } > + > return 0; > } >
On Fri, Nov 20, 2020 at 12:17:21PM +0800, Anand Jain wrote: > On 10/11/20 7:26 pm, Naohiro Aota wrote: > > + unsigned int flags) > > +{ > > + if (btrfs_is_zoned(fs_info) && (flags & FS_NOCOW_FL)) > > > > + return -EPERM; > > nit: > Should it be -EINVAL instead? I am not sure. May be David can fix > while integrating. IIRC we've discussed that in some previous iteration. EPERM should be interpreted as that it's not permitted right now, but otherwise it is a valid operation/flag. The constraint is the zoned device. As an example: deleting default subvolume is not permitted (EPERM), but otherwise subvolume deletion is a valid operation. So, EINVAL is for invalid combination of parameters or a request for something that does not make sense at all.
On 24/11/20 1:21 am, David Sterba wrote: > On Fri, Nov 20, 2020 at 12:17:21PM +0800, Anand Jain wrote: >> On 10/11/20 7:26 pm, Naohiro Aota wrote: >>> + unsigned int flags) >>> +{ >>> + if (btrfs_is_zoned(fs_info) && (flags & FS_NOCOW_FL)) >> >> >>> + return -EPERM; >> >> nit: >> Should it be -EINVAL instead? I am not sure. May be David can fix >> while integrating. > > IIRC we've discussed that in some previous iteration. EPERM should be > interpreted as that it's not permitted right now, but otherwise it is a > valid operation/flag. The constraint is the zoned device. > > As an example: deleting default subvolume is not permitted (EPERM), but > otherwise subvolume deletion is a valid operation. > > So, EINVAL is for invalid combination of parameters or a request for > something that does not make sense at all. > Ok. Thanks.
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index ab408a23ba32..d13b522e7bb2 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -193,6 +193,15 @@ static int check_fsflags(unsigned int old_flags, unsigned int flags) return 0; } +static int check_fsflags_compatible(struct btrfs_fs_info *fs_info, + unsigned int flags) +{ + if (btrfs_is_zoned(fs_info) && (flags & FS_NOCOW_FL)) + return -EPERM; + + return 0; +} + static int btrfs_ioctl_setflags(struct file *file, void __user *arg) { struct inode *inode = file_inode(file); @@ -230,6 +239,10 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg) if (ret) goto out_unlock; + ret = check_fsflags_compatible(fs_info, fsflags); + if (ret) + goto out_unlock; + binode_flags = binode->flags; if (fsflags & FS_SYNC_FL) binode_flags |= BTRFS_INODE_SYNC; diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c index d6b8165e2c91..bd153932606e 100644 --- a/fs/btrfs/zoned.c +++ b/fs/btrfs/zoned.c @@ -290,5 +290,10 @@ int btrfs_check_mountopts_zoned(struct btrfs_fs_info *info) return -EINVAL; } + if (btrfs_test_opt(info, NODATACOW)) { + btrfs_err(info, "zoned: NODATACOW not supported"); + return -EINVAL; + } + return 0; }