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@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
This commit is contained in:
Chunwei Chen 2019-10-28 09:49:44 -07:00 committed by Tony Hutter
parent 635603a1c2
commit c6eaa8b7f9
3 changed files with 41 additions and 26 deletions

View File

@ -8419,24 +8419,12 @@ typedef struct hist_cbdata {
boolean_t internal; boolean_t internal;
} hist_cbdata_t; } hist_cbdata_t;
/* static void
* Print out the command history for a specific pool. print_history_records(nvlist_t *nvhis, hist_cbdata_t *cb)
*/
static int
get_history_one(zpool_handle_t *zhp, void *data)
{ {
nvlist_t *nvhis;
nvlist_t **records; nvlist_t **records;
uint_t numrecords; uint_t numrecords;
int ret, i; int 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);
verify(nvlist_lookup_nvlist_array(nvhis, ZPOOL_HIST_RECORD, verify(nvlist_lookup_nvlist_array(nvhis, ZPOOL_HIST_RECORD,
&records, &numrecords) == 0); &records, &numrecords) == 0);
@ -8540,8 +8528,32 @@ get_history_one(zpool_handle_t *zhp, void *data)
(void) printf("]"); (void) printf("]");
(void) printf("\n"); (void) printf("\n");
} }
(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); nvlist_free(nvhis);
}
(void) printf("\n");
return (ret); return (ret);
} }

View File

@ -425,7 +425,8 @@ typedef enum {
extern char *zpool_vdev_name(libzfs_handle_t *, zpool_handle_t *, nvlist_t *, extern char *zpool_vdev_name(libzfs_handle_t *, zpool_handle_t *, nvlist_t *,
int name_flags); int name_flags);
extern int zpool_upgrade(zpool_handle_t *, uint64_t); 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, extern int zpool_events_next(libzfs_handle_t *, nvlist_t **, int *, unsigned,
int); int);
extern int zpool_events_clear(libzfs_handle_t *, int *); extern int zpool_events_clear(libzfs_handle_t *, int *);

View File

@ -4305,33 +4305,37 @@ get_history(zpool_handle_t *zhp, char *buf, uint64_t *off, uint64_t *len)
* Retrieve the command history of a pool. * Retrieve the command history of a pool.
*/ */
int 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; char *buf;
int buflen = 128 * 1024; int buflen = 128 * 1024;
uint64_t off = 0;
nvlist_t **records = NULL; nvlist_t **records = NULL;
uint_t numrecords = 0; uint_t numrecords = 0;
int err, i; int err, i;
uint64_t start = *off;
buf = malloc(buflen); buf = malloc(buflen);
if (buf == NULL) if (buf == NULL)
return (ENOMEM); return (ENOMEM);
do { /* process about 1MB a time */
while (*off - start < 1024 * 1024) {
uint64_t bytes_read = buflen; uint64_t bytes_read = buflen;
uint64_t leftover; uint64_t leftover;
if ((err = get_history(zhp, buf, &off, &bytes_read)) != 0) if ((err = get_history(zhp, buf, off, &bytes_read)) != 0)
break; break;
/* if nothing else was read in, we're at EOF, just return */ /* if nothing else was read in, we're at EOF, just return */
if (!bytes_read) if (!bytes_read) {
*eof = B_TRUE;
break; break;
}
if ((err = zpool_history_unpack(buf, bytes_read, if ((err = zpool_history_unpack(buf, bytes_read,
&leftover, &records, &numrecords)) != 0) &leftover, &records, &numrecords)) != 0)
break; break;
off -= leftover; *off -= leftover;
if (leftover == bytes_read) { if (leftover == bytes_read) {
/* /*
* no progress made, because buffer is not big enough * no progress made, because buffer is not big enough
@ -4343,9 +4347,7 @@ zpool_get_history(zpool_handle_t *zhp, nvlist_t **nvhisp)
if (buf == NULL) if (buf == NULL)
return (ENOMEM); return (ENOMEM);
} }
}
/* CONSTCOND */
} while (1);
free(buf); free(buf);