From d79b4b3e4ad722bf457efe9401d7f267a8dfcc6c Mon Sep 17 00:00:00 2001 From: Ned Bass Date: Thu, 28 Sep 2017 17:13:13 -0700 Subject: [PATCH] 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. Fixes #6694 Signed-off-by: Ned Bass --- module/zfs/dmu_send.c | 6 +- tests/runfiles/linux.run | 2 +- .../tests/functional/rsend/Makefile.am | 3 +- .../functional/rsend/send_freeobjects.ksh | 81 +++++++++++++++++++ 4 files changed, 86 insertions(+), 6 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/rsend/send_freeobjects.ksh diff --git a/module/zfs/dmu_send.c b/module/zfs/dmu_send.c index 359ef99d35fd..fc63b6e1a6a8 100644 --- 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 b4853586a6c3..d425406b761f 100644 --- 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 199acb68bb68..8833d1d760fe 100644 --- 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 index 000000000000..6533352a9a14 --- /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"