libbe(3): Fix error handling with respect to be_exists

Some paths through be_exists will set the error state, others will not
There are multiple reasons that a call can fail, so clean it up a bit: all
paths now return an appropriate error code so the caller can attempt to
distinguish between a BE legitimately not existing and just having the wrong
mountpoint. The caller is expected to bubble the error through to the
internal error handler as needed.

This fixes some unfriendliness with bectl(8)'s activate subcommand, where
it might fail due to a bad mountpoint but the only message output is a
generic "failed to activate" message.

Approved by:	re (gjb)

lib/libbe/be.c

@@ -270,8 +270,8 @@ be_snapshot(libbe_handle_t *lbh, const char *source, const char *snap_name,
 
 	be_root_concat(lbh, source, buf);
 
-	if (!be_exists(lbh, buf))
-		return (BE_ERR_NOENT);
+	if ((err = be_exists(lbh, buf)) != 0)
+		return (set_error(lbh, err));
 
 	if (snap_name != NULL) {
 		if (strlcat(buf, "@", sizeof(buf)) >= sizeof(buf))
@@ -528,10 +528,10 @@ be_validate_snap(libbe_handle_t *lbh, const char *snap_name)
 
 	if ((err = zfs_prop_get(zfs_hdl, ZFS_PROP_MOUNTPOINT, buf,
 	    sizeof(buf), NULL, NULL, 0, 1)) != 0)
-		err = BE_ERR_INVORIGIN;
+		err = BE_ERR_BADMOUNT;
 
 	if ((err != 0) && (strncmp(buf, "/", sizeof(buf)) != 0))
-		err = BE_ERR_INVORIGIN;
+		err = BE_ERR_BADMOUNT;
 
 	zfs_close(zfs_hdl);
 
@@ -935,8 +935,8 @@ be_activate(libbe_handle_t *lbh, const char *bootenv, bool temporary)
 	be_root_concat(lbh, bootenv, be_path);
 
 	/* Note: be_exists fails if mountpoint is not / */
-	if (!be_exists(lbh, be_path))
-		return (BE_ERR_NOENT);
+	if ((err = be_exists(lbh, be_path)) != 0)
+		return (set_error(lbh, err));
 
 	if (temporary) {
 		config = zpool_get_config(lbh->active_phandle, NULL);

lib/libbe/be.h

@@ -49,7 +49,7 @@ typedef enum be_error {
 	BE_ERR_BADPATH,		/* path not suitable for operation */
 	BE_ERR_PATHBUSY,	/* requested path is busy */
 	BE_ERR_PATHLEN,         /* provided name exceeds maximum length limit */
-	BE_ERR_INVORIGIN,       /* snapshot origin's mountpoint is not '/' */
+	BE_ERR_BADMOUNT,        /* mountpoint is not '/' */
 	BE_ERR_NOORIGIN,        /* could not open snapshot's origin */
 	BE_ERR_MOUNTED,         /* boot environment is already mounted */
 	BE_ERR_NOMOUNT,         /* boot environment is not mounted */
@@ -118,7 +118,7 @@ void libbe_print_on_error(libbe_handle_t *, bool);
 int be_root_concat(libbe_handle_t *, const char *, char *);
 int be_validate_name(libbe_handle_t * __unused, const char *);
 int be_validate_snap(libbe_handle_t *, const char *);
-bool be_exists(libbe_handle_t *, char *);
+int be_exists(libbe_handle_t *, char *);
 
 int be_export(libbe_handle_t *, const char *, int fd);
 int be_import(libbe_handle_t *, const char *, int fd);

lib/libbe/be_access.c

@@ -114,8 +114,8 @@ be_mount(libbe_handle_t *lbh, char *bootenv, char *mountpoint, int flags,
 	if ((err = be_root_concat(lbh, bootenv, be)) != 0)
 		return (set_error(lbh, err));
 
-	if (!be_exists(lbh, bootenv))
-		return (set_error(lbh, BE_ERR_NOENT));
+	if ((err = be_exists(lbh, bootenv)) != 0)
+		return (set_error(lbh, err));
 
 	if (is_mounted(lbh->lzh, be, NULL))
 		return (set_error(lbh, BE_ERR_MOUNTED));

lib/libbe/be_error.c

@@ -75,8 +75,8 @@ libbe_error_description(libbe_handle_t *lbh)
 	case BE_ERR_PATHLEN:
 		return ("provided path name exceeds maximum length limit");
 
-	case BE_ERR_INVORIGIN:
-		return ("snapshot origin's mountpoint is not \"/\"");
+	case BE_ERR_BADMOUNT:
+		return ("mountpoint is not \"/\"");
 
 	case BE_ERR_NOORIGIN:
 		return ("could not open snapshot's origin");

lib/libbe/be_info.c

@@ -285,7 +285,7 @@ be_prop_list_free(nvlist_t *be_list)
 /*
  * Usage
  */
-bool
+int
 be_exists(libbe_handle_t *lbh, char *be)
 {
 	char buf[BE_MAXPATHLEN];
@@ -296,25 +296,23 @@ be_exists(libbe_handle_t *lbh, char *be)
 	be_root_concat(lbh, be, buf);
 
 	if (!zfs_dataset_exists(lbh->lzh, buf, ZFS_TYPE_DATASET))
-		return (false);
+		return (BE_ERR_NOENT);
 
 	/* Also check if it's mounted at / */
-	if (be_prop_list_alloc(&dsprops) != 0) {
-		set_error(lbh, BE_ERR_UNKNOWN);
-		return (false);
-	}
+	if (be_prop_list_alloc(&dsprops) != 0)
+		return (BE_ERR_UNKNOWN);
 
 	if (be_get_dataset_props(lbh, buf, dsprops) != 0) {
 		nvlist_free(dsprops);
-		return (false);
+		return (BE_ERR_UNKNOWN);
 	}
 
 	if (nvlist_lookup_string(dsprops, "mountpoint", &mntpoint) == 0) {
 		valid = (strcmp(mntpoint, "/") == 0);
 		nvlist_free(dsprops);
-		return (valid);
+		return (valid ? BE_ERR_SUCCESS : BE_ERR_BADMOUNT);
 	}
 
 	nvlist_free(dsprops);
-	return (false);
+	return (BE_ERR_BADMOUNT);
 }

lib/libbe/libbe.3

@@ -28,7 +28,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd August 24, 2018
+.Dd August 31, 2018
 .Dt LIBBE 3
 .Os
 .Sh NAME
@@ -111,7 +111,7 @@
 .Ft int
 .Fn be_validate_snap "libbe_handle_t *hdl" "const char *snap"
 .Pp
-.Ft bool
+.Ft int
 .Fn be_exists "libbe_handle_t *hdl" "char *be_name"
 .Pp
 .Ft int
@@ -352,6 +352,8 @@ The
 function will check whether the given boot environment exists and has a
 mountpoint of
 .Pa / .
+This function does not set the internal library error state, but will return
+the appropriate error.
 .Pp
 The
 .Fn be_export
@@ -444,7 +446,7 @@ BE_ERR_DESTROYMNT,
 BE_ERR_BADPATH,
 BE_ERR_PATHBUSY,
 BE_ERR_PATHLEN,
-BE_ERR_INVORIGIN,
+BE_ERR_BADMOUNT,
 BE_ERR_NOORIGIN,
 BE_ERR_MOUNTED,
 BE_ERR_NOMOUNT,