]> git.proxmox.com Git - mirror_zfs.git/commitdiff
ZTS: switch to rsync for directory diffs
authorAleksa Sarai <cyphar@cyphar.com>
Tue, 1 Mar 2022 18:05:32 +0000 (05:05 +1100)
committerGitHub <noreply@github.com>
Tue, 1 Mar 2022 18:05:32 +0000 (10:05 -0800)
While "diff -r" is the most straightforward way of comparing directory
trees for differences, it has two major issues:

 * File metadata is not compared, which means that subtle bugs may be
   missed even if a test is written that exercises the buggy behaviour.
 * diff(1) doesn't know how to compare special files -- it assumes they
   are always different, which means that a test using diff(1) on
   special files will always fail (resulting in such tests not being
   added).

rsync can be used in a very similar manner to diff (with the -ni flags),
but has the additional benefit of being able to detect and resolve many
more differences between directory trees. In addition, rsync has a
standard set of features and flags while diffs feature set depends on
whether you're using GNU or BSD binutils.

Note that for several of the test cases we expect that file timestamps
will not match. For example, the ctime for a file creation or modify
event is stored in the intent log but not the mtime. Thus when replaying
the log the correct ctime is set but the current mtime is used. This is
the expected behavior, so to prevent these tests from failing, there's a
replay_directory_diff function which ignores those kinds of changes.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Closes #12588

16 files changed:
.github/workflows/build-dependencies.txt
tests/zfs-tests/include/commands.cfg
tests/zfs-tests/include/libtest.shlib
tests/zfs-tests/tests/functional/cli_root/zfs_receive/zfs_receive_010_pos.ksh
tests/zfs-tests/tests/functional/cli_root/zfs_send/zfs_send_007_pos.ksh
tests/zfs-tests/tests/functional/cli_root/zfs_snapshot/zfs_snapshot_009_pos.ksh
tests/zfs-tests/tests/functional/cli_root/zpool_import/zpool_import_errata4.ksh
tests/zfs-tests/tests/functional/features/large_dnode/large_dnode_005_pos.ksh
tests/zfs-tests/tests/functional/redacted_send/redacted_incrementals.ksh
tests/zfs-tests/tests/functional/redacted_send/redacted_largeblocks.ksh
tests/zfs-tests/tests/functional/rsend/recv_dedup.ksh
tests/zfs-tests/tests/functional/rsend/rsend.kshlib
tests/zfs-tests/tests/functional/slog/slog_replay_fs_001.ksh
tests/zfs-tests/tests/functional/slog/slog_replay_fs_002.ksh
tests/zfs-tests/tests/functional/snapshot/snapshot_002_pos.ksh
tests/zfs-tests/tests/functional/snapshot/snapshot_006_pos.ksh

index 1dc745d5f6429cbba2578e57e9c1dba20444e27c..e591399001f57a97e29cc794b4f59e849d49b811 100644 (file)
@@ -38,6 +38,7 @@ python3-dev
 python3-packaging
 python3-setuptools
 rng-tools
+rsync
 samba
 sysstat
 uuid-dev
index 51052e801e8701d38ddba6909a42b965a860d296..fa3d12d669f05086b096343a3113f538cbac8fdf 100644 (file)
@@ -77,6 +77,7 @@ export SYSTEM_FILES_COMMON='arp
     readlink
     rm
     rmdir
+    rsync
     scp
     script
     sed
index b229a161518be2f163b285dc8ba03176b3c77e5f..bb8cb5c8c7b11bae559463480e355094491249ac 100644 (file)
@@ -4303,3 +4303,86 @@ function wait_for_children #children
        done
        return $rv
 }
