[PATCH] btrfs-progs: convert, warn if converting a fs which won't mount

[PATCH] btrfs-progs: convert, warn if converting a fs which won't mount

[Anand Jain]

On aarch64 with pagesize 64k, btrfs-convert of ext4 is successful,
but it won't mount because we don't yet support subpage blocksize/
sectorsize.

 BTRFS error (device vda): sectorsize 4096 not supported yet, only support 65536

So in this case during convert provide a warning and a 10s delay to
terminate the command.

For example:

WARNING: Blocksize 4096 is not equal to the pagesize 65536,
         converted filesystem won't mount on this system.
         The operation will start in 10 seconds. Use Ctrl-c to stop it.
10 9 8 7 6 5 4^C

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 convert/main.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/convert/main.c b/convert/main.c
index a04ec7a36abf..f936ec37d30a 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -1140,6 +1140,21 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize,
 		error("block size is too small: %u < 4096", blocksize);
 		goto fail;
 	}
+	if (blocksize != getpagesize()) {
+		int delay = 10;
+
+		warning("Blocksize %u is not equal to the pagesize %u,\n\
+         converted filesystem won't mount on this system.\n\
+         The operation will start in %d seconds. Use Ctrl-C to stop it.",
+			blocksize, getpagesize(), delay);
+
+		while (delay) {
+			printf("%2d", delay--);
+			fflush(stdout);
+			sleep(1);
+		}
+	}
+
 	if (btrfs_check_nodesize(nodesize, blocksize, features))
 		goto fail;
 	fd = open(devname, O_RDWR);
-- 
1.8.3.1

Re: [PATCH] btrfs-progs: convert, warn if converting a fs which won't mount

[Nikolay Borisov]

On 28.02.20 г. 10:03 ч., Anand Jain wrote:
> On aarch64 with pagesize 64k, btrfs-convert of ext4 is successful,
> but it won't mount because we don't yet support subpage blocksize/
> sectorsize.
> 
>  BTRFS error (device vda): sectorsize 4096 not supported yet, only support 65536
> 
> So in this case during convert provide a warning and a 10s delay to
> terminate the command.
> 
> For example:
> 
> WARNING: Blocksize 4096 is not equal to the pagesize 65536,
>          converted filesystem won't mount on this system.
>          The operation will start in 10 seconds. Use Ctrl-c to stop it.
> 10 9 8 7 6 5 4^C

What's the point of the delay? Just refuse to start the operation and quit.

> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  convert/main.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/convert/main.c b/convert/main.c
> index a04ec7a36abf..f936ec37d30a 100644
> --- a/convert/main.c
> +++ b/convert/main.c
> @@ -1140,6 +1140,21 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize,
>  		error("block size is too small: %u < 4096", blocksize);
>  		goto fail;
>  	}
> +	if (blocksize != getpagesize()) {
> +		int delay = 10;
> +
> +		warning("Blocksize %u is not equal to the pagesize %u,\n\
> +         converted filesystem won't mount on this system.\n\
> +         The operation will start in %d seconds. Use Ctrl-C to stop it.",
> +			blocksize, getpagesize(), delay);
> +
> +		while (delay) {
> +			printf("%2d", delay--);
> +			fflush(stdout);
> +			sleep(1);
> +		}
> +	}
> +
>  	if (btrfs_check_nodesize(nodesize, blocksize, features))
>  		goto fail;
>  	fd = open(devname, O_RDWR);
> 

Re: [PATCH] btrfs-progs: convert, warn if converting a fs which won't mount

[Roman Mamedov]

On Fri, 28 Feb 2020 10:13:41 +0200
Nikolay Borisov <nborisov@suse.com> wrote:

