From 948423a3d1b6b396b46c8edbee9d77515a62589b Mon Sep 17 00:00:00 2001 From: Matthew Ahrens Date: Fri, 31 Jul 2020 08:59:00 -0700 Subject: [PATCH] zfs promote does not delete livelist of origin When a clone is promoted, its livelist is no longer accurate, so it is discarded. If the clone's origin is also a clone (i.e. we are promoting a clone of a clone), then the origin's livelist is also no longer accurate, so it should be discarded, but the code doesn't actually do that. Consider a pool with: * Filesystem A * Clone B, a clone of A * Clone C, a clone of B If we promote C, it discards C's livelist. It should discard B's livelist, but that is not happening. The impact is that when B is destroyed, we use the livelist to find the blocks to free, but the livelist is no longer correct so we end up freeing blocks that are still in use by C. The incorrectly-freed blocks can be reallocated causing checksum errors. And when C is destroyed it can double-free the incorrectly-freed blocks. The problem is that we remove the livelist of `origin_ds->ds_dir`, but the origin snapshot has already been moved to the promoted dsl_dir. So this is actually trying to remove the livelist of the promoted dsl_dir, which was already removed. As explained in a comment in the beginning of `dsl_dataset_promote_sync()`, we need to use the saved `odd` for the origin's dsl_dir. Reviewed-by: Pavel Zakharov Reviewed-by: Brian Behlendorf Reviewed-by: George Wilson Reviewed by: Sara Hartse Signed-off-by: Matthew Ahrens Closes #10652 --- module/zfs/dsl_dataset.c | 4 +-- .../zfs_destroy_clone_livelist.ksh | 26 ++++++++++++++++++- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c index c04f47d36..aac8bf76e 100644 --- a/module/zfs/dsl_dataset.c +++ b/module/zfs/dsl_dataset.c @@ -21,7 +21,7 @@ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2011, 2018 by Delphix. All rights reserved. + * Copyright (c) 2011, 2020 by Delphix. All rights reserved. * Copyright (c) 2014, Joyent, Inc. All rights reserved. * Copyright (c) 2014 RackTop Systems. * Copyright (c) 2014 Spectra Logic Corporation, All rights reserved. @@ -3735,7 +3735,7 @@ dsl_dataset_promote_sync(void *arg, dmu_tx_t *tx) * as well. */ dsl_dir_remove_livelist(dd, tx, B_TRUE); - dsl_dir_remove_livelist(origin_ds->ds_dir, tx, B_TRUE); + dsl_dir_remove_livelist(odd, tx, B_TRUE); /* log history record */ spa_history_log_internal_ds(hds, "promote", tx, " "); diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_destroy/zfs_destroy_clone_livelist.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_destroy/zfs_destroy_clone_livelist.ksh index 6d8984fe9..2da58ec96 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_destroy/zfs_destroy_clone_livelist.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_destroy/zfs_destroy_clone_livelist.ksh @@ -11,7 +11,7 @@ # # -# Copyright (c) 2018 by Delphix. All rights reserved. +# Copyright (c) 2018, 2020 by Delphix. All rights reserved. # # DESCRIPTION @@ -32,6 +32,7 @@ # - same as 1. but with multiple clones # 4. Multiple clones with populated livelists # - same as 2. but with multiple clones +# 5. Clone of clone with populated livelists with promote . $STF_SUITE/include/libtest.shlib . $STF_SUITE/tests/functional/cli_root/zfs_destroy/zfs_destroy_common.kshlib @@ -120,6 +121,28 @@ function test_promote log_must zfs destroy -R $TESTPOOL/$TESTCLONE } +function test_clone_clone_promote +{ + log_must zfs create $TESTPOOL/fs + log_must dd if=/dev/zero of=/$TESTPOOL/fs/file bs=128k count=100 + log_must zfs snapshot $TESTPOOL/fs@snap + log_must zfs clone $TESTPOOL/fs@snap $TESTPOOL/clone + log_must dd if=/dev/zero of=/$TESTPOOL/clone/clonefile bs=128k count=10 + log_must zfs snapshot $TESTPOOL/clone@csnap + log_must zfs clone $TESTPOOL/clone@csnap $TESTPOOL/cloneclone + + check_livelist_exists clone + check_livelist_exists cloneclone + + # Promote should remove both clones' livelists + log_must zfs promote $TESTPOOL/cloneclone + check_livelist_gone + + # This destroy should not use a livelist + log_must zfs destroy $TESTPOOL/clone + log_must zdb -bcc $TESTPOOL +} + ORIGINAL_MAX=$(get_tunable LIVELIST_MAX_ENTRIES) log_onexit cleanup @@ -135,6 +158,7 @@ test_one test_multiple_empty test_multiple test_promote +test_clone_clone_promote log_pass "Clone with the livelist feature enabled could be destroyed," \ "also could be promoted and destroyed as expected."