* [btrfs-progs PATCH 0/4] tests: do not fail if dm-thin is missing @ 2019-12-17 20:31 Marcos Paulo de Souza 2019-12-17 20:31 ` [btrfs-progs PATCH 1/4] tests: common: Add check_dm_target_support helper Marcos Paulo de Souza ` (3 more replies) 0 siblings, 4 replies; 14+ messages in thread From: Marcos Paulo de Souza @ 2019-12-17 20:31 UTC (permalink / raw) Cc: dsterba, linux-btrfs, Marcos Paulo de Souza, wqu From: Marcos Paulo de Souza <mpdesouza@suse.com> Hi there, these patchset is trying to fix the issue 192[1] by checking if dm-thin exists, and if it's not available, skip the test. In the last patch, the same is done for dmsetup. Feel free to ignore the last patch if you think we should stop the tests if dmsetup isn't available. Thanks! [1]: https://github.com/kdave/btrfs-progs/issues/192 Marcos Paulo de Souza (4): tests: common: Add check_dm_target_support helper tests: mkfs: 017: Use check_dm_target_support helper tests: mkfs: 005: Use check_dm_target_support helper tests: Do not fail is dmsetup is missing tests/common | 18 ++++++++++++++++++ .../005-long-device-name-for-ssd/test.sh | 2 +- .../test.sh | 2 +- 3 files changed, 20 insertions(+), 2 deletions(-) -- 2.23.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [btrfs-progs PATCH 1/4] tests: common: Add check_dm_target_support helper 2019-12-17 20:31 [btrfs-progs PATCH 0/4] tests: do not fail if dm-thin is missing Marcos Paulo de Souza @ 2019-12-17 20:31 ` Marcos Paulo de Souza 2019-12-18 0:26 ` Qu Wenruo 2019-12-17 20:31 ` [btrfs-progs PATCH 2/4] tests: mkfs: 017: Use " Marcos Paulo de Souza ` (2 subsequent siblings) 3 siblings, 1 reply; 14+ messages in thread From: Marcos Paulo de Souza @ 2019-12-17 20:31 UTC (permalink / raw) Cc: dsterba, linux-btrfs, Marcos Paulo de Souza, wqu From: Marcos Paulo de Souza <mpdesouza@suse.com> This function will be used later to test if dm-thin is supported. Inspired by fstests. Suggested-by: Qu Wenruo <wqu@suse.com> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> --- tests/common | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/common b/tests/common index ca098444..f138b17e 100644 --- a/tests/common +++ b/tests/common @@ -322,6 +322,19 @@ check_global_prereq() fi } +# check if the targets passed as arguments are available, and if not just skip +# the test +check_dm_target_support() +{ + for target in "$@"; do + $SUDO_HELPER modprobe dm-$target >/dev/null 2>&1 + $SUDO_HELPER dmsetup targets 2>&1 | grep -q ^$target + if [ $? -ne 0 ]; then + _not_run "This test requires dm $target support." + fi + done +} + check_image() { local image -- 2.23.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [btrfs-progs PATCH 1/4] tests: common: Add check_dm_target_support helper 2019-12-17 20:31 ` [btrfs-progs PATCH 1/4] tests: common: Add check_dm_target_support helper Marcos Paulo de Souza @ 2019-12-18 0:26 ` Qu Wenruo 2019-12-18 15:58 ` David Sterba 0 siblings, 1 reply; 14+ messages in thread From: Qu Wenruo @ 2019-12-18 0:26 UTC (permalink / raw) To: Marcos Paulo de Souza; +Cc: dsterba, linux-btrfs, Marcos Paulo de Souza, wqu [-- Attachment #1.1: Type: text/plain, Size: 1535 bytes --] On 2019/12/18 上午4:31, Marcos Paulo de Souza wrote: > From: Marcos Paulo de Souza <mpdesouza@suse.com> > > This function will be used later to test if dm-thin is supported. > Inspired by fstests. > > Suggested-by: Qu Wenruo <wqu@suse.com> > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> Looks mostly fine, but a small nitpick for the SUDO_HELPER usage. > --- > tests/common | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/tests/common b/tests/common > index ca098444..f138b17e 100644 > --- a/tests/common > +++ b/tests/common > @@ -322,6 +322,19 @@ check_global_prereq() > fi > } > > +# check if the targets passed as arguments are available, and if not just skip > +# the test > +check_dm_target_support() > +{ > + for target in "$@"; do > + $SUDO_HELPER modprobe dm-$target >/dev/null 2>&1 To utilize $SUDO_HELPER, we need to call setup_root_helper() in the first place, just like all the other $SUDO_HELPER users in `tests/common`. Although nowadays it feels a little unnecessary, since the functionality is introduced because I'm a lazybone who doesn't bother to startup a VM to do proper test, but uses current unprivileged user to utilize self tests. Maybe it's time to get rid of SUDO_HELPER ? Thanks, Qu > + $SUDO_HELPER dmsetup targets 2>&1 | grep -q ^$target > + if [ $? -ne 0 ]; then > + _not_run "This test requires dm $target support." > + fi > + done > +} > + > check_image() > { > local image > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 520 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [btrfs-progs PATCH 1/4] tests: common: Add check_dm_target_support helper 2019-12-18 0:26 ` Qu Wenruo @ 2019-12-18 15:58 ` David Sterba 2019-12-18 16:05 ` Marcos Paulo de Souza 0 siblings, 1 reply; 14+ messages in thread From: David Sterba @ 2019-12-18 15:58 UTC (permalink / raw) To: Qu Wenruo Cc: Marcos Paulo de Souza, dsterba, linux-btrfs, Marcos Paulo de Souza, wqu On Wed, Dec 18, 2019 at 08:26:09AM +0800, Qu Wenruo wrote: > > +# check if the targets passed as arguments are available, and if not just skip > > +# the test > > +check_dm_target_support() > > +{ > > + for target in "$@"; do > > + $SUDO_HELPER modprobe dm-$target >/dev/null 2>&1 > > To utilize $SUDO_HELPER, we need to call setup_root_helper() in the > first place, just like all the other $SUDO_HELPER users in `tests/common`. > > Although nowadays it feels a little unnecessary, since the functionality > is introduced because I'm a lazybone who doesn't bother to startup a VM > to do proper test, but uses current unprivileged user to utilize self tests. > > Maybe it's time to get rid of SUDO_HELPER ? No, that should stay, my local testing relies on that heavily. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [btrfs-progs PATCH 1/4] tests: common: Add check_dm_target_support helper 2019-12-18 15:58 ` David Sterba @ 2019-12-18 16:05 ` Marcos Paulo de Souza 0 siblings, 0 replies; 14+ messages in thread From: Marcos Paulo de Souza @ 2019-12-18 16:05 UTC (permalink / raw) To: dsterba, Qu Wenruo Cc: Marcos Paulo de Souza, dsterba, linux-btrfs, Marcos Paulo de Souza, wqu On Wed, 2019-12-18 at 16:58 +0100, David Sterba wrote: > On Wed, Dec 18, 2019 at 08:26:09AM +0800, Qu Wenruo wrote: > > > +# check if the targets passed as arguments are available, and if > not just skip > > > +# the test > > > +check_dm_target_support() > > > +{ > > > + for target in "$@"; do > > > + $SUDO_HELPER modprobe dm-$target >/dev/null 2>&1 > > > > To utilize $SUDO_HELPER, we need to call setup_root_helper() in the > > first place, just like all the other $SUDO_HELPER users in > `tests/common`. > > > > Although nowadays it feels a little unnecessary, since the > functionality > > is introduced because I'm a lazybone who doesn't bother to startup > a VM > > to do proper test, but uses current unprivileged user to utilize > self tests. > > > > Maybe it's time to get rid of SUDO_HELPER ? > > No, that should stay, my local testing relies on that heavily. An updated version keeping SUDO_HELPER and adding setup_root_helper is in [1]. All other patches are the same ones, already reviewed by Qu. [1]: https://github.com/marcosps/btrfs-progs/tree/mpdesouza_mkfs_fixes ^ permalink raw reply [flat|nested] 14+ messages in thread
* [btrfs-progs PATCH 2/4] tests: mkfs: 017: Use check_dm_target_support helper 2019-12-17 20:31 [btrfs-progs PATCH 0/4] tests: do not fail if dm-thin is missing Marcos Paulo de Souza 2019-12-17 20:31 ` [btrfs-progs PATCH 1/4] tests: common: Add check_dm_target_support helper Marcos Paulo de Souza @ 2019-12-17 20:31 ` Marcos Paulo de Souza 2019-12-18 0:26 ` Qu Wenruo 2019-12-17 20:31 ` [btrfs-progs PATCH 3/4] tests: mkfs: 005: " Marcos Paulo de Souza 2019-12-17 20:31 ` [btrfs-progs PATCH 4/4] tests: Do not fail is dmsetup is missing Marcos Paulo de Souza 3 siblings, 1 reply; 14+ messages in thread From: Marcos Paulo de Souza @ 2019-12-17 20:31 UTC (permalink / raw) Cc: dsterba, linux-btrfs, Marcos Paulo de Souza, wqu From: Marcos Paulo de Souza <mpdesouza@suse.com> If dm-thin or dm-linear are not supported, let's skip the test altogether instead of throwing an error. Issue: #192 Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> --- .../017-small-backing-size-thin-provision-device/test.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/mkfs-tests/017-small-backing-size-thin-provision-device/test.sh b/tests/mkfs-tests/017-small-backing-size-thin-provision-device/test.sh index 32640ce5..91851945 100755 --- a/tests/mkfs-tests/017-small-backing-size-thin-provision-device/test.sh +++ b/tests/mkfs-tests/017-small-backing-size-thin-provision-device/test.sh @@ -7,6 +7,7 @@ source "$TEST_TOP/common" check_prereq mkfs.btrfs check_global_prereq udevadm check_global_prereq dmsetup +check_dm_target_support linear thin setup_root_helper prepare_test_dev -- 2.23.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [btrfs-progs PATCH 2/4] tests: mkfs: 017: Use check_dm_target_support helper 2019-12-17 20:31 ` [btrfs-progs PATCH 2/4] tests: mkfs: 017: Use " Marcos Paulo de Souza @ 2019-12-18 0:26 ` Qu Wenruo 0 siblings, 0 replies; 14+ messages in thread From: Qu Wenruo @ 2019-12-18 0:26 UTC (permalink / raw) To: Marcos Paulo de Souza; +Cc: dsterba, linux-btrfs, Marcos Paulo de Souza, wqu [-- Attachment #1.1: Type: text/plain, Size: 1071 bytes --] On 2019/12/18 上午4:31, Marcos Paulo de Souza wrote: > From: Marcos Paulo de Souza <mpdesouza@suse.com> > > If dm-thin or dm-linear are not supported, let's skip the test altogether > instead of throwing an error. > > Issue: #192 > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > .../017-small-backing-size-thin-provision-device/test.sh | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/mkfs-tests/017-small-backing-size-thin-provision-device/test.sh b/tests/mkfs-tests/017-small-backing-size-thin-provision-device/test.sh > index 32640ce5..91851945 100755 > --- a/tests/mkfs-tests/017-small-backing-size-thin-provision-device/test.sh > +++ b/tests/mkfs-tests/017-small-backing-size-thin-provision-device/test.sh > @@ -7,6 +7,7 @@ source "$TEST_TOP/common" > check_prereq mkfs.btrfs > check_global_prereq udevadm > check_global_prereq dmsetup > +check_dm_target_support linear thin > > setup_root_helper > prepare_test_dev > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 520 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* [btrfs-progs PATCH 3/4] tests: mkfs: 005: Use check_dm_target_support helper 2019-12-17 20:31 [btrfs-progs PATCH 0/4] tests: do not fail if dm-thin is missing Marcos Paulo de Souza 2019-12-17 20:31 ` [btrfs-progs PATCH 1/4] tests: common: Add check_dm_target_support helper Marcos Paulo de Souza 2019-12-17 20:31 ` [btrfs-progs PATCH 2/4] tests: mkfs: 017: Use " Marcos Paulo de Souza @ 2019-12-17 20:31 ` Marcos Paulo de Souza 2019-12-18 0:27 ` Qu Wenruo 2019-12-17 20:31 ` [btrfs-progs PATCH 4/4] tests: Do not fail is dmsetup is missing Marcos Paulo de Souza 3 siblings, 1 reply; 14+ messages in thread From: Marcos Paulo de Souza @ 2019-12-17 20:31 UTC (permalink / raw) Cc: dsterba, linux-btrfs, Marcos Paulo de Souza, wqu From: Marcos Paulo de Souza <mpdesouza@suse.com> This way we ensure the linear target is available and skip the test. Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> --- tests/mkfs-tests/005-long-device-name-for-ssd/test.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh b/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh index e7a1ac45..329deaf2 100755 --- a/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh +++ b/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh @@ -5,6 +5,7 @@ source "$TEST_TOP/common" check_prereq mkfs.btrfs check_global_prereq dmsetup +check_dm_target_support linear setup_root_helper prepare_test_dev -- 2.23.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [btrfs-progs PATCH 3/4] tests: mkfs: 005: Use check_dm_target_support helper 2019-12-17 20:31 ` [btrfs-progs PATCH 3/4] tests: mkfs: 005: " Marcos Paulo de Souza @ 2019-12-18 0:27 ` Qu Wenruo 0 siblings, 0 replies; 14+ messages in thread From: Qu Wenruo @ 2019-12-18 0:27 UTC (permalink / raw) To: Marcos Paulo de Souza; +Cc: dsterba, linux-btrfs, Marcos Paulo de Souza, wqu [-- Attachment #1.1: Type: text/plain, Size: 907 bytes --] On 2019/12/18 上午4:31, Marcos Paulo de Souza wrote: > From: Marcos Paulo de Souza <mpdesouza@suse.com> > > This way we ensure the linear target is available and skip the test. > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > tests/mkfs-tests/005-long-device-name-for-ssd/test.sh | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh b/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh > index e7a1ac45..329deaf2 100755 > --- a/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh > +++ b/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh > @@ -5,6 +5,7 @@ source "$TEST_TOP/common" > > check_prereq mkfs.btrfs > check_global_prereq dmsetup > +check_dm_target_support linear > > setup_root_helper > prepare_test_dev > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 520 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* [btrfs-progs PATCH 4/4] tests: Do not fail is dmsetup is missing 2019-12-17 20:31 [btrfs-progs PATCH 0/4] tests: do not fail if dm-thin is missing Marcos Paulo de Souza ` (2 preceding siblings ...) 2019-12-17 20:31 ` [btrfs-progs PATCH 3/4] tests: mkfs: 005: " Marcos Paulo de Souza @ 2019-12-17 20:31 ` Marcos Paulo de Souza 2019-12-18 0:30 ` Qu Wenruo 3 siblings, 1 reply; 14+ messages in thread From: Marcos Paulo de Souza @ 2019-12-17 20:31 UTC (permalink / raw) Cc: dsterba, linux-btrfs, Marcos Paulo de Souza, wqu From: Marcos Paulo de Souza <mpdesouza@suse.com> Move the check of dmsetup to check_dm_target_support, and adapt the only two places checking if dmsetup is present in the system. Now we skip the test if dmsetup isn't available, instead of marking the test as failed. Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> --- tests/common | 9 +++++++-- tests/mkfs-tests/005-long-device-name-for-ssd/test.sh | 1 - .../017-small-backing-size-thin-provision-device/test.sh | 1 - 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/common b/tests/common index f138b17e..dc2d084e 100644 --- a/tests/common +++ b/tests/common @@ -322,10 +322,15 @@ check_global_prereq() fi } -# check if the targets passed as arguments are available, and if not just skip -# the test +# check if dmsetup and targets passed as arguments are available, and skip the +# test if they aren't. check_dm_target_support() { + which dmsetup &> /dev/null + if [ $? -ne 0 ]; then + _not_run "This test requires dmsetup tool."; + fi + for target in "$@"; do $SUDO_HELPER modprobe dm-$target >/dev/null 2>&1 $SUDO_HELPER dmsetup targets 2>&1 | grep -q ^$target diff --git a/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh b/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh index 329deaf2..2df88db4 100755 --- a/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh +++ b/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh @@ -4,7 +4,6 @@ source "$TEST_TOP/common" check_prereq mkfs.btrfs -check_global_prereq dmsetup check_dm_target_support linear setup_root_helper diff --git a/tests/mkfs-tests/017-small-backing-size-thin-provision-device/test.sh b/tests/mkfs-tests/017-small-backing-size-thin-provision-device/test.sh index 91851945..83f34ecc 100755 --- a/tests/mkfs-tests/017-small-backing-size-thin-provision-device/test.sh +++ b/tests/mkfs-tests/017-small-backing-size-thin-provision-device/test.sh @@ -6,7 +6,6 @@ source "$TEST_TOP/common" check_prereq mkfs.btrfs check_global_prereq udevadm -check_global_prereq dmsetup check_dm_target_support linear thin setup_root_helper -- 2.23.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [btrfs-progs PATCH 4/4] tests: Do not fail is dmsetup is missing 2019-12-17 20:31 ` [btrfs-progs PATCH 4/4] tests: Do not fail is dmsetup is missing Marcos Paulo de Souza @ 2019-12-18 0:30 ` Qu Wenruo 2019-12-18 0:44 ` Marcos Paulo de Souza 0 siblings, 1 reply; 14+ messages in thread From: Qu Wenruo @ 2019-12-18 0:30 UTC (permalink / raw) To: Marcos Paulo de Souza; +Cc: dsterba, linux-btrfs, Marcos Paulo de Souza, wqu [-- Attachment #1.1: Type: text/plain, Size: 2596 bytes --] On 2019/12/18 上午4:31, Marcos Paulo de Souza wrote: > From: Marcos Paulo de Souza <mpdesouza@suse.com> > > Move the check of dmsetup to check_dm_target_support, and adapt the only > two places checking if dmsetup is present in the system. Now we skip the > test if dmsetup isn't available, instead of marking the test as failed. > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> Looks good overall, just a small nitpick inlined below. > --- > tests/common | 9 +++++++-- > tests/mkfs-tests/005-long-device-name-for-ssd/test.sh | 1 - > .../017-small-backing-size-thin-provision-device/test.sh | 1 - > 3 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/tests/common b/tests/common > index f138b17e..dc2d084e 100644 > --- a/tests/common > +++ b/tests/common > @@ -322,10 +322,15 @@ check_global_prereq() > fi > } > > -# check if the targets passed as arguments are available, and if not just skip > -# the test > +# check if dmsetup and targets passed as arguments are available, and skip the > +# test if they aren't. > check_dm_target_support() > { > + which dmsetup &> /dev/null > + if [ $? -ne 0 ]; then > + _not_run "This test requires dmsetup tool."; > + fi What about using existing check_global_prereq()? Despite that, Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > + > for target in "$@"; do > $SUDO_HELPER modprobe dm-$target >/dev/null 2>&1 > $SUDO_HELPER dmsetup targets 2>&1 | grep -q ^$target > diff --git a/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh b/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh > index 329deaf2..2df88db4 100755 > --- a/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh > +++ b/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh > @@ -4,7 +4,6 @@ > source "$TEST_TOP/common" > > check_prereq mkfs.btrfs > -check_global_prereq dmsetup > check_dm_target_support linear > > setup_root_helper > diff --git a/tests/mkfs-tests/017-small-backing-size-thin-provision-device/test.sh b/tests/mkfs-tests/017-small-backing-size-thin-provision-device/test.sh > index 91851945..83f34ecc 100755 > --- a/tests/mkfs-tests/017-small-backing-size-thin-provision-device/test.sh > +++ b/tests/mkfs-tests/017-small-backing-size-thin-provision-device/test.sh > @@ -6,7 +6,6 @@ source "$TEST_TOP/common" > > check_prereq mkfs.btrfs > check_global_prereq udevadm > -check_global_prereq dmsetup > check_dm_target_support linear thin > > setup_root_helper > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 520 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [btrfs-progs PATCH 4/4] tests: Do not fail is dmsetup is missing 2019-12-18 0:30 ` Qu Wenruo @ 2019-12-18 0:44 ` Marcos Paulo de Souza 2019-12-18 0:48 ` Qu Wenruo 0 siblings, 1 reply; 14+ messages in thread From: Marcos Paulo de Souza @ 2019-12-18 0:44 UTC (permalink / raw) To: Qu Wenruo, Marcos Paulo de Souza Cc: dsterba, linux-btrfs, Marcos Paulo de Souza, wqu On Wed, 2019-12-18 at 08:30 +0800, Qu Wenruo wrote: > > On 2019/12/18 上午4:31, Marcos Paulo de Souza wrote: > > From: Marcos Paulo de Souza <mpdesouza@suse.com> > > > > Move the check of dmsetup to check_dm_target_support, and adapt the > only > > two places checking if dmsetup is present in the system. Now we > skip the > > test if dmsetup isn't available, instead of marking the test as > failed. > > > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> > > Looks good overall, just a small nitpick inlined below. > > > --- > > tests/common | 9 > +++++++-- > > tests/mkfs-tests/005-long-device-name-for-ssd/test.sh | 1 - > > .../017-small-backing-size-thin-provision-device/test.sh | 1 - > > 3 files changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/tests/common b/tests/common > > index f138b17e..dc2d084e 100644 > > --- a/tests/common > > +++ b/tests/common > > @@ -322,10 +322,15 @@ check_global_prereq() > > fi > > } > > > > -# check if the targets passed as arguments are available, and if > not just skip > > -# the test > > +# check if dmsetup and targets passed as arguments are available, > and skip the > > +# test if they aren't. > > check_dm_target_support() > > { > > + which dmsetup &> /dev/null > > + if [ $? -ne 0 ]; then > > + _not_run "This test requires dmsetup tool."; > > + fi > > What about using existing check_global_prereq()? check_global_prereq call _fail when the executable is not found in $PATH, that's why I open coded the implementation and just called _not_run. > > Despite that, > > Reviewed-by: Qu Wenruo <wqu@suse.com> > > Thanks, > Qu > > > + > > for target in "$@"; do > > $SUDO_HELPER modprobe dm-$target >/dev/null 2>&1 > > $SUDO_HELPER dmsetup targets 2>&1 | grep -q ^$target > > diff --git a/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh > b/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh > > index 329deaf2..2df88db4 100755 > > --- a/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh > > +++ b/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh > > @@ -4,7 +4,6 @@ > > source "$TEST_TOP/common" > > > > check_prereq mkfs.btrfs > > -check_global_prereq dmsetup > > check_dm_target_support linear > > > > setup_root_helper > > diff --git a/tests/mkfs-tests/017-small-backing-size-thin- > provision-device/test.sh b/tests/mkfs-tests/017-small-backing-size- > thin-provision-device/test.sh > > index 91851945..83f34ecc 100755 > > --- a/tests/mkfs-tests/017-small-backing-size-thin-provision- > device/test.sh > > +++ b/tests/mkfs-tests/017-small-backing-size-thin-provision- > device/test.sh > > @@ -6,7 +6,6 @@ source "$TEST_TOP/common" > > > > check_prereq mkfs.btrfs > > check_global_prereq udevadm > > -check_global_prereq dmsetup > > check_dm_target_support linear thin > > > > setup_root_helper > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [btrfs-progs PATCH 4/4] tests: Do not fail is dmsetup is missing 2019-12-18 0:44 ` Marcos Paulo de Souza @ 2019-12-18 0:48 ` Qu Wenruo 2019-12-18 16:03 ` David Sterba 0 siblings, 1 reply; 14+ messages in thread From: Qu Wenruo @ 2019-12-18 0:48 UTC (permalink / raw) To: Marcos Paulo de Souza, Marcos Paulo de Souza Cc: dsterba, linux-btrfs, Marcos Paulo de Souza, wqu [-- Attachment #1.1: Type: text/plain, Size: 3240 bytes --] On 2019/12/18 上午8:44, Marcos Paulo de Souza wrote: > On Wed, 2019-12-18 at 08:30 +0800, Qu Wenruo wrote: >> >> On 2019/12/18 上午4:31, Marcos Paulo de Souza wrote: >>> From: Marcos Paulo de Souza <mpdesouza@suse.com> >>> >>> Move the check of dmsetup to check_dm_target_support, and adapt the >> only >>> two places checking if dmsetup is present in the system. Now we >> skip the >>> test if dmsetup isn't available, instead of marking the test as >> failed. >>> >>> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> >> >> Looks good overall, just a small nitpick inlined below. >> >>> --- >>> tests/common | 9 >> +++++++-- >>> tests/mkfs-tests/005-long-device-name-for-ssd/test.sh | 1 - >>> .../017-small-backing-size-thin-provision-device/test.sh | 1 - >>> 3 files changed, 7 insertions(+), 4 deletions(-) >>> >>> diff --git a/tests/common b/tests/common >>> index f138b17e..dc2d084e 100644 >>> --- a/tests/common >>> +++ b/tests/common >>> @@ -322,10 +322,15 @@ check_global_prereq() >>> fi >>> } >>> >>> -# check if the targets passed as arguments are available, and if >> not just skip >>> -# the test >>> +# check if dmsetup and targets passed as arguments are available, >> and skip the >>> +# test if they aren't. >>> check_dm_target_support() >>> { >>> + which dmsetup &> /dev/null >>> + if [ $? -ne 0 ]; then >>> + _not_run "This test requires dmsetup tool."; >>> + fi >> >> What about using existing check_global_prereq()? > > check_global_prereq call _fail when the executable is not found in > $PATH, that's why I open coded the implementation and just called > _not_run. Makes sense. Although it would be even better to change from "_fail" to "_not_run" for check_global_prereq(). That could be a new patch. Thanks, Qu > >> >> Despite that, >> >> Reviewed-by: Qu Wenruo <wqu@suse.com> >> >> Thanks, >> Qu >> >>> + >>> for target in "$@"; do >>> $SUDO_HELPER modprobe dm-$target >/dev/null 2>&1 >>> $SUDO_HELPER dmsetup targets 2>&1 | grep -q ^$target >>> diff --git a/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh >> b/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh >>> index 329deaf2..2df88db4 100755 >>> --- a/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh >>> +++ b/tests/mkfs-tests/005-long-device-name-for-ssd/test.sh >>> @@ -4,7 +4,6 @@ >>> source "$TEST_TOP/common" >>> >>> check_prereq mkfs.btrfs >>> -check_global_prereq dmsetup >>> check_dm_target_support linear >>> >>> setup_root_helper >>> diff --git a/tests/mkfs-tests/017-small-backing-size-thin- >> provision-device/test.sh b/tests/mkfs-tests/017-small-backing-size- >> thin-provision-device/test.sh >>> index 91851945..83f34ecc 100755 >>> --- a/tests/mkfs-tests/017-small-backing-size-thin-provision- >> device/test.sh >>> +++ b/tests/mkfs-tests/017-small-backing-size-thin-provision- >> device/test.sh >>> @@ -6,7 +6,6 @@ source "$TEST_TOP/common" >>> >>> check_prereq mkfs.btrfs >>> check_global_prereq udevadm >>> -check_global_prereq dmsetup >>> check_dm_target_support linear thin >>> >>> setup_root_helper >>> >> > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 520 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [btrfs-progs PATCH 4/4] tests: Do not fail is dmsetup is missing 2019-12-18 0:48 ` Qu Wenruo @ 2019-12-18 16:03 ` David Sterba 0 siblings, 0 replies; 14+ messages in thread From: David Sterba @ 2019-12-18 16:03 UTC (permalink / raw) To: Qu Wenruo Cc: Marcos Paulo de Souza, Marcos Paulo de Souza, dsterba, linux-btrfs, Marcos Paulo de Souza, wqu On Wed, Dec 18, 2019 at 08:48:21AM +0800, Qu Wenruo wrote: > >>> +# check if dmsetup and targets passed as arguments are available, > >> and skip the > >>> +# test if they aren't. > >>> check_dm_target_support() > >>> { > >>> + which dmsetup &> /dev/null > >>> + if [ $? -ne 0 ]; then > >>> + _not_run "This test requires dmsetup tool."; > >>> + fi > >> > >> What about using existing check_global_prereq()? > > > > check_global_prereq call _fail when the executable is not found in > > $PATH, that's why I open coded the implementation and just called > > _not_run. > > Makes sense. > > Although it would be even better to change from "_fail" to "_not_run" > for check_global_prereq(). I'd rather keep it as _fail, the utilities checked by this helper is mostly some system tool or other filesystem tool, all should be present for the testing. I don't want the testsuite to skip random tests without a good reason. We can add a script or make target to make the checks in advance and not when some test fails after a long time. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-12-18 16:03 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-17 20:31 [btrfs-progs PATCH 0/4] tests: do not fail if dm-thin is missing Marcos Paulo de Souza 2019-12-17 20:31 ` [btrfs-progs PATCH 1/4] tests: common: Add check_dm_target_support helper Marcos Paulo de Souza 2019-12-18 0:26 ` Qu Wenruo 2019-12-18 15:58 ` David Sterba 2019-12-18 16:05 ` Marcos Paulo de Souza 2019-12-17 20:31 ` [btrfs-progs PATCH 2/4] tests: mkfs: 017: Use " Marcos Paulo de Souza 2019-12-18 0:26 ` Qu Wenruo 2019-12-17 20:31 ` [btrfs-progs PATCH 3/4] tests: mkfs: 005: " Marcos Paulo de Souza 2019-12-18 0:27 ` Qu Wenruo 2019-12-17 20:31 ` [btrfs-progs PATCH 4/4] tests: Do not fail is dmsetup is missing Marcos Paulo de Souza 2019-12-18 0:30 ` Qu Wenruo 2019-12-18 0:44 ` Marcos Paulo de Souza 2019-12-18 0:48 ` Qu Wenruo 2019-12-18 16:03 ` David Sterba
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.