Print this page
3740 Poor ZFS send / receive performance due to snapshot hold / release processing
Submitted by: Steven Hartland <steven.hartland@multiplay.co.uk>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>

@@ -23,10 +23,11 @@
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2012 by Delphix. All rights reserved.
  * Copyright (c) 2012 DEY Storage Systems, Inc.  All rights reserved.
  * Copyright 2012 Nexenta Systems, Inc. All rights reserved.
  * Copyright (c) 2013 Martin Matuska. All rights reserved.
+ * Copyright (c) 2013 Steven Hartland. All rights reserved.
  */
 
 #include <ctype.h>
 #include <errno.h>
 #include <libintl.h>

@@ -3102,22 +3103,18 @@
 
 static int
 zfs_check_snap_cb(zfs_handle_t *zhp, void *arg)
 {
         struct destroydata *dd = arg;
-        zfs_handle_t *szhp;
         char name[ZFS_MAXNAMELEN];
         int rv = 0;
 
         (void) snprintf(name, sizeof (name),
             "%s@%s", zhp->zfs_name, dd->snapname);
 
-        szhp = make_dataset_handle(zhp->zfs_hdl, name);
-        if (szhp) {
+        if (lzc_exists(name))
                 verify(nvlist_add_boolean(dd->nvl, name) == 0);
-                zfs_close(szhp);
-        }
 
         rv = zfs_iter_filesystems(zhp, zfs_check_snap_cb, dd);
         zfs_close(zhp);
         return (rv);
 }

@@ -3133,11 +3130,11 @@
 
         dd.snapname = snapname;
         verify(nvlist_alloc(&dd.nvl, NV_UNIQUE_NAME, 0) == 0);
         (void) zfs_check_snap_cb(zfs_handle_dup(zhp), &dd);
 
