From b48502fe1a0fe681f5ae605f67954585d6507a09 Mon Sep 17 00:00:00 2001 From: Andriy Gapon Date: Wed, 2 Nov 2016 17:33:22 +0000 Subject: [PATCH 1/6] 5752 dump_nvlist() is not aware of boolean array illumos/illumos-gate@ee3c499ad1e4fc884a11b2bc6490787b788bf84a https://github.com/illumos/illumos-gate/commit/ee3c499ad1e4fc884a11b2bc6490787b788bf84a https://www.illumos.org/issues/5752 dump_nvlist() is not aware of the boolean array value type: bad config type 24 for "foobar" Reviewed by: Dan Kimmel Reviewed by: Matthew Ahrens Reviewed by: Will Andrews Approved by: Robert Mustacchi Author: Andriy Gapon --- lib/libnvpair/libnvpair.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/libnvpair/libnvpair.c b/lib/libnvpair/libnvpair.c index 0415568e9de9..d4d20000280b 100644 --- a/lib/libnvpair/libnvpair.c +++ b/lib/libnvpair/libnvpair.c @@ -794,6 +794,7 @@ dump_nvlist(nvlist_t *list, int indent) { nvpair_t *elem = NULL; boolean_t bool_value; + boolean_t *bool_array_value; nvlist_t *nvlist_value; nvlist_t **nvlist_array_value; uint_t i, count; @@ -854,6 +855,16 @@ dump_nvlist(nvlist_t *list, int indent) NVP(elem, string, char *, char *, "'%s'"); break; + case DATA_TYPE_BOOLEAN_ARRAY: + (void) nvpair_value_boolean_array(elem, + &bool_array_value, &count); + for (i = 0; i < count; i++) { + (void) printf("%*s%s[%d]: %s\n", indent, "", + nvpair_name(elem), i, + bool_array_value[i] ? "true" : "false"); + } + break; + case DATA_TYPE_BYTE_ARRAY: NVPA(elem, byte_array, uchar_t, int, "%u"); break; From 3033242f0e72cba7aed30cf8654537afc909011d Mon Sep 17 00:00:00 2001 From: Andriy Gapon Date: Wed, 2 Nov 2016 17:34:33 +0000 Subject: [PATCH 2/6] 5778 nvpair_type_is_array() does not recognize DATA_TYPE_INT8_ARRAY illumos/illumos-gate@bf4d553b8a4685dc5ba4549cc9ba6d94e9306a81 https://github.com/illumos/illumos-gate/commit/bf4d553b8a4685dc5ba4549cc9ba6d94e9306a81 https://www.illumos.org/issues/5778 DATA_TYPE_INT8_ARRAY is missing from the array check in nvpair_type_is_array() Reviewed by: Matthew Ahrens Reviewed by: Prakash Surya Approved by: Dan McDonald Author: Andriy Gapon --- common/nvpair/nvpair.c | 1 + 1 file changed, 1 insertion(+) diff --git a/common/nvpair/nvpair.c b/common/nvpair/nvpair.c index 9b5d51334805..691de5d61b69 100644 --- a/common/nvpair/nvpair.c +++ b/common/nvpair/nvpair.c @@ -1230,6 +1230,7 @@ nvpair_type_is_array(nvpair_t *nvp) data_type_t type = NVP_TYPE(nvp); if ((type == DATA_TYPE_BYTE_ARRAY) || + (type == DATA_TYPE_INT8_ARRAY) || (type == DATA_TYPE_UINT8_ARRAY) || (type == DATA_TYPE_INT16_ARRAY) || (type == DATA_TYPE_UINT16_ARRAY) || From 94ce2dcaaba02bd9cd77dcf238a977527cabdc39 Mon Sep 17 00:00:00 2001 From: Andriy Gapon Date: Tue, 22 Nov 2016 11:46:22 +0000 Subject: [PATCH 3/6] 6412 zfs receive: -u can be ignored sometimes illumos/illumos-gate@9185393f2919d8b897f6142d8a9fa0429c285dc1 https://github.com/illumos/illumos-gate/commit/9185393f2919d8b897f6142d8a9fa0429c285dc1 https://www.illumos.org/issues/6412 It seems that zfs receive -F -u would mount a received filesystem after receiving a full stream if a destination filesystem already existed (and, thus, got destroyed and re-created) and was mounted. How to reproduce: $ zfs create rpool/sandbox $ zfs create rpool/sandbox/from $ zfs create rpool/sandbox/to $ zfs snap rpool/sandbox/from@snap $ zfs send rpool/sandbox/from@snap | zfs recv -v -F -u rpool/sandbox/to receiving full stream of rpool/sandbox/from@snap into rpool/sandbox/to@snap received 41.7KB stream in 1 seconds (41.7KB/sec) $ zfs get mounted rpool/sandbox/to NAME PROPERTY VALUE SOURCE rpool/tmp/sandbox/to mounted yes - This behavior can be problematic if the mountpoint property changes either because it had a non-inherited value or the stream contains properties because it has been generated with either -R or -p. Reviewed by: Matthew Ahrens Reviewed by: Paul Dagnelie Approved by: Richard Lowe Author: Andriy Gapon --- lib/libzfs/common/libzfs_sendrecv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/libzfs/common/libzfs_sendrecv.c b/lib/libzfs/common/libzfs_sendrecv.c index 70114fa81219..4d376a407590 100644 --- a/lib/libzfs/common/libzfs_sendrecv.c +++ b/lib/libzfs/common/libzfs_sendrecv.c @@ -3533,7 +3533,8 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap, } if (clp) { - err |= changelist_postfix(clp); + if (!flags->nomount) + err |= changelist_postfix(clp); changelist_free(clp); } From d414bfe23b6528291ff88a2eb45d6e14b63fc6a5 Mon Sep 17 00:00:00 2001 From: Andriy Gapon Date: Tue, 22 Nov 2016 11:47:27 +0000 Subject: [PATCH 4/6] 7180 potential race between zfs_suspend_fs+zfs_resume_fs and zfs_ioc_rename illumos/illumos-gate@690041b9caf801816f2d0bac90bc7cecefb73523 https://github.com/illumos/illumos-gate/commit/690041b9caf801816f2d0bac90bc7cecefb73523 https://www.illumos.org/issues/7180 If a filesystem is not unmounted while the rename is being performed, then, for example, a concurrect zfs rollback may call zfs_suspend_fs followed by zfs_resume_fs on the same filesystem. The latter takes the filesystem's name as an argument. If the filesystem name changes as a result of the rename, then dmu_objset_hold(osname, zfsvfs, &os) call in zfs_resume_fs would fail resulting in a kernel panic. So far I have been able to reproduce this problem on FreeBSD where zfs rename has -u option that skips the unmounting before doing the renaming. But I think that in theory the same problem can occur on illumos as well, because the unmounting is done in userland before invoking the rename ioctl and there could be a race with, e.g., zfs mount. panic: solaris assert: dmu_objset_hold(osname, zfsvfs, &zfsvfs->z_os) == 0 (0x2 == 0x0), file: /usr/devel/svn/head/sys/cddl/contrib/opensolaris/uts/common/fs/ zfs/zfs_vfsops.c, line: 2210 KDB: stack backtrace: db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe004df30710 vpanic() at vpanic+0x182/frame 0xfffffe004df30790 panic() at panic+0x43/frame 0xfffffe004df307f0 assfail3() at assfail3+0x2c/frame 0xfffffe004df30810 zfs_resume_fs() at zfs_resume_fs+0xb9/frame 0xfffffe004df30860 zfs_ioc_rollback() at zfs_ioc_rollback+0x61/frame 0xfffffe004df308a0 zfsdev_ioctl() at zfsdev_ioctl+0x65c/frame 0xfffffe004df30940 devfs_ioctl_f() at devfs_ioctl_f+0x156/frame 0xfffffe004df309a0 kern_ioctl() at kern_ioctl+0x246/frame 0xfffffe004df30a00 sys_ioctl() at sys_ioctl+0x171/frame 0xfffffe004df30ae0 amd64_syscall() at amd64_syscall+0x2db/frame 0xfffffe004df30bf0 Xfast_syscall() at Xfast_syscall+0xfb/frame 0xfffffe004df30bf0 Reviewed by: Matt Ahrens Reviewed by: Pavel Zakharov Approved by: Richard Lowe Author: Andriy Gapon --- uts/common/fs/zfs/sys/zfs_vfsops.h | 2 +- uts/common/fs/zfs/zfs_ioctl.c | 14 +++++++++++--- uts/common/fs/zfs/zfs_vfsops.c | 13 ++++++------- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/uts/common/fs/zfs/sys/zfs_vfsops.h b/uts/common/fs/zfs/sys/zfs_vfsops.h index b382d27bdfd8..3278171372ba 100644 --- a/uts/common/fs/zfs/sys/zfs_vfsops.h +++ b/uts/common/fs/zfs/sys/zfs_vfsops.h @@ -137,7 +137,7 @@ typedef struct zfid_long { extern uint_t zfs_fsyncer_key; extern int zfs_suspend_fs(zfsvfs_t *zfsvfs); -extern int zfs_resume_fs(zfsvfs_t *zfsvfs, const char *osname); +extern int zfs_resume_fs(zfsvfs_t *zfsvfs, struct dsl_dataset *ds); extern int zfs_userspace_one(zfsvfs_t *zfsvfs, zfs_userquota_prop_t type, const char *domain, uint64_t rid, uint64_t *valuep); extern int zfs_userspace_many(zfsvfs_t *zfsvfs, zfs_userquota_prop_t type, diff --git a/uts/common/fs/zfs/zfs_ioctl.c b/uts/common/fs/zfs/zfs_ioctl.c index 48be8dceb906..e03c19d99b04 100644 --- a/uts/common/fs/zfs/zfs_ioctl.c +++ b/uts/common/fs/zfs/zfs_ioctl.c @@ -3645,12 +3645,15 @@ zfs_ioc_rollback(const char *fsname, nvlist_t *args, nvlist_t *outnvl) int error; if (getzfsvfs(fsname, &zfsvfs) == 0) { + dsl_dataset_t *ds; + + ds = dmu_objset_ds(zfsvfs->z_os); error = zfs_suspend_fs(zfsvfs); if (error == 0) { int resume_err; error = dsl_dataset_rollback(fsname, zfsvfs, outnvl); - resume_err = zfs_resume_fs(zfsvfs, fsname); + resume_err = zfs_resume_fs(zfsvfs, ds); error = error ? error : resume_err; } VFS_RELE(zfsvfs->z_vfs); @@ -4275,8 +4278,10 @@ zfs_ioc_recv(zfs_cmd_t *zc) if (getzfsvfs(tofs, &zfsvfs) == 0) { /* online recv */ + dsl_dataset_t *ds; int end_err; + ds = dmu_objset_ds(zfsvfs->z_os); error = zfs_suspend_fs(zfsvfs); /* * If the suspend fails, then the recv_end will @@ -4284,7 +4289,7 @@ zfs_ioc_recv(zfs_cmd_t *zc) */ end_err = dmu_recv_end(&drc, zfsvfs); if (error == 0) - error = zfs_resume_fs(zfsvfs, tofs); + error = zfs_resume_fs(zfsvfs, ds); error = error ? error : end_err; VFS_RELE(zfsvfs->z_vfs); } else { @@ -4808,11 +4813,14 @@ zfs_ioc_userspace_upgrade(zfs_cmd_t *zc) * objset needs to be closed & reopened (to grow the * objset_phys_t). Suspend/resume the fs will do that. */ + dsl_dataset_t *ds; + + ds = dmu_objset_ds(zfsvfs->z_os); error = zfs_suspend_fs(zfsvfs); if (error == 0) { dmu_objset_refresh_ownership(zfsvfs->z_os, zfsvfs); - error = zfs_resume_fs(zfsvfs, zc->zc_name); + error = zfs_resume_fs(zfsvfs, ds); } } if (error == 0) diff --git a/uts/common/fs/zfs/zfs_vfsops.c b/uts/common/fs/zfs/zfs_vfsops.c index 7b5d87d6ae50..e9821ff9329f 100644 --- a/uts/common/fs/zfs/zfs_vfsops.c +++ b/uts/common/fs/zfs/zfs_vfsops.c @@ -2020,7 +2020,7 @@ zfs_suspend_fs(zfsvfs_t *zfsvfs) * zfsvfs, held, and long held on entry. */ int -zfs_resume_fs(zfsvfs_t *zfsvfs, const char *osname) +zfs_resume_fs(zfsvfs_t *zfsvfs, dsl_dataset_t *ds) { int err; znode_t *zp; @@ -2029,14 +2029,13 @@ zfs_resume_fs(zfsvfs_t *zfsvfs, const char *osname) ASSERT(RW_WRITE_HELD(&zfsvfs->z_teardown_inactive_lock)); /* - * We already own this, so just hold and rele it to update the - * objset_t, as the one we had before may have been evicted. + * We already own this, so just update the objset_t, as the one we + * had before may have been evicted. */ objset_t *os; - VERIFY0(dmu_objset_hold(osname, zfsvfs, &os)); - VERIFY3P(os->os_dsl_dataset->ds_owner, ==, zfsvfs); - VERIFY(dsl_dataset_long_held(os->os_dsl_dataset)); - dmu_objset_rele(os, zfsvfs); + VERIFY3P(ds->ds_owner, ==, zfsvfs); + VERIFY(dsl_dataset_long_held(ds)); + VERIFY0(dmu_objset_from_ds(ds, &os)); err = zfsvfs_init(zfsvfs, os); if (err != 0) From 7f74889d89a07719a32fdfccd9948d809d9a7fc3 Mon Sep 17 00:00:00 2001 From: Andriy Gapon Date: Tue, 22 Nov 2016 11:49:55 +0000 Subject: [PATCH 5/6] 7199, 7200 dsl_dataset_rollback_sync may try to free already free blocks 7199 dsl_dataset_rollback_sync may try to free already free blocks 7200 no blocks must be born in a txg after a snaphot is created illumos/illumos-gate@bfaed0b91e57062c38bc16b4f89db3c8f0052a9b https://github.com/illumos/illumos-gate/commit/bfaed0b91e57062c38bc16b4f89db3c8f0052a9b https://www.illumos.org/issues/7199 dsl_dataset_rollback_sync may try to free already freed blocks when it calls dsl_destroy_head_sync_impl to destroy a temporary clone. That happens if a snapshot to which we are rolling back and from which the clone is created has some ZIL records. https://www.illumos.org/issues/7200 No new blocks must be born in a dataset in the same TXG after a snapshot of the dataset is taken. Those blocks would have the same blk_birth as the dataset's ds_prev_snap_txg and as such they would be presumed to belong o the snapshot while in fact they do not. All the datasets must be clean before sync tasks are run, so the described scenario may happen only if one of the sync tasks dirties the dataset and another sync task takes its snapshot. Then, there will be another sync pass because of the dirty data and the new blocks will be born in the same TXG when the data is written out. It seems that almost all of the existing sync tasks modify only MOS and do not dirty any objsets. The only exception that I've been able to identify so far is the rollback which can modify an objset when it zeroes out the objset's ZIL. Reviewed by: Matthew Ahrens Reviewed by: Brad Lewis Approved by: Gordon Ross Author: Andriy Gapon --- uts/common/fs/zfs/dsl_dataset.c | 62 ++++++++++++++++++++++++++--- uts/common/fs/zfs/dsl_pool.c | 14 +------ uts/common/fs/zfs/sys/dsl_dataset.h | 1 + 3 files changed, 59 insertions(+), 18 deletions(-) diff --git a/uts/common/fs/zfs/dsl_dataset.c b/uts/common/fs/zfs/dsl_dataset.c index b143b3900ae7..0b033ca52bf7 100644 --- a/uts/common/fs/zfs/dsl_dataset.c +++ b/uts/common/fs/zfs/dsl_dataset.c @@ -81,6 +81,8 @@ extern inline dsl_dataset_phys_t *dsl_dataset_phys(dsl_dataset_t *ds); extern int spa_asize_inflation; +static zil_header_t zero_zil; + /* * Figure out how much of this delta should be propogated to the dsl_dir * layer. If there's a refreservation, that space has already been @@ -125,6 +127,7 @@ dsl_dataset_block_born(dsl_dataset_t *ds, const blkptr_t *bp, dmu_tx_t *tx) return; } + ASSERT3U(bp->blk_birth, >, dsl_dataset_phys(ds)->ds_prev_snap_txg); dmu_buf_will_dirty(ds->ds_dbuf, tx); mutex_enter(&ds->ds_lock); delta = parent_delta(ds, used); @@ -889,8 +892,20 @@ dsl_dataset_zero_zil(dsl_dataset_t *ds, dmu_tx_t *tx) objset_t *os; VERIFY0(dmu_objset_from_ds(ds, &os)); - bzero(&os->os_zil_header, sizeof (os->os_zil_header)); - dsl_dataset_dirty(ds, tx); + if (bcmp(&os->os_zil_header, &zero_zil, sizeof (zero_zil)) != 0) { + dsl_pool_t *dp = ds->ds_dir->dd_pool; + zio_t *zio; + + bzero(&os->os_zil_header, sizeof (os->os_zil_header)); + + zio = zio_root(dp->dp_spa, NULL, NULL, ZIO_FLAG_MUSTSUCCEED); + dsl_dataset_sync(ds, zio, tx); + VERIFY0(zio_wait(zio)); + + /* dsl_dataset_sync_done will drop this reference. */ + dmu_buf_add_ref(ds->ds_dbuf, ds); + dsl_dataset_sync_done(ds, tx); + } } uint64_t @@ -1030,8 +1045,10 @@ dsl_dataset_dirty(dsl_dataset_t *ds, dmu_tx_t *tx) if (dsl_dataset_phys(ds)->ds_next_snap_obj != 0) panic("dirtying snapshot!"); - dp = ds->ds_dir->dd_pool; + /* Must not dirty a dataset in the same txg where it got snapshotted. */ + ASSERT3U(tx->tx_txg, >, dsl_dataset_phys(ds)->ds_prev_snap_txg); + dp = ds->ds_dir->dd_pool; if (txg_list_add(&dp->dp_dirty_datasets, ds, tx->tx_txg)) { /* up the hold count until we can be written out */ dmu_buf_add_ref(ds->ds_dbuf, ds); @@ -1286,8 +1303,6 @@ void dsl_dataset_snapshot_sync_impl(dsl_dataset_t *ds, const char *snapname, dmu_tx_t *tx) { - static zil_header_t zero_zil; - dsl_pool_t *dp = ds->ds_dir->dd_pool; dmu_buf_t *dbuf; dsl_dataset_phys_t *dsphys; @@ -1306,6 +1321,10 @@ dsl_dataset_snapshot_sync_impl(dsl_dataset_t *ds, const char *snapname, bcmp(&os->os_phys->os_zil_header, &zero_zil, sizeof (zero_zil)) == 0); + /* Should not snapshot a dirty dataset. */ + ASSERT(!txg_list_member(&ds->ds_dir->dd_pool->dp_dirty_datasets, + ds, tx->tx_txg)); + dsl_fs_ss_count_adjust(ds->ds_dir, 1, DD_FIELD_SNAPSHOT_COUNT, tx); /* @@ -1652,6 +1671,27 @@ dsl_dataset_sync(dsl_dataset_t *ds, zio_t *zio, dmu_tx_t *tx) } } +static int +deadlist_enqueue_cb(void *arg, const blkptr_t *bp, dmu_tx_t *tx) +{ + dsl_deadlist_t *dl = arg; + dsl_deadlist_insert(dl, bp, tx); + return (0); +} + +void +dsl_dataset_sync_done(dsl_dataset_t *ds, dmu_tx_t *tx) +{ + objset_t *os = ds->ds_objset; + + bplist_iterate(&ds->ds_pending_deadlist, + deadlist_enqueue_cb, &ds->ds_deadlist, tx); + + ASSERT(!dmu_objset_is_dirty(os, dmu_tx_get_txg(tx))); + + dmu_buf_rele(ds->ds_dbuf, ds); +} + static void get_clones_stat(dsl_dataset_t *ds, nvlist_t *nv) { @@ -2151,6 +2191,18 @@ dsl_dataset_rollback_check(void *arg, dmu_tx_t *tx) return (SET_ERROR(EINVAL)); } + /* + * No rollback to a snapshot created in the current txg, because + * the rollback may dirty the dataset and create blocks that are + * not reachable from the rootbp while having a birth txg that + * falls into the snapshot's range. + */ + if (dmu_tx_is_syncing(tx) && + dsl_dataset_phys(ds)->ds_prev_snap_txg >= tx->tx_txg) { + dsl_dataset_rele(ds, FTAG); + return (SET_ERROR(EAGAIN)); + } + /* must not have any bookmarks after the most recent snapshot */ nvlist_t *proprequest = fnvlist_alloc(); fnvlist_add_boolean(proprequest, zfs_prop_to_name(ZFS_PROP_CREATETXG)); diff --git a/uts/common/fs/zfs/dsl_pool.c b/uts/common/fs/zfs/dsl_pool.c index 35d7d5b42107..9e23c7a62896 100644 --- a/uts/common/fs/zfs/dsl_pool.c +++ b/uts/common/fs/zfs/dsl_pool.c @@ -424,14 +424,6 @@ dsl_pool_mos_diduse_space(dsl_pool_t *dp, mutex_exit(&dp->dp_lock); } -static int -deadlist_enqueue_cb(void *arg, const blkptr_t *bp, dmu_tx_t *tx) -{ - dsl_deadlist_t *dl = arg; - dsl_deadlist_insert(dl, bp, tx); - return (0); -} - static void dsl_pool_sync_mos(dsl_pool_t *dp, dmu_tx_t *tx) { @@ -532,11 +524,7 @@ dsl_pool_sync(dsl_pool_t *dp, uint64_t txg) * - release hold from dsl_dataset_dirty() */ while ((ds = list_remove_head(&synced_datasets)) != NULL) { - objset_t *os = ds->ds_objset; - bplist_iterate(&ds->ds_pending_deadlist, - deadlist_enqueue_cb, &ds->ds_deadlist, tx); - ASSERT(!dmu_objset_is_dirty(os, txg)); - dmu_buf_rele(ds->ds_dbuf, ds); + dsl_dataset_sync_done(ds, tx); } while ((dd = txg_list_remove(&dp->dp_dirty_dirs, txg)) != NULL) { dsl_dir_sync(dd, tx); diff --git a/uts/common/fs/zfs/sys/dsl_dataset.h b/uts/common/fs/zfs/sys/dsl_dataset.h index 18466bb405e7..22c67b48a915 100644 --- a/uts/common/fs/zfs/sys/dsl_dataset.h +++ b/uts/common/fs/zfs/sys/dsl_dataset.h @@ -274,6 +274,7 @@ boolean_t dsl_dataset_modified_since_snap(dsl_dataset_t *ds, dsl_dataset_t *snap); void dsl_dataset_sync(dsl_dataset_t *os, zio_t *zio, dmu_tx_t *tx); +void dsl_dataset_sync_done(dsl_dataset_t *os, dmu_tx_t *tx); void dsl_dataset_block_born(dsl_dataset_t *ds, const blkptr_t *bp, dmu_tx_t *tx); From 5041986419d5eca8cf10dbfb5341d123ec1608a8 Mon Sep 17 00:00:00 2001 From: Andriy Gapon Date: Tue, 22 Nov 2016 11:50:52 +0000 Subject: [PATCH 6/6] 6428 set canmount=off on unmounted filesystem tries to unmount children illumos/illumos-gate@c079fa4d202eff15e318131c52755d214ffa2da7 https://github.com/illumos/illumos-gate/commit/c079fa4d202eff15e318131c52755d214ffa2da7 https://www.illumos.org/issues/6428 Scenario: $ zfs create rpool/p $ zfs set canmount=noauto rpool/p $ zfs umount rpool/p $ zfs create rpool/p/c $ zfs get -r mounted,canmount rpool/p NAME PROPERTY VALUE SOURCE rpool/p mounted no - rpool/p canmount noauto local rpool/p/c mounted yes - rpool/p/c canmount on default In another shell ensure that rpool/p/c is in use, for example: $ cd /rpool/p/c Then: $ zfs set canmount=off rpool/p cannot unmount '/rpool/p/c': Device busy But there is no reason to try to unmount rpool/p/c in this scenario. Reviewed by: Matthew Ahrens Approved by: Gordon Ross Author: Andriy Gapon --- lib/libzfs/common/libzfs_dataset.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/libzfs/common/libzfs_dataset.c b/lib/libzfs/common/libzfs_dataset.c index aa557ae80b8a..7327c2999133 100644 --- a/lib/libzfs/common/libzfs_dataset.c +++ b/lib/libzfs/common/libzfs_dataset.c @@ -1597,8 +1597,9 @@ zfs_prop_set_list(zfs_handle_t *zhp, nvlist_t *props) * its canmount property to 'on' or 'noauto'. We only use * the changelist logic to unmount when setting canmount=off. */ - if (!(prop == ZFS_PROP_CANMOUNT && - fnvpair_value_uint64(elem) != ZFS_CANMOUNT_OFF)) { + if (prop != ZFS_PROP_CANMOUNT || + (fnvpair_value_uint64(elem) == ZFS_CANMOUNT_OFF && + zfs_is_mounted(zhp, NULL))) { cls[cl_idx] = changelist_gather(zhp, prop, 0, 0); if (cls[cl_idx] == NULL) goto error;