On Fri, 2022-01-07 at 01:09 +0800, Zorro Lang wrote: > On Thu, Jan 06, 2022 at 06:23:16AM -0500, Jeff Layton wrote: > > Cephfs is introducing a new mount device syntax. Fix the fstests > > infrastructure to handle the new syntax correctly. > > > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > --- > > common/config | 8 ++++++++ > > common/rc | 34 ++++++++++++++++++++++++++++++++-- > > 2 files changed, 40 insertions(+), 2 deletions(-) > > > > v2: more rigorous check for ceph mount device in _check_device > > > > diff --git a/common/config b/common/config > > index e0a5c5df58ff..2b357746476b 100644 > > --- a/common/config > > +++ b/common/config > > @@ -537,6 +537,14 @@ _check_device() > > # 9p and virtiofs mount tags are just plain strings, so anything is allowed > > # tmpfs doesn't use mount source, ignore > > ;; > > + ceph) > > + # ceph has two different possible syntaxes for mount devices. The > > + # network URL check above catches the legacy syntax. Check for the > > + # new-style syntax here. > > + if ( echo $dev | grep -qEv "=/" ); then > > + _fatal "common/config: $name ($dev) is not a valid ceph mount string" > > + fi > > + ;; > > I have not objection with this, if the maintainer prefer: > > - if [ -b "$dev" ] || ( echo $dev | grep -qE ":|//" ); then > + if [ -b "$dev" ] || ( echo $dev | grep -qE ":|//|=/" ); then > # block device or a network url > return 0 > fi > The "=/" part of the syntax is (so far) ceph-specific. I don't think we want to accept that for (e.g.) NFS, so it's probably best to allow that only when FSTYP=ceph. > > overlay) > > if [ ! -d "$dev" ]; then > > _fatal "common/config: $name ($dev) is not a directory for overlay" > > diff --git a/common/rc b/common/rc > > index 7973ceb5fdf8..4fa0b818d840 100644 > > --- a/common/rc > > +++ b/common/rc > > @@ -1592,7 +1592,7 @@ _require_scratch_nocheck() > > _notrun "this test requires a valid \$SCRATCH_MNT" > > fi > > ;; > > - nfs*|ceph) > > + nfs*) > > echo $SCRATCH_DEV | grep -q ":/" > /dev/null 2>&1 > > if [ -z "$SCRATCH_DEV" -o "$?" != "0" ]; then > > _notrun "this test requires a valid \$SCRATCH_DEV" > > @@ -1601,6 +1601,21 @@ _require_scratch_nocheck() > > _notrun "this test requires a valid \$SCRATCH_MNT" > > fi > > ;; > > + ceph) > > + if [ -z "$SCRATCH_DEV" ]; then > > + _notrun "this test requires a valid \$SCRATCH_DEV" > > + fi > > + echo $SCRATCH_DEV | grep -q "=/" > /dev/null 2>&1 > > + if [ "$?" != "0" ]; then > > + echo $SCRATCH_DEV | grep -q ":/" > /dev/null 2>&1 > > + if [ "$?" != "0" ]; then > > Why not combine two "if condition" into one, likes: > echo $SCRATCH_DEV | grep -Eq "=/|:/" >/dev/null 2>&1 > if [ "$?" != "0" ] .... > > (Sorry I didn't metion this last time...) > > Same below: > Fair enough -- that's more efficient too. I'll respin with that. > > + _notrun "this test requires a valid \$SCRATCH_DEV" > > + fi > > + fi > > + if [ ! -d "$SCRATCH_MNT" ]; then > > + _notrun "this test requires a valid \$SCRATCH_MNT" > > + fi > > + ;; > > pvfs2) > > echo $SCRATCH_DEV | grep -q "://" > /dev/null 2>&1 > > if [ -z "$SCRATCH_DEV" -o "$?" != "0" ]; then > > @@ -1770,7 +1785,7 @@ _require_test() > > _notrun "this test requires a valid \$TEST_DIR" > > fi > > ;; > > - nfs*|ceph) > > + nfs*) > > echo $TEST_DEV | grep -q ":/" > /dev/null 2>&1 > > if [ -z "$TEST_DEV" -o "$?" != "0" ]; then > > _notrun "this test requires a valid \$TEST_DEV" > > @@ -1779,6 +1794,21 @@ _require_test() > > _notrun "this test requires a valid \$TEST_DIR" > > fi > > ;; > > + ceph) > > + if [ -z "$TEST_DEV" ]; then > > + _notrun "this test requires a valid \$TEST_DEV" > > + fi > > + echo $TEST_DEV | grep -q "=/" > /dev/null 2>&1 > > + if [ "$?" != "0" ]; then > > + echo $TEST_DEV | grep -q ":/" > /dev/null 2>&1 > > + if [ "$?" != "0" ]; then > > + _notrun "this test requires a valid \$TEST_DEV" > > + fi > > + fi > > + if [ ! -d "$TEST_DIR" ]; then > > + _notrun "this test requires a valid \$TEST_DIR" > > + fi > > + ;; > > cifs) > > echo $TEST_DEV | grep -q "//" > /dev/null 2>&1 > > if [ -z "$TEST_DEV" -o "$?" != "0" ]; then > > -- > > 2.33.1 > > > Thanks, -- Jeff Layton <jlayton@xxxxxxxxxx>
