Skip to content

Add "zpool status -vv" #17502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add "zpool status -vv" #17502

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b07f519
Add "zpool status -vv"
asomers Jul 1, 2025
1e469dc
Respond to review comments:
asomers Aug 20, 2025
c9f179f
Condense error reports to a single line for each file.
asomers Aug 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 72 additions & 21 deletions cmd/zpool/zpool_main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2590,7 +2590,7 @@ typedef struct status_cbdata {
int cb_name_flags;
int cb_namewidth;
boolean_t cb_allpools;
boolean_t cb_verbose;
int cb_verbosity;
boolean_t cb_literal;
boolean_t cb_explain;
boolean_t cb_first;
Expand Down Expand Up @@ -3322,7 +3322,7 @@ print_class_vdevs(zpool_handle_t *zhp, status_cbdata_t *cb, nvlist_t *nv,
nvlist_t **child;
boolean_t printed = B_FALSE;

assert(zhp != NULL || !cb->cb_verbose);
assert(zhp != NULL || cb->cb_verbosity == 0);

if (nvlist_lookup_nvlist_array(nv, ZPOOL_CONFIG_CHILDREN, &child,
&children) != 0)
Expand Down Expand Up @@ -9478,7 +9478,7 @@ class_vdevs_nvlist(zpool_handle_t *zhp, status_cbdata_t *cb, nvlist_t *nv,
if (!cb->cb_flat_vdevs)
class_obj = fnvlist_alloc();

assert(zhp != NULL || !cb->cb_verbose);
assert(zhp != NULL || cb->cb_verbosity == 0);

if (nvlist_lookup_nvlist_array(nv, ZPOOL_CONFIG_CHILDREN, &child,
&children) != 0)
Expand Down Expand Up @@ -9586,26 +9586,38 @@ static void
errors_nvlist(zpool_handle_t *zhp, status_cbdata_t *cb, nvlist_t *item)
{
uint64_t nerr;
int verbosity = cb->cb_verbosity;
Copy link
Contributor

@behlendorf behlendorf Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use cb->cb_verbosity throughout and remove the local variable.

Copy link
Contributor Author

@asomers asomers Aug 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only reason I created the local variable is so I didn't have to split a long line at 80 columns. IMHO it looks better this way. But I'll change it if you want me to.

Copy link
Contributor

@behlendorf behlendorf Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd just like to make sure we're using either the local variable or cb->cb_verbosity consistently in this function. I don't feel strongly about which one, so if you want to stick with the local variable we should update the other places it's used.

nvlist_t *config = zpool_get_config(zhp, NULL);
if (nvlist_lookup_uint64(config, ZPOOL_CONFIG_ERRCOUNT,
&nerr) == 0) {
nice_num_str_nvlist(item, ZPOOL_CONFIG_ERRCOUNT, nerr,
cb->cb_literal, cb->cb_json_as_int, ZFS_NICENUM_1024);
if (nerr != 0 && cb->cb_verbose) {
if (nerr != 0 && cb->cb_verbosity > 0) {
nvlist_t *nverrlist = NULL;
if (zpool_get_errlog(zhp, &nverrlist) == 0) {
if (zpool_get_errlog(zhp, &nverrlist, verbosity) == 0) {
int i = 0;
int count = 0;
size_t len = MAXPATHLEN * 2;
nvpair_t *elem = NULL;
char **errl = NULL, *pathbuf = NULL;
nvlist_t **errnvl = NULL;

for (nvpair_t *pair =
nvlist_next_nvpair(nverrlist, NULL);
pair != NULL;
pair = nvlist_next_nvpair(nverrlist, pair))
count++;
char **errl = (char **)malloc(
count * sizeof (char *));
if (cb->cb_verbosity < 2)
errl = calloc(count, sizeof (char *));
else {
pathbuf = safe_malloc(len);
errnvl = calloc(count,
sizeof (nvlist_t *));
}
if (errl == NULL && errnvl == NULL) {
perror("calloc");
exit(1);
}

while ((elem = nvlist_next_nvpair(nverrlist,
elem)) != NULL) {
Expand All @@ -9618,16 +9630,46 @@ errors_nvlist(zpool_handle_t *zhp, status_cbdata_t *cb, nvlist_t *item)
ZPOOL_ERR_DATASET, &dsobj) == 0);
verify(nvlist_lookup_uint64(nv,
ZPOOL_ERR_OBJECT, &obj) == 0);
errl[i] = safe_malloc(len);
zpool_obj_to_path(zhp, dsobj, obj,
errl[i++], len);
if (cb->cb_verbosity < 2) {
errl[i] = safe_malloc(len);
zpool_obj_to_path(zhp, dsobj,
obj, errl[i++], len);
} else {
uint64_t lvl, blkid;

errnvl[i] = fnvlist_alloc();
lvl = fnvlist_lookup_uint64(nv,
ZPOOL_ERR_LEVEL);
blkid = fnvlist_lookup_uint64(
nv, ZPOOL_ERR_BLKID);
zpool_obj_to_path(zhp, dsobj,
obj, pathbuf, len);
fnvlist_add_string(errnvl[i],
"path", pathbuf);
fnvlist_add_uint64(errnvl[i],
"level", lvl);
fnvlist_add_uint64(errnvl[i++],
"record", blkid);
}
Copy link
Member

@amotin amotin Aug 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't we always do it the verbose way here to simplify the code, and just not print the additional information later? It does not look like very performance-sensitive code path.

Copy link
Contributor Author

@asomers asomers Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, because if verbosity is < 2, then nverrlist won't contain the "level" and "blockid" fields. And I don't want for zpool_get_errlog to always supply those fields, because it could result in enormous nvlists if a single file has many corrupt records.

Copy link
Contributor

@behlendorf behlendorf Aug 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enormous nvlists if a single file has many corrupt records.

Along these same lines I'm a bit concerned with logging a line per block. That could be overwhelming.

Thinking about this from a user perspective, I really don't care about how ZFS decided to internally layout the file (objid, level, blkid). What is useful to me are the file offsets which are corrupt. That's a little more work to generate but shouldn't be too bad. Maybe something like:

errors: Permanent errors have been detected in the following files:

        /testpool/randfile.7 393216-524288,1048576-1179647,1966080-2097151
        /testpool/randfile.5 917504-1048575
        ...

Copy link
Member

@amotin amotin Aug 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I don't want for zpool_get_errlog to always supply those fields

I didn't mean to always supply all the ranges. Merely using the same data structure with nvlists, just with one vs many entries per file, if it allow to make code cleaner. But if no, I won't insist.

}
nvlist_free(nverrlist);
fnvlist_add_string_array(item, "errlist",
(const char **)errl, count);
for (int i = 0; i < count; ++i)
free(errl[i]);
free(errl);
if (cb->cb_verbosity < 2) {
fnvlist_add_string_array(item,
"errlist", (const char **)errl,
count);
for (int i = 0; i < count; ++i)
free(errl[i]);
free(errl);
} else {
fnvlist_add_nvlist_array(item,
"errlist",
(const nvlist_t **)errnvl,
count);
for (int i = 0; i < count; ++i)
free(errnvl[i]);
free(errnvl);
free(pathbuf);
}
} else
fnvlist_add_string(item, "errlist",
strerror(errno));
Expand Down Expand Up @@ -10304,14 +10346,14 @@ print_checkpoint_status(pool_checkpoint_stat_t *pcs)
}

static void
print_error_log(zpool_handle_t *zhp)
print_error_log(zpool_handle_t *zhp, int verbosity)
{
nvlist_t *nverrlist = NULL;
nvpair_t *elem;
char *pathname;
size_t len = MAXPATHLEN * 2;

if (zpool_get_errlog(zhp, &nverrlist) != 0)
if (zpool_get_errlog(zhp, &nverrlist, verbosity) != 0)
return;

(void) printf("errors: Permanent errors have been "
Expand All @@ -10329,7 +10371,16 @@ print_error_log(zpool_handle_t *zhp)
verify(nvlist_lookup_uint64(nv, ZPOOL_ERR_OBJECT,
&obj) == 0);
zpool_obj_to_path(zhp, dsobj, obj, pathname, len);
(void) printf("%7s %s\n", "", pathname);
if (verbosity > 1) {
uint64_t level, blkid;

blkid = fnvlist_lookup_uint64(nv, ZPOOL_ERR_BLKID);
level = fnvlist_lookup_uint64(nv, ZPOOL_ERR_LEVEL);
(void) printf("%7s %s L%lu record %lu\n", "", pathname,
level, blkid);
} else {
(void) printf("%7s %s\n", "", pathname);
}
}
free(pathname);
nvlist_free(nverrlist);
Expand Down Expand Up @@ -11027,14 +11078,14 @@ status_callback(zpool_handle_t *zhp, void *data)
if (nerr == 0) {
(void) printf(gettext(
"errors: No known data errors\n"));
} else if (!cbp->cb_verbose) {
} else if (0 == cbp->cb_verbosity) {
color_start(ANSI_RED);
(void) printf(gettext("errors: %llu data "
"errors, use '-v' for a list\n"),
(u_longlong_t)nerr);
color_end();
} else {
print_error_log(zhp);
print_error_log(zhp, cbp->cb_verbosity);
}
}

Expand Down Expand Up @@ -11161,7 +11212,7 @@ zpool_do_status(int argc, char **argv)
get_timestamp_arg(*optarg);
break;
case 'v':
cb.cb_verbose = B_TRUE;
cb.cb_verbosity++;
break;
case 'j':
cb.cb_json = B_TRUE;
Expand Down
2 changes: 1 addition & 1 deletion include/libzfs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ _LIBZFS_H zpool_status_t zpool_import_status(nvlist_t *, const char **,
_LIBZFS_H nvlist_t *zpool_get_config(zpool_handle_t *, nvlist_t **);
_LIBZFS_H nvlist_t *zpool_get_features(zpool_handle_t *);
_LIBZFS_H int zpool_refresh_stats(zpool_handle_t *, boolean_t *);
_LIBZFS_H int zpool_get_errlog(zpool_handle_t *, nvlist_t **);
_LIBZFS_H int zpool_get_errlog(zpool_handle_t *, nvlist_t **, int);
_LIBZFS_H void zpool_add_propname(zpool_handle_t *, const char *);

/*
Expand Down
2 changes: 2 additions & 0 deletions include/sys/fs/zfs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1723,6 +1723,8 @@ typedef enum {
#define ZPOOL_ERR_LIST "error list"
#define ZPOOL_ERR_DATASET "dataset"
#define ZPOOL_ERR_OBJECT "object"
#define ZPOOL_ERR_LEVEL "level"
#define ZPOOL_ERR_BLKID "blkid"

#define HIS_MAX_RECORD_LEN (MAXPATHLEN + MAXPATHLEN + 1)

Expand Down
1 change: 1 addition & 0 deletions lib/libzfs/libzfs.abi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7103,6 +7103,7 @@
<function-decl name='zpool_get_errlog' mangled-name='zpool_get_errlog' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='zpool_get_errlog'>
<parameter type-id='4c81de99' name='zhp'/>
<parameter type-id='857bb57e' name='nverrlistp'/>
<parameter type-id='95e97e5e' name='verbosity'/>
<return type-id='95e97e5e'/>
</function-decl>
<function-decl name='zpool_upgrade' mangled-name='zpool_upgrade' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='zpool_upgrade'>
Expand Down
24 changes: 22 additions & 2 deletions lib/libzfs/libzfs_pool.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4618,7 +4618,7 @@ zpool_add_propname(zpool_handle_t *zhp, const char *propname)
* caller.
*/
int
zpool_get_errlog(zpool_handle_t *zhp, nvlist_t **nverrlistp)
zpool_get_errlog(zpool_handle_t *zhp, nvlist_t **nverrlistp, int verbosity)
{
zfs_cmd_t zc = {"\0"};
libzfs_handle_t *hdl = zhp->zpool_hdl;
Expand Down Expand Up @@ -4674,8 +4674,16 @@ zpool_get_errlog(zpool_handle_t *zhp, nvlist_t **nverrlistp)
for (uint64_t i = 0; i < zblen; i++) {
nvlist_t *nv;

/* ignoring zb_blkid and zb_level for now */
/* filter out duplicate records */
if (i > 0 && zb[i-1].zb_objset == zb[i].zb_objset &&
zb[i-1].zb_object == zb[i].zb_object &&
zb[i-1].zb_level == zb[i].zb_level &&
zb[i-1].zb_blkid == zb[i].zb_blkid)
continue;

/* filter out duplicate files */
if (verbosity < 2 && i > 0 &&
zb[i-1].zb_objset == zb[i].zb_objset &&
zb[i-1].zb_object == zb[i].zb_object)
continue;

Expand All @@ -4691,6 +4699,18 @@ zpool_get_errlog(zpool_handle_t *zhp, nvlist_t **nverrlistp)
nvlist_free(nv);
goto nomem;
}
if (verbosity > 1) {
if (nvlist_add_uint64(nv, ZPOOL_ERR_LEVEL,
zb[i].zb_level) != 0) {
nvlist_free(nv);
goto nomem;
}
if (nvlist_add_uint64(nv, ZPOOL_ERR_BLKID,
zb[i].zb_blkid) != 0) {
nvlist_free(nv);
goto nomem;
}
}
if (nvlist_add_nvlist(*nverrlistp, "ejk", nv) != 0) {
nvlist_free(nv);
goto nomem;
Expand Down
6 changes: 4 additions & 2 deletions man/man8/zpool-status.8
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
.\" Copyright 2017 Nexenta Systems, Inc.
.\" Copyright (c) 2017 Open-E, Inc. All Rights Reserved.
.\"
.Dd February 14, 2024
.Dd July 1, 2025
.Dt ZPOOL-STATUS 8
.Os
.
Expand Down Expand Up @@ -156,7 +156,9 @@ See
Display vdev TRIM status.
.It Fl v
Displays verbose data error information, printing out a complete list of all
data errors since the last complete pool scrub.
files containing data errors since the last complete pool scrub.
Specified twice, prints out the complete list of all corrupt records within
each corrupt file.
If the head_errlog feature is enabled and files containing errors have been
removed then the respective filenames will not be reported in subsequent runs
of this command.
Expand Down
Loading