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 <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Signed-off-by: Umer Saleem <[email protected]>
Closes openzfs#13969

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) {
 

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);
 

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,

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);
 }

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 *);

lib/libzfs/libzfs.abi

@@ -191,6 +191,7 @@
     <elf-symbol name='getzoneid' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='is_mounted' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='is_mpath_whole_disk' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+    <elf-symbol name='libpc_error_description' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='libspl_assertf' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='libspl_set_assert_ok' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='libzfs_add_handle' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
@@ -4569,7 +4570,32 @@
       </data-member>
     </class-decl>
     <typedef-decl name='importargs_t' type-id='7ac83801' id='7a842a6b'/>
+    <class-decl name='libpc_handle' size-in-bits='8448' is-struct='yes' visibility='default' id='7c8737f0'>
+      <data-member access='public' layout-offset-in-bits='0'>
+        <var-decl name='lpc_error' type-id='95e97e5e' visibility='default'/>
+      </data-member>
+      <data-member access='public' layout-offset-in-bits='32'>
+        <var-decl name='lpc_printerr' type-id='c19b74c3' visibility='default'/>
+      </data-member>
+      <data-member access='public' layout-offset-in-bits='64'>
+        <var-decl name='lpc_open_access_error' type-id='c19b74c3' visibility='default'/>
+      </data-member>
+      <data-member access='public' layout-offset-in-bits='96'>
+        <var-decl name='lpc_desc_active' type-id='c19b74c3' visibility='default'/>
+      </data-member>
+      <data-member access='public' layout-offset-in-bits='128'>
+        <var-decl name='lpc_desc' type-id='b54ce520' visibility='default'/>
+      </data-member>
+      <data-member access='public' layout-offset-in-bits='8320'>
+        <var-decl name='lpc_ops' type-id='f095e320' visibility='default'/>
+      </data-member>
+      <data-member access='public' layout-offset-in-bits='8384'>
+        <var-decl name='lpc_lib_handle' type-id='eaa32e2f' visibility='default'/>
+      </data-member>
+    </class-decl>
+    <typedef-decl name='libpc_handle_t' type-id='7c8737f0' id='8a70a786'/>
     <pointer-type-def type-id='7a842a6b' size-in-bits='64' id='07ee4a58'/>
+    <pointer-type-def type-id='8a70a786' size-in-bits='64' id='5507783b'/>
     <pointer-type-def type-id='b1e62775' size-in-bits='64' id='f095e320'/>
     <function-decl name='zpool_read_label' mangled-name='zpool_read_label' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='zpool_read_label'>
       <parameter type-id='95e97e5e' name='fd'/>
@@ -4578,17 +4604,15 @@
       <return type-id='95e97e5e'/>
     </function-decl>
     <function-decl name='zpool_search_import' mangled-name='zpool_search_import' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='zpool_search_import'>
-      <parameter type-id='eaa32e2f' name='hdl'/>
+      <parameter type-id='5507783b' name='hdl'/>
       <parameter type-id='07ee4a58' name='import'/>
-      <parameter type-id='f095e320' name='pco'/>
       <return type-id='5ce45b60'/>
     </function-decl>
     <function-decl name='zpool_find_config' mangled-name='zpool_find_config' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='zpool_find_config'>
-      <parameter type-id='eaa32e2f' name='hdl'/>
+      <parameter type-id='5507783b' name='hdl'/>
       <parameter type-id='80f4b756' name='target'/>
       <parameter type-id='857bb57e' name='configp'/>
       <parameter type-id='07ee4a58' name='args'/>
-      <parameter type-id='f095e320' name='pco'/>
       <return type-id='95e97e5e'/>
     </function-decl>
   </abi-instr>

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,15 +143,15 @@ 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));
 }
 
 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,17 +1267,17 @@ 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);
 	}
 
 	dirp = opendir(path);
 	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;