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

[Eryu Guan <eguan@redhat.com>]

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.

Some comments and/or commit log to clarify that might be good :)

Thanks,
Eryu