From d9ac17a57f66bdbd482ab09ab238cb941821e13f Mon Sep 17 00:00:00 2001 From: Umer Saleem Date: Mon, 26 Sep 2022 18:40:43 +0500 Subject: [PATCH] Expose libzutil error info in libpc_handle_t In libzutil, for zpool_search_import and zpool_find_config, we use libpc_handle_t internally, which does not maintain error code and it is not exposed in the interface. Due to this, the error information is not propagated to the caller. Instead, an error message is printed on stderr. This commit adds lpc_error field in libpc_handle_t and exposes it in the interface, which can be used by the users of libzutil to get the appropriate error information and handle it accordingly. Users of the API can also control if they want to print the error message on stderr. Reviewed-by: Brian Behlendorf Reviewed-by: Ryan Moeller Signed-off-by: Umer Saleem Closes #13969 --- cmd/zdb/zdb.c | 8 +++- cmd/zhack.c | 8 +++- cmd/zpool/zpool_main.c | 9 +++- cmd/ztest.c | 8 +++- include/libzutil.h | 27 +++++++++-- lib/libzfs/libzfs.abi | 32 +++++++++++-- lib/libzutil/zutil_import.c | 92 ++++++++++++++++++++++--------------- lib/libzutil/zutil_import.h | 15 ------ 8 files changed, 130 insertions(+), 69 deletions(-) diff --git a/cmd/zdb/zdb.c b/cmd/zdb/zdb.c index 96db9c4b9a..18a94da1f7 100644 --- a/cmd/zdb/zdb.c +++ b/cmd/zdb/zdb.c @@ -8778,8 +8778,12 @@ main(int argc, char **argv) args.path = searchdirs; args.can_be_active = B_TRUE; - error = zpool_find_config(NULL, target_pool, &cfg, &args, - &libzpool_config_ops); + libpc_handle_t lpch = { + .lpc_lib_handle = NULL, + .lpc_ops = &libzpool_config_ops, + .lpc_printerr = B_TRUE + }; + error = zpool_find_config(&lpch, target_pool, &cfg, &args); if (error == 0) { diff --git a/cmd/zhack.c b/cmd/zhack.c index 8797a53e47..a1063ab147 100644 --- a/cmd/zhack.c +++ b/cmd/zhack.c @@ -140,8 +140,12 @@ zhack_import(char *target, boolean_t readonly) g_importargs.can_be_active = readonly; g_pool = strdup(target); - error = zpool_find_config(NULL, target, &config, &g_importargs, - &libzpool_config_ops); + libpc_handle_t lpch = { + .lpc_lib_handle = NULL, + .lpc_ops = &libzpool_config_ops, + .lpc_printerr = B_TRUE + }; + error = zpool_find_config(&lpch, target, &config, &g_importargs); if (error) fatal(NULL, FTAG, "cannot import '%s'", target); diff --git a/cmd/zpool/zpool_main.c b/cmd/zpool/zpool_main.c index 39ea615f63..8a4f3dd162 100644 --- a/cmd/zpool/zpool_main.c +++ b/cmd/zpool/zpool_main.c @@ -3773,7 +3773,12 @@ zpool_do_import(int argc, char **argv) idata.scan = do_scan; idata.policy = policy; - pools = zpool_search_import(g_zfs, &idata, &libzfs_config_ops); + libpc_handle_t lpch = { + .lpc_lib_handle = g_zfs, + .lpc_ops = &libzfs_config_ops, + .lpc_printerr = B_TRUE + }; + pools = zpool_search_import(&lpch, &idata); if (pools != NULL && pool_exists && (argc == 1 || strcmp(argv[0], argv[1]) == 0)) { @@ -3829,7 +3834,7 @@ zpool_do_import(int argc, char **argv) */ idata.scan = B_TRUE; nvlist_free(pools); - pools = zpool_search_import(g_zfs, &idata, &libzfs_config_ops); + pools = zpool_search_import(&lpch, &idata); err = import_pools(pools, props, mntopts, flags, argc >= 1 ? argv[0] : NULL, diff --git a/cmd/ztest.c b/cmd/ztest.c index e630d33531..668a267b66 100644 --- a/cmd/ztest.c +++ b/cmd/ztest.c @@ -7471,8 +7471,12 @@ ztest_import_impl(void) args.path = searchdirs; args.can_be_active = B_FALSE; - VERIFY0(zpool_find_config(NULL, ztest_opts.zo_pool, &cfg, &args, - &libzpool_config_ops)); + libpc_handle_t lpch = { + .lpc_lib_handle = NULL, + .lpc_ops = &libzpool_config_ops, + .lpc_printerr = B_TRUE + }; + VERIFY0(zpool_find_config(&lpch, ztest_opts.zo_pool, &cfg, &args)); VERIFY0(spa_import(ztest_opts.zo_pool, cfg, NULL, flags)); fnvlist_free(cfg); } diff --git a/include/libzutil.h b/include/libzutil.h index 0b4075c160..617dd0cd17 100644 --- a/include/libzutil.h +++ b/include/libzutil.h @@ -59,6 +59,15 @@ typedef const struct pool_config_ops { _LIBZUTIL_H pool_config_ops_t libzfs_config_ops; _LIBZUTIL_H pool_config_ops_t libzpool_config_ops; +typedef enum lpc_error { + LPC_SUCCESS = 0, /* no error -- success */ + LPC_BADCACHE = 2000, /* out of memory */ + LPC_BADPATH, /* must be an absolute path */ + LPC_NOMEM, /* out of memory */ + LPC_EACCESS, /* some devices require root privileges */ + LPC_UNKNOWN +} lpc_error_t; + typedef struct importargs { char **path; /* a list of paths to search */ int paths; /* number of paths to search */ @@ -70,10 +79,20 @@ typedef struct importargs { nvlist_t *policy; /* load policy (max txg, rewind, etc.) */ } importargs_t; -_LIBZUTIL_H nvlist_t *zpool_search_import(void *, importargs_t *, - pool_config_ops_t *); -_LIBZUTIL_H int zpool_find_config(void *, const char *, nvlist_t **, - importargs_t *, pool_config_ops_t *); +typedef struct libpc_handle { + int lpc_error; + boolean_t lpc_printerr; + boolean_t lpc_open_access_error; + boolean_t lpc_desc_active; + char lpc_desc[1024]; + pool_config_ops_t *lpc_ops; + void *lpc_lib_handle; +} libpc_handle_t; + +_LIBZUTIL_H const char *libpc_error_description(libpc_handle_t *); +_LIBZUTIL_H nvlist_t *zpool_search_import(libpc_handle_t *, importargs_t *); +_LIBZUTIL_H int zpool_find_config(libpc_handle_t *, const char *, nvlist_t **, + importargs_t *); _LIBZUTIL_H const char * const * zpool_default_search_paths(size_t *count); _LIBZUTIL_H int zpool_read_label(int, nvlist_t **, int *); diff --git a/lib/libzfs/libzfs.abi b/lib/libzfs/libzfs.abi index 3471bcac94..87b65a4cfe 100644 --- a/lib/libzfs/libzfs.abi +++ b/lib/libzfs/libzfs.abi @@ -191,6 +191,7 @@ + @@ -4569,7 +4570,32 @@ + + + + + + + + + + + + + + + + + + + + + + + + + @@ -4578,17 +4604,15 @@ - + - - + - diff --git a/lib/libzutil/zutil_import.c b/lib/libzutil/zutil_import.c index e3f1c89425..fbf390587f 100644 --- a/lib/libzutil/zutil_import.c +++ b/lib/libzutil/zutil_import.c @@ -71,6 +71,30 @@ #include "zutil_import.h" +const char * +libpc_error_description(libpc_handle_t *hdl) +{ + if (hdl->lpc_desc[0] != '\0') + return (hdl->lpc_desc); + + switch (hdl->lpc_error) { + case LPC_BADCACHE: + return (dgettext(TEXT_DOMAIN, "invalid or missing cache file")); + case LPC_BADPATH: + return (dgettext(TEXT_DOMAIN, "must be an absolute path")); + case LPC_NOMEM: + return (dgettext(TEXT_DOMAIN, "out of memory")); + case LPC_EACCESS: + return (dgettext(TEXT_DOMAIN, "some devices require root " + "privileges")); + case LPC_UNKNOWN: + return (dgettext(TEXT_DOMAIN, "unknown error")); + default: + assert(hdl->lpc_error == 0); + return (dgettext(TEXT_DOMAIN, "no error")); + } +} + static __attribute__((format(printf, 2, 3))) void zutil_error_aux(libpc_handle_t *hdl, const char *fmt, ...) { @@ -85,28 +109,27 @@ zutil_error_aux(libpc_handle_t *hdl, const char *fmt, ...) } static void -zutil_verror(libpc_handle_t *hdl, const char *error, const char *fmt, +zutil_verror(libpc_handle_t *hdl, lpc_error_t error, const char *fmt, va_list ap) { char action[1024]; (void) vsnprintf(action, sizeof (action), fmt, ap); + hdl->lpc_error = error; if (hdl->lpc_desc_active) hdl->lpc_desc_active = B_FALSE; else hdl->lpc_desc[0] = '\0'; - if (hdl->lpc_printerr) { - if (hdl->lpc_desc[0] != '\0') - error = hdl->lpc_desc; - - (void) fprintf(stderr, "%s: %s\n", action, error); - } + if (hdl->lpc_printerr) + (void) fprintf(stderr, "%s: %s\n", action, + libpc_error_description(hdl)); } static __attribute__((format(printf, 3, 4))) int -zutil_error_fmt(libpc_handle_t *hdl, const char *error, const char *fmt, ...) +zutil_error_fmt(libpc_handle_t *hdl, lpc_error_t error, + const char *fmt, ...) { va_list ap; @@ -120,7 +143,7 @@ zutil_error_fmt(libpc_handle_t *hdl, const char *error, const char *fmt, ...) } static int -zutil_error(libpc_handle_t *hdl, const char *error, const char *msg) +zutil_error(libpc_handle_t *hdl, lpc_error_t error, const char *msg) { return (zutil_error_fmt(hdl, error, "%s", msg)); } @@ -128,7 +151,7 @@ zutil_error(libpc_handle_t *hdl, const char *error, const char *msg) static int zutil_no_memory(libpc_handle_t *hdl) { - zutil_error(hdl, EZFS_NOMEM, "internal error"); + zutil_error(hdl, LPC_NOMEM, "internal error"); exit(1); } @@ -1244,8 +1267,8 @@ zpool_find_import_scan_dir(libpc_handle_t *hdl, pthread_mutex_t *lock, return (0); zutil_error_aux(hdl, "%s", strerror(error)); - (void) zutil_error_fmt(hdl, EZFS_BADPATH, dgettext( - TEXT_DOMAIN, "cannot resolve path '%s'"), dir); + (void) zutil_error_fmt(hdl, LPC_BADPATH, dgettext(TEXT_DOMAIN, + "cannot resolve path '%s'"), dir); return (error); } @@ -1253,8 +1276,8 @@ zpool_find_import_scan_dir(libpc_handle_t *hdl, pthread_mutex_t *lock, if (dirp == NULL) { error = errno; zutil_error_aux(hdl, "%s", strerror(error)); - (void) zutil_error_fmt(hdl, EZFS_BADPATH, - dgettext(TEXT_DOMAIN, "cannot open '%s'"), path); + (void) zutil_error_fmt(hdl, LPC_BADPATH, dgettext(TEXT_DOMAIN, + "cannot open '%s'"), path); return (error); } @@ -1315,8 +1338,8 @@ zpool_find_import_scan_path(libpc_handle_t *hdl, pthread_mutex_t *lock, } zutil_error_aux(hdl, "%s", strerror(error)); - (void) zutil_error_fmt(hdl, EZFS_BADPATH, dgettext( - TEXT_DOMAIN, "cannot resolve path '%s'"), dir); + (void) zutil_error_fmt(hdl, LPC_BADPATH, dgettext(TEXT_DOMAIN, + "cannot resolve path '%s'"), dir); goto out; } @@ -1353,7 +1376,7 @@ zpool_find_import_scan(libpc_handle_t *hdl, pthread_mutex_t *lock, continue; zutil_error_aux(hdl, "%s", strerror(error)); - (void) zutil_error_fmt(hdl, EZFS_BADPATH, dgettext( + (void) zutil_error_fmt(hdl, LPC_BADPATH, dgettext( TEXT_DOMAIN, "cannot resolve path '%s'"), dir[i]); goto error; } @@ -1574,16 +1597,16 @@ zpool_find_import_cached(libpc_handle_t *hdl, importargs_t *iarg) if ((fd = open(iarg->cachefile, O_RDONLY | O_CLOEXEC)) < 0) { zutil_error_aux(hdl, "%s", strerror(errno)); - (void) zutil_error(hdl, EZFS_BADCACHE, - dgettext(TEXT_DOMAIN, "failed to open cache file")); + (void) zutil_error(hdl, LPC_BADCACHE, dgettext(TEXT_DOMAIN, + "failed to open cache file")); return (NULL); } if (fstat64(fd, &statbuf) != 0) { zutil_error_aux(hdl, "%s", strerror(errno)); (void) close(fd); - (void) zutil_error(hdl, EZFS_BADCACHE, - dgettext(TEXT_DOMAIN, "failed to get size of cache file")); + (void) zutil_error(hdl, LPC_BADCACHE, dgettext(TEXT_DOMAIN, + "failed to get size of cache file")); return (NULL); } @@ -1595,8 +1618,7 @@ zpool_find_import_cached(libpc_handle_t *hdl, importargs_t *iarg) if (read(fd, buf, statbuf.st_size) != statbuf.st_size) { (void) close(fd); free(buf); - (void) zutil_error(hdl, EZFS_BADCACHE, - dgettext(TEXT_DOMAIN, + (void) zutil_error(hdl, LPC_BADCACHE, dgettext(TEXT_DOMAIN, "failed to read cache file contents")); return (NULL); } @@ -1605,8 +1627,7 @@ zpool_find_import_cached(libpc_handle_t *hdl, importargs_t *iarg) if (nvlist_unpack(buf, statbuf.st_size, &raw, 0) != 0) { free(buf); - (void) zutil_error(hdl, EZFS_BADCACHE, - dgettext(TEXT_DOMAIN, + (void) zutil_error(hdl, LPC_BADCACHE, dgettext(TEXT_DOMAIN, "invalid or corrupt cache file contents")); return (NULL); } @@ -1777,25 +1798,20 @@ zpool_find_import(libpc_handle_t *hdl, importargs_t *iarg) nvlist_t * -zpool_search_import(void *hdl, importargs_t *import, pool_config_ops_t *pco) +zpool_search_import(libpc_handle_t *hdl, importargs_t *import) { - libpc_handle_t handle = { 0 }; nvlist_t *pools = NULL; - handle.lpc_lib_handle = hdl; - handle.lpc_ops = pco; - handle.lpc_printerr = B_TRUE; - verify(import->poolname == NULL || import->guid == 0); if (import->cachefile != NULL) - pools = zpool_find_import_cached(&handle, import); + pools = zpool_find_import_cached(hdl, import); else - pools = zpool_find_import(&handle, import); + pools = zpool_find_import(hdl, import); if ((pools == NULL || nvlist_empty(pools)) && - handle.lpc_open_access_error && geteuid() != 0) { - (void) zutil_error(&handle, EZFS_EACESS, dgettext(TEXT_DOMAIN, + hdl->lpc_open_access_error && geteuid() != 0) { + (void) zutil_error(hdl, LPC_EACCESS, dgettext(TEXT_DOMAIN, "no pools found")); } @@ -1819,8 +1835,8 @@ pool_match(nvlist_t *cfg, char *tgt) } int -zpool_find_config(void *hdl, const char *target, nvlist_t **configp, - importargs_t *args, pool_config_ops_t *pco) +zpool_find_config(libpc_handle_t *hdl, const char *target, nvlist_t **configp, + importargs_t *args) { nvlist_t *pools; nvlist_t *match = NULL; @@ -1834,7 +1850,7 @@ zpool_find_config(void *hdl, const char *target, nvlist_t **configp, if ((sepp = strpbrk(targetdup, "/@")) != NULL) *sepp = '\0'; - pools = zpool_search_import(hdl, args, pco); + pools = zpool_search_import(hdl, args); if (pools != NULL) { nvpair_t *elem = NULL; diff --git a/lib/libzutil/zutil_import.h b/lib/libzutil/zutil_import.h index 482315e441..f851a91132 100644 --- a/lib/libzutil/zutil_import.h +++ b/lib/libzutil/zutil_import.h @@ -28,26 +28,11 @@ #ifndef _LIBZUTIL_ZUTIL_IMPORT_H_ #define _LIBZUTIL_ZUTIL_IMPORT_H_ -#define EZFS_BADCACHE "invalid or missing cache file" -#define EZFS_BADPATH "must be an absolute path" -#define EZFS_NOMEM "out of memory" -#define EZFS_EACESS "some devices require root privileges" - #define IMPORT_ORDER_PREFERRED_1 1 #define IMPORT_ORDER_PREFERRED_2 2 #define IMPORT_ORDER_SCAN_OFFSET 10 #define IMPORT_ORDER_DEFAULT 100 -typedef struct libpc_handle { - boolean_t lpc_printerr; - boolean_t lpc_open_access_error; - boolean_t lpc_desc_active; - char lpc_desc[1024]; - pool_config_ops_t *lpc_ops; - void *lpc_lib_handle; -} libpc_handle_t; - - int label_paths(libpc_handle_t *hdl, nvlist_t *label, char **path, char **devid); int zpool_find_import_blkid(libpc_handle_t *hdl, pthread_mutex_t *lock,