> 
> 
> On 28.02.20 г. 10:03 ч., Anand Jain wrote:
> > On aarch64 with pagesize 64k, btrfs-convert of ext4 is successful,
> > but it won't mount because we don't yet support subpage blocksize/
> > sectorsize.
> > 
> >  BTRFS error (device vda): sectorsize 4096 not supported yet, only support 65536
> > 
> > So in this case during convert provide a warning and a 10s delay to
> > terminate the command.
> > 
> > For example:
> > 
> > WARNING: Blocksize 4096 is not equal to the pagesize 65536,
> >          converted filesystem won't mount on this system.
> >          The operation will start in 10 seconds. Use Ctrl-c to stop it.
> > 10 9 8 7 6 5 4^C
> 
> What's the point of the delay? Just refuse to start the operation and quit.

IMO there should be a way to proceed with convert, if the user knows what they
are doing; maybe refuse the operation, but provide an "-f" "--force" option to
proceed anyway?

As for these 10 second delays, they always seemed a bit odd and unusual among
Linux filesystem tools or Unix CLI software in general.

-- 
With respect,
Roman

Re: [PATCH] btrfs-progs: convert, warn if converting a fs which won't mount

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1865 bytes --]



On 2020/2/28 下午4:03, Anand Jain wrote:
> On aarch64 with pagesize 64k, btrfs-convert of ext4 is successful,
> but it won't mount because we don't yet support subpage blocksize/
> sectorsize.
> 
>  BTRFS error (device vda): sectorsize 4096 not supported yet, only support 65536
> 
> So in this case during convert provide a warning and a 10s delay to
> terminate the command.

This is no different than calling mkfs.btrfs -s 64k on x86 system.
And I see no warning from mkfs.btrfs.

Thus I don't see the point of only introducing such warning to
btrfs-convert.

Thanks,
Qu

> 
> For example:
> 
> WARNING: Blocksize 4096 is not equal to the pagesize 65536,
>          converted filesystem won't mount on this system.
>          The operation will start in 10 seconds. Use Ctrl-c to stop it.
> 10 9 8 7 6 5 4^C
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  convert/main.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/convert/main.c b/convert/main.c
> index a04ec7a36abf..f936ec37d30a 100644
> --- a/convert/main.c
> +++ b/convert/main.c
> @@ -1140,6 +1140,21 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize,
>  		error("block size is too small: %u < 4096", blocksize);
>  		goto fail;
>  	}
> +	if (blocksize != getpagesize()) {
> +		int delay = 10;
> +
> +		warning("Blocksize %u is not equal to the pagesize %u,\n\
> +         converted filesystem won't mount on this system.\n\
> +         The operation will start in %d seconds. Use Ctrl-C to stop it.",
> +			blocksize, getpagesize(), delay);
> +
> +		while (delay) {
> +			printf("%2d", delay--);
> +			fflush(stdout);
> +			sleep(1);
> +		}
> +	}
> +
>  	if (btrfs_check_nodesize(nodesize, blocksize, features))
>  		goto fail;
>  	fd = open(devname, O_RDWR);
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] btrfs-progs: convert, warn if converting a fs which won't mount

[Anand Jain]

On 2/28/20 4:27 PM, Qu Wenruo wrote:
> 
> 
> On 2020/2/28 下午4:03, Anand Jain wrote:
>> On aarch64 with pagesize 64k, btrfs-convert of ext4 is successful,
>> but it won't mount because we don't yet support subpage blocksize/
>> sectorsize.
>>
>>   BTRFS error (device vda): sectorsize 4096 not supported yet, only support 65536
>>
>> So in this case during convert provide a warning and a 10s delay to
>> terminate the command.
> 
> This is no different than calling mkfs.btrfs -s 64k on x86 system.
> And I see no warning from mkfs.btrfs.
> 
> Thus I don't see the point of only introducing such warning to
> btrfs-convert.
> 

I have equal weight-age on the choices if blocksize != pagesize viz..
   delay and warn (this patch)
   quit (Nikolay).
   keep it as it is without warning (Qu).

  Here we are dealing with already user data. Should it be different
  from mkfs?
  Quit is fine, but convert tool should it be system neutral?

  I am not sure.

  David, any idea?

Thanks, Anand

> Thanks,
> Qu
> 
>>
>> For example:
>>
>> WARNING: Blocksize 4096 is not equal to the pagesize 65536,
>>           converted filesystem won't mount on this system.
>>           The operation will start in 10 seconds. Use Ctrl-c to stop it.
>> 10 9 8 7 6 5 4^C
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   convert/main.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/convert/main.c b/convert/main.c
>> index a04ec7a36abf..f936ec37d30a 100644
>> --- a/convert/main.c
>> +++ b/convert/main.c
>> @@ -1140,6 +1140,21 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize,
>>   		error("block size is too small: %u < 4096", blocksize);
>>   		goto fail;
>>   	}
>> +	if (blocksize != getpagesize()) {
>> +		int delay = 10;
>> +
>> +		warning("Blocksize %u is not equal to the pagesize %u,\n\
>> +         converted filesystem won't mount on this system.\n\
>> +         The operation will start in %d seconds. Use Ctrl-C to stop it.",
>> +			blocksize, getpagesize(), delay);
>> +
>> +		while (delay) {
>> +			printf("%2d", delay--);
>> +			fflush(stdout);
>> +			sleep(1);
>> +		}
>> +	}
>> +
>>   	if (btrfs_check_nodesize(nodesize, blocksize, features))
>>   		goto fail;
>>   	fd = open(devname, O_RDWR);
>>
> 

Re: [PATCH] btrfs-progs: convert, warn if converting a fs which won't mount

[Qu Wenruo]

On 2020/2/28 下午5:06, Anand Jain wrote:
>
>
> On 2/28/20 4:27 PM, Qu Wenruo wrote:
>>
>>
>> On 2020/2/28 下午4:03, Anand Jain wrote:
>>> On aarch64 with pagesize 64k, btrfs-convert of ext4 is successful,
>>> but it won't mount because we don't yet support subpage blocksize/
>>> sectorsize.
>>>
>>>   BTRFS error (device vda): sectorsize 4096 not supported yet, only
>>> support 65536
>>>
>>> So in this case during convert provide a warning and a 10s delay to
>>> terminate the command.
>>
>> This is no different than calling mkfs.btrfs -s 64k on x86 system.
>> And I see no warning from mkfs.btrfs.
>>
>> Thus I don't see the point of only introducing such warning to
>> btrfs-convert.
>>
>
> I have equal weight-age on the choices if blocksize != pagesize viz..
>   delay and warn (this patch)
>   quit (Nikolay).
>   keep it as it is without warning (Qu).
>
>  Here we are dealing with already user data. Should it be different
>  from mkfs?
>  Quit is fine, but convert tool should it be system neutral?

If we can't mount, btrfs-convert can easily do a revert without anything
touched for the original fs.

And, convert tool is not that system neutral.
It needs the source fs has a sector size that matches btrfs.

E.g. ext2 with 512/1K/2K sector size can't be supported by btrfs-convert
at all.

Thanks,
Qu

>
>  I am not sure.
>
>  David, any idea?
>
> Thanks, Anand
>
>> Thanks,
>> Qu
>>
>>>
>>> For example:
>>>
>>> WARNING: Blocksize 4096 is not equal to the pagesize 65536,
>>>           converted filesystem won't mount on this system.
>>>           The operation will start in 10 seconds. Use Ctrl-c to stop it.
>>> 10 9 8 7 6 5 4^C
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>> ---
>>>   convert/main.c | 15 +++++++++++++++
>>>   1 file changed, 15 insertions(+)
>>>
>>> diff --git a/convert/main.c b/convert/main.c
>>> index a04ec7a36abf..f936ec37d30a 100644
>>> --- a/convert/main.c
>>> +++ b/convert/main.c
>>> @@ -1140,6 +1140,21 @@ static int do_convert(const char *devname, u32
>>> convert_flags, u32 nodesize,
>>>           error("block size is too small: %u < 4096", blocksize);
>>>           goto fail;
>>>       }
>>> +    if (blocksize != getpagesize()) {
>>> +        int delay = 10;
>>> +
>>> +        warning("Blocksize %u is not equal to the pagesize %u,\n\
>>> +         converted filesystem won't mount on this system.\n\
>>> +         The operation will start in %d seconds. Use Ctrl-C to stop
>>> it.",
>>> +            blocksize, getpagesize(), delay);
>>> +
>>> +        while (delay) {
>>> +            printf("%2d", delay--);
>>> +            fflush(stdout);
>>> +            sleep(1);
>>> +        }
>>> +    }
>>> +
>>>       if (btrfs_check_nodesize(nodesize, blocksize, features))
>>>           goto fail;
>>>       fd = open(devname, O_RDWR);
>>>
>>

