From: Eryu Guan Subject: Re: [fstests PATCH] generic: add test for executables on read-only DAX mounts Date: Wed, 30 Aug 2017 18:59:30 +0800 Message-ID: <20170830105930.GE27835@eguan.usersys.redhat.com> References: <20170829213721.GA27686@linux.intel.com> <20170829223715.26507-1-ross.zwisler@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: fstests@vger.kernel.org, linux-ext4@vger.kernel.org, linux-nvdimm@lists.01.org, linux-fsdevel@vger.kernel.org, Randy Dodgen , Christoph Hellwig , Theodore Ts'o To: Ross Zwisler Return-path: Content-Disposition: inline In-Reply-To: <20170829223715.26507-1-ross.zwisler@linux.intel.com> Sender: fstests-owner@vger.kernel.org List-Id: linux-ext4.vger.kernel.org On Tue, Aug 29, 2017 at 04:37:15PM -0600, Ross Zwisler wrote: > This adds a regression test for the following kernel patch: > > commit 42d4a99b09cb ("ext4: fix fault handling when mounted with -o dax,ro") > > The above patch fixes an issue with ext4 where executables cannot be run on > read-only filesystems mounted with the DAX option. > > This issue does not appear to be present in ext2 or XFS, as they both pass > the test. I've also confirmed outside of the test that they are both > indeed able to execute binaries on read-only DAX mounts. > > Thanks to Randy Dodgen for the bug report and reproduction steps. > > Signed-off-by: Ross Zwisler > Cc: Randy Dodgen > Cc: Christoph Hellwig > Cc: Theodore Ts'o > > --- > > Sorry if we collided, Randy, but I was 90% done with the test by the time I > saw your mail. :) > --- > tests/generic/452 | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/generic/452.out | 2 ++ > tests/generic/group | 1 + > 3 files changed, 90 insertions(+) > create mode 100755 tests/generic/452 > create mode 100644 tests/generic/452.out > > diff --git a/tests/generic/452 b/tests/generic/452 > new file mode 100755 > index 0000000..54dda8c > --- /dev/null > +++ b/tests/generic/452 > @@ -0,0 +1,87 @@ > +#! /bin/bash > +# FS QA Test 452 > +# > +# This is a regression test for kernel patch: > +# commit 42d4a99b09cb ("ext4: fix fault handling when mounted with -o dax,ro") > +# created by Ross Zwisler > +# > +#----------------------------------------------------------------------- > +# Copyright (c) 2017 Intel Corporation. All Rights Reserved. > +# > +# This program is free software; you can redistribute it and/or > +# modify it under the terms of the GNU General Public License as > +# published by the Free Software Foundation. > +# > +# This program is distributed in the hope that it would be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program; if not, write the Free Software Foundation, > +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > +#----------------------------------------------------------------------- > +# > + > +seq=`basename $0` > +seqres=$RESULT_DIR/$seq > +echo "QA output created by $seq" > + > +here=`pwd` > +tmp=/tmp/$$ > +status=1 # failure is the default! > +trap "_cleanup; exit \$status" 0 1 2 3 15 > + > +_cleanup() > +{ > + cd / > + rm -f $tmp.* > +} > + > +# get standard environment, filters and checks > +. ./common/rc > +. ./common/filter > + > +# remove previous $seqres.full before test > +rm -f $seqres.full > + > +# Modify as appropriate. > +_supported_fs generic > +_supported_os Linux > +_require_scratch > + > +# real QA test starts here > +# format and mount > +_scratch_mkfs > $seqres.full 2>&1 > +_scratch_mount >> $seqres.full 2>&1 Need to _notrun if scratch device was mounted with "noexec" option. _exclude_scratch_mount_option "noexec" > + > +LS=$(which ls --skip-alias --skip-functions) 2>/dev/null > +if [ $? -ne 0 ]; then > + status=$? > + echo "Couldn't find 'ls'!?" > + exit > +fi > + > +cp $LS $SCRATCH_MNT > +SCRATCH_LS=$SCRATCH_MNT/ls > + > +$SCRATCH_LS >/dev/null 2>&1 > +if [ $? -ne 0 ]; then > + status=$? > + echo "$SCRATCH_LS not working before remount" > + exit > +fi > + > +_scratch_remount ro > + > +$SCRATCH_LS >/dev/null 2>&1 > +if [ $? -ne 0 ]; then > + status=$? > + echo "$SCRATCH_LS not working after remount" > + exit > +fi Hmm, I don't think all these checks on return value are needed, just print out the error messages on failure and that will break the golden image, as this is not a destructive test, continuing with errors only fail the test :) I think this can be simplified to something like: LS=$(which ls ....) SCRATCH_LS=$SCRATCH_MNT/ls_on_scratch cp $LS $SCRATCH_LS $SCRATCH_LS $SCRATCH_LS | _filter_scratch _scratch_remount ro $SCRATCH_LS $SCRATCH_LS | _filter_scratch And update .out file accordingly. Thanks, Eryu > + > +# success, all done > +echo "Silence is golden" > +status=0 > +exit > diff --git a/tests/generic/452.out b/tests/generic/452.out > new file mode 100644 > index 0000000..db92441 > --- /dev/null > +++ b/tests/generic/452.out > @@ -0,0 +1,2 @@ > +QA output created by 452 > +Silence is golden > diff --git a/tests/generic/group b/tests/generic/group > index 044ec3f..45f5789 100644 > --- a/tests/generic/group > +++ b/tests/generic/group > @@ -453,3 +453,4 @@ > 449 auto quick acl enospc > 450 auto quick rw > 451 auto quick rw aio > +452 auto quick > -- > 2.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe fstests" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html