-        if (nvlist_next_nvpair(dd.nvl, NULL) == NULL) {
+        if (nvlist_empty(dd.nvl)) {
                 ret = zfs_standard_error_fmt(zhp->zfs_hdl, ENOENT,
                     dgettext(TEXT_DOMAIN, "cannot destroy '%s@%s'"),
                     zhp->zfs_name, snapname);
         } else {
                 ret = zfs_destroy_snaps_nvl(zhp->zfs_hdl, dd.nvl, defer);

@@ -3158,11 +3155,11 @@
         ret = lzc_destroy_snaps(snaps, defer, &errlist);
 
         if (ret == 0)
                 return (0);
 
-        if (nvlist_next_nvpair(errlist, NULL) == NULL) {
+        if (nvlist_empty(errlist)) {
                 char errbuf[1024];
                 (void) snprintf(errbuf, sizeof (errbuf),
                     dgettext(TEXT_DOMAIN, "cannot destroy snapshots"));
 
                 ret = zfs_standard_error(hdl, ret, errbuf);

@@ -4080,66 +4077,76 @@
 
 static int
 zfs_hold_one(zfs_handle_t *zhp, void *arg)
 {
         struct holdarg *ha = arg;
-        zfs_handle_t *szhp;
         char name[ZFS_MAXNAMELEN];
         int rv = 0;
 
         (void) snprintf(name, sizeof (name),
             "%s@%s", zhp->zfs_name, ha->snapname);
 
-        szhp = make_dataset_handle(zhp->zfs_hdl, name);
-        if (szhp) {
+        if (lzc_exists(name))
                 fnvlist_add_string(ha->nvl, name, ha->tag);
-                zfs_close(szhp);
-        }
 
         if (ha->recursive)
                 rv = zfs_iter_filesystems(zhp, zfs_hold_one, ha);
         zfs_close(zhp);
         return (rv);
 }
 
 int
 zfs_hold(zfs_handle_t *zhp, const char *snapname, const char *tag,
-    boolean_t recursive, boolean_t enoent_ok, int cleanup_fd)
+    boolean_t recursive, int cleanup_fd)
 {
         int ret;
         struct holdarg ha;
-        nvlist_t *errors;
-        libzfs_handle_t *hdl = zhp->zfs_hdl;
-        char errbuf[1024];
-        nvpair_t *elem;
 
         ha.nvl = fnvlist_alloc();
         ha.snapname = snapname;
         ha.tag = tag;
         ha.recursive = recursive;
         (void) zfs_hold_one(zfs_handle_dup(zhp), &ha);
 
-        if (nvlist_next_nvpair(ha.nvl, NULL) == NULL) {
+        if (nvlist_empty(ha.nvl)) {
+                char errbuf[1024];
+
                 fnvlist_free(ha.nvl);
                 ret = ENOENT;
-                if (!enoent_ok) {
                         (void) snprintf(errbuf, sizeof (errbuf),
                             dgettext(TEXT_DOMAIN,
                             "cannot hold snapshot '%s@%s'"),
                             zhp->zfs_name, snapname);
-                        (void) zfs_standard_error(hdl, ret, errbuf);
-                }
+                (void) zfs_standard_error(zhp->zfs_hdl, ret, errbuf);
                 return (ret);
         }
 
-        ret = lzc_hold(ha.nvl, cleanup_fd, &errors);
+        ret = zfs_hold_nvl(zhp, cleanup_fd, ha.nvl);
         fnvlist_free(ha.nvl);
 
-        if (ret == 0)
+        return (ret);
+}
+
+int
+zfs_hold_nvl(zfs_handle_t *zhp, int cleanup_fd, nvlist_t *holds)
+{
+        int ret;
+        nvlist_t *errors;
+        libzfs_handle_t *hdl = zhp->zfs_hdl;
+        char errbuf[1024];
+        nvpair_t *elem;
+
+        errors = NULL;
+        ret = lzc_hold(holds, cleanup_fd, &errors);
+
+        if (ret == 0) {
+                /* There may be errors even in the success case. */
+                fnvlist_free(errors);
                 return (0);
+        }
 
-        if (nvlist_next_nvpair(errors, NULL) == NULL) {
+        if (nvlist_empty(errors)) {
                 /* no hold-specific errors */
                 (void) snprintf(errbuf, sizeof (errbuf),
                     dgettext(TEXT_DOMAIN, "cannot hold"));
                 switch (ret) {
                 case ENOTSUP:

@@ -4175,48 +4182,35 @@
                         (void) zfs_error(hdl, EZFS_BADTYPE, errbuf);
                         break;
                 case EEXIST:
                         (void) zfs_error(hdl, EZFS_REFTAG_HOLD, errbuf);
                         break;
-                case ENOENT:
-                        if (enoent_ok)
-                                return (ENOENT);
-                        /* FALLTHROUGH */
                 default:
                         (void) zfs_standard_error(hdl,
                             fnvpair_value_int32(elem), errbuf);
                 }
         }
 
         fnvlist_free(errors);
         return (ret);
 }
 
-struct releasearg {
-        nvlist_t *nvl;
-        const char *snapname;
-        const char *tag;
-        boolean_t recursive;
-};
-
 static int
 zfs_release_one(zfs_handle_t *zhp, void *arg)
 {
         struct holdarg *ha = arg;
-        zfs_handle_t *szhp;
         char name[ZFS_MAXNAMELEN];
         int rv = 0;
 
         (void) snprintf(name, sizeof (name),
             "%s@%s", zhp->zfs_name, ha->snapname);
 
-        szhp = make_dataset_handle(zhp->zfs_hdl, name);
-        if (szhp) {
+        if (lzc_exists(name)) {
                 nvlist_t *holds = fnvlist_alloc();
                 fnvlist_add_boolean(holds, ha->tag);
                 fnvlist_add_nvlist(ha->nvl, name, holds);
-                zfs_close(szhp);
+                fnvlist_free(holds);
         }
 
         if (ha->recursive)
                 rv = zfs_iter_filesystems(zhp, zfs_release_one, ha);
         zfs_close(zhp);

@@ -4227,22 +4221,22 @@
 zfs_release(zfs_handle_t *zhp, const char *snapname, const char *tag,
     boolean_t recursive)
 {
         int ret;
         struct holdarg ha;
-        nvlist_t *errors;
+        nvlist_t *errors = NULL;
         nvpair_t *elem;
         libzfs_handle_t *hdl = zhp->zfs_hdl;
         char errbuf[1024];
 
         ha.nvl = fnvlist_alloc();
         ha.snapname = snapname;
         ha.tag = tag;
         ha.recursive = recursive;
         (void) zfs_release_one(zfs_handle_dup(zhp), &ha);
 
-        if (nvlist_next_nvpair(ha.nvl, NULL) == NULL) {
+        if (nvlist_empty(ha.nvl)) {
                 fnvlist_free(ha.nvl);
                 ret = ENOENT;
                 (void) snprintf(errbuf, sizeof (errbuf),
                     dgettext(TEXT_DOMAIN,
                     "cannot release hold from snapshot '%s@%s'"),

@@ -4252,14 +4246,17 @@
         }
 
         ret = lzc_release(ha.nvl, &errors);
         fnvlist_free(ha.nvl);
 
-        if (ret == 0)
+        if (ret == 0) {
+                /* There may be errors even in the success case. */
+                fnvlist_free(errors);
                 return (0);
+        }
 
-        if (nvlist_next_nvpair(errors, NULL) == NULL) {
+        if (nvlist_empty(errors)) {
                 /* no hold-specific errors */
                 (void) snprintf(errbuf, sizeof (errbuf), dgettext(TEXT_DOMAIN,
                     "cannot release"));
                 switch (errno) {
                 case ENOTSUP: