Skip to content

Commit

Permalink
Fix zpool history unbounded memory usage
Browse files Browse the repository at this point in the history
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 openzfs#9516
  • Loading branch information
davidchenntnx authored and ahrens committed Mar 18, 2020
1 parent f30abc5 commit 2b6c163
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 26 deletions.
44 changes: 28 additions & 16 deletions cmd/zpool/zpool_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -8528,24 +8528,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);
Expand Down Expand Up @@ -8649,8 +8637,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);
}
Expand Down
3 changes: 2 additions & 1 deletion include/libzfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,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_events_next(libzfs_handle_t *, nvlist_t **, int *, unsigned,
int);
extern int zpool_events_clear(libzfs_handle_t *, int *);
Expand Down
20 changes: 11 additions & 9 deletions lib/libzfs/libzfs_pool.c
Original file line number Diff line number Diff line change
Expand Up @@ -4330,33 +4330,37 @@ get_history(zpool_handle_t *zhp, char *buf, uint64_t *off, uint64_t *len)
* 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;
int buflen = 128 * 1024;
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 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)
if (!bytes_read) {
*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
Expand All @@ -4368,9 +4372,7 @@ zpool_get_history(zpool_handle_t *zhp, nvlist_t **nvhisp)
if (buf == NULL)
return (ENOMEM);
}

/* CONSTCOND */
} while (1);
}

free(buf);

Expand Down

0 comments on commit 2b6c163

Please sign in to comment.