[PATCH] Btrfs: clear compress-force when remounting with compress option

[PATCH] Btrfs: clear compress-force when remounting with compress option

[Wang Shilong]

Steps to reproduce:
 # mkfs.btrfs -f /dev/sdb
 # mount /dev/sdb /mnt -o compress-force=lzo
 # mount /dev/sdb /mnt -o remount,compress=zlib
 # cat /proc/mounts

Remounting from compress-force to compress could not clear compress-force
option. The problem is there is no way for users to clear compress-force
option separately.

Fix this problem by clearing @FORCE_COMPRESS flag when remounting to
compress=xxx.

Suggested-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com>
Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 fs/btrfs/super.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 4662d92..c3bb35f 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -513,6 +513,14 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
 					btrfs_info(root->fs_info,
 						   "btrfs: use %s compression",
 						   compress_type);
+				/*
+				 * If we remount from compress-force=xxx to
+				 * compress=xxx, we need clear FORCE_COMPRESS
+				 * flag, otherwise, there is no way for users
+				 * to disable forcible compression separately.
+				 */
+				btrfs_clear_and_info(root, FORCE_COMPRESS,
+						     "compress is not forced anymore");
 			}
 			break;
 		case Opt_ssd:
-- 
1.9.0

Re: [PATCH] Btrfs: clear compress-force when remounting with compress option

[David Sterba]

On Fri, May 23, 2014 at 11:36:52AM +0800, Wang Shilong wrote:
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -513,6 +513,14 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
>  					btrfs_info(root->fs_info,
>  						   "btrfs: use %s compression",
>  						   compress_type);
> +				/*
> +				 * If we remount from compress-force=xxx to
> +				 * compress=xxx, we need clear FORCE_COMPRESS
> +				 * flag, otherwise, there is no way for users
> +				 * to disable forcible compression separately.
> +				 */
> +				btrfs_clear_and_info(root, FORCE_COMPRESS,
> +						     "compress is not forced anymore");

This adds another message and does not make sense if it's not during
remount.

The message "btrfs: use %s compression" could be printed
unconditionally, we want to see that the compression is requested via
mount option.

Otherwise, the variable 'compress' does nothing useful and can be
removed as well.


>  			}
>  			break;
>  		case Opt_ssd:

Re: [PATCH] Btrfs: clear compress-force when remounting with compress option

[Wang Shilong]

Hi David,

On 05/28/2014 12:22 AM, David Sterba wrote:
> On Fri, May 23, 2014 at 11:36:52AM +0800, Wang Shilong wrote:
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -513,6 +513,14 @@ int btrfs_parse_options(struct btrfs_root *root, char *options)
>>   					btrfs_info(root->fs_info,
>>   						   "btrfs: use %s compression",
>>   						   compress_type);
>> +				/*
>> +				 * If we remount from compress-force=xxx to
>> +				 * compress=xxx, we need clear FORCE_COMPRESS
>> +				 * flag, otherwise, there is no way for users
>> +				 * to disable forcible compression separately.
>> +				 */
>> +				btrfs_clear_and_info(root, FORCE_COMPRESS,
>> +						     "compress is not forced anymore");
> This adds another message and does not make sense if it's not during
> remount.
The message is output in the following cases:

case1:
    # mount -o compress-force=lzo /dev/sdb /mnt
    # mount -o remount,compress=lzo  /dev/sdb /mnt

case2:
    # mount -o compress-force=lzo /dev/sdb /mnt
    # mount -o remount,compress-force=no /dev/sdb /mnt

Message won't be printed except remounting, any other case
that i was missing?

> The message "btrfs: use %s compression" could be printed
> unconditionally, we want to see that the compression is requested via
> mount option.
>
> Otherwise, the variable 'compress' does nothing useful and can be
> removed as well.
See many other mount options, if we change it, it will call
btrfs_clear_and_info(....)...

I don't have any taste that whether we need add or remove
this message, if you dosen't like this,  i could remove it.

Thanks,
Wang
>>   			}
>>   			break;
>>   		case Opt_ssd:
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> .
>