d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

MFOpenZFS: Fix zpool history unbounded memory usage

Browse Source 
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@datto.com>
Reviewed-by: Matt Ahrens <matt@delphix.com>
Reviewed-by: Igor Kozhukhov <igor@dilos.org>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chunwei Chen <david.chen@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@gmail.com>
Discussed with:	freqlabs
MFC after:	2 weeks
Differential Revision:	https://reviews.freebsd.org/D25745
openzfs/zfs@7125a109dc
This commit is contained in:
Mark Johnston 2020-07-23 14:21:45 +00:00
parent cbef26ed16
commit 4cbba6ae24
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=363447
3 changed files with 41 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
cddl/contrib/opensolaris/cmd/zpool/zpool_main.c
View File

@ -6226,24 +6226,12 @@ 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;
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);
int i;
verify(nvlist_lookup_nvlist_array(nvhis, ZPOOL_HIST_RECORD,
&records, &numrecords) == 0);
@ -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);
}

3
cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h
View File

@ -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 *,

20
cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c
View File

@ -4124,33 +4124,37 @@ zpool_history_unpack(char *buf, uint64_t bytes_read, uint64_t *leftover,
* 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 **nvhisp)
break;
}
}
/* CONSTCOND */
} while (1);
}
free(buf);