receive_freeobjects() skips freeing some objects

When receiving a FREEOBJECTS record, receive_freeobjects()
incorrectly skips a freed object in some cases. Specifically, this
happens when the first object in the range to be freed doesn't exist,
but the second object does. This leaves an object allocated on disk
on the receiving side which is unallocated on the sending side, which
may cause receiving subsequent incremental streams to fail.

The bug was caused by an incorrect increment of the object index
variable when current object being freed doesn't exist.  The
increment is incorrect because incrementing the object index is
handled by a call to dmu_object_next() in the increment portion of
the for loop statement.

Add test case that exposes this bug.

Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ned Bass <bass6@llnl.gov>
Closes #6694
Closes #6695

diff --git a/module/zfs/dmu_send.c b/module/zfs/dmu_send.c
index 359ef99d35fdb243269dd0f79978955e6489c319..fc63b6e1a6a887aa86e482f7932d18ba866f3d8a 100644 (file)
--- a/module/zfs/dmu_send.c
+++ b/module/zfs/dmu_send.c
@@ -2563,12 +2563,10 @@ receive_freeobjects(struct receive_writer_arg *rwa,
                int err;
 
                err = dmu_object_info(rwa->os, obj, &doi);
-               if (err == ENOENT) {
-                       obj++;
+               if (err == ENOENT)
                        continue;
-               } else if (err != 0) {
+               else if (err != 0)
                        return (err);
-               }
 
                err = dmu_free_long_object(rwa->os, obj);
                if (err != 0)

diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run
index b4853586a6c3e72610d6ba0e2389f421662ce84b..d425406b761f1360187fbe0339e26be10c9af8e1 100644 (file)
--- a/tests/runfiles/linux.run
+++ b/tests/runfiles/linux.run
@@ -525,7 +525,7 @@ tests = ['rsend_001_pos', 'rsend_002_pos', 'rsend_003_pos', 'rsend_004_pos',
     'send-c_lz4_disabled', 'send-c_recv_lz4_disabled',
     'send-c_mixed_compression', 'send-c_stream_size_estimate', 'send-cD',
     'send-c_embedded_blocks', 'send-c_resume', 'send-cpL_varied_recsize',
-    'send-c_recv_dedup', 'send_encrypted_heirarchy']
+    'send-c_recv_dedup', 'send_encrypted_heirarchy', 'send_freeobjects']
 
 [tests/functional/scrub_mirror]
 tests = ['scrub_mirror_001_pos', 'scrub_mirror_002_pos',

diff --git a/tests/zfs-tests/tests/functional/rsend/Makefile.am b/tests/zfs-tests/tests/functional/rsend/Makefile.am
index 199acb68bb685c5a997602069405eb2a5d1eca43..8833d1d760fef698a4bb31bf56e3c9aec0c38377 100644 (file)
--- a/tests/zfs-tests/tests/functional/rsend/Makefile.am
+++ b/tests/zfs-tests/tests/functional/rsend/Makefile.am
@@ -38,4 +38,5 @@ dist_pkgdata_SCRIPTS = \
        send-c_verify_ratio.ksh \
        send-c_volume.ksh \
        send-c_zstreamdump.ksh \
-       send-cpL_varied_recsize.ksh
+       send-cpL_varied_recsize.ksh \
+       send_freeobjects.ksh

diff --git a/tests/zfs-tests/tests/functional/rsend/send_freeobjects.ksh b/tests/zfs-tests/tests/functional/rsend/send_freeobjects.ksh
new file mode 100755 (executable)
index 0000000..6533352
--- /dev/null
+++ b/tests/zfs-tests/tests/functional/rsend/send_freeobjects.ksh
@@ -0,0 +1,81 @@
+#!/bin/ksh
+
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright (c) 2017 by Lawrence Livermore National Security, LLC.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/rsend/rsend.kshlib
+
+#
+# Description:
+# Verify FREEOBJECTS record frees sequential objects (See
+# https://github.com/zfsonlinux/zfs/issues/6694)
+#
+# Strategy:
+# 1. Create three files with sequential object numbers, f1 f2 and f3
+# 2. Delete f2
+# 3. Take snapshot A
+# 4. Delete f3
+# 5. Take snapshot B
+# 6. Receive a full send of A
+# 7. Receive an incremental send of B
+# 8. Fail test if f3 exists on received snapshot B
+#
+
+verify_runnable "both"
+
+log_assert "Verify FREEOBJECTS record frees sequential objects"
+
+sendds=sendfo
+recvds=recvfo
+f1=/$POOL/$sendds/f1
+f2=/$POOL/$sendds/f2
+f3=/$POOL/$sendds/f3
+
+#
+# We need to set xattr=sa and dnodesize=legacy to guarantee sequential
+# object numbers for this test. Otherwise, if we used directory-based
+# xattrs, SELinux extended attributes might consume intervening object
+# numbers.
+#
+log_must zfs create -o xattr=sa -o dnodesize=legacy $POOL/$sendds
+
+tries=100
+for ((i=0; i<$tries; i++)); do
+       touch $f1 $f2 $f3
+       o1=$(ls -li $f1 | awk '{print $1}')
+       o2=$(ls -li $f2 | awk '{print $1}')
+       o3=$(ls -li $f3 | awk '{print $1}')
+
+       if [[ $o2 -ne $(( $o1 + 1 )) ]] || [[ $o3 -ne $(( $o2 + 1 )) ]]; then
+               rm -f $f1 $f2 $f3
+       else
+               break
+       fi
+done
+
+if [[ $i -eq $tries ]]; then
+       log_fail "Failed to create three sequential objects"
+fi
+
+log_must rm $f2
+log_must zfs snap $POOL/$sendds@A
+log_must rm $f3
+log_must zfs snap $POOL/$sendds@B
+log_must eval "zfs send $POOL/$sendds@A | zfs recv $POOL/$recvds"
+log_must eval "zfs send -i $POOL/$sendds@A $POOL/$sendds@B |" \
+       "zfs recv $POOL/$recvds"
+log_mustnot zdb $POOL/$recvds@B $o3
+log_pass "Verify FREEOBJECTS record frees sequential objects"