[PATCH v2] common/rc: don't clear superblock for zoned scratch pools

[PATCH v2] common/rc: don't clear superblock for zoned scratch pools

[Johannes Thumshirn]

_require_scratch_dev_pool() zeros the first 100 sectors of each device to
clear eventual remains of older filesystems.

On zoned devices this won't work as a plain dd will end up creating
unaligned write errors failing all subsequent actions on the device.

For zoned devices it is enough to simply reset the first two zones of the
device to achieve the same result.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 common/rc | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/common/rc b/common/rc
index 654730b21ead..739abbdbfc8c 100644
--- a/common/rc
+++ b/common/rc
@@ -3459,9 +3459,15 @@ _require_scratch_dev_pool()
 		            exit 1
 		        fi
 		fi
-		# to help better debug when something fails, we remove
-		# traces of previous btrfs FS on the dev.
-		dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
+		# To help better debug when something fails, we remove
+		# traces of previous btrfs FS on the dev. For zoned devices we
+		# can't use dd as it'll lead to unaligned writes so simply
+		# reset the first two zones.
+		if [ "`_zone_type "$i"`" = "none" ]; then
+			dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
+		else
+			blkzone reset -c 2 $i
+		fi
 	done
 }
 
-- 
2.39.1

Re: [PATCH v2] common/rc: don't clear superblock for zoned scratch pools

[David Disseldorp]

Hi Johannes,

On Mon, 27 Feb 2023 00:27:17 -0800, Johannes Thumshirn wrote:

> _require_scratch_dev_pool() zeros the first 100 sectors of each device to
> clear eventual remains of older filesystems.
> 
> On zoned devices this won't work as a plain dd will end up creating
> unaligned write errors failing all subsequent actions on the device.
> 
> For zoned devices it is enough to simply reset the first two zones of the
> device to achieve the same result.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  common/rc | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 654730b21ead..739abbdbfc8c 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3459,9 +3459,15 @@ _require_scratch_dev_pool()
>  		            exit 1
>  		        fi
>  		fi
> -		# to help better debug when something fails, we remove
> -		# traces of previous btrfs FS on the dev.
> -		dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
> +		# To help better debug when something fails, we remove
> +		# traces of previous btrfs FS on the dev. For zoned devices we
> +		# can't use dd as it'll lead to unaligned writes so simply
> +		# reset the first two zones.
> +		if [ "`_zone_type "$i"`" = "none" ]; then
> +			dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
> +		else
> +			blkzone reset -c 2 $i
> +		fi

IIUC, any output from blkzone reset will cause test failures - is that
an intention here, or should output go to /dev/null like dd?

Looks fine otherwise.

Cheers, David

Re: [PATCH v2] common/rc: don't clear superblock for zoned scratch pools

[Johannes Thumshirn]

On 27.02.23 10:22, David Disseldorp wrote:
>> -		# to help better debug when something fails, we remove
>> -		# traces of previous btrfs FS on the dev.
>> -		dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
>> +		# To help better debug when something fails, we remove
>> +		# traces of previous btrfs FS on the dev. For zoned devices we
>> +		# can't use dd as it'll lead to unaligned writes so simply
>> +		# reset the first two zones.
>> +		if [ "`_zone_type "$i"`" = "none" ]; then
>> +			dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
>> +		else
>> +			blkzone reset -c 2 $i
>> +		fi
> 
> IIUC, any output from blkzone reset will cause test failures - is that
> an intention here, or should output go to /dev/null like dd?
> 
> Looks fine otherwise.

If all is well, blkzone reset won't have any output. So I guess we're fine.

Thanks :)

Re: [PATCH v2] common/rc: don't clear superblock for zoned scratch pools

[David Disseldorp]

On Mon, 27 Feb 2023 09:31:53 +0000, Johannes Thumshirn wrote:

> On 27.02.23 10:22, David Disseldorp wrote:
> >> -		# to help better debug when something fails, we remove
> >> -		# traces of previous btrfs FS on the dev.
> >> -		dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
> >> +		# To help better debug when something fails, we remove
> >> +		# traces of previous btrfs FS on the dev. For zoned devices we
> >> +		# can't use dd as it'll lead to unaligned writes so simply
> >> +		# reset the first two zones.
> >> +		if [ "`_zone_type "$i"`" = "none" ]; then
> >> +			dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
> >> +		else
> >> +			blkzone reset -c 2 $i
> >> +		fi  
> > 
> > IIUC, any output from blkzone reset will cause test failures - is that
> > an intention here, or should output go to /dev/null like dd?
> > 
> > Looks fine otherwise.  
> 
> If all is well, blkzone reset won't have any output. So I guess we're fine.

Thanks for the confirmation. In that case:
Reviewed-by: David Disseldorp <ddiss@suse.de>

Re: [PATCH v2] common/rc: don't clear superblock for zoned scratch pools

[Naohiro Aota]

On Mon, Feb 27, 2023 at 12:27:17AM -0800, Johannes Thumshirn wrote:
> _require_scratch_dev_pool() zeros the first 100 sectors of each device to
> clear eventual remains of older filesystems.
> 
> On zoned devices this won't work as a plain dd will end up creating
> unaligned write errors failing all subsequent actions on the device.
> 
> For zoned devices it is enough to simply reset the first two zones of the
> device to achieve the same result.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  common/rc | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 654730b21ead..739abbdbfc8c 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3459,9 +3459,15 @@ _require_scratch_dev_pool()
>  		            exit 1
>  		        fi
>  		fi
> -		# to help better debug when something fails, we remove
> -		# traces of previous btrfs FS on the dev.
> -		dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
> +		# To help better debug when something fails, we remove
> +		# traces of previous btrfs FS on the dev. For zoned devices we
> +		# can't use dd as it'll lead to unaligned writes so simply
> +		# reset the first two zones.
> +		if [ "`_zone_type "$i"`" = "none" ]; then
> +			dd if=/dev/zero of=$i bs=4096 count=100 > /dev/null 2>&1
> +		else
> +			blkzone reset -c 2 $i

We can use "$BLKZONE_PROG reset ..." to honor common/config.

With that,

Reviewed-by: Naohiro Aota <naohiro.aota@wdc.com>

> +		fi
>  	done
>  }
>  
> -- 
> 2.39.1
>