Date: Sat, 1 Sep 2018 02:22:27 +0000 (UTC) From: Kyle Evans <kevans@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r338417 - head/lib/libbe Message-ID: <201809010222.w812MRc5006148@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: kevans Date: Sat Sep 1 02:22:26 2018 New Revision: 338417 URL: https://svnweb.freebsd.org/changeset/base/338417 Log: 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) Modified: head/lib/libbe/be.c head/lib/libbe/be.h head/lib/libbe/be_access.c head/lib/libbe/be_error.c head/lib/libbe/be_info.c head/lib/libbe/libbe.3 Modified: head/lib/libbe/be.c ============================================================================== --- head/lib/libbe/be.c Fri Aug 31 21:45:05 2018 (r338416) +++ head/lib/libbe/be.c Sat Sep 1 02:22:26 2018 (r338417) @@ -270,8 +270,8 @@ be_snapshot(libbe_handle_t *lbh, const char *source, c 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 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, 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); Modified: head/lib/libbe/be.h ============================================================================== --- head/lib/libbe/be.h Fri Aug 31 21:45:05 2018 (r338416) +++ head/lib/libbe/be.h Sat Sep 1 02:22:26 2018 (r338417) @@ -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); Modified: head/lib/libbe/be_access.c ============================================================================== --- head/lib/libbe/be_access.c Fri Aug 31 21:45:05 2018 (r338416) +++ head/lib/libbe/be_access.c Sat Sep 1 02:22:26 2018 (r338417) @@ -114,8 +114,8 @@ be_mount(libbe_handle_t *lbh, char *bootenv, char *mou 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)); Modified: head/lib/libbe/be_error.c ============================================================================== --- head/lib/libbe/be_error.c Fri Aug 31 21:45:05 2018 (r338416) +++ head/lib/libbe/be_error.c Sat Sep 1 02:22:26 2018 (r338417) @@ -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"); Modified: head/lib/libbe/be_info.c ============================================================================== --- head/lib/libbe/be_info.c Fri Aug 31 21:45:05 2018 (r338416) +++ head/lib/libbe/be_info.c Sat Sep 1 02:22:26 2018 (r338417) @@ -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); } Modified: head/lib/libbe/libbe.3 ============================================================================== --- head/lib/libbe/libbe.3 Fri Aug 31 21:45:05 2018 (r338416) +++ head/lib/libbe/libbe.3 Sat Sep 1 02:22:26 2018 (r338417) @@ -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,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201809010222.w812MRc5006148>