zfs: add PR 6695
don't accidently skip over objects which should be freed
This commit is contained in:
		
							parent
							
								
									e2f4edc81e
								
							
						
					
					
						commit
						f2be26ec96
					
				
							
								
								
									
										203
									
								
								zfs-patches/0010-include-upstream-PR-6695.patch
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										203
									
								
								zfs-patches/0010-include-upstream-PR-6695.patch
									
									
									
									
									
										Normal file
									
								
							| @ -0,0 +1,203 @@ | ||||
| From c47a0ade3f46171a3c1e459c379550b6dc3a0913 Mon Sep 17 00:00:00 2001 | ||||
| From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= <f.gruenbichler@proxmox.com> | ||||
| Date: Thu, 12 Oct 2017 12:28:32 +0200 | ||||
| Subject: [PATCH 10/11] include upstream PR #6695 | ||||
| MIME-Version: 1.0 | ||||
| Content-Type: text/plain; charset=UTF-8 | ||||
| Content-Transfer-Encoding: 8bit | ||||
| 
 | ||||
| receive_freeobjects() skips freeing some objects | ||||
| 
 | ||||
| Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> | ||||
| ---
 | ||||
|  ...ve_freeobjects-skips-freeing-some-objects.patch | 168 +++++++++++++++++++++ | ||||
|  debian/patches/series                              |   1 + | ||||
|  2 files changed, 169 insertions(+) | ||||
|  create mode 100644 debian/patches/0010-receive_freeobjects-skips-freeing-some-objects.patch | ||||
| 
 | ||||
| diff --git a/debian/patches/0010-receive_freeobjects-skips-freeing-some-objects.patch b/debian/patches/0010-receive_freeobjects-skips-freeing-some-objects.patch
 | ||||
| new file mode 100644 | ||||
| index 000000000..3470be026
 | ||||
| --- /dev/null
 | ||||
| +++ b/debian/patches/0010-receive_freeobjects-skips-freeing-some-objects.patch
 | ||||
| @@ -0,0 +1,168 @@
 | ||||
| +From: Ned Bass <bass6@llnl.gov>
 | ||||
| +Date: Mon, 2 Oct 2017 15:36:04 -0700
 | ||||
| +Subject: 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
 | ||||
| +
 | ||||
| +(backported from commit 39f56627ae988d09b4e3803c01c22b2026b2310e)
 | ||||
| +
 | ||||
| +Conflicts:
 | ||||
| +	tests/runfiles/linux.run
 | ||||
| +---
 | ||||
| + tests/zfs-tests/tests/functional/rsend/Makefile.am |  3 +-
 | ||||
| + module/zfs/dmu_send.c                              |  6 +-
 | ||||
| + tests/runfiles/linux.run                           |  2 +-
 | ||||
| + .../tests/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/tests/zfs-tests/tests/functional/rsend/Makefile.am b/tests/zfs-tests/tests/functional/rsend/Makefile.am
 | ||||
| +index b9f8dba..60ca776 100644
 | ||||
| +--- a/tests/zfs-tests/tests/functional/rsend/Makefile.am
 | ||||
| ++++ b/tests/zfs-tests/tests/functional/rsend/Makefile.am
 | ||||
| +@@ -37,4 +37,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/module/zfs/dmu_send.c b/module/zfs/dmu_send.c
 | ||||
| +index a2ace9b..0c53ced 100644
 | ||||
| +--- a/module/zfs/dmu_send.c
 | ||||
| ++++ b/module/zfs/dmu_send.c
 | ||||
| +@@ -2233,12 +2233,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 a12fc2d..5583a25 100644
 | ||||
| +--- a/tests/runfiles/linux.run
 | ||||
| ++++ b/tests/runfiles/linux.run
 | ||||
| +@@ -501,7 +501,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-c_recv_dedup', 'send_freeobjects']
 | ||||
| + 
 | ||||
| + [tests/functional/scrub_mirror]
 | ||||
| + tests = ['scrub_mirror_001_pos', 'scrub_mirror_002_pos',
 | ||||
| +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 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"
 | ||||
| diff --git a/debian/patches/series b/debian/patches/series
 | ||||
| index d6f6f87f9..3c8f56483 100644
 | ||||
| --- a/debian/patches/series
 | ||||
| +++ b/debian/patches/series
 | ||||
| @@ -7,3 +7,4 @@ enable-zed.patch
 | ||||
|  0008-dracut-make-module-setup.sh-shebang-explicit.patch | ||||
|  0009-add-man-page-reference-to-systemd-units.patch | ||||
|  0010-fix-install-path-of-zpool.d-scripts.patch | ||||
| +0010-receive_freeobjects-skips-freeing-some-objects.patch
 | ||||
| -- 
 | ||||
| 2.14.1 | ||||
| 
 | ||||
| @ -6,3 +6,4 @@ | ||||
| 0007-fix-install-path-of-zpool.d-scripts.patch | ||||
| 0008-zfsutils-linux-install-new-files.patch | ||||
| 0009-dh_install-switch-to-fail-missing.patch | ||||
| 0010-include-upstream-PR-6695.patch | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user
	 Fabian Grünbichler
						Fabian Grünbichler