Re: [PATCH] btrfs-progs: convert, warn if converting a fs which won't mount

[David Sterba]

On Fri, Feb 28, 2020 at 05:06:52PM +0800, Anand Jain wrote:
> On 2/28/20 4:27 PM, Qu Wenruo wrote:
> > On 2020/2/28 下午4:03, Anand Jain wrote:
> >> On aarch64 with pagesize 64k, btrfs-convert of ext4 is successful,
> >> but it won't mount because we don't yet support subpage blocksize/
> >> sectorsize.
> >>
> >>   BTRFS error (device vda): sectorsize 4096 not supported yet, only support 65536
> >>
> >> So in this case during convert provide a warning and a 10s delay to
> >> terminate the command.
> > 
> > This is no different than calling mkfs.btrfs -s 64k on x86 system.
> > And I see no warning from mkfs.btrfs.
> > 
> > Thus I don't see the point of only introducing such warning to
> > btrfs-convert.
> > 
> 
> I have equal weight-age on the choices if blocksize != pagesize viz..
>    delay and warn (this patch)
>    quit (Nikolay).
>    keep it as it is without warning (Qu).
> 
>   Here we are dealing with already user data. Should it be different
>   from mkfs?
>   Quit is fine, but convert tool should it be system neutral?

The delays should be used in exceptional cases, now we have it for check
--repair and for unfiltered balance. Both on user request because
expecting users to know everything in advance what the commands do has
shown to be too optimistic.

Refusing to allow the conversion does not make much sense for usability,
mising the unmounted and mounted constraints.

A warning might be in place but there's nothing wrong to let the user do
the conversion.

I've tried mkfs.ext4 with 64k block size and it warns and in the
interactive session wants to confirm that by the user:

  $ mkfs.ext4 -b 64k img
  Warning: blocksize 65536 not usable on most systems.
  mke2fs 1.45.5 (07-Jan-2020)
  img contains a ext4 file system
	  created on Tue Mar  3 18:41:46 2020
  Proceed anyway? (y,N) y
  mkfs.ext4: 65536-byte blocks too big for system (max 4096)
  Proceed anyway? (y,N) y
  Warning: 65536-byte blocks too big for system (max 4096), forced to continue
  Creating filesystem with 32768 64k blocks and 32768 inodes

  Allocating group tables: done
  Writing inode tables: done
  Creating journal (4096 blocks): done
  Writing superblocks and filesystem accounting information: done

Re: [PATCH] btrfs-progs: convert, warn if converting a fs which won't mount

[Anand Jain]