+
+#
+# Compare two directory trees recursively in a manner similar to diff(1), but
+# using rsync. If there are any discrepancies, a summary of the differences are
+# output and a non-zero error is returned.
+#
+# If you're comparing a directory after a ZIL replay, you should set
+# LIBTEST_DIFF_ZIL_REPLAY=1 or use replay_directory_diff which will cause
+# directory_diff to ignore mtime changes (the ZIL replay won't fix up mtime
+# information).
+#
+function directory_diff # dir_a dir_b
+{
+       dir_a="$1"
+       dir_b="$2"
+       zil_replay="${LIBTEST_DIFF_ZIL_REPLAY:-0}"
+
+       # If one of the directories doesn't exist, return 2. This is to match the
+       # semantics of diff.
+       if ! [ -d "$dir_a" -a -d "$dir_b" ]; then
+               return 2
+       fi
+
+       # Run rsync with --dry-run --itemize-changes to get something akin to diff
+       # output, but rsync is far more thorough in detecting differences (diff
+       # doesn't compare file metadata, and cannot handle special files).
+       #
+       # Also make sure to filter out non-user.* xattrs when comparing. On
+       # SELinux-enabled systems the copied tree will probably have different
+       # SELinux labels.
+       args=("-nicaAHX" '--filter=-x! user.*' "--delete")
+
+       # NOTE: Quite a few rsync builds do not support --crtimes which would be
+       # necessary to verify that creation times are being maintained properly.
+       # Unfortunately because of this we cannot use it unconditionally but we can
+       # check if this rsync build supports it and use it then. This check is
+       # based on the same check in the rsync test suite (testsuite/crtimes.test).
+       #
+       # We check ctimes even with zil_replay=1 because the ZIL does store
+       # creation times and we should make sure they match (if the creation times
+       # do not match there is a "c" entry in one of the columns).
+       if ( rsync --version | grep -q "[, ] crtimes" >/dev/null ); then
+               args+=("--crtimes")
+       else
+               echo "NOTE: This rsync package does not support --crtimes (-N)."
+       fi
+
+       # If we are testing a ZIL replay, we need to ignore timestamp changes.
+       # Unfortunately --no-times doesn't do what we want -- it will still tell
+       # you if the timestamps don't match but rsync will set the timestamps to
+       # the current time (leading to an itemised change entry). It's simpler to
+       # just filter out those lines.
+       if [ "$zil_replay" -eq 0 ]; then
+               filter=("cat")
+       else
+               # Different rsync versions have different numbers of columns. So just
+               # require that aside from the first two, all other columns must be
+               # blank (literal ".") or a timestamp field ("[tT]").
+               filter=("grep" "-v" '^\..[.Tt]\+ ')
+       fi
+
+       diff="$(rsync "${args[@]}" "$dir_a/" "$dir_b/" | "${filter[@]}")"
+       rv=0
+       if [ -n "$diff" ]; then
+               echo "$diff"
+               rv=1
+       fi
+       return $rv
+}
+
+#
+# Compare two directory trees recursively, without checking whether the mtimes
+# match (creation times will be checked if the available rsync binary supports
+# it). This is necessary for ZIL replay checks (because the ZIL does not
+# contain mtimes and thus after a ZIL replay, mtimes won't match).
+#
+# This is shorthand for LIBTEST_DIFF_ZIL_REPLAY=1 directory_diff <...>.
+#
+function replay_directory_diff # dir_a dir_b
+{
+       LIBTEST_DIFF_ZIL_REPLAY=1 directory_diff "$@"
+       return $?
+}
index 84485977aa235944097885e8bd30b19d6fad2e4f..47e23e6ebca7fb87611dd0460003e719de9498a3 100755 (executable)
@@ -166,12 +166,12 @@ log_must zfs destroy -fr $rfs
 cat $TESTDIR/zr010p | log_must zfs receive -o origin=$fs2@s1 $rfs
 mntpnt_old=$(get_prop mountpoint $fs)
 mntpnt_new=$(get_prop mountpoint $rfs)
-log_must diff -r $mntpnt_old $mntpnt_new
+log_must directory_diff $mntpnt_old $mntpnt_new
 log_must zfs destroy -r $rfs
 
 cat $TESTDIR/zr010p2 | log_must zfs receive -o origin=$fs@s1 $rfs
 mntpnt_old=$(get_prop mountpoint $fs2)
 mntpnt_new=$(get_prop mountpoint $rfs)
