[btrfs-progs,1/4] tests: common: Add check_dm_target_support helper
diff mbox series

Message ID 20191217203155.24206-2-marcos.souza.org@gmail.com
State New
Headers show
Series
  • tests: do not fail if dm-thin is missing
Related show

Commit Message

Marcos Paulo de Souza Dec. 17, 2019, 8:31 p.m. UTC
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(+)

Comments

Qu Wenruo Dec. 18, 2019, 12:26 a.m. UTC | #1
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
>
David Sterba Dec. 18, 2019, 3:58 p.m. UTC | #2
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.
Marcos Paulo de Souza Dec. 18, 2019, 4:05 p.m. UTC | #3
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

Patch
diff mbox series

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