Re: [PATCH] btrfs: add missing check for nocow and compression inode flags

David Sterba
2020-07-10T13:28:37Z

On Fri, Jul 10, 2020 at 10:17:55AM +0000, Johannes Thumshirn wrote:
> On 10/07/2020 12:06, David Sterba wrote:
> > +static int check_fsflags(unsigned int old_flags, unsigned int flags)
> > {
> > if (flags & ~(FS_IMMUTABLE_FL | FS_APPEND_FL | \
> > FS_NOATIME_FL | FS_NODUMP_FL | \
> > @@ -174,9 +177,19 @@ static int check_fsflags(unsigned int flags)
> > FS_NOCOW_FL))
> > return -EOPNOTSUPP;
> >
> > + /* COMPR and NOCOMP on new/old are valid */
> > if ((flags & FS_NOCOMP_FL) && (flags & FS_COMPR_FL))
> > return -EINVAL;
> >
> > + if ((flags & FS_COMPR_FL) && (flags & FS_NOCOW_FL))
> > + return -EINVAL;
> > +
> > + /* NOCOW and compression options are mutually exclusive */
> > + if ((old_flags & FS_NOCOW_FL) && (flags & (FS_COMPR_FL | FS_NOCOMP_FL)))
> > + return -EINVAL;
> > + if ((flags & FS_NOCOW_FL) && (old_flags & (FS_COMPR_FL | FS_NOCOMP_FL)))
> > + return -EINVAL;
> > +
>
>
> If we'd pass in fs_info to check_fsflags() we could also validate against mount options
> which are incompatible with inode flags. Like -o nodatacow and FS_COMPR_FL or
> -o auth_key and FS_NOCOW_FL.

Same question was asked on IRC too, mount options are independent and
take lower precedence than the inode attributes. A scenario where user
wants to set a nodatacow attribute when the filesystem is mounted with
compress= is valid and can be quite common.