Illumos #3829

3829 fix for 3740 changed behavior of zfs destroy/hold/release ioctl
Reviewed by: Matt Amdur <matt.amdur@delphix.com>
Reviewed by: Christopher Siden <christopher.siden@delphix.com>
Approved by: Richard Lowe <richlowe@richlowe.net>

References:
  https://www.illumos.org/issues/3829
  illumos/illumos-gate@bb6e70758d0c30c09f148026d6e686e21cfc8d18

Ported-by: Richard Yao <ryao@gentoo.org>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue #1775

diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c
index 4b4f8d8c9262e8512a924fe18b97404206076131..3bbd653c66cbb64310c3999e3ea9776b499cbd92 100644 (file)
--- 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 <pawel@dawidek.net>.
  * 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 d3918a4edd7be9f96208170a53b88c511b2bde2c..53c813d4a28f2e070cde203be378932709405d11 100644 (file)
--- 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.
- *
- * The return value will be ENOENT if none of the snapshots for the requested
- * 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, 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 b1fd270dac418439a26033ea60de760f8896e151..1c4c3deec78282cd50d1a20f6c3c70e15f637f2e 100644 (file)
--- 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 f33292787df32117996b38742a2376e9a00ac39a..db9540d13d18cb63a419c16dd69d098bc08369b7 100644 (file)
--- 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);
 }