On 3/4/20 1:44 AM, David Sterba wrote:
> On Fri, Feb 28, 2020 at 05:06:52PM +0800, Anand Jain wrote:
>> On 2/28/20 4:27 PM, Qu Wenruo wrote:
>>> On 2020/2/28 下午4:03, Anand Jain wrote:
>>>> On aarch64 with pagesize 64k, btrfs-convert of ext4 is successful,
>>>> but it won't mount because we don't yet support subpage blocksize/
>>>> sectorsize.
>>>>
>>>>    BTRFS error (device vda): sectorsize 4096 not supported yet, only support 65536
>>>>
>>>> So in this case during convert provide a warning and a 10s delay to
>>>> terminate the command.
>>>
>>> This is no different than calling mkfs.btrfs -s 64k on x86 system.
>>> And I see no warning from mkfs.btrfs.
>>>
>>> Thus I don't see the point of only introducing such warning to
>>> btrfs-convert.
>>>
>>
>> I have equal weight-age on the choices if blocksize != pagesize viz..
>>     delay and warn (this patch)
>>     quit (Nikolay).
>>     keep it as it is without warning (Qu).
>>
>>    Here we are dealing with already user data. Should it be different
>>    from mkfs?
>>    Quit is fine, but convert tool should it be system neutral?
> 
> The delays should be used in exceptional cases, now we have it for check
> --repair and for unfiltered balance. Both on user request because
> expecting users to know everything in advance what the commands do has
> shown to be too optimistic.
> 
> Refusing to allow the conversion does not make much sense for usability,
> mising the unmounted and mounted constraints.
> 
> A warning might be in place but there's nothing wrong to let the user do
> the conversion.
> 
> I've tried mkfs.ext4 with 64k block size and it warns and in the
> interactive session wants to confirm that by the user:
> 
>    $ mkfs.ext4 -b 64k img
>    Warning: blocksize 65536 not usable on most systems.
>    mke2fs 1.45.5 (07-Jan-2020)
>    img contains a ext4 file system
> 	  created on Tue Mar  3 18:41:46 2020
>    Proceed anyway? (y,N) y
>    mkfs.ext4: 65536-byte blocks too big for system (max 4096)
>    Proceed anyway? (y,N) y
>    Warning: 65536-byte blocks too big for system (max 4096), forced to continue
>    Creating filesystem with 32768 64k blocks and 32768 inodes
> 
>    Allocating group tables: done
>    Writing inode tables: done
>    Creating journal (4096 blocks): done
>    Writing superblocks and filesystem accounting information: done
> 

Just warn is reasonable. But I don't think you meant to introduce
interactive part similar to mkfs.ext4 in btrfs-convert? we don't have it
anywhere in btrfs-progs. As the btrfs-convert is not an exceptional case
(though it deals with the user data) removing the delay makes sense,
mover over the conversion and the rollback does not take much time in
general.

Thanks, Anand

Re: [PATCH] btrfs-progs: convert, warn if converting a fs which won't mount

[Qu Wenruo]

On 2020/3/4 上午10:14, Anand Jain wrote:
>
>
> On 3/4/20 1:44 AM, David Sterba wrote:
>> On Fri, Feb 28, 2020 at 05:06:52PM +0800, Anand Jain wrote:
>>> On 2/28/20 4:27 PM, Qu Wenruo wrote:
>>>> On 2020/2/28 下午4:03, Anand Jain wrote:
>>>>> On aarch64 with pagesize 64k, btrfs-convert of ext4 is successful,
>>>>> but it won't mount because we don't yet support subpage blocksize/
>>>>> sectorsize.
>>>>>
>>>>>    BTRFS error (device vda): sectorsize 4096 not supported yet,
>>>>> only support 65536
>>>>>
>>>>> So in this case during convert provide a warning and a 10s delay to
>>>>> terminate the command.
>>>>
>>>> This is no different than calling mkfs.btrfs -s 64k on x86 system.
>>>> And I see no warning from mkfs.btrfs.
>>>>
>>>> Thus I don't see the point of only introducing such warning to
>>>> btrfs-convert.
>>>>
>>>
>>> I have equal weight-age on the choices if blocksize != pagesize viz..
>>>     delay and warn (this patch)
>>>     quit (Nikolay).
>>>     keep it as it is without warning (Qu).
>>>
>>>    Here we are dealing with already user data. Should it be different
>>>    from mkfs?
>>>    Quit is fine, but convert tool should it be system neutral?
>>
>> The delays should be used in exceptional cases, now we have it for check
>> --repair and for unfiltered balance. Both on user request because
>> expecting users to know everything in advance what the commands do has
>> shown to be too optimistic.
>>
>> Refusing to allow the conversion does not make much sense for usability,
>> mising the unmounted and mounted constraints.
>>
>> A warning might be in place but there's nothing wrong to let the user do
>> the conversion.
>>
>> I've tried mkfs.ext4 with 64k block size and it warns and in the
>> interactive session wants to confirm that by the user:
>>
>>    $ mkfs.ext4 -b 64k img
>>    Warning: blocksize 65536 not usable on most systems.
>>    mke2fs 1.45.5 (07-Jan-2020)
>>    img contains a ext4 file system
>>       created on Tue Mar  3 18:41:46 2020
>>    Proceed anyway? (y,N) y
>>    mkfs.ext4: 65536-byte blocks too big for system (max 4096)
>>    Proceed anyway? (y,N) y
>>    Warning: 65536-byte blocks too big for system (max 4096), forced to
>> continue
>>    Creating filesystem with 32768 64k blocks and 32768 inodes
>>
>>    Allocating group tables: done
>>    Writing inode tables: done
>>    Creating journal (4096 blocks): done
>>    Writing superblocks and filesystem accounting information: done
>>
>
> Just warn is reasonable. But I don't think you meant to introduce
> interactive part similar to mkfs.ext4 in btrfs-convert? we don't have it
> anywhere in btrfs-progs. As the btrfs-convert is not an exceptional case
> (though it deals with the user data) removing the delay makes sense,
> mover over the conversion and the rollback does not take much time in
> general.
>
> Thanks, Anand

