aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndriy Gapon <avg@FreeBSD.org>2019-09-19 09:43:56 +0000
committerAndriy Gapon <avg@FreeBSD.org>2019-09-19 09:43:56 +0000
commit6caa629e735a800bf034dc5821c2516bb35ce28b (patch)
tree1e96544eccd2dea67865701190c79a170da7ac83
parent9a2ed10014863e84ee404f8b62dcbdbd7f01fb45 (diff)
downloadsrc-6caa629e735a800bf034dc5821c2516bb35ce28b.tar.gz
src-6caa629e735a800bf034dc5821c2516bb35ce28b.zip
fix dsl_scan_ds_clone_swapped logic
It was incorrect with respect to swapping dataset IDs both in the on-disk ZAP object and the in-memory queue. In both cases, if only ds1 was already present, then it would be first replaced with ds2 and then ds2 would be replaced back with ds1. Also, both cases did not properly handle a situation where both ds1 and ds2 are already queued. A duplicate insertion would be attempted and its failure would result in a panic. This change has also been submitted to ZoL as zfsonlinux/zfs@dd262c9 PR: 239566 Reported by: pascal.guitierrez@gmail.com MFC after: 4 days Sponsored by: CyberSecure
Notes
Notes: svn path=/head/; revision=352506
-rw-r--r--sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_scan.c100
1 files changed, 69 insertions, 31 deletions
diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_scan.c b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_scan.c
index a32df14e7507..2966df66da85 100644
--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_scan.c
+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_scan.c
@@ -2014,16 +2014,17 @@ ds_clone_swapped_bookmark(dsl_dataset_t *ds1, dsl_dataset_t *ds2,
}
/*
- * Called when a parent dataset and its clone are swapped. If we were
+ * Called when an origin dataset and its clone are swapped. If we were
* currently traversing the dataset, we need to switch to traversing the
- * newly promoted parent.
+ * newly promoted clone.
*/
void
dsl_scan_ds_clone_swapped(dsl_dataset_t *ds1, dsl_dataset_t *ds2, dmu_tx_t *tx)
{
dsl_pool_t *dp = ds1->ds_dir->dd_pool;
dsl_scan_t *scn = dp->dp_scan;
- uint64_t mintxg;
+ uint64_t mintxg1, mintxg2;
+ boolean_t ds1_queued, ds2_queued;
if (!dsl_scan_is_running(scn))
return;
@@ -2031,44 +2032,81 @@ dsl_scan_ds_clone_swapped(dsl_dataset_t *ds1, dsl_dataset_t *ds2, dmu_tx_t *tx)
ds_clone_swapped_bookmark(ds1, ds2, &scn->scn_phys.scn_bookmark);
ds_clone_swapped_bookmark(ds1, ds2, &scn->scn_phys_cached.scn_bookmark);
- if (scan_ds_queue_contains(scn, ds1->ds_object, &mintxg)) {
- scan_ds_queue_remove(scn, ds1->ds_object);
- scan_ds_queue_insert(scn, ds2->ds_object, mintxg);
+ /*
+ * Handle the in-memory scan queue.
+ */
+ ds1_queued = scan_ds_queue_contains(scn, ds1->ds_object, &mintxg1);
+ ds2_queued = scan_ds_queue_contains(scn, ds2->ds_object, &mintxg2);
+
+ /* Sanity checking. */
+ if (ds1_queued) {
+ ASSERT3U(mintxg1, ==, dsl_dataset_phys(ds1)->ds_prev_snap_txg);
+ ASSERT3U(mintxg1, ==, dsl_dataset_phys(ds2)->ds_prev_snap_txg);
+ }
+ if (ds2_queued) {
+ ASSERT3U(mintxg2, ==, dsl_dataset_phys(ds1)->ds_prev_snap_txg);
+ ASSERT3U(mintxg2, ==, dsl_dataset_phys(ds2)->ds_prev_snap_txg);
}
- if (scan_ds_queue_contains(scn, ds2->ds_object, &mintxg)) {
+
+ if (ds1_queued && ds2_queued) {
+ /*
+ * If both are queued, we don't need to do anything.
+ * The swapping code below would not handle this case correctly,
+ * since we can't insert ds2 if it is already there. That's
+ * because scan_ds_queue_insert() prohibits a duplicate insert
+ * and panics.
+ */
+ } else if (ds1_queued) {
+ scan_ds_queue_remove(scn, ds1->ds_object);
+ scan_ds_queue_insert(scn, ds2->ds_object, mintxg1);
+ } else if (ds2_queued) {
scan_ds_queue_remove(scn, ds2->ds_object);
- scan_ds_queue_insert(scn, ds1->ds_object, mintxg);
+ scan_ds_queue_insert(scn, ds1->ds_object, mintxg2);
}
- if (zap_lookup_int_key(dp->dp_meta_objset, scn->scn_phys.scn_queue_obj,
- ds1->ds_object, &mintxg) == 0) {
- int err;
- ASSERT3U(mintxg, ==, dsl_dataset_phys(ds1)->ds_prev_snap_txg);
- ASSERT3U(mintxg, ==, dsl_dataset_phys(ds2)->ds_prev_snap_txg);
- VERIFY3U(0, ==, zap_remove_int(dp->dp_meta_objset,
+ /*
+ * Handle the on-disk scan queue.
+ * The on-disk state is an out-of-date version of the in-memory state,
+ * so the in-memory and on-disk values for ds1_queued and ds2_queued may
+ * be different. Therefore we need to apply the swap logic to the
+ * on-disk state independently of the in-memory state.
+ */
+ ds1_queued = zap_lookup_int_key(dp->dp_meta_objset,
+ scn->scn_phys.scn_queue_obj, ds1->ds_object, &mintxg1) == 0;
+ ds2_queued = zap_lookup_int_key(dp->dp_meta_objset,
+ scn->scn_phys.scn_queue_obj, ds2->ds_object, &mintxg2) == 0;
+
+ /* Sanity checking. */
+ if (ds1_queued) {
+ ASSERT3U(mintxg1, ==, dsl_dataset_phys(ds1)->ds_prev_snap_txg);
+ ASSERT3U(mintxg1, ==, dsl_dataset_phys(ds2)->ds_prev_snap_txg);
+ }
+ if (ds2_queued) {
+ ASSERT3U(mintxg2, ==, dsl_dataset_phys(ds1)->ds_prev_snap_txg);
+ ASSERT3U(mintxg2, ==, dsl_dataset_phys(ds2)->ds_prev_snap_txg);
+ }
+
+ if (ds1_queued && ds2_queued) {
+ /*
+ * If both are queued, we don't need to do anything.
+ * Alternatively, we could check for EEXIST from
+ * zap_add_int_key() and back out to the original state, but
+ * that would be more work than checking for this case upfront.
+ */
+ } else if (ds1_queued) {
+ VERIFY3S(0, ==, zap_remove_int(dp->dp_meta_objset,
scn->scn_phys.scn_queue_obj, ds1->ds_object, tx));
- err = zap_add_int_key(dp->dp_meta_objset,
- scn->scn_phys.scn_queue_obj, ds2->ds_object, mintxg, tx);
- VERIFY(err == 0 || err == EEXIST);
- if (err == EEXIST) {
- /* Both were there to begin with */
- VERIFY(0 == zap_add_int_key(dp->dp_meta_objset,
- scn->scn_phys.scn_queue_obj,
- ds1->ds_object, mintxg, tx));
- }
+ VERIFY3S(0, ==, zap_add_int_key(dp->dp_meta_objset,
+ scn->scn_phys.scn_queue_obj, ds2->ds_object, mintxg1, tx));
zfs_dbgmsg("clone_swap ds %llu; in queue; "
"replacing with %llu",
(u_longlong_t)ds1->ds_object,
(u_longlong_t)ds2->ds_object);
- }
- if (zap_lookup_int_key(dp->dp_meta_objset, scn->scn_phys.scn_queue_obj,
- ds2->ds_object, &mintxg) == 0) {
- ASSERT3U(mintxg, ==, dsl_dataset_phys(ds1)->ds_prev_snap_txg);
- ASSERT3U(mintxg, ==, dsl_dataset_phys(ds2)->ds_prev_snap_txg);
- VERIFY3U(0, ==, zap_remove_int(dp->dp_meta_objset,
+ } else if (ds2_queued) {
+ VERIFY3S(0, ==, zap_remove_int(dp->dp_meta_objset,
scn->scn_phys.scn_queue_obj, ds2->ds_object, tx));
- VERIFY(0 == zap_add_int_key(dp->dp_meta_objset,
- scn->scn_phys.scn_queue_obj, ds1->ds_object, mintxg, tx));
+ VERIFY3S(0, ==, zap_add_int_key(dp->dp_meta_objset,
+ scn->scn_phys.scn_queue_obj, ds1->ds_object, mintxg2, tx));
zfs_dbgmsg("clone_swap ds %llu; in queue; "
"replacing with %llu",
(u_longlong_t)ds2->ds_object,