| Message ID | 20191217203155.24206-2-marcos.souza.org@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tests: do not fail if dm-thin is missing | expand |
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 >
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.
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
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