+1 for warning only, especially when btrfs-convert is revertable, unlike
mkfs.

Thanks,
Qu

Re: [PATCH] btrfs-progs: convert, warn if converting a fs which won't mount

[David Sterba]

On Wed, Mar 04, 2020 at 10:14:20AM +0800, Anand Jain wrote:
> > the conversion.
> > 
> > I've tried mkfs.ext4 with 64k block size and it warns and in the
> > interactive session wants to confirm that by the user:
> > 
> >    $ mkfs.ext4 -b 64k img
> >    Warning: blocksize 65536 not usable on most systems.
> >    mke2fs 1.45.5 (07-Jan-2020)
> >    img contains a ext4 file system
> > 	  created on Tue Mar  3 18:41:46 2020
> >    Proceed anyway? (y,N) y
> >    mkfs.ext4: 65536-byte blocks too big for system (max 4096)
> >    Proceed anyway? (y,N) y
> >    Warning: 65536-byte blocks too big for system (max 4096), forced to continue
> >    Creating filesystem with 32768 64k blocks and 32768 inodes
> > 
> >    Allocating group tables: done
> >    Writing inode tables: done
> >    Creating journal (4096 blocks): done
> >    Writing superblocks and filesystem accounting information: done
> > 
> 
> Just warn is reasonable. But I don't think you meant to introduce
> interactive part similar to mkfs.ext4 in btrfs-convert?

No I haven't meant that. So let's go with the warning.

Re: [PATCH] btrfs-progs: convert, warn if converting a fs which won't mount

[Anand Jain]

On 3/4/20 9:32 PM, David Sterba wrote:
> On Wed, Mar 04, 2020 at 10:14:20AM +0800, Anand Jain wrote:
>>> the conversion.
>>>
>>> I've tried mkfs.ext4 with 64k block size and it warns and in the
>>> interactive session wants to confirm that by the user:
>>>
>>>     $ mkfs.ext4 -b 64k img
>>>     Warning: blocksize 65536 not usable on most systems.
>>>     mke2fs 1.45.5 (07-Jan-2020)
>>>     img contains a ext4 file system
>>> 	  created on Tue Mar  3 18:41:46 2020
>>>     Proceed anyway? (y,N) y
>>>     mkfs.ext4: 65536-byte blocks too big for system (max 4096)
>>>     Proceed anyway? (y,N) y
>>>     Warning: 65536-byte blocks too big for system (max 4096), forced to continue
>>>     Creating filesystem with 32768 64k blocks and 32768 inodes
>>>
>>>     Allocating group tables: done
>>>     Writing inode tables: done
>>>     Creating journal (4096 blocks): done
>>>     Writing superblocks and filesystem accounting information: done
>>>
>>
>> Just warn is reasonable. But I don't think you meant to introduce
>> interactive part similar to mkfs.ext4 in btrfs-convert?
> 
> No I haven't meant that. So let's go with the warning.
> 
Ok. Patch v2 is in ML.

Thanks, Anand