On Wed, Aug 02, 2023 at 12:15:35PM -0700, Luis Chamberlain wrote: > The filesystem configuration file does not allow you to use symlinks to > real devices given the existing sanity checks verify that the target end > device matches the source. Device mapper links work but not symlinks for > real drives do not. > > Using a symlink is desirable if you want to enable persistent tests > across reboots. For example you may want to use /dev/disk/by-id/nvme-eui.* > so to ensure that the same drives are used even after reboot. This > is very useful if you are testing for example with a virtualized > environment and are using PCIe passthrough with other qemu NVMe drives > with one or many NVMe drives. > > To enable support just add a helper to canonicalize devices prior to > running the tests. > > This allows one test runner, kdevops, which I just extended with > support to use real NVMe drives it has support now to use nvme EUI > symlinks and fallbacks to nvme model + serial symlinks as not all > NVMe drives support EUIs. The drives it uses for the filesystem > configuration optionally is with NVMe eui symlinks so to allow > the same drives to be used over reboots. > > For instance this works today with real nvme drives: > > mkfs.xfs -f /dev/nvme0n1 > mount /dev/nvme0n1 /mnt > TEST_DIR=/mnt TEST_DEV=/dev/nvme0n1 FSTYP=xfs ./check generic/110 > > FSTYP -- xfs (debug) > PLATFORM -- Linux/x86_64 flax-mtr01 6.5.0-rc3-djwx #rc3 SMP PREEMPT_DYNAMIC Wed Jul 26 14:26:48 PDT 2023 > > generic/110 2s > Ran: generic/110 > Passed all 1 tests > > But this does not: > > TEST_DIR=/mnt TEST_DEV=/dev/disk/by-id/nvme-eui.0035385411904c1e FSTYP=xfs ./check generic/110 > mount: /mnt: /dev/disk/by-id/nvme-eui.0035385411904c1e already mounted on /mnt. > common/rc: retrying test device mount with external set > mount: /mnt: /dev/disk/by-id/nvme-eui.0035385411904c1e already mounted on /mnt. > common/rc: could not mount /dev/disk/by-id/nvme-eui.0035385411904c1e on /mnt > > umount /mnt > TEST_DIR=/mnt TEST_DEV=/dev/disk/by-id/nvme-eui.0035385411904c1e FSTYP=xfs ./check generic/110 > TEST_DEV=/dev/disk/by-id/nvme-eui.0035385411904c1e is mounted but not on TEST_DIR=/mnt - aborting > Already mounted result: > /dev/disk/by-id/nvme-eui.0035385411904c1e /mnt > > This fixes this. This allows the same real drives for a test to be > used over and over after reboots. > > Use readlink -e because that support exists since 2004: > > https://github.com/coreutils/coreutils/commit/e0b8973bd4b146b5fb39641a4ee7984e922c3ff5 > > realpath is much newer than readlink, it's first commit including > support for -e dates back to 2011: > > https://github.com/coreutils/coreutils/commit/77ea441f79aa115f79b47d9c1fc9c0004c5c7111 > > Signed-off-by: Luis Chamberlain <mcgrof@xxxxxxxxxx> > --- > > Changes on v2: > > - Enhanced the commit log to describe the existing status quo where > at least device mapper symlinks work but not for real drives. Also > provide an example output of the issue and use case as implied by > Darrick. > - Added CANON_DEVS to disable this by default, document it > - simplify _canonicalize_devices() with as many one liners as possible > - use readlink -e because my history scavanging has found it has existed for > 7 years longer thjan realpath -e support. Documen this on the commit > log as well. > > README | 3 +++ > check | 1 + > common/config | 32 +++++++++++++++++++++++++++++++- > 3 files changed, 35 insertions(+), 1 deletion(-) > > diff --git a/README b/README > index 1ca506492bf0..97ef63d6d693 100644 > --- a/README > +++ b/README > @@ -268,6 +268,9 @@ Misc: > this option is supported for all filesystems currently only -overlay is > expected to run without issues. For other filesystems additional patches > and fixes to the test suite might be needed. > + - set CANON_DEVS=yes to canonicalize device symlinks. This will let you > + for example use something like TEST_DEV/dev/disk/by-id/nvme-* so the > + device remains persistent between reboots. This is disabled by default. > > ______________________ > USING THE FSQA SUITE > diff --git a/check b/check > index 0bf5b22e061a..577e09655844 100755 > --- a/check > +++ b/check > @@ -711,6 +711,7 @@ function run_section() > fi > > get_next_config $section > + _canonicalize_devices > > mkdir -p $RESULT_BASE > if [ ! -d $RESULT_BASE ]; then > diff --git a/common/config b/common/config > index 6c8cb3a5ba68..7d74c285ac71 100644 > --- a/common/config > +++ b/common/config > @@ -25,6 +25,9 @@ > # KEEP_DMESG - whether to keep all dmesg for each test case. > # yes: keep all dmesg > # no: only keep dmesg with error/warning (default) > +# CANON_DEVS - whether or not to canonicalize device symlinks > +# yes: canonicalize device symlinks > +# no (default) do not canonicalize device if they are symlinks > # > # - These can be added to $HOST_CONFIG_DIR (witch default to ./config) > # below or a separate local configuration file can be used (using > @@ -644,6 +647,32 @@ _canonicalize_mountpoint() > echo "$parent/$base" > } > > +# Enables usage of /dev/disk/by-id/ symlinks to persist target devices > +# over reboots > +_canonicalize_devices() > +{ > + if [ "$CANON_DEVS" != "yes" ]; then > + return > + fi > + [ -L "$TEST_DEV" ] && TEST_DEV=$(readlink -e "$TEST_DEV") > + [ -L $SCRATCH_DEV ] && SCRATCH_DEV=$(readlink -e "$SCRATCH_DEV") With or without "" will be different... > + [ -L $TEST_LOGDEV ] && TEST_LOGDEV=$(readlink -e "$TEST_LOGDEV") > + [ -L $TEST_RTDEV ] && TEST_RTDEV=$(readlink -e "$TEST_RTDEV") > + [ -L $SCRATCH_RTDEV ] && SCRATCH_RTDEV=$(readlink -e "$SCRATCH_RTDEV") > + [ -L $LOGWRITES_DEV ] && LOGWRITES_DEV=$(readlink -e "$LOGWRITES_DEV") You've give "" to $TEST_DEV, others should be same. > + if [ ! -z "$SCRATCH_DEV_POOL" ]; then > + NEW_SCRATCH_POOL="" If the NEW_SCRATCH_POOL isn't used in other places, it can be a local variable as a tmp variable. Thanks, Zorro > + for i in $SCRATCH_DEV_POOL; do > + if [ -L $i ]; then > + NEW_SCRATCH_POOL="$NEW_SCRATCH_POOL $(readlink -e $i)" > + else > + NEW_SCRATCH_POOL="$NEW_SCRATCH_POOL $i)" > + fi > + done > + SCRATCH_DEV_POOL="$NEW_SCRATCH_POOL" > + fi > +} > + > # On check -overlay, for the non multi section config case, this > # function is called on every test, before init_rc(). > # When SCRATCH/TEST_* vars are defined in config file, config file > @@ -774,7 +803,6 @@ get_next_config() { > fi > > parse_config_section $1 > - > if [ ! -z "$OLD_FSTYP" ] && [ $OLD_FSTYP != $FSTYP ]; then > [ -z "$MOUNT_OPTIONS" ] && _mount_opts > [ -z "$TEST_FS_MOUNT_OPTS" ] && _test_mount_opts > @@ -890,5 +918,7 @@ else > fi > fi > > +_canonicalize_devices > + > # make sure this script returns success > /bin/true > -- > 2.39.2 >
