From 1a077756e8ba946a55f999fa1cb5f0e7dcb9aa81 Mon Sep 17 00:00:00 2001 From: Matthew Ahrens Date: Thu, 20 Jun 2013 14:43:17 -0800 Subject: [PATCH] Illumos #3829 3829 fix for 3740 changed behavior of zfs destroy/hold/release ioctl Reviewed by: Matt Amdur Reviewed by: Christopher Siden Approved by: Richard Lowe References: https://www.illumos.org/issues/3829 illumos/illumos-gate@bb6e70758d0c30c09f148026d6e686e21cfc8d18 Ported-by: Richard Yao Signed-off-by: Brian Behlendorf Issue #1775 --- lib/libzfs/libzfs_dataset.c | 27 +++++++++++++++++++-------- lib/libzfs_core/libzfs_core.c | 23 ++++++----------------- module/zfs/dsl_destroy.c | 3 --- module/zfs/dsl_userhold.c | 8 -------- 4 files changed, 25 insertions(+), 36 deletions(-) diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c index 4b4f8d8c9..3bbd653c6 100644 --- a/lib/libzfs/libzfs_dataset.c +++ b/lib/libzfs/libzfs_dataset.c @@ -21,7 +21,7 @@ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2012 by Delphix. All rights reserved. + * Copyright (c) 2013 by Delphix. All rights reserved. * Copyright (c) 2012 DEY Storage Systems, Inc. All rights reserved. * Copyright (c) 2012 Pawel Jakub Dawidek . * Copyright 2012 Nexenta Systems, Inc. All rights reserved. @@ -4412,6 +4412,7 @@ struct holdarg { const char *snapname; const char *tag; boolean_t recursive; + int error; }; static int @@ -4539,15 +4540,20 @@ zfs_release_one(zfs_handle_t *zhp, void *arg) struct holdarg *ha = arg; char name[ZFS_MAXNAMELEN]; int rv = 0; + nvlist_t *existing_holds; (void) snprintf(name, sizeof (name), "%s@%s", zhp->zfs_name, ha->snapname); - if (lzc_exists(name)) { - nvlist_t *holds = fnvlist_alloc(); - fnvlist_add_boolean(holds, ha->tag); - fnvlist_add_nvlist(ha->nvl, name, holds); - fnvlist_free(holds); + if (lzc_get_holds(name, &existing_holds) != 0) { + ha->error = ENOENT; + } else if (!nvlist_exists(existing_holds, ha->tag)) { + ha->error = ESRCH; + } else { + nvlist_t *torelease = fnvlist_alloc(); + fnvlist_add_boolean(torelease, ha->tag); + fnvlist_add_nvlist(ha->nvl, name, torelease); + fnvlist_free(torelease); } if (ha->recursive) @@ -4571,16 +4577,21 @@ zfs_release(zfs_handle_t *zhp, const char *snapname, const char *tag, ha.snapname = snapname; ha.tag = tag; ha.recursive = recursive; + ha.error = 0; (void) zfs_release_one(zfs_handle_dup(zhp), &ha); if (nvlist_empty(ha.nvl)) { fnvlist_free(ha.nvl); - ret = ENOENT; + ret = ha.error; (void) snprintf(errbuf, sizeof (errbuf), dgettext(TEXT_DOMAIN, "cannot release hold from snapshot '%s@%s'"), zhp->zfs_name, snapname); - (void) zfs_standard_error(hdl, ret, errbuf); + if (ret == ESRCH) { + (void) zfs_error(hdl, EZFS_REFTAG_RELE, errbuf); + } else { + (void) zfs_standard_error(hdl, ret, errbuf); + } return (ret); } diff --git a/lib/libzfs_core/libzfs_core.c b/lib/libzfs_core/libzfs_core.c index d3918a4ed..53c813d4a 100644 --- a/lib/libzfs_core/libzfs_core.c +++ b/lib/libzfs_core/libzfs_core.c @@ -255,11 +255,8 @@ lzc_snapshot(nvlist_t *snaps, nvlist_t *props, nvlist_t **errlist) * marked for deferred destruction, and will be destroyed when the last hold * or clone is removed/destroyed. * - * The return value will be ENOENT if none of the snapshots existed. - * * The return value will be 0 if all snapshots were destroyed (or marked for - * later destruction if 'defer' is set) or didn't exist to begin with and - * at least one snapshot was destroyed. + * later destruction if 'defer' is set) or didn't exist to begin with. * * Otherwise the return value will be the errno of a (unspecified) snapshot * that failed, no snapshots will be destroyed, and the errlist will have an @@ -350,15 +347,10 @@ lzc_exists(const char *dataset) * or imported. * * Holds for snapshots which don't exist will be skipped and have an entry - * added to errlist, but will not cause an overall failure, except in the - * case that all holds where skipped. + * added to errlist, but will not cause an overall failure. * - * The return value will be ENOENT if none of the snapshots for the requested - * holds existed. - * - * The return value will be 0 if the nvl holds was empty or all holds, for - * snapshots that existed, were succesfully created and at least one hold - * was created. + * The return value will be 0 if all holds, for snapshots that existed, + * were succesfully created. * * Otherwise the return value will be the errno of a (unspecified) hold that * failed and no holds will be created. @@ -402,13 +394,10 @@ lzc_hold(nvlist_t *holds, int cleanup_fd, nvlist_t **errlist) * The value is a nvlist whose keys are the holds to remove. * * Holds which failed to release because they didn't exist will have an entry - * added to errlist, but will not cause an overall failure, except in the - * case that all releases where skipped. - * - * The return value will be ENOENT if none of the specified holds existed. + * added to errlist, but will not cause an overall failure. * * The return value will be 0 if the nvl holds was empty or all holds that - * existed, were successfully removed and at least one hold was removed. + * existed, were successfully removed. * * Otherwise the return value will be the errno of a (unspecified) hold that * failed to release and no holds will be released. diff --git a/module/zfs/dsl_destroy.c b/module/zfs/dsl_destroy.c index b1fd270da..1c4c3deec 100644 --- a/module/zfs/dsl_destroy.c +++ b/module/zfs/dsl_destroy.c @@ -129,9 +129,6 @@ dsl_destroy_snapshot_check(void *arg, dmu_tx_t *tx) if (pair != NULL) return (fnvpair_value_int32(pair)); - if (nvlist_empty(dsda->dsda_successful_snaps)) - return (SET_ERROR(ENOENT)); - return (0); } diff --git a/module/zfs/dsl_userhold.c b/module/zfs/dsl_userhold.c index f33292787..db9540d13 100644 --- a/module/zfs/dsl_userhold.c +++ b/module/zfs/dsl_userhold.c @@ -127,10 +127,6 @@ dsl_dataset_user_hold_check(void *arg, dmu_tx_t *tx) } } - /* Return ENOENT if no holds would be created. */ - if (nvlist_empty(dduha->dduha_chkholds)) - return (SET_ERROR(ENOENT)); - return (0); } @@ -472,10 +468,6 @@ dsl_dataset_user_release_check(void *arg, dmu_tx_t *tx) } } - /* Return ENOENT if none of the holds existed. */ - if (nvlist_empty(ddura->ddura_chkholds)) - return (SET_ERROR(ENOENT)); - return (0); }