Message ID | 20220419175811.602126-1-ridave@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | cephfs: add ceph-fuse suport for ceph-fuse | expand |
On Tue, 2022-04-19 at 23:28 +0530, Rishabh Dave wrote: > Currently tests in xfstests-dev can be executed against CephFS only by > mounting CephFS using kernel driver. Attempting to run tests against > CephFS using FUSE doesn't work because xfstests-dev would remount CephFS > using kernel. This patch adds the ability for xfstest-dev code to mount > CephFS using FUSE. > > Fixes: https://tracker.ceph.com/issues/55354 > Signed-off-by: Rishabh Dave <ridave@redhat.com> > --- > common/config | 2 ++ > common/rc | 46 ++++++++++++++++++++++++++++++++++------------ > 2 files changed, 36 insertions(+), 12 deletions(-) > > diff --git a/common/config b/common/config > index 1033b890..efcf12e9 100644 > --- a/common/config > +++ b/common/config > @@ -556,6 +556,8 @@ _check_device() > _fatal "common/config: $name ($dev) is not a character device" > fi > ;; > + ceph-fuse) > + ;; > *) > _fatal "common/config: $name ($dev) is not a block device or a network filesystem" > esac > diff --git a/common/rc b/common/rc > index 553ae350..868b3ddd 100644 > --- a/common/rc > +++ b/common/rc > @@ -500,10 +500,14 @@ _test_mount() > { > local mount_ret > > - if [ "$FSTYP" == "overlay" ]; then > + if [ "$FSTYP" == "ceph-fuse" ]; then > + $CEPH_FUSE_BIN_PATH $TEST_FS_MOUNT_OPTS $TEST_DIR 2> /dev/null > + return $? > + elif [ "$FSTYP" == "overlay" ]; then > _overlay_test_mount $* > return $? > fi > + > _test_options mount > _mount -t $FSTYP $TEST_OPTIONS $TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS $* $TEST_DEV $TEST_DIR > mount_ret=$? > @@ -1446,7 +1450,8 @@ _fs_type() > # have to bother with this quirk. > # > _df_device $1 | $AWK_PROG '{ print $2 }' | \ > - sed -e 's/nfs4/nfs/' -e 's/fuse.glusterfs/glusterfs/' > + sed -e 's/nfs4/nfs/' -e 's/fuse.glusterfs/glusterfs/' \ > + -e 's/fuse.ceph-fuse/ceph-fuse/' > } > > # return the FS mount options of a mounted device > @@ -1595,6 +1600,24 @@ _supported_fs() > _notrun "not suitable for this filesystem type: $FSTYP" > } > > +_check_if_dev_already_mounted() > +{ > + local dev=$1 > + local mnt=$2 > + > + # find $dev as the source, and print result in "$dev $mnt" format > + local mount_rec=`findmnt -rncv -S $dev -o SOURCE,TARGET` > + [ -n "$mount_rec" ] || return 1 # 1 = not mounted > + > + # if it's mounted, make sure its on $mnt > + if [ "$mount_rec" != "$dev $mnt" ]; then > + echo "$devname=$dev is mounted but not on $mntname=$mnt - aborting" > + echo "Already mounted result:" > + echo $mount_rec > + return 2 # 2 = mounted on wrong mnt > + fi > +} > + > # check if a FS on a device is mounted > # if so, verify that it is mounted on mount point > # if fstype is given as argument, verify that it is also > @@ -1608,16 +1631,14 @@ _check_mounted_on() > local mnt=$4 > local type=$5 > > - # find $dev as the source, and print result in "$dev $mnt" format > - local mount_rec=`findmnt -rncv -S $dev -o SOURCE,TARGET` > - [ -n "$mount_rec" ] || return 1 # 1 = not mounted > + # this check doesn't work on ceph-fuse > + if [ "$dev" != "ceph-fuse" ]; then > + _check_if_dev_already_mounted $dev $mnt > + dev_already_mounted=$? > Shouldn't there be a "fi" above? > - # if it's mounted, make sure its on $mnt > - if [ "$mount_rec" != "$dev $mnt" ]; then > - echo "$devname=$dev is mounted but not on $mntname=$mnt - aborting" > - echo "Already mounted result:" > - echo $mount_rec > - return 2 # 2 = mounted on wrong mnt > + if [ $dev_already_mounted -ne 0 ]; then > + return $dev_already_mounted > + fi > fi > > if [ -n "$type" -a "`_fs_type $dev`" != "$type" ]; then > @@ -1869,6 +1890,7 @@ _require_test() > if [ ! -d "$TEST_DIR" ]; then > _notrun "this test requires a valid \$TEST_DIR" > fi > + ceph-fuse) > ;; > cifs) > echo $TEST_DEV | grep -q "//" > /dev/null 2>&1 > @@ -3204,7 +3226,7 @@ _check_test_fs() > virtiofs) > # no way to check consistency for virtiofs > ;; > - ceph) > + ceph|ceph-fuse) > # no way to check consistency for CephFS > ;; > glusterfs)
Hi Jeff, Thanks for the review! On Tue, 19 Apr 2022 at 23:34, Jeff Layton <jlayton@redhat.com> wrote: > > > - # find $dev as the source, and print result in "$dev $mnt" format > > - local mount_rec=`findmnt -rncv -S $dev -o SOURCE,TARGET` > > - [ -n "$mount_rec" ] || return 1 # 1 = not mounted > > + # this check doesn't work on ceph-fuse > > + if [ "$dev" != "ceph-fuse" ]; then > > + _check_if_dev_already_mounted $dev $mnt > > + dev_already_mounted=$? > > > > Shouldn't there be a "fi" above? > > > - # if it's mounted, make sure its on $mnt > > - if [ "$mount_rec" != "$dev $mnt" ]; then > > - echo "$devname=$dev is mounted but not on $mntname=$mnt - aborting" > > - echo "Already mounted result:" > > - echo $mount_rec > > - return 2 # 2 = mounted on wrong mnt > > + if [ $dev_already_mounted -ne 0 ]; then > > + return $dev_already_mounted > > + fi > > fi > Here's the "fi" for the "if" you referred to above.
On Tue, 2022-04-19 at 23:50 +0530, Rishabh Dave wrote: > Hi Jeff, > > Thanks for the review! > > On Tue, 19 Apr 2022 at 23:34, Jeff Layton <jlayton@redhat.com> wrote: > > > > > - # find $dev as the source, and print result in "$dev $mnt" format > > > - local mount_rec=`findmnt -rncv -S $dev -o SOURCE,TARGET` > > > - [ -n "$mount_rec" ] || return 1 # 1 = not mounted > > > + # this check doesn't work on ceph-fuse > > > + if [ "$dev" != "ceph-fuse" ]; then > > > + _check_if_dev_already_mounted $dev $mnt > > > + dev_already_mounted=$? > > > > > > > Shouldn't there be a "fi" above? > > > > > - # if it's mounted, make sure its on $mnt > > > - if [ "$mount_rec" != "$dev $mnt" ]; then > > > - echo "$devname=$dev is mounted but not on $mntname=$mnt - aborting" > > > - echo "Already mounted result:" > > > - echo $mount_rec > > > - return 2 # 2 = mounted on wrong mnt > > > + if [ $dev_already_mounted -ne 0 ]; then > > > + return $dev_already_mounted > > > + fi > > > fi > > > > Here's the "fi" for the "if" you referred to above. > Oops, I misread the patch! This looks fine to me: Reviewed-by: Jeff Layton <jlayton@redhat.com>
On Tue, Apr 19, 2022 at 11:28:11PM +0530, Rishabh Dave wrote: > Currently tests in xfstests-dev can be executed against CephFS only by > mounting CephFS using kernel driver. Attempting to run tests against > CephFS using FUSE doesn't work because xfstests-dev would remount CephFS > using kernel. This patch adds the ability for xfstest-dev code to mount > CephFS using FUSE. > > Fixes: https://tracker.ceph.com/issues/55354 > Signed-off-by: Rishabh Dave <ridave@redhat.com> > --- > common/config | 2 ++ > common/rc | 46 ++++++++++++++++++++++++++++++++++------------ > 2 files changed, 36 insertions(+), 12 deletions(-) > > diff --git a/common/config b/common/config > index 1033b890..efcf12e9 100644 > --- a/common/config > +++ b/common/config > @@ -556,6 +556,8 @@ _check_device() > _fatal "common/config: $name ($dev) is not a character device" > fi > ;; > + ceph-fuse) > + ;; > *) > _fatal "common/config: $name ($dev) is not a block device or a network filesystem" > esac > diff --git a/common/rc b/common/rc > index 553ae350..868b3ddd 100644 > --- a/common/rc > +++ b/common/rc [snip] > > if [ -n "$type" -a "`_fs_type $dev`" != "$type" ]; then > @@ -1869,6 +1890,7 @@ _require_test() > if [ ! -d "$TEST_DIR" ]; then > _notrun "this test requires a valid \$TEST_DIR" > fi This place missed a ";;", it'll cause all cases fail directly when they try to ". common/rc". As this patch has been RVB, I'll fix it when I merge it. But better to give a patch a mimimum test before sending (except it's simple enough:), to make sure it won't break the most fundamental things at least, it's hard for me to test all fs that fstests supports before merging, Thanks for understanding :) Thanks, Zorro > + ceph-fuse) > ;; > cifs) > echo $TEST_DEV | grep -q "//" > /dev/null 2>&1 > @@ -3204,7 +3226,7 @@ _check_test_fs() > virtiofs) > # no way to check consistency for virtiofs > ;; > - ceph) > + ceph|ceph-fuse) > # no way to check consistency for CephFS > ;; > glusterfs) > -- > 2.34.1 >
On Wed, 27 Apr 2022 at 14:18, Zorro Lang <zlang@kernel.org> wrote: > > > diff --git a/common/rc b/common/rc > > index 553ae350..868b3ddd 100644 > > --- a/common/rc > > +++ b/common/rc > > [snip] > > > > > if [ -n "$type" -a "`_fs_type $dev`" != "$type" ]; then > > @@ -1869,6 +1890,7 @@ _require_test() > > if [ ! -d "$TEST_DIR" ]; then > > _notrun "this test requires a valid \$TEST_DIR" > > fi > > This place missed a ";;", it'll cause all cases fail directly when they try to > ". common/rc". As this patch has been RVB, I'll fix it when I merge it. But better > to give a patch a mimimum test before sending (except it's simple enough:), to make > sure it won't break the most fundamental things at least, it's hard for me to test > all fs that fstests supports before merging, Thanks for understanding :) > Sorry for the inconvenience. I guess I made a last minute error because normally I test my patch against CephFS. Should I be running more tests, besides just this one? Please let me know if so because I am new to xfstests-dev. I've resent my patch under subject "[PATCH test] common: add support for ceph-fuse". It should've been [PATCH v2] since it's a newer version of this patch but I guess I made a mistake while running "git send-email" (I'm new to it). Thanks for the patch review!
diff --git a/common/config b/common/config index 1033b890..efcf12e9 100644 --- a/common/config +++ b/common/config @@ -556,6 +556,8 @@ _check_device() _fatal "common/config: $name ($dev) is not a character device" fi ;; + ceph-fuse) + ;; *) _fatal "common/config: $name ($dev) is not a block device or a network filesystem" esac diff --git a/common/rc b/common/rc index 553ae350..868b3ddd 100644 --- a/common/rc +++ b/common/rc @@ -500,10 +500,14 @@ _test_mount() { local mount_ret - if [ "$FSTYP" == "overlay" ]; then + if [ "$FSTYP" == "ceph-fuse" ]; then + $CEPH_FUSE_BIN_PATH $TEST_FS_MOUNT_OPTS $TEST_DIR 2> /dev/null + return $? + elif [ "$FSTYP" == "overlay" ]; then _overlay_test_mount $* return $? fi + _test_options mount _mount -t $FSTYP $TEST_OPTIONS $TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS $* $TEST_DEV $TEST_DIR mount_ret=$? @@ -1446,7 +1450,8 @@ _fs_type() # have to bother with this quirk. # _df_device $1 | $AWK_PROG '{ print $2 }' | \ - sed -e 's/nfs4/nfs/' -e 's/fuse.glusterfs/glusterfs/' + sed -e 's/nfs4/nfs/' -e 's/fuse.glusterfs/glusterfs/' \ + -e 's/fuse.ceph-fuse/ceph-fuse/' } # return the FS mount options of a mounted device @@ -1595,6 +1600,24 @@ _supported_fs() _notrun "not suitable for this filesystem type: $FSTYP" } +_check_if_dev_already_mounted() +{ + local dev=$1 + local mnt=$2 + + # find $dev as the source, and print result in "$dev $mnt" format + local mount_rec=`findmnt -rncv -S $dev -o SOURCE,TARGET` + [ -n "$mount_rec" ] || return 1 # 1 = not mounted + + # if it's mounted, make sure its on $mnt + if [ "$mount_rec" != "$dev $mnt" ]; then + echo "$devname=$dev is mounted but not on $mntname=$mnt - aborting" + echo "Already mounted result:" + echo $mount_rec + return 2 # 2 = mounted on wrong mnt + fi +} + # check if a FS on a device is mounted # if so, verify that it is mounted on mount point # if fstype is given as argument, verify that it is also @@ -1608,16 +1631,14 @@ _check_mounted_on() local mnt=$4 local type=$5 - # find $dev as the source, and print result in "$dev $mnt" format - local mount_rec=`findmnt -rncv -S $dev -o SOURCE,TARGET` - [ -n "$mount_rec" ] || return 1 # 1 = not mounted + # this check doesn't work on ceph-fuse + if [ "$dev" != "ceph-fuse" ]; then + _check_if_dev_already_mounted $dev $mnt + dev_already_mounted=$? - # if it's mounted, make sure its on $mnt - if [ "$mount_rec" != "$dev $mnt" ]; then - echo "$devname=$dev is mounted but not on $mntname=$mnt - aborting" - echo "Already mounted result:" - echo $mount_rec - return 2 # 2 = mounted on wrong mnt + if [ $dev_already_mounted -ne 0 ]; then + return $dev_already_mounted + fi fi if [ -n "$type" -a "`_fs_type $dev`" != "$type" ]; then @@ -1869,6 +1890,7 @@ _require_test() if [ ! -d "$TEST_DIR" ]; then _notrun "this test requires a valid \$TEST_DIR" fi + ceph-fuse) ;; cifs) echo $TEST_DEV | grep -q "//" > /dev/null 2>&1 @@ -3204,7 +3226,7 @@ _check_test_fs() virtiofs) # no way to check consistency for virtiofs ;; - ceph) + ceph|ceph-fuse) # no way to check consistency for CephFS ;; glusterfs)
Currently tests in xfstests-dev can be executed against CephFS only by mounting CephFS using kernel driver. Attempting to run tests against CephFS using FUSE doesn't work because xfstests-dev would remount CephFS using kernel. This patch adds the ability for xfstest-dev code to mount CephFS using FUSE. Fixes: https://tracker.ceph.com/issues/55354 Signed-off-by: Rishabh Dave <ridave@redhat.com> --- common/config | 2 ++ common/rc | 46 ++++++++++++++++++++++++++++++++++------------ 2 files changed, 36 insertions(+), 12 deletions(-)