From owner-svn-src-all@freebsd.org Wed Feb 21 15:12:15 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 69353F0AAAB; Wed, 21 Feb 2018 15:12:15 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 08889807E6; Wed, 21 Feb 2018 15:12:15 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 038861A2FC; Wed, 21 Feb 2018 15:12:15 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w1LFCEiK023001; Wed, 21 Feb 2018 15:12:14 GMT (envelope-from avg@FreeBSD.org) Received: (from avg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w1LFCEc6022998; Wed, 21 Feb 2018 15:12:14 GMT (envelope-from avg@FreeBSD.org) Message-Id: <201802211512.w1LFCEc6022998@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: avg set sender to avg@FreeBSD.org using -f From: Andriy Gapon Date: Wed, 21 Feb 2018 15:12:14 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r329719 - in head: cddl/contrib/opensolaris/lib/libzfs/common sys/cddl/contrib/opensolaris/uts/common/fs/zfs X-SVN-Group: head X-SVN-Commit-Author: avg X-SVN-Commit-Paths: in head: cddl/contrib/opensolaris/lib/libzfs/common sys/cddl/contrib/opensolaris/uts/common/fs/zfs X-SVN-Commit-Revision: 329719 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Feb 2018 15:12:15 -0000 Author: avg Date: Wed Feb 21 15:12:14 2018 New Revision: 329719 URL: https://svnweb.freebsd.org/changeset/base/329719 Log: MFV r329718: 8520 7198 lzc_rollback_to should support rolling back to origin illumos/illumos-gate@95643f75d23914a3e332adc9661ed51749e9858d https://github.com/illumos/illumos-gate/commit/95643f75d23914a3e332adc9661ed51749e9858d https://www.illumos.org/issues/8520 lzc_rollback_to() should support rolling back to a clone's origin. The current checks in zfs_ioc_rollback() would not allow that because the origin snapshot belongs to a different filesystem. The overly restrictive check was introduced in 7600, but it was not a regression as none of the existing tools provided a way to rollback to the origin. https://www.illumos.org/issues/7198 EINVAL is returned when a dataset does not have any snapshots, so there is nothing to roll back to. Although the code in zfs_do_rollback checks for that condition in advance, it's still possible that the snapshot(s) gets removed after the check and before the rollback sync task is executed. At the moment zfs command would crash when that happens. Reviewed by: Matthew Ahrens Approved by: Dan McDonald Author: Andriy Gapon MFC after: 2 weeks Modified: head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c Directory Properties: head/cddl/contrib/opensolaris/ (props changed) head/cddl/contrib/opensolaris/lib/libzfs/ (props changed) head/sys/cddl/contrib/opensolaris/ (props changed) Modified: head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c ============================================================================== --- head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c Wed Feb 21 15:10:33 2018 (r329718) +++ head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c Wed Feb 21 15:12:14 2018 (r329719) @@ -4054,17 +4054,31 @@ zfs_rollback(zfs_handle_t *zhp, zfs_handle_t *snap, bo * a new snapshot is created before this request is processed. */ err = lzc_rollback_to(zhp->zfs_name, snap->zfs_name); - if (err == EXDEV) { - zfs_error_aux(zhp->zfs_hdl, dgettext(TEXT_DOMAIN, - "'%s' is not the latest snapshot"), snap->zfs_name); - (void) zfs_error_fmt(zhp->zfs_hdl, EZFS_BUSY, + if (err != 0) { + char errbuf[1024]; + + (void) snprintf(errbuf, sizeof (errbuf), dgettext(TEXT_DOMAIN, "cannot rollback '%s'"), zhp->zfs_name); - return (err); - } else if (err != 0) { - (void) zfs_standard_error_fmt(zhp->zfs_hdl, errno, - dgettext(TEXT_DOMAIN, "cannot rollback '%s'"), - zhp->zfs_name); + switch (err) { + case EEXIST: + zfs_error_aux(zhp->zfs_hdl, dgettext(TEXT_DOMAIN, + "there is a snapshot or bookmark more recent " + "than '%s'"), snap->zfs_name); + (void) zfs_error(zhp->zfs_hdl, EZFS_EXISTS, errbuf); + break; + case ESRCH: + zfs_error_aux(zhp->zfs_hdl, dgettext(TEXT_DOMAIN, + "'%s' is not found among snapshots of '%s'"), + snap->zfs_name, zhp->zfs_name); + (void) zfs_error(zhp->zfs_hdl, EZFS_NOENT, errbuf); + break; + case EINVAL: + (void) zfs_error(zhp->zfs_hdl, EZFS_BADTYPE, errbuf); + break; + default: + (void) zfs_standard_error(zhp->zfs_hdl, err, errbuf); + } return (err); } Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c Wed Feb 21 15:10:33 2018 (r329718) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c Wed Feb 21 15:12:14 2018 (r329719) @@ -2553,7 +2553,7 @@ dsl_dataset_rollback_check(void *arg, dmu_tx_t *tx) /* must have a most recent snapshot */ if (dsl_dataset_phys(ds)->ds_prev_snap_txg < TXG_INITIAL) { dsl_dataset_rele(ds, FTAG); - return (SET_ERROR(EINVAL)); + return (SET_ERROR(ESRCH)); } /* @@ -2573,11 +2573,46 @@ dsl_dataset_rollback_check(void *arg, dmu_tx_t *tx) * the latest snapshot is it. */ if (ddra->ddra_tosnap != NULL) { - char namebuf[ZFS_MAX_DATASET_NAME_LEN]; + dsl_dataset_t *snapds; - dsl_dataset_name(ds->ds_prev, namebuf); - if (strcmp(namebuf, ddra->ddra_tosnap) != 0) - return (SET_ERROR(EXDEV)); + /* Check if the target snapshot exists at all. */ + error = dsl_dataset_hold(dp, ddra->ddra_tosnap, FTAG, &snapds); + if (error != 0) { + /* + * ESRCH is used to signal that the target snapshot does + * not exist, while ENOENT is used to report that + * the rolled back dataset does not exist. + * ESRCH is also used to cover other cases where the + * target snapshot is not related to the dataset being + * rolled back such as being in a different pool. + */ + if (error == ENOENT || error == EXDEV) + error = SET_ERROR(ESRCH); + dsl_dataset_rele(ds, FTAG); + return (error); + } + ASSERT(snapds->ds_is_snapshot); + + /* Check if the snapshot is the latest snapshot indeed. */ + if (snapds != ds->ds_prev) { + /* + * Distinguish between the case where the only problem + * is intervening snapshots (EEXIST) vs the snapshot + * not being a valid target for rollback (ESRCH). + */ + if (snapds->ds_dir == ds->ds_dir || + (dsl_dir_is_clone(ds->ds_dir) && + dsl_dir_phys(ds->ds_dir)->dd_origin_obj == + snapds->ds_object)) { + error = SET_ERROR(EEXIST); + } else { + error = SET_ERROR(ESRCH); + } + dsl_dataset_rele(snapds, FTAG); + dsl_dataset_rele(ds, FTAG); + return (error); + } + dsl_dataset_rele(snapds, FTAG); } /* must not have any bookmarks after the most recent snapshot */ @@ -2586,8 +2621,10 @@ dsl_dataset_rollback_check(void *arg, dmu_tx_t *tx) nvlist_t *bookmarks = fnvlist_alloc(); error = dsl_get_bookmarks_impl(ds, proprequest, bookmarks); fnvlist_free(proprequest); - if (error != 0) + if (error != 0) { + dsl_dataset_rele(ds, FTAG); return (error); + } for (nvpair_t *pair = nvlist_next_nvpair(bookmarks, NULL); pair != NULL; pair = nvlist_next_nvpair(bookmarks, pair)) { nvlist_t *valuenv = Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c Wed Feb 21 15:10:33 2018 (r329718) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c Wed Feb 21 15:12:14 2018 (r329719) @@ -3852,11 +3852,14 @@ zfs_ioc_rollback(const char *fsname, nvlist_t *innvl, (void) nvlist_lookup_string(innvl, "target", &target); if (target != NULL) { - int fslen = strlen(fsname); + const char *cp = strchr(target, '@'); - if (strncmp(fsname, target, fslen) != 0) - return (SET_ERROR(EINVAL)); - if (target[fslen] != '@') + /* + * The snap name must contain an @, and the part after it must + * contain only valid characters. + */ + if (cp == NULL || + zfs_component_namecheck(cp + 1, NULL, NULL) != 0) return (SET_ERROR(EINVAL)); }