diff mbox series

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

Message ID 20230227082717.325929-1-johannes.thumshirn@wdc.com (mailing list archive)
State New, archived
Headers show
Series [v2] common/rc: don't clear superblock for zoned scratch pools | expand

Commit Message

Johannes Thumshirn Feb. 27, 2023, 8:27 a.m. UTC
_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(-)

Comments

David Disseldorp Feb. 27, 2023, 9:23 a.m. UTC | #1
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
Johannes Thumshirn Feb. 27, 2023, 9:31 a.m. UTC | #2
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 :)
David Disseldorp Feb. 27, 2023, 10:14 a.m. UTC | #3
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>
Naohiro Aota March 2, 2023, 7:17 a.m. UTC | #4
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
>
diff mbox series

Patch

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
 }