svn commit: r363447 - in head/cddl/contrib/opensolaris: cmd/zpool lib/libzfs/common

[Mark Johnston]

Author: markj
Date: Thu Jul 23 14:21:45 2020
New Revision: 363447
URL: https://svnweb.freebsd.org/changeset/base/363447

Log:
  MFOpenZFS: Fix zpool history unbounded memory usage
  
  In original implementation, zpool history will read the whole history
  before printing anything, causing memory usage goes unbounded. We fix
  this by breaking it into read-print iterations.
  
  Reviewed-by: Tom Caputi <tcaputi at datto.com>
  Reviewed-by: Matt Ahrens <matt at delphix.com>
  Reviewed-by: Igor Kozhukhov <igor at dilos.org>
  Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
  Signed-off-by: Chunwei Chen <david.chen at nutanix.com>
  Closes #9516
  
  Note, this change changes the libzfs.so ABI by modifying the prototype
  of zpool_get_history().  Since libzfs is effectively private to the base
  system it is anticipated that this will not be a problem.
  
  PR:		247557
  Obtained from:	OpenZFS
  Reported and tested by:	Sam Vaughan <samjvaughan at gmail.com>
  Discussed with:	freqlabs
  MFC after:	2 weeks
  Differential Revision:	https://reviews.freebsd.org/D25745
  openzfs/zfs at 7125a109dcc55628336ff3f58e59e503f4d7694d

Modified:
  head/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c
  head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h
  head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c

Modified: head/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c
==============================================================================
--- head/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c	Thu Jul 23 14:03:37 2020	(r363446)
+++ head/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c	Thu Jul 23 14:21:45 2020	(r363447)
@@ -6226,25 +6226,13 @@ typedef struct hist_cbdata {
 	boolean_t internal;
 } hist_cbdata_t;
 
-/*
- * Print out the command history for a specific pool.
- */
-static int
-get_history_one(zpool_handle_t *zhp, void *data)
+static void
+print_history_records(nvlist_t *nvhis, hist_cbdata_t *cb)
 {
-	nvlist_t *nvhis;
 	nvlist_t **records;
 	uint_t numrecords;
-	int ret, i;
-	hist_cbdata_t *cb = (hist_cbdata_t *)data;
+	int i;
 
-	cb->first = B_FALSE;
-
-	(void) printf(gettext("History for '%s':\n"), zpool_get_name(zhp));
-
-	if ((ret = zpool_get_history(zhp, &nvhis)) != 0)
-		return (ret);
-
 	verify(nvlist_lookup_nvlist_array(nvhis, ZPOOL_HIST_RECORD,
 	    &records, &numrecords) == 0);
 	for (i = 0; i < numrecords; i++) {
@@ -6344,8 +6332,32 @@ get_history_one(zpool_handle_t *zhp, void *data)
 		(void) printf("]");
 		(void) printf("\n");
 	}
+}
+
+/*
+ * Print out the command history for a specific pool.
+ */
+static int
+get_history_one(zpool_handle_t *zhp, void *data)
+{
+	nvlist_t *nvhis;
+	int ret;
+	hist_cbdata_t *cb = (hist_cbdata_t *)data;
+	uint64_t off = 0;
+	boolean_t eof = B_FALSE;
+
+	cb->first = B_FALSE;
+
+	(void) printf(gettext("History for '%s':\n"), zpool_get_name(zhp));
+
+	while (!eof) {
+		if ((ret = zpool_get_history(zhp, &nvhis, &off, &eof)) != 0)
+			return (ret);
+
+		print_history_records(nvhis, cb);
+		nvlist_free(nvhis);
+	}
 	(void) printf("\n");
-	nvlist_free(nvhis);
 
 	return (ret);
 }

Modified: head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h
==============================================================================
--- head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h	Thu Jul 23 14:03:37 2020	(r363446)
+++ head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h	Thu Jul 23 14:21:45 2020	(r363447)
@@ -441,7 +441,8 @@ typedef enum {
 extern char *zpool_vdev_name(libzfs_handle_t *, zpool_handle_t *, nvlist_t *,
     int name_flags);
 extern int zpool_upgrade(zpool_handle_t *, uint64_t);
-extern int zpool_get_history(zpool_handle_t *, nvlist_t **);
+extern int zpool_get_history(zpool_handle_t *, nvlist_t **, uint64_t *,
+    boolean_t *);
 extern int zpool_history_unpack(char *, uint64_t, uint64_t *,
     nvlist_t ***, uint_t *);
 extern void zpool_obj_to_path(zpool_handle_t *, uint64_t, uint64_t, char *,

Modified: head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c
==============================================================================
--- head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c	Thu Jul 23 14:03:37 2020	(r363446)
+++ head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c	Thu Jul 23 14:21:45 2020	(r363447)
@@ -4124,33 +4124,37 @@ zpool_history_unpack(char *buf, uint64_t bytes_read, u
  * Retrieve the command history of a pool.
  */
 int
-zpool_get_history(zpool_handle_t *zhp, nvlist_t **nvhisp)
+zpool_get_history(zpool_handle_t *zhp, nvlist_t **nvhisp, uint64_t *off,
+    boolean_t *eof)
 {
 	char *buf;
 	uint64_t buflen = HIS_BUF_LEN_DEF;
-	uint64_t off = 0;
 	nvlist_t **records = NULL;
 	uint_t numrecords = 0;
 	int err, i;
+	uint64_t start = *off;
 
 	buf = malloc(buflen);
 	if (buf == NULL)
 		return (ENOMEM);
-	do {
+	/* process about 1MB at a time */
+	while (*off - start < 1024 * 1024) {
 		uint64_t bytes_read = buflen;
 		uint64_t leftover;
 
-		if ((err = get_history(zhp, buf, &off, &bytes_read)) != 0)
+		if ((err = get_history(zhp, buf, off, &bytes_read)) != 0)
 			break;
 
 		/* if nothing else was read in, we're at EOF, just return */
-		if (bytes_read == 0)
+		if (bytes_read == 0) {
+			*eof = B_TRUE;
 			break;
+		}
 
 		if ((err = zpool_history_unpack(buf, bytes_read,
 		    &leftover, &records, &numrecords)) != 0)
 			break;
-		off -= leftover;
+		*off -= leftover;
 		if (leftover == bytes_read) {
 			/*
 			 * no progress made, because buffer is not big enough
@@ -4165,9 +4169,7 @@ zpool_get_history(zpool_handle_t *zhp, nvlist_t **nvhi
 				break;
 			}
 		}
-
-		/* CONSTCOND */
-	} while (1);
+	}
 
 	free(buf);