Re: [PATCH 2/2] btrfs: fix vanished compression property after failed set

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 13, 2019 at 06:49:27PM +0800, Anand Jain wrote:
> 
> 
> On 3/13/19 6:33 PM, Anand Jain wrote:
> > 
> > 
> > 
> > On 3/13/19 1:36 PM, Anand Jain wrote:
> >> The compression property resets to NULL, instead of the old value if we
> >> fail to set the new compression parameter.
> >>
> >> btrfs prop get /btrfs compression
> >>    compression=lzo
> >> btrfs prop set /btrfs compression zli
> >>    ERROR: failed to set compression for /btrfs: Invalid argument
> >> btrfs prop get /btrfs compression
> >>
> >> This is because the compression property ->validate() is successful for
> >> 'zli' as the strncmp() used the len passed from the userland.
> >>
> >> Fix it by using the expected string length in strncmp().
> >>
> >> Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx>
> >> ---
> >>   fs/btrfs/props.c | 6 +++---
> >>   1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
> >> index ef6502a94712..7aa362c2fbcf 100644
> >> --- a/fs/btrfs/props.c
> >> +++ b/fs/btrfs/props.c
> >> @@ -277,11 +277,11 @@ static int prop_compression_validate(struct 
> >> inode *inode, const char *value,
> >>       if (!value)
> >>           return 0;
> >> -    if (!strncmp("lzo", value, len))
> >> +    if (!strncmp("lzo", value, 3))
> >>           return 0;
> >> -    else if (!strncmp("zlib", value, len))
> >> +    else if (!strncmp("zlib", value, 4))
> >>           return 0;
> >> -    else if (!strncmp("zstd", value, len))
> >> +    else if (!strncmp("zstd", value, 4))
> >>           return 0;
> >>
> > 
> > 
> >    Nack.
> > Now some junk value after expected string is not an error.
> > such as..
> > 
> > btrfs prop set /btrfs compression lzo110
> 
>   mount(8) compression and the property compression parameter have
>   diverged, the compression levels are set able only from
>   mount(8) option? We should rather make it consistent?

Yes the properties should be able to get to the same result as the mount
option, but this needs more changes as the level is not extracted from
the property and passed to the compression (in compress_file_range).



[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux