fstests.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH xfstests] overlay: Enable character device to be the base fs partition
@ 2019-09-24  9:40 Zhihao Cheng
  2019-09-24 11:15 ` Zhihao Cheng
  2019-09-24 12:33 ` Amir Goldstein
  0 siblings, 2 replies; 6+ messages in thread
From: Zhihao Cheng @ 2019-09-24  9:40 UTC (permalink / raw)
  To: guaneryu, amir73il, david.oberhollenzer, ebiggers, yi.zhang
  Cc: fstests, linux-kernel, chengzhihao1

When running overlay tests using character devices as base fs partitions,
all overlay usecase results become 'notrun'. Function
'_overay_config_override' (common/config) detects that the current base
fs partition is not a block device and will set FSTYP to base fs. The
overlay usecase will check the current FSTYP, and if it is not 'overlay'
or 'generic', it will skip the execution.

For example, using UBIFS as base fs skips all overlay usecases:

  FSTYP         -- ubifs       # FSTYP should be overridden as 'overlay'
  MKFS_OPTIONS  -- /dev/ubi0_1 # Character device
  MOUNT_OPTIONS -- -t ubifs /dev/ubi0_1 /tmp/scratch

  overlay/001	[not run] not suitable for this filesystem type: ubifs
  overlay/002	[not run] not suitable for this filesystem type: ubifs
  overlay/003	[not run] not suitable for this filesystem type: ubifs
  ...

When checking that the base fs partition is a block/character device,
FSTYP is overwritten as 'overlay'. This patch allows the base fs
partition to be a character device that can also execute overlay
usecases (such as ubifs).

Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 common/config | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/config b/common/config
index 4c86a49..a22acdb 100644
--- a/common/config
+++ b/common/config
@@ -550,7 +550,7 @@ _overlay_config_override()
 	#    the new OVL_BASE_SCRATCH/TEST_DEV/MNT vars are set to the values
 	#    of the configured base fs and SCRATCH/TEST_DEV vars are set to the
 	#    overlayfs base and mount dirs inside base fs mount.
-	[ -b "$TEST_DEV" ] || return 0
+	[ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || return 0
 
 	# Config file may specify base fs type, but we obay -overlay flag
 	[ "$FSTYP" == overlay ] || export OVL_BASE_FSTYP="$FSTYP"
@@ -570,7 +570,7 @@ _overlay_config_override()
 	export TEST_DIR="$OVL_BASE_TEST_DIR/$OVL_MNT"
 	export MOUNT_OPTIONS="$OVERLAY_MOUNT_OPTIONS"
 
-	[ -b "$SCRATCH_DEV" ] || return 0
+	[ -b "$SCRATCH_DEV" ] || [ -c "$SCRATCH_DEV" ] || return 0
 
 	# Store original base fs vars
 	export OVL_BASE_SCRATCH_DEV="$SCRATCH_DEV"
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH xfstests] overlay: Enable character device to be the base fs partition
  2019-09-24  9:40 [PATCH xfstests] overlay: Enable character device to be the base fs partition Zhihao Cheng
@ 2019-09-24 11:15 ` Zhihao Cheng
  2019-09-24 12:33 ` Amir Goldstein
  1 sibling, 0 replies; 6+ messages in thread
From: Zhihao Cheng @ 2019-09-24 11:15 UTC (permalink / raw)
  To: guaneryu, amir73il, david.oberhollenzer, ebiggers, yi.zhang
  Cc: fstests, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 3049 bytes --]

After incorporating patches, use overlay usecases to test character device-based base fs, and all overlay usecases are executed:

FSTYP         -- overlay  # FSTYP has be overridden as 'overlay'
PLATFORM      -- Linux/x86_64 localhost
MKFS_OPTIONS  -- /tmp/scratch
MOUNT_OPTIONS -- /tmp/scratch /tmp/scratch/ovl-mnt

overlay/001	[not run] This test requires at least 8GB free on /tmp/scratch to run
overlay/002	 1s
overlay/003	 0s
overlay/004	 0s
overlay/005	 1s
overlay/006	 0s
overlay/007	 0s
overlay/008	 0s
overlay/009	 0s
overlay/010	 0s
overlay/011	 1s
overlay/012	 0s
overlay/013	 0s
overlay/014	 1s
...

Attachments:
setup.sh: Create character device for base fs (UBIFS)
local.config: Xfstests local configuration

在 2019/9/24 17:40, Zhihao Cheng 写道:
> When running overlay tests using character devices as base fs partitions,
> all overlay usecase results become 'notrun'. Function
> '_overay_config_override' (common/config) detects that the current base
> fs partition is not a block device and will set FSTYP to base fs. The
> overlay usecase will check the current FSTYP, and if it is not 'overlay'
> or 'generic', it will skip the execution.
> 
> For example, using UBIFS as base fs skips all overlay usecases:
> 
>   FSTYP         -- ubifs       # FSTYP should be overridden as 'overlay'
>   MKFS_OPTIONS  -- /dev/ubi0_1 # Character device
>   MOUNT_OPTIONS -- -t ubifs /dev/ubi0_1 /tmp/scratch
> 
>   overlay/001	[not run] not suitable for this filesystem type: ubifs
>   overlay/002	[not run] not suitable for this filesystem type: ubifs
>   overlay/003	[not run] not suitable for this filesystem type: ubifs
>   ...
> 
> When checking that the base fs partition is a block/character device,
> FSTYP is overwritten as 'overlay'. This patch allows the base fs
> partition to be a character device that can also execute overlay
> usecases (such as ubifs).
> 
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
> ---
>  common/config | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/common/config b/common/config
> index 4c86a49..a22acdb 100644
> --- a/common/config
> +++ b/common/config
> @@ -550,7 +550,7 @@ _overlay_config_override()
>  	#    the new OVL_BASE_SCRATCH/TEST_DEV/MNT vars are set to the values
>  	#    of the configured base fs and SCRATCH/TEST_DEV vars are set to the
>  	#    overlayfs base and mount dirs inside base fs mount.
> -	[ -b "$TEST_DEV" ] || return 0
> +	[ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || return 0
>  
>  	# Config file may specify base fs type, but we obay -overlay flag
>  	[ "$FSTYP" == overlay ] || export OVL_BASE_FSTYP="$FSTYP"
> @@ -570,7 +570,7 @@ _overlay_config_override()
>  	export TEST_DIR="$OVL_BASE_TEST_DIR/$OVL_MNT"
>  	export MOUNT_OPTIONS="$OVERLAY_MOUNT_OPTIONS"
>  
> -	[ -b "$SCRATCH_DEV" ] || return 0
> +	[ -b "$SCRATCH_DEV" ] || [ -c "$SCRATCH_DEV" ] || return 0
>  
>  	# Store original base fs vars
>  	export OVL_BASE_SCRATCH_DEV="$SCRATCH_DEV"
> 

[-- Attachment #2: local.config --]
[-- Type: text/plain, Size: 205 bytes --]

export FSTYP=ubifs
export TEST_DEV=/dev/ubi0_0
export TEST_DIR=/tmp/test
export TEST_FS_MOUNT_OPTS="-t ubifs"
export SCRATCH_DEV=/dev/ubi0_1
export SCRATCH_MNT=/tmp/scratch
export MOUNT_OPTIONS="-t ubifs"

[-- Attachment #3: setup.sh --]
[-- Type: text/plain, Size: 801 bytes --]

#!/bin/bash

set -e

TMP=/tmp/test
SMP=/tmp/scratch
umount $TMP $SMP 2>/dev/null || true
mkdir -p $TMP $SMP

modprobe -r ubifs 2>/dev/null || true
for i in $(seq 0 1)
do
	ubidetach -p /dev/mtd$i 2>/dev/null || true
done
modprobe -r ubi 2>/dev/null || true
modprobe -r nandsim 2>/dev/null || true

mtd=/dev/mtd0
ubi=/dev/ubi0

ARCH=$(uname -m)
if test "$ARCH" == ppc || test "$ARCH" == armv7l
then
	# 512MB, 8-bits, page size 4KB, OOB 128B, block 128KB
	ID="0x20,0xac,0x00,0x16"
	TSIZE=128MiB
	SSIZE=350MiB
else
	# 2GB, 8-bits, page size 4KB, OOB 128B, block 128KB
	ID="0x20,0xa5,0x00,0x16"
	TSIZE=400MiB
	SSIZE=1500MiB
fi
modprobe nandsim id_bytes=$ID
flash_erase -q -j $mtd 0 0
modprobe ubi
modprobe ubifs

ubiattach -p $mtd
ubimkvol $ubi -N test -s $TSIZE
ubimkvol $ubi -N scratch -s $SSIZE

exit 0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH xfstests] overlay: Enable character device to be the base fs partition
  2019-09-24  9:40 [PATCH xfstests] overlay: Enable character device to be the base fs partition Zhihao Cheng
  2019-09-24 11:15 ` Zhihao Cheng
@ 2019-09-24 12:33 ` Amir Goldstein
  2019-09-24 14:19   ` Zhihao Cheng
  1 sibling, 1 reply; 6+ messages in thread
From: Amir Goldstein @ 2019-09-24 12:33 UTC (permalink / raw)
  To: Zhihao Cheng
  Cc: Eryu Guan, David Oberhollenzer, Eric Biggers, zhangyi (F),
	fstests, linux-kernel

On Tue, Sep 24, 2019 at 12:34 PM Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>
> When running overlay tests using character devices as base fs partitions,
> all overlay usecase results become 'notrun'. Function
> '_overay_config_override' (common/config) detects that the current base
> fs partition is not a block device and will set FSTYP to base fs. The
> overlay usecase will check the current FSTYP, and if it is not 'overlay'
> or 'generic', it will skip the execution.
>
> For example, using UBIFS as base fs skips all overlay usecases:
>
>   FSTYP         -- ubifs       # FSTYP should be overridden as 'overlay'
>   MKFS_OPTIONS  -- /dev/ubi0_1 # Character device
>   MOUNT_OPTIONS -- -t ubifs /dev/ubi0_1 /tmp/scratch
>
>   overlay/001   [not run] not suitable for this filesystem type: ubifs
>   overlay/002   [not run] not suitable for this filesystem type: ubifs
>   overlay/003   [not run] not suitable for this filesystem type: ubifs
>   ...
>
> When checking that the base fs partition is a block/character device,
> FSTYP is overwritten as 'overlay'. This patch allows the base fs
> partition to be a character device that can also execute overlay
> usecases (such as ubifs).
>
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
> ---
>  common/config | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/common/config b/common/config
> index 4c86a49..a22acdb 100644
> --- a/common/config
> +++ b/common/config
> @@ -550,7 +550,7 @@ _overlay_config_override()
>         #    the new OVL_BASE_SCRATCH/TEST_DEV/MNT vars are set to the values
>         #    of the configured base fs and SCRATCH/TEST_DEV vars are set to the
>         #    overlayfs base and mount dirs inside base fs mount.
> -       [ -b "$TEST_DEV" ] || return 0
> +       [ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || return 0
>
>         # Config file may specify base fs type, but we obay -overlay flag
>         [ "$FSTYP" == overlay ] || export OVL_BASE_FSTYP="$FSTYP"
> @@ -570,7 +570,7 @@ _overlay_config_override()
>         export TEST_DIR="$OVL_BASE_TEST_DIR/$OVL_MNT"
>         export MOUNT_OPTIONS="$OVERLAY_MOUNT_OPTIONS"
>
> -       [ -b "$SCRATCH_DEV" ] || return 0
> +       [ -b "$SCRATCH_DEV" ] || [ -c "$SCRATCH_DEV" ] || return 0
>
>         # Store original base fs vars
>         export OVL_BASE_SCRATCH_DEV="$SCRATCH_DEV"
> --
> 2.7.4
>

Looks fine.

One nit: there is a message in _require_scratch_shutdown():
_notrun "$SCRATCH_DEV is not a block device"
for when $OVL_BASE_SCRATCH_DEV is not defined.

Could probably use a better describing error anyway.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH xfstests] overlay: Enable character device to be the base fs partition
  2019-09-24 12:33 ` Amir Goldstein
@ 2019-09-24 14:19   ` Zhihao Cheng
  2019-09-25  3:15     ` Eryu Guan
  0 siblings, 1 reply; 6+ messages in thread
From: Zhihao Cheng @ 2019-09-24 14:19 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Eryu Guan, David Oberhollenzer, Eric Biggers, zhangyi (F),
	fstests, linux-kernel

As far as I know, _require_scratch_shutdown() is called after _overay_config_override(), at this moment, FSTYP equals to base fs. According the implementation of _require_scratch_shutdown:
3090 _require_scratch_shutdown()
3091 {
3092     [ -x src/godown ] || _notrun "src/godown executable not found"
3093
3094     _scratch_mkfs > /dev/null 2>&1 || _notrun "_scratch_mkfs failed on $SCRATCH_DEV"
3095     _scratch_mount
3096
3097     if [ $FSTYP = "overlay" ]; then                                            # FSTYP = base fs
3098         if [ -z $OVL_BASE_SCRATCH_DEV ]; then
3099             # In lagacy overlay usage, it may specify directory as
3100             # SCRATCH_DEV, in this case OVL_BASE_SCRATCH_DEV
3101             # will be null, so check OVL_BASE_SCRATCH_DEV before
3102             # running shutdown to avoid shutting down base fs accidently.
3103             _notrun "$SCRATCH_DEV is not a block device"
3104         else
3105             src/godown -f $OVL_BASE_SCRATCH_MNT 2>&1 \
3106             || _notrun "Underlying filesystem does not support shutdown"
3107         fi
3108     else
3109         src/godown -f $SCRATCH_MNT 2>&1 \
3110             || _notrun "$FSTYP does not support shutdown"                      # Executes this path
3111     fi
3112
3113     _scratch_unmount
3114 }
So, we can't get output: _notrun "$SCRATCH_DEV is not a block device". Instead, the verbose should like:
  after _overlay_config_override FSTYP=ubifs    # Additional print message
  FSTYP         -- ubifs
  PLATFORM      -- Linux/x86_64
  MKFS_OPTIONS  -- /dev/ubi0_1
  MOUNT_OPTIONS -- -t ubifs /dev/ubi0_1 /tmp/scratch

  generic/042	[not run] ubifs does not support shutdown

But I'll consider describing error more concisely in v2.

在 2019/9/24 20:33, Amir Goldstein 写道:
> On Tue, Sep 24, 2019 at 12:34 PM Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>
>> When running overlay tests using character devices as base fs partitions,
>> all overlay usecase results become 'notrun'. Function
>> '_overay_config_override' (common/config) detects that the current base
>> fs partition is not a block device and will set FSTYP to base fs. The
>> overlay usecase will check the current FSTYP, and if it is not 'overlay'
>> or 'generic', it will skip the execution.
>>
>> For example, using UBIFS as base fs skips all overlay usecases:
>>
>>   FSTYP         -- ubifs       # FSTYP should be overridden as 'overlay'
>>   MKFS_OPTIONS  -- /dev/ubi0_1 # Character device
>>   MOUNT_OPTIONS -- -t ubifs /dev/ubi0_1 /tmp/scratch
>>
>>   overlay/001   [not run] not suitable for this filesystem type: ubifs
>>   overlay/002   [not run] not suitable for this filesystem type: ubifs
>>   overlay/003   [not run] not suitable for this filesystem type: ubifs
>>   ...
>>
>> When checking that the base fs partition is a block/character device,
>> FSTYP is overwritten as 'overlay'. This patch allows the base fs
>> partition to be a character device that can also execute overlay
>> usecases (such as ubifs).
>>
>> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
>> ---
>>  common/config | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/config b/common/config
>> index 4c86a49..a22acdb 100644
>> --- a/common/config
>> +++ b/common/config
>> @@ -550,7 +550,7 @@ _overlay_config_override()
>>         #    the new OVL_BASE_SCRATCH/TEST_DEV/MNT vars are set to the values
>>         #    of the configured base fs and SCRATCH/TEST_DEV vars are set to the
>>         #    overlayfs base and mount dirs inside base fs mount.
>> -       [ -b "$TEST_DEV" ] || return 0
>> +       [ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || return 0
>>
>>         # Config file may specify base fs type, but we obay -overlay flag
>>         [ "$FSTYP" == overlay ] || export OVL_BASE_FSTYP="$FSTYP"
>> @@ -570,7 +570,7 @@ _overlay_config_override()
>>         export TEST_DIR="$OVL_BASE_TEST_DIR/$OVL_MNT"
>>         export MOUNT_OPTIONS="$OVERLAY_MOUNT_OPTIONS"
>>
>> -       [ -b "$SCRATCH_DEV" ] || return 0
>> +       [ -b "$SCRATCH_DEV" ] || [ -c "$SCRATCH_DEV" ] || return 0
>>
>>         # Store original base fs vars
>>         export OVL_BASE_SCRATCH_DEV="$SCRATCH_DEV"
>> --
>> 2.7.4
>>
> 
> Looks fine.
> 
> One nit: there is a message in _require_scratch_shutdown():
> _notrun "$SCRATCH_DEV is not a block device"
> for when $OVL_BASE_SCRATCH_DEV is not defined.
> 
> Could probably use a better describing error anyway.
> 
> Thanks,
> Amir.
> 
> .
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH xfstests] overlay: Enable character device to be the base fs partition
  2019-09-24 14:19   ` Zhihao Cheng
@ 2019-09-25  3:15     ` Eryu Guan
  2019-09-25  3:20       ` Zhihao Cheng
  0 siblings, 1 reply; 6+ messages in thread
From: Eryu Guan @ 2019-09-25  3:15 UTC (permalink / raw)
  To: Zhihao Cheng
  Cc: Amir Goldstein, Eryu Guan, David Oberhollenzer, Eric Biggers,
	zhangyi (F),
	fstests, linux-kernel

On Tue, Sep 24, 2019 at 10:19:38PM +0800, Zhihao Cheng wrote:
> As far as I know, _require_scratch_shutdown() is called after _overay_config_override(), at this moment, FSTYP equals to base fs. According the implementation of _require_scratch_shutdown:
> 3090 _require_scratch_shutdown()
> 3091 {
> 3092     [ -x src/godown ] || _notrun "src/godown executable not found"
> 3093
> 3094     _scratch_mkfs > /dev/null 2>&1 || _notrun "_scratch_mkfs failed on $SCRATCH_DEV"
> 3095     _scratch_mount
> 3096
> 3097     if [ $FSTYP = "overlay" ]; then                                            # FSTYP = base fs
> 3098         if [ -z $OVL_BASE_SCRATCH_DEV ]; then
> 3099             # In lagacy overlay usage, it may specify directory as
> 3100             # SCRATCH_DEV, in this case OVL_BASE_SCRATCH_DEV
> 3101             # will be null, so check OVL_BASE_SCRATCH_DEV before
> 3102             # running shutdown to avoid shutting down base fs accidently.
> 3103             _notrun "$SCRATCH_DEV is not a block device"
> 3104         else
> 3105             src/godown -f $OVL_BASE_SCRATCH_MNT 2>&1 \
> 3106             || _notrun "Underlying filesystem does not support shutdown"
> 3107         fi
> 3108     else
> 3109         src/godown -f $SCRATCH_MNT 2>&1 \
> 3110             || _notrun "$FSTYP does not support shutdown"                      # Executes this path
> 3111     fi
> 3112
> 3113     _scratch_unmount
> 3114 }
> So, we can't get output: _notrun "$SCRATCH_DEV is not a block device". Instead, the verbose should like:
>   after _overlay_config_override FSTYP=ubifs    # Additional print message
>   FSTYP         -- ubifs
>   PLATFORM      -- Linux/x86_64
>   MKFS_OPTIONS  -- /dev/ubi0_1
>   MOUNT_OPTIONS -- -t ubifs /dev/ubi0_1 /tmp/scratch
> 
>   generic/042	[not run] ubifs does not support shutdown
> 
> But I'll consider describing error more concisely in v2.
> 
> 在 2019/9/24 20:33, Amir Goldstein 写道:
> > On Tue, Sep 24, 2019 at 12:34 PM Zhihao Cheng <chengzhihao1@huawei.com> wrote:
> >>
> >> When running overlay tests using character devices as base fs partitions,
> >> all overlay usecase results become 'notrun'. Function
> >> '_overay_config_override' (common/config) detects that the current base
> >> fs partition is not a block device and will set FSTYP to base fs. The
> >> overlay usecase will check the current FSTYP, and if it is not 'overlay'
> >> or 'generic', it will skip the execution.
> >>
> >> For example, using UBIFS as base fs skips all overlay usecases:
> >>
> >>   FSTYP         -- ubifs       # FSTYP should be overridden as 'overlay'
> >>   MKFS_OPTIONS  -- /dev/ubi0_1 # Character device
> >>   MOUNT_OPTIONS -- -t ubifs /dev/ubi0_1 /tmp/scratch
> >>
> >>   overlay/001   [not run] not suitable for this filesystem type: ubifs
> >>   overlay/002   [not run] not suitable for this filesystem type: ubifs
> >>   overlay/003   [not run] not suitable for this filesystem type: ubifs
> >>   ...
> >>
> >> When checking that the base fs partition is a block/character device,
> >> FSTYP is overwritten as 'overlay'. This patch allows the base fs
> >> partition to be a character device that can also execute overlay
> >> usecases (such as ubifs).
> >>
> >> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
> >> ---
> >>  common/config | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/common/config b/common/config
> >> index 4c86a49..a22acdb 100644
> >> --- a/common/config
> >> +++ b/common/config
> >> @@ -550,7 +550,7 @@ _overlay_config_override()
> >>         #    the new OVL_BASE_SCRATCH/TEST_DEV/MNT vars are set to the values
> >>         #    of the configured base fs and SCRATCH/TEST_DEV vars are set to the
> >>         #    overlayfs base and mount dirs inside base fs mount.
> >> -       [ -b "$TEST_DEV" ] || return 0
> >> +       [ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || return 0
> >>
> >>         # Config file may specify base fs type, but we obay -overlay flag
> >>         [ "$FSTYP" == overlay ] || export OVL_BASE_FSTYP="$FSTYP"
> >> @@ -570,7 +570,7 @@ _overlay_config_override()
> >>         export TEST_DIR="$OVL_BASE_TEST_DIR/$OVL_MNT"
> >>         export MOUNT_OPTIONS="$OVERLAY_MOUNT_OPTIONS"
> >>
> >> -       [ -b "$SCRATCH_DEV" ] || return 0
> >> +       [ -b "$SCRATCH_DEV" ] || [ -c "$SCRATCH_DEV" ] || return 0
> >>
> >>         # Store original base fs vars
> >>         export OVL_BASE_SCRATCH_DEV="$SCRATCH_DEV"
> >> --
> >> 2.7.4
> >>
> > 
> > Looks fine.
> > 
> > One nit: there is a message in _require_scratch_shutdown():
> > _notrun "$SCRATCH_DEV is not a block device"
> > for when $OVL_BASE_SCRATCH_DEV is not defined.
> > 
> > Could probably use a better describing error anyway.

I think what Amir suggested is that, as you add char device support to
overlay base device, the message in _require_scratch_shutdown() should
be updated accordingly, not the commit log.

Thanks,
Eryu

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH xfstests] overlay: Enable character device to be the base fs partition
  2019-09-25  3:15     ` Eryu Guan
@ 2019-09-25  3:20       ` Zhihao Cheng
  0 siblings, 0 replies; 6+ messages in thread
From: Zhihao Cheng @ 2019-09-25  3:20 UTC (permalink / raw)
  To: Eryu Guan
  Cc: Amir Goldstein, Eryu Guan, David Oberhollenzer, Eric Biggers,
	zhangyi (F),
	fstests, linux-kernel

Oh, You are right, I understood it wrong. Thanks for reminding.

在 2019/9/25 11:15, Eryu Guan 写道:
> On Tue, Sep 24, 2019 at 10:19:38PM +0800, Zhihao Cheng wrote:
>> As far as I know, _require_scratch_shutdown() is called after _overay_config_override(), at this moment, FSTYP equals to base fs. According the implementation of _require_scratch_shutdown:
>> 3090 _require_scratch_shutdown()
>> 3091 {
>> 3092     [ -x src/godown ] || _notrun "src/godown executable not found"
>> 3093
>> 3094     _scratch_mkfs > /dev/null 2>&1 || _notrun "_scratch_mkfs failed on $SCRATCH_DEV"
>> 3095     _scratch_mount
>> 3096
>> 3097     if [ $FSTYP = "overlay" ]; then                                            # FSTYP = base fs
>> 3098         if [ -z $OVL_BASE_SCRATCH_DEV ]; then
>> 3099             # In lagacy overlay usage, it may specify directory as
>> 3100             # SCRATCH_DEV, in this case OVL_BASE_SCRATCH_DEV
>> 3101             # will be null, so check OVL_BASE_SCRATCH_DEV before
>> 3102             # running shutdown to avoid shutting down base fs accidently.
>> 3103             _notrun "$SCRATCH_DEV is not a block device"
>> 3104         else
>> 3105             src/godown -f $OVL_BASE_SCRATCH_MNT 2>&1 \
>> 3106             || _notrun "Underlying filesystem does not support shutdown"
>> 3107         fi
>> 3108     else
>> 3109         src/godown -f $SCRATCH_MNT 2>&1 \
>> 3110             || _notrun "$FSTYP does not support shutdown"                      # Executes this path
>> 3111     fi
>> 3112
>> 3113     _scratch_unmount
>> 3114 }
>> So, we can't get output: _notrun "$SCRATCH_DEV is not a block device". Instead, the verbose should like:
>>   after _overlay_config_override FSTYP=ubifs    # Additional print message
>>   FSTYP         -- ubifs
>>   PLATFORM      -- Linux/x86_64
>>   MKFS_OPTIONS  -- /dev/ubi0_1
>>   MOUNT_OPTIONS -- -t ubifs /dev/ubi0_1 /tmp/scratch
>>
>>   generic/042	[not run] ubifs does not support shutdown
>>
>> But I'll consider describing error more concisely in v2.
>>
>> 在 2019/9/24 20:33, Amir Goldstein 写道:
>>> On Tue, Sep 24, 2019 at 12:34 PM Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>>>
>>>> When running overlay tests using character devices as base fs partitions,
>>>> all overlay usecase results become 'notrun'. Function
>>>> '_overay_config_override' (common/config) detects that the current base
>>>> fs partition is not a block device and will set FSTYP to base fs. The
>>>> overlay usecase will check the current FSTYP, and if it is not 'overlay'
>>>> or 'generic', it will skip the execution.
>>>>
>>>> For example, using UBIFS as base fs skips all overlay usecases:
>>>>
>>>>   FSTYP         -- ubifs       # FSTYP should be overridden as 'overlay'
>>>>   MKFS_OPTIONS  -- /dev/ubi0_1 # Character device
>>>>   MOUNT_OPTIONS -- -t ubifs /dev/ubi0_1 /tmp/scratch
>>>>
>>>>   overlay/001   [not run] not suitable for this filesystem type: ubifs
>>>>   overlay/002   [not run] not suitable for this filesystem type: ubifs
>>>>   overlay/003   [not run] not suitable for this filesystem type: ubifs
>>>>   ...
>>>>
>>>> When checking that the base fs partition is a block/character device,
>>>> FSTYP is overwritten as 'overlay'. This patch allows the base fs
>>>> partition to be a character device that can also execute overlay
>>>> usecases (such as ubifs).
>>>>
>>>> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
>>>> ---
>>>>  common/config | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/common/config b/common/config
>>>> index 4c86a49..a22acdb 100644
>>>> --- a/common/config
>>>> +++ b/common/config
>>>> @@ -550,7 +550,7 @@ _overlay_config_override()
>>>>         #    the new OVL_BASE_SCRATCH/TEST_DEV/MNT vars are set to the values
>>>>         #    of the configured base fs and SCRATCH/TEST_DEV vars are set to the
>>>>         #    overlayfs base and mount dirs inside base fs mount.
>>>> -       [ -b "$TEST_DEV" ] || return 0
>>>> +       [ -b "$TEST_DEV" ] || [ -c "$TEST_DEV" ] || return 0
>>>>
>>>>         # Config file may specify base fs type, but we obay -overlay flag
>>>>         [ "$FSTYP" == overlay ] || export OVL_BASE_FSTYP="$FSTYP"
>>>> @@ -570,7 +570,7 @@ _overlay_config_override()
>>>>         export TEST_DIR="$OVL_BASE_TEST_DIR/$OVL_MNT"
>>>>         export MOUNT_OPTIONS="$OVERLAY_MOUNT_OPTIONS"
>>>>
>>>> -       [ -b "$SCRATCH_DEV" ] || return 0
>>>> +       [ -b "$SCRATCH_DEV" ] || [ -c "$SCRATCH_DEV" ] || return 0
>>>>
>>>>         # Store original base fs vars
>>>>         export OVL_BASE_SCRATCH_DEV="$SCRATCH_DEV"
>>>> --
>>>> 2.7.4
>>>>
>>>
>>> Looks fine.
>>>
>>> One nit: there is a message in _require_scratch_shutdown():
>>> _notrun "$SCRATCH_DEV is not a block device"
>>> for when $OVL_BASE_SCRATCH_DEV is not defined.
>>>
>>> Could probably use a better describing error anyway.
> 
> I think what Amir suggested is that, as you add char device support to
> overlay base device, the message in _require_scratch_shutdown() should
> be updated accordingly, not the commit log.
> 
> Thanks,
> Eryu
> 
> .
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-09-25  3:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-24  9:40 [PATCH xfstests] overlay: Enable character device to be the base fs partition Zhihao Cheng
2019-09-24 11:15 ` Zhihao Cheng
2019-09-24 12:33 ` Amir Goldstein
2019-09-24 14:19   ` Zhihao Cheng
2019-09-25  3:15     ` Eryu Guan
2019-09-25  3:20       ` Zhihao Cheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).