-log_must diff -r $mntpnt_old $mntpnt_new
+log_must directory_diff $mntpnt_old $mntpnt_new
 
 log_pass "zfs receive of full send as clone works"
index 675afa72f5af46a4b07bca271a2814106020cdda..306fabc8cef4a76f7891ab600293808718135fd8 100755 (executable)
@@ -83,7 +83,7 @@ test_pool ()
        cat $streamfile | log_must zfs receive $POOL/recvfs
 
        recv_mntpnt=$(get_prop mountpoint "$POOL/recvfs")
-       log_must diff -r $mntpnt $recv_mntpnt
+       log_must directory_diff $mntpnt $recv_mntpnt
        log_must zfs destroy -rf $POOL/fs
        log_must zfs destroy -rf $POOL/recvfs
 }
index 6fedba9e5b276287d0c3e20fa55a39e03bcfb259..4ff539a377d4549c5a374b7256bf3df71014e992 100755 (executable)
@@ -93,11 +93,8 @@ for i in 1 2 3; do
 done
 log_note "verify snapshot contents"
 for ds in $datasets; do
-       diff -q -r /$ds /$ds/.zfs/snapshot/snap > /dev/null 2>&1
-       if [[ $? -eq 1 ]]; then
-               log_fail "snapshot contents are different from" \
-                   "the filesystem"
-       fi
+       [ -d "/$ds/.zfs/snapshot/snap" ] && \
+               log_must directory_diff /$ds /$ds/.zfs/snapshot/snap
 done
 
 # We subtract 3 + 7 + 7 + 1 = 18 for three slashes (/), strlen("TESTFSA") == 7,
index a0f063a8dc83fc59a14581dcced140f6d1f01c00..a199c2a7fb9d4d23ac4a9786e29d32250ea70225 100755 (executable)
@@ -122,7 +122,7 @@ block_device_wait
 
 old_mntpnt=$(get_prop mountpoint $POOL_NAME/testfs)
 new_mntpnt=$(get_prop mountpoint $POOL_NAME/fixed/testfs)
-log_must diff -r "$old_mntpnt" "$new_mntpnt"
+log_must directory_diff "$old_mntpnt" "$new_mntpnt"
 log_must diff /dev/zvol/$POOL_NAME/testvol /dev/zvol/$POOL_NAME/fixed/testvol
 
 log_must has_ivset_guid $POOL_NAME/fixed/testfs@snap1
index 2be98942634f3545cbb427eb501e1580c2843f96..03e2db4b808208295fc7f38c24cd60f69a390457 100755 (executable)
@@ -68,7 +68,7 @@ if [[ "$dnsize" != "1K" ]]; then
 fi
 
 log_must eval "zfs recv -F $TEST_RECV_FS < $TEST_STREAMINCR"
-log_must diff -r /$TEST_SEND_FS /$TEST_RECV_FS
+log_must directory_diff /$TEST_SEND_FS /$TEST_RECV_FS
 log_must zfs umount $TEST_SEND_FS
 log_must zfs umount $TEST_RECV_FS
 
index 1d2ed3a687beefc2b66b45bc8000a5d44f332c61..18ea5d68fcf8aad0112c8f81e21a124d8293d7cf 100755 (executable)
@@ -51,10 +51,10 @@ log_must eval "zfs receive $POOL2/rfs <$stream"
 # Verify receipt of normal incrementals to redaction list members.
 log_must eval "zfs send -i $sendfs@snap0 $POOL/stride3@snap >$stream"
 log_must eval "zfs recv $POOL2/rstride3 <$stream"
-log_must diff -r /$POOL/stride3 /$POOL2/rstride3
+log_must directory_diff /$POOL/stride3 /$POOL2/rstride3
 log_must eval "zfs send -i $sendfs@snap0 $POOL/stride5@snap >$stream"
 log_must eval "zfs recv $POOL2/rstride5 <$stream"
-log_must diff -r /$POOL/stride5 /$POOL2/rstride5
+log_must directory_diff /$POOL/stride5 /$POOL2/rstride5
 
 # But not a normal child that we weren't redacted with respect to.
 log_must eval "zfs send -i $sendfs@snap0 $POOL/hole@snap >$stream"
@@ -73,7 +73,7 @@ log_must mount_redacted -f $POOL2/rint
 # Verify we can receive grandchildren on the child.
 log_must eval "zfs send -i $POOL/int@snap $POOL/rm@snap >$stream"
 log_must eval "zfs receive $POOL2/rrm <$stream"
-log_must diff -r /$POOL/rm /$POOL2/rrm
+log_must directory_diff /$POOL/rm /$POOL2/rrm
 
 # But not a grandchild that the received child wasn't redacted with respect to.
 log_must eval "zfs send -i $POOL/int@snap $POOL/write@snap >$stream"
@@ -92,13 +92,13 @@ log_mustnot zfs redact $POOL/int@snap book6 $POOL/hole@snap
 # Verify we can receive a full clone of the grandchild on the child.
 log_must eval "zfs send $POOL/write@snap >$stream"
 log_must eval "zfs recv -o origin=$POOL2/rint@snap $POOL2/rwrite <$stream"
-log_must diff -r /$POOL/write /$POOL2/rwrite
+log_must directory_diff /$POOL/write /$POOL2/rwrite
 
 # Along with other origins.
 log_must eval "zfs recv -o origin=$POOL2/rfs@snap0 $POOL2/rwrite1 <$stream"
-log_must diff -r /$POOL/write /$POOL2/rwrite1
+log_must directory_diff /$POOL/write /$POOL2/rwrite1
 log_must eval "zfs recv -o origin=$POOL2@init $POOL2/rwrite2 <$stream"
-log_must diff -r /$POOL/write /$POOL2/rwrite2
+log_must directory_diff /$POOL/write /$POOL2/rwrite2
 log_must zfs destroy -R $POOL2/rwrite2
 
 log_must zfs destroy -R $POOL2/rfs
@@ -140,7 +140,7 @@ unmount_redacted $POOL2/rfs
 # sending from the bookmark.
 log_must eval "zfs send -i $sendfs#book7 $POOL/hole1@snap >$stream"
 log_must eval "zfs recv $POOL2/rhole1 <$stream"
-log_must diff -r /$POOL/hole1 /$POOL2/rhole1
+log_must directory_diff /$POOL/hole1 /$POOL2/rhole1
 
 # Verify we can receive an intermediate clone redacted with respect to a
 # non-subset if we send from the bookmark.
index caccdd360061277e8e0cab822a8c4d515bc37d43..ef85bb31dcea86dd3a9b7c8b2498255e987221b6 100755 (executable)
@@ -58,6 +58,6 @@ unmount_redacted $recvfs
 log_must eval "zfs send -L -i $sendfs@snap $clone@snap1 >$stream"
 log_must stream_has_features $stream large_blocks
 log_must eval "zfs recv $recvfs/new <$stream"
-log_must diff -r $clone_mnt $recv_mnt/new
+log_must directory_diff $clone_mnt $recv_mnt/new
 
 log_pass "Large blocks and redacted send work correctly together."
index e6e282a1c6f8b0d817d4830f8e3461e6bdbb9b28..ba23c4f6aa3cfcc122624a2ca51c1dda996b02cf 100755 (executable)
@@ -48,6 +48,7 @@ log_must eval "zstream redup $sendfile | zfs recv -d $TESTPOOL/recv"
 
 log_must mkdir /$TESTPOOL/tar
 log_must tar --directory /$TESTPOOL/tar -xzf $tarfile
-log_must diff -r /$TESTPOOL/tar /$TESTPOOL/recv
+# The recv'd filesystem is called "/fs", so only compare that subdirectory.
+log_must directory_diff /$TESTPOOL/tar/fs /$TESTPOOL/recv/fs
 
 log_pass "zfs can receive dedup send streams with 'zstream redup'"
index 516d41263294e65a6e6bc2922228877e345dcb73..2c8085f226b08d7e888b0b46ef174c0720f1c76a 100644 (file)
@@ -212,7 +212,7 @@ function cmp_ds_cont
        srcdir=$(get_prop mountpoint $src_fs)
        dstdir=$(get_prop mountpoint $dst_fs)
 
-       diff -r $srcdir $dstdir > /dev/null 2>&1
+       replay_directory_diff $srcdir $dstdir
        return $?
 }
 
@@ -627,12 +627,12 @@ function file_check
 
        if [[ -d /$recvfs/.zfs/snapshot/a && -d \
            /$sendfs/.zfs/snapshot/a ]]; then
-               diff -r /$recvfs/.zfs/snapshot/a /$sendfs/.zfs/snapshot/a
+               directory_diff /$recvfs/.zfs/snapshot/a /$sendfs/.zfs/snapshot/a
                [[ $? -eq 0 ]] || log_fail "Differences found in snap a"
        fi
        if [[ -d /$recvfs/.zfs/snapshot/b && -d \
            /$sendfs/.zfs/snapshot/b ]]; then
-               diff -r /$recvfs/.zfs/snapshot/b /$sendfs/.zfs/snapshot/b
+               directory_diff /$recvfs/.zfs/snapshot/b /$sendfs/.zfs/snapshot/b
                [[ $? -eq 0 ]] || log_fail "Differences found in snap b"
        fi
 }
index 0b78a099f0b951003faac465275163d9a6b509dc..0b78a9645a60294e2dc4cff4f5a7f34b269f3b6d 100755 (executable)
@@ -178,8 +178,8 @@ log_must rm /$TESTPOOL/$TESTFS/link_and_unlink.link
 #
 # 4. Copy TESTFS to temporary location (TESTDIR/copy)
 #
-log_must mkdir -p $TESTDIR/copy
-log_must cp -a /$TESTPOOL/$TESTFS/* $TESTDIR/copy/
+log_must mkdir -p $TESTDIR
+log_must rsync -aHAX /$TESTPOOL/$TESTFS/ $TESTDIR/copy
 
 #
 # 5. Unmount filesystem and export the pool
@@ -213,7 +213,7 @@ log_must ls_xattr /$TESTPOOL/$TESTFS/xattr.dir
 log_must ls_xattr /$TESTPOOL/$TESTFS/xattr.file
 
 log_note "Verify working set diff:"
-log_must diff -r /$TESTPOOL/$TESTFS $TESTDIR/copy
+log_must replay_directory_diff $TESTDIR/copy /$TESTPOOL/$TESTFS
 
 log_note "Verify file checksum:"
 typeset checksum1=$(sha256digest /$TESTPOOL/$TESTFS/payload)
index 3c3ccdf4ad23ee9103f9b7c5eb9b701395d9f162..50ccc60893229694153d085b4125c8d4c7713b4a 100755 (executable)
@@ -98,8 +98,8 @@ log_must eval 'for i in $(seq $NFILES); do zfs set dnodesize=${dnsize[$RANDOM %
 #
 # 4. Copy TESTFS to temporary location (TESTDIR/copy)
 #
-log_must mkdir -p $TESTDIR/copy
-log_must cp -a /$TESTPOOL/$TESTFS/* $TESTDIR/copy/
+log_must mkdir -p $TESTDIR
+log_must rsync -aHAX /$TESTPOOL/$TESTFS/ $TESTDIR/copy
 
 #
 # 5. Unmount filesystem and export the pool
@@ -132,6 +132,6 @@ log_note "Verify number of files"
 log_must test "$(ls /$TESTPOOL/$TESTFS/dir0 | wc -l)" -eq $NFILES
 
 log_note "Verify working set diff:"
-log_must diff -r /$TESTPOOL/$TESTFS $TESTDIR/copy
+log_must replay_directory_diff $TESTDIR/copy /$TESTPOOL/$TESTFS
 
 log_pass "Replay of intent log succeeds."
index 124a7db9c6e612252e94846963cdc793ec1a1410..42fbbd9a7a2beaef0343e8e46158258c89499b3c 100755 (executable)
@@ -55,24 +55,28 @@ function cleanup
                cd $CWD || log_fail "Could not cd $CWD"
        fi
 
-        snapexists $SNAPFS
-        if [[ $? -eq 0 ]]; then
-                log_must zfs destroy $SNAPFS
-        fi
+       snapexists $SNAPFS
+       if [[ $? -eq 0 ]]; then
+               log_must zfs destroy $SNAPFS
+       fi
 
-        if [[ -e $SNAPDIR ]]; then
-                log_must rm -rf $SNAPDIR > /dev/null 2>&1
-        fi
+       if [[ -e $SNAPDIR ]]; then
+               log_must rm -rf $SNAPDIR > /dev/null 2>&1
+       fi
 
-        if [[ -e $TESTDIR ]]; then
-                log_must rm -rf $TESTDIR/* > /dev/null 2>&1
-        fi
+       if [[ -e $TESTDIR ]]; then
+               log_must rm -rf $TESTDIR/* > /dev/null 2>&1
+       fi
 
+       if [[ -d "$SNAPSHOT_TARDIR" ]]; then
+               log_must rm -rf $SNAPSHOT_TARDIR > /dev/null 2>&1
+       fi
 }
 
 log_assert "Verify an archive of a file system is identical to " \
     "an archive of its snapshot."
 
+SNAPSHOT_TARDIR="$(mktemp -d /tmp/zfstests_snapshot_002.XXXXXX)"
 log_onexit cleanup
 
 typeset -i COUNT=21
@@ -87,14 +91,13 @@ typeset i=1
 while [ $i -lt $COUNT ]; do
        log_must file_write -o $OP -f $TESTDIR/file$i \
            -b $BLOCKSZ -c $NUM_WRITES -d $DATA
-
        (( i = i + 1 ))
 done
 
 log_note "Create a tarball from $TESTDIR contents..."
 CWD=$PWD
 cd $TESTDIR || log_fail "Could not cd $TESTDIR"
-log_must tar cf $TESTDIR/tarball.original.tar file*
+log_must tar cf $SNAPSHOT_TARDIR/original.tar .
 cd $CWD || log_fail "Could not cd $CWD"
 
 log_note "Create a snapshot and mount it..."
@@ -106,7 +109,7 @@ log_must rm -f $TESTDIR/file* > /dev/null 2>&1
 log_note "Create tarball of snapshot..."
 CWD=$PWD
 cd $SNAPDIR || log_fail "Could not cd $SNAPDIR"
-log_must tar cf $TESTDIR/tarball.snapshot.tar file*
+log_must tar cf $SNAPSHOT_TARDIR/snapshot.tar .
 cd $CWD || log_fail "Could not cd $CWD"
 
 log_must mkdir $TESTDIR/original
@@ -114,16 +117,12 @@ log_must mkdir $TESTDIR/snapshot
 
 CWD=$PWD
 cd $TESTDIR/original || log_fail "Could not cd $TESTDIR/original"
-log_must tar xf $TESTDIR/tarball.original.tar
+log_must tar xf $SNAPSHOT_TARDIR/original.tar
 
 cd $TESTDIR/snapshot || log_fail "Could not cd $TESTDIR/snapshot"
-log_must tar xf $TESTDIR/tarball.snapshot.tar
+log_must tar xf $SNAPSHOT_TARDIR/snapshot.tar
 
 cd $CWD || log_fail "Could not cd $CWD"
 
-diff -q -r $TESTDIR/original $TESTDIR/snapshot > /dev/null 2>&1
-if [[ $? -eq 1 ]]; then
-       log_fail "Directory structures differ."
-fi
-
+log_must directory_diff $TESTDIR/original $TESTDIR/snapshot
 log_pass "Directory structures match."
index 68a616c02a6c4ef5fedd1e58957abfdda908480d..d2a304670981d202c3d512f1117b0bc16f87dfbd 100755 (executable)
@@ -54,24 +54,28 @@ function cleanup
                cd $CWD || log_fail "Could not cd $CWD"
        fi
 
-        snapexists $SNAPCTR
-        if [[ $? -eq 0 ]]; then
-                log_must zfs destroy $SNAPCTR
-        fi
+       snapexists $SNAPCTR
+       if [[ $? -eq 0 ]]; then
+               log_must zfs destroy $SNAPCTR
+       fi
 
-        if [[ -e $SNAPDIR1 ]]; then
-                log_must rm -rf $SNAPDIR1 > /dev/null 2>&1
-        fi
+       if [[ -e $SNAPDIR1 ]]; then
+               log_must rm -rf $SNAPDIR1 > /dev/null 2>&1
+       fi
 
-        if [[ -e $TESTDIR1 ]]; then
-                log_must rm -rf $TESTDIR1/* > /dev/null 2>&1
-        fi
+       if [[ -e $TESTDIR1 ]]; then
+               log_must rm -rf $TESTDIR1/* > /dev/null 2>&1
+       fi
 
+       if [[ -d "$SNAPSHOT_TARDIR" ]]; then
+               log_must rm -rf $SNAPSHOT_TARDIR > /dev/null 2>&1
+       fi
 }
 
 log_assert "Verify that an archive of a dataset is identical to " \
    "an archive of the dataset's snapshot."
 
+SNAPSHOT_TARDIR="$(mktemp -d /tmp/zfstests_snapshot_006.XXXXXX)"
 log_onexit cleanup
 
 typeset -i COUNT=21
@@ -85,14 +89,13 @@ typeset i=1
 while [ $i -lt $COUNT ]; do
        log_must file_write -o $OP -f $TESTDIR1/file$i \
            -b $BLOCKSZ -c $NUM_WRITES -d $DATA
-
        (( i = i + 1 ))
 done
 
 log_note "Create a tarball from $TESTDIR1 contents..."
 CWD=$PWD
 cd $TESTDIR1 || log_fail "Could not cd $TESTDIR1"
-log_must tar cf $TESTDIR1/tarball.original.tar file*
+log_must tar cf $SNAPSHOT_TARDIR/original.tar .
 cd $CWD || log_fail "Could not cd $CWD"
 
 log_note "Create a snapshot and mount it..."
@@ -104,7 +107,7 @@ log_must rm -f $TESTDIR1/file* > /dev/null 2>&1
 log_note "Create tarball of snapshot..."
 CWD=$PWD
 cd $SNAPDIR1 || log_fail "Could not cd $SNAPDIR1"
-log_must tar cf $TESTDIR1/tarball.snapshot.tar file*
+log_must tar cf $SNAPSHOT_TARDIR/snapshot.tar .
 cd $CWD || log_fail "Could not cd $CWD"
 
 log_must mkdir $TESTDIR1/original
@@ -112,16 +115,12 @@ log_must mkdir $TESTDIR1/snapshot
 
 CWD=$PWD
 cd $TESTDIR1/original || log_fail "Could not cd $TESTDIR1/original"
-log_must tar xf $TESTDIR1/tarball.original.tar
+log_must tar xf $SNAPSHOT_TARDIR/original.tar
 
 cd $TESTDIR1/snapshot || log_fail "Could not cd $TESTDIR1/snapshot"
-log_must tar xf $TESTDIR1/tarball.snapshot.tar
+log_must tar xf $SNAPSHOT_TARDIR/snapshot.tar
 
 cd $CWD || log_fail "Could not cd $CWD"
 
-diff -q -r $TESTDIR1/original $TESTDIR1/snapshot > /dev/null 2>&1
-if [[ $? -eq 1 ]]; then
-       log_fail "Directory structures differ."
-fi
-
+log_must directory_diff $TESTDIR1/original $TESTDIR1/snapshot
 log_pass "Directory structures match."