Re: [PATCH] xfs/288: test xfs_growfs to ensure rejection if target isn't mounted

["Bill O'Donnell" <billodo@redhat.com>]

On Mon, Apr 24, 2017 at 02:01:20PM -0700, Darrick J. Wong wrote:
> On Mon, Apr 24, 2017 at 02:53:14PM -0500, Bill O'Donnell wrote:
> > On Mon, Apr 24, 2017 at 02:38:02PM -0500, Eric Sandeen wrote:
> > > On 4/24/17 11:34 AM, Darrick J. Wong wrote:
> > > > On Mon, Apr 24, 2017 at 10:09:35AM -0500, Bill O'Donnell wrote:
> > > >> The manpage for xfs_growfs specifies that the file/dir to be
> > > >> grown must be mounted. This is a test to ensure that if the
> > > >> target isn't mounted, the command is rejected. The test covers
> > > >> absolute paths and relative paths, including symbolic links
> > > >> and bind mounts.
> > > >>
> > > >> Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> > > >> ---
> > > >>  tests/xfs/288     | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >>  tests/xfs/288.out | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >>  tests/xfs/group   |   1 +
> > > >>  3 files changed, 241 insertions(+)
> > > >>  create mode 100755 tests/xfs/288
> > > >>  create mode 100644 tests/xfs/288.out
> > > >>
> > > >> diff --git a/tests/xfs/288 b/tests/xfs/288
> > > >> new file mode 100755
> > > >> index 0000000..628f753
> > > >> --- /dev/null
> > > >> +++ b/tests/xfs/288
> > > >> @@ -0,0 +1,121 @@
> > > >> +#! /bin/bash
> > > >> +# FS QA Test 288
> > > >> +#
> > > >> +# Test to ensure xfs_growfs command rejects non-existent mount points
> > > >> +#
> > > >> +#-----------------------------------------------------------------------
> > > >> +# Copyright (c) 2017 Red Hat, Inc.  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()
> > > >> +{
> > > >> +    umount $tmpdir
> > > >> +    umount $tmpbind
> > > >> +    rmdir $tmpdir
> > > >> +    rm -f $tmpsymlink
> > > >> +    rmdir $tmpbind
> > > >> +    rm -f $tmp
> > > >> +    rm -f $tmpfile
> > > >> +}
> > > >> +
> > > >> +# get standard environment, filters and checks
> > > >> +. ./common/rc
> > > >> +. ./common/filter
> > > >> +
> > > >> +# remove previous $seqres.full before test
> > > >> +rm -f $seqres.full
> > > >> +
> > > >> +# real QA test starts here
> > > >> +
> > > >> +# Modify as appropriate.
> > > >> +_supported_fs xfs
> > > >> +_supported_os Linux
> > > >> +_require_test
> > > >> +_require_loop
> > > >> +
> > > >> +tmpfile=$TEST_DIR/fsfile
> > > >> +tmpdir=$TEST_DIR/tmpdir
> > > >> +tmpsymlink=$TEST_DIR/tmpsymlink.$$
> > > >> +tmpbind=$TEST_DIR/tmpbind.$$
> > > >> +
> > > >> +mkdir -p $tmpdir || _fail "!!! failed to create temp mount dir"
> > > >> +
> > > >> +echo "=== mkfs.xfs ==="
> > > >> +mkfs.xfs -d file,name=$tmpfile,size=64m -f
> > > > 
> > > > $MKFS_XFS_PROG -d file...
> > > > 
> > > >> +
> > > >> +echo "=== truncate ==="
> > > >> +truncate --size=128m $tmpfile
> > > >> +
> > > >> +echo "=== xfs_growfs - unmounted, command should be rejected ==="
> > > >> +xfs_growfs $tmpdir
> > > > 
> > > > $XFS_GROWFS_PROG $tmpdir
> > > > 
> > > >> +
> > > >> +echo "=== xfs_growfs - check relative path, unmounted ==="
> > > >> +cd $TEST_DIR
> > > >> +xfs_growfs ./tmpdir
> > > >> +
> > > >> +echo "=== xfs_growfs - no path, unmounted ==="
> > > >> +cd $TEST_DIR
> > > >> +xfs_growfs tmpdir
> > > >> +
> > > >> +echo "=== mount ==="
> > > >> +mount -o loop $tmpfile $tmpdir || _fail "!!! failed to loopback mount"
> > > >> +
> > > >> +echo "=== xfs_growfs - mounted ==="
> > > >> +xfs_growfs $tmpdir
> > > >> +
> > > >> +echo "=== xfs_growfs - check relative path ==="
> > > >> +cd $TEST_DIR
> > > >> +xfs_growfs ./tmpdir
> > > > 
> > > > So this growfs invocation should succeed, correct?  I see agcount
> > > > doubling but $tmpfile doesn't get expanded... such that all the
> > > > following growfses have no space to grow the fs, but they also don't
> > > > spit out error messages.
> > > > 
> > > > (Just talking myself through this test...)
> > > 
> > > There may be no need to actually grow the filesystem to test this
> > > behavior; nothing about this part of the tool differentiates between
> > > xfs_growfs -n (information on current filesystem, no change) and
> > > bare xfs_growfs (i.e. actually grow it ...) right?  And same man page
> > > for both, specifying "shall be given a mount point to operate on."
> > > 
> > > > Also wondering if this test ought to keep expanding $testfile one AG at
> > > > a time prior to every growfs test so that test success is keyed off of
> > > > seeing a "data blocks changed from XXX to YYY" message, just in case the
> > > > layout ever shifts...
> > > 
> > > *shrug* using xfs_growfs -n will test the "does it pass or fail based on
> > > on whether it's given a mountpoint" and the actual geometry output won't
> > > really matter much...
> > > 
> > > Growing it is fine too, but it may not be needed.  As long as it's consistent,
> > > I think.
> > 
> > I could make it do that, but yeah, my goal was to merely test
> > the command response behavior, not concerning the test wih the
> > machinations of growfs under the hood.
> 
> Ok, that's fine then. :)

Thanks for reviewing. The test does need a bit of work. I'll post a v2.
-Bill

> 
> --D
> 
> > 
> > Thanks-
> > Bill
> > 
> > > --
> > > 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