socsvn commit: r324821 - soc2017/kneitinger/libbe-head/lib/libbe
kneitinger at FreeBSD.org
kneitinger at FreeBSD.org
Wed Jul 19 12:00:54 UTC 2017
Author: kneitinger
Date: Wed Jul 19 12:00:51 2017
New Revision: 324821
URL: http://svnweb.FreeBSD.org/socsvn/?view=rev&rev=324821
Log:
libbe(3): refactor library to avoid redundant error checking
Modified:
soc2017/kneitinger/libbe-head/lib/libbe/be.c
soc2017/kneitinger/libbe-head/lib/libbe/be.h
soc2017/kneitinger/libbe-head/lib/libbe/be_access.c
soc2017/kneitinger/libbe-head/lib/libbe/be_impl.h
soc2017/kneitinger/libbe-head/lib/libbe/be_info.c
Modified: soc2017/kneitinger/libbe-head/lib/libbe/be.c
==============================================================================
--- soc2017/kneitinger/libbe-head/lib/libbe/be.c Wed Jul 19 09:59:32 2017 (r324820)
+++ soc2017/kneitinger/libbe-head/lib/libbe/be.c Wed Jul 19 12:00:51 2017 (r324821)
@@ -26,6 +26,7 @@
* SUCH DAMAGE.
*/
+#include <sys/stat.h>
#include <sys/types.h>
#include <ctype.h>
@@ -44,60 +45,61 @@
libbe_handle_t *
libbe_init(void)
{
+ char buf[BE_MAXPATHLEN];
+ struct stat sb;
+ dev_t root_dev;
libbe_handle_t *lbh;
- char buf[MAXPATHLEN];
+ char *pos;
+ // TODO: use errno here??
- if ((lbh = calloc(1, sizeof(libbe_handle_t))) == NULL) {
+ /* Verify that /boot and / are mounted on the same filesystem */
+ if (stat("/", &sb) != 0) {
return (NULL);
}
+ root_dev = sb.st_dev;
- if ((lbh->lzh = libzfs_init()) == NULL) {
- free(lbh);
+ if (stat("/boot", &sb) != 0) {
return (NULL);
}
- /* Obtain path to active boot environment */
- if ((kenv(KENV_GET, "zfs_be_active", buf, MAXPATHLEN)) == -1) {
- libzfs_fini(lbh->lzh);
- free(lbh);
+ if (root_dev != sb.st_dev) {
return (NULL);
}
- /* Remove leading 'zfs:' if present, otherwise use value as-is */
- char *path = strrchr(buf, ':');
- path = path ? path + 1 : buf;
+ if ((lbh = calloc(1, sizeof(libbe_handle_t))) == NULL) {
+ return (NULL);
+ }
- /* Obtain zfs_handle_t to the active bootenv */
- if ((lbh->be_active = zfs_open(lbh->lzh, path,
- ZFS_TYPE_DATASET)) == NULL) {
- libzfs_fini(lbh->lzh);
+ if ((lbh->lzh = libzfs_init()) == NULL) {
free(lbh);
return (NULL);
}
- /* Obtain path to boot environment root */
- if ((kenv(KENV_GET, "zfs_be_root", buf, MAXPATHLEN)) == -1) {
- zfs_close(lbh->be_active);
+ /* Obtain path to active boot environment */
+ if ((kenv(KENV_GET, "zfs_be_active", lbh->active,
+ BE_MAXPATHLEN)) == -1) {
libzfs_fini(lbh->lzh);
free(lbh);
return (NULL);
}
+ /* Remove leading 'zfs:' if present, otherwise use value as-is */
+ if ((pos = strrchr(lbh->active, ':')) != NULL) {
+ strncpy(lbh->active, pos + sizeof(char), BE_MAXPATHLEN);
+ }
- /* Obtain zfs_handle_t to the bootenv root */
- if ((lbh->be_root =
- zfs_open(lbh->lzh, buf, ZFS_TYPE_DATASET)) == NULL) {
- zfs_close(lbh->be_active);
+ /* Obtain path to boot environment root */
+ if ((kenv(KENV_GET, "zfs_be_root", lbh->root, BE_MAXPATHLEN)) == -1) {
libzfs_fini(lbh->lzh);
free(lbh);
return (NULL);
}
- /* TODO: verify that /boot is mounted on the active be */
-
- prop_list_builder(lbh);
-
+ /* Remove leading 'zfs:' if present, otherwise use value as-is */
+ if ((pos = strrchr(lbh->root, ':')) != NULL) {
+ strncpy(lbh->root, pos + sizeof(char), BE_MAXPATHLEN);
+ }
return (lbh);
}
@@ -109,10 +111,7 @@
void
libbe_close(libbe_handle_t *lbh)
{
- zfs_close(lbh->be_active);
- zfs_close(lbh->be_root);
libzfs_fini(lbh->lzh);
- prop_list_free(lbh);
free(lbh);
}
@@ -126,14 +125,14 @@
be_destroy(libbe_handle_t *lbh, char *name, int options)
{
zfs_handle_t *fs;
- char path[MAXPATHLEN];
+ char path[BE_MAXPATHLEN];
char *p = path;
int mounted;
int force = options & BE_DESTROY_FORCE;
int err = BE_ERR_SUCCESS;
- snprintf(path, MAXPATHLEN, "%s/%s", zfs_get_name(lbh->be_root), name);
+ snprintf(path, BE_MAXPATHLEN, "%s/%s", lbh->root, name);
if (!zfs_dataset_exists(lbh->lzh, path, ZFS_TYPE_DATASET)) {
return (set_error(lbh, BE_ERR_NOENT));
@@ -141,7 +140,7 @@
fs = zfs_open(lbh->lzh, p, ZFS_TYPE_DATASET);
- if (strcmp(path, zfs_get_name(lbh->be_active)) != 0) {
+ if (strcmp(path, lbh->active) != 0) {
return (set_error(lbh, BE_ERR_DESTROYACT));
}
@@ -162,8 +161,8 @@
be_create(libbe_handle_t *lbh, char *name)
{
int err, pos;
- char be_path[MAXPATHLEN];
- char snap_name[MAXPATHLEN];
+ char be_path[BE_MAXPATHLEN];
+ char snap_name[BE_MAXPATHLEN];
time_t rawtime;
zfs_handle_t *snap_hdl;
@@ -176,9 +175,11 @@
return (err);
}
- pos = snprintf(be_path, MAXPATHLEN, "%s/%s", be_root_path(lbh), name);
+ // TODO: be_root_concat
+ pos =
+ snprintf(be_path, BE_MAXPATHLEN, "%s/%s", be_root_path(lbh), name);
- if ((pos < 0) || (pos >= MAXPATHLEN)) {
+ if ((pos < 0) || (pos >= BE_MAXPATHLEN)) {
err = set_error(lbh, BE_ERR_PATHLEN);
}
@@ -189,8 +190,8 @@
time(&rawtime);
- pos = snprintf(snap_name, MAXPATHLEN, "%s", be_active_path(lbh));
- strftime(snap_name + pos, MAXPATHLEN - pos,
+ pos = snprintf(snap_name, BE_MAXPATHLEN, "%s", be_active_path(lbh));
+ strftime(snap_name + pos, BE_MAXPATHLEN - pos,
"@%F-%T", localtime(&rawtime));
@@ -200,6 +201,7 @@
return (-1);
}
+ // TODO: verify not null!!
snap_hdl = zfs_open(lbh->lzh, snap_name, ZFS_TYPE_SNAPSHOT);
// TODO: should any props be in the last arg (nvlist)?
@@ -219,8 +221,8 @@
be_create_from_existing(libbe_handle_t *lbh, char *name, char *snap)
{
int err, pos;
- char be_path[MAXPATHLEN];
- char snap_name[MAXPATHLEN];
+ char be_path[BE_MAXPATHLEN];
+ char snap_name[BE_MAXPATHLEN];
time_t rawtime;
zfs_handle_t *snap_hdl;
@@ -242,9 +244,10 @@
return (err);
}
- pos = snprintf(be_path, MAXPATHLEN, "%s/%s", be_root_path(lbh), name);
+ pos =
+ snprintf(be_path, BE_MAXPATHLEN, "%s/%s", be_root_path(lbh), name);
- if ((pos < 0) || (pos >= MAXPATHLEN)) {
+ if ((pos < 0) || (pos >= BE_MAXPATHLEN)) {
err = set_error(lbh, BE_ERR_PATHLEN);
}
@@ -276,12 +279,12 @@
be_validate_snap(libbe_handle_t *lbh, char *snap_name)
{
zfs_handle_t *zfs_hdl;
- char buf[MAXPATHLEN];
+ char buf[BE_MAXPATHLEN];
char *snap_delim;
char *mountpoint;
int err = 0;
- if (strlen(snap_name) >= MAXPATHLEN) {
+ if (strlen(snap_name) >= BE_MAXPATHLEN) {
return (BE_ERR_PATHLEN);
}
@@ -289,7 +292,7 @@
return (BE_ERR_NOENT);
}
- strncpy(buf, snap_name, MAXPATHLEN);
+ strncpy(buf, snap_name, BE_MAXPATHLEN);
if ((snap_delim = strchr(buf, '@')) == NULL) {
return (BE_ERR_INVALIDNAME);
@@ -301,12 +304,12 @@
return (BE_ERR_NOORIGIN);
}
- if (err = zfs_prop_get(zfs_hdl, ZFS_PROP_MOUNTPOINT, buf, MAXPATHLEN,
+ if (err = zfs_prop_get(zfs_hdl, ZFS_PROP_MOUNTPOINT, buf, BE_MAXPATHLEN,
NULL, NULL, 0, 1)) {
err = BE_ERR_INVORIGIN;
}
- if ((err != 0) && (strncmp(buf, "/", MAXPATHLEN) != 0)) {
+ if ((err != 0) && (strncmp(buf, "/", BE_MAXPATHLEN) != 0)) {
err = BE_ERR_INVORIGIN;
}
@@ -317,21 +320,35 @@
/*
- * Appends the name argument on the root boot environment path and copies the
- * resulting string into the result buffer. Returns 0 upon success and
- * BE_ERR_PATHLEN if the resulting path is longer than MAXPATHLEN. Does not set
- * internal library error state.
+ * Idempotently appends the name argument to the root boot environment path
+ * and copies the resulting string into the result buffer (which is assumed
+ * to be at least BE_MAXPATHLEN characters long. Returns BE_ERR_SUCCESS upon
+ * success, BE_ERR_PATHLEN if the resulting path is longer than BE_MAXPATHLEN,
+ * or BE_ERR_INVALIDNAME if the name is a path that does not begin with
+ * zfs_be_root. Does not set internal library error state.
*/
int
be_root_concat(libbe_handle_t *lbh, char *name, char *result)
{
- // TODO: make idempotent. As in: if it already is of the form:
- // pool/beroot/be, return that
+ size_t name_len, root_len;
+
+ name_len = strlen(name);
+
+ /* Act idempotently; return be name if it is already a full path */
+ if (strrchr(name, '/') != NULL) {
+ if (strstr(name, lbh->root) != name) {
+ return (BE_ERR_INVALIDNAME);
+ }
- const char *root = be_root_path(lbh);
- int n = snprintf(result, MAXPATHLEN, "%s/%s", root, name);
+ if (name_len >= BE_MAXPATHLEN) {
+ return (BE_ERR_PATHLEN);
+ }
- if ((n >= 0) && (n < MAXPATHLEN)) {
+ strncpy(result, name, BE_MAXPATHLEN);
+ return (BE_ERR_SUCCESS);
+ } else if (name_len + root_len + 1 < BE_MAXPATHLEN) {
+ root_len = strlen(lbh->root);
+ snprintf(result, BE_MAXPATHLEN, "%s/%s", lbh->root, name);
return (BE_ERR_SUCCESS);
}
@@ -367,8 +384,8 @@
int
be_rename(libbe_handle_t *lbh, char *old, char *new)
{
- char full_old[MAXPATHLEN];
- char full_new[MAXPATHLEN];
+ char full_old[BE_MAXPATHLEN];
+ char full_new[BE_MAXPATHLEN];
zfs_handle_t *zfs_hdl;
int err;
Modified: soc2017/kneitinger/libbe-head/lib/libbe/be.h
==============================================================================
--- soc2017/kneitinger/libbe-head/lib/libbe/be.h Wed Jul 19 09:59:32 2017 (r324820)
+++ soc2017/kneitinger/libbe-head/lib/libbe/be.h Wed Jul 19 12:00:51 2017 (r324821)
@@ -32,6 +32,8 @@
#include <libzfs.h>
#include <stdbool.h>
+#define BE_MAXPATHLEN ZFS_MAX_DATASET_NAME_LEN
+
typedef struct libbe_handle libbe_handle_t;
typedef enum be_error {
Modified: soc2017/kneitinger/libbe-head/lib/libbe/be_access.c
==============================================================================
--- soc2017/kneitinger/libbe-head/lib/libbe/be_access.c Wed Jul 19 09:59:32 2017 (r324820)
+++ soc2017/kneitinger/libbe-head/lib/libbe/be_access.c Wed Jul 19 12:00:51 2017 (r324821)
@@ -35,7 +35,7 @@
int
be_mount(libbe_handle_t *lbh, char *bootenv, char *mountpoint, int flags)
{
- char be[MAXPATHLEN];
+ char be[BE_MAXPATHLEN];
zfs_handle_t *zfs_hdl;
char *path;
int mntflags;
@@ -75,7 +75,7 @@
be_unmount(libbe_handle_t *lbh, char *bootenv, int flags)
{
int err, mntflags;
- char be[MAXPATHLEN];
+ char be[BE_MAXPATHLEN];
struct statfs *mntbuf;
int mntsize;
char *mntpath;
Modified: soc2017/kneitinger/libbe-head/lib/libbe/be_impl.h
==============================================================================
--- soc2017/kneitinger/libbe-head/lib/libbe/be_impl.h Wed Jul 19 09:59:32 2017 (r324820)
+++ soc2017/kneitinger/libbe-head/lib/libbe/be_impl.h Wed Jul 19 12:00:51 2017 (r324821)
@@ -36,16 +36,12 @@
struct libbe_handle {
libzfs_handle_t *lzh;
- zfs_handle_t *be_root;
- zfs_handle_t *be_active;
+ char root[BE_MAXPATHLEN];
+ char active[BE_MAXPATHLEN];
be_error_t error;
- nvlist_t *list;
bool print_on_err;
};
-int prop_list_builder(libbe_handle_t *);
-void prop_list_free(libbe_handle_t *);
-
int set_error(libbe_handle_t *, be_error_t);
#endif /* _LIBBE_IMPL_H */
Modified: soc2017/kneitinger/libbe-head/lib/libbe/be_info.c
==============================================================================
--- soc2017/kneitinger/libbe-head/lib/libbe/be_info.c Wed Jul 19 09:59:32 2017 (r324820)
+++ soc2017/kneitinger/libbe-head/lib/libbe/be_info.c Wed Jul 19 12:00:51 2017 (r324821)
@@ -29,7 +29,13 @@
#include "be.h"
#include "be_impl.h"
+typedef struct prop_data {
+ nvlist_t *list;
+ libbe_handle_t *lbh;
+} prop_data_t;
+
static int prop_list_builder_cb(zfs_handle_t *, void *);
+static int prop_list_builder(prop_data_t *);
/*
* Returns the name of the active boot environment
@@ -37,9 +43,7 @@
const char *
be_active_name(libbe_handle_t *lbh)
{
- const char *full_path = zfs_get_name(lbh->be_active);
-
- return (strrchr(full_path, '/') + 1);
+ return (strrchr(lbh->active, '/') + sizeof(char));
}
@@ -49,7 +53,7 @@
const char *
be_active_path(libbe_handle_t *lbh)
{
- return (zfs_get_name(lbh->be_active));
+ return (lbh->active);
}
@@ -59,7 +63,7 @@
const char *
be_root_path(libbe_handle_t *lbh)
{
- return (zfs_get_name(lbh->be_root));
+ return (lbh->root);
}
@@ -70,9 +74,12 @@
nvlist_t *
be_get_bootenv_props(libbe_handle_t *lbh)
{
- // TODO: Should there be a dirty flag that re-calcs the list if an op
- // has changed it?
- return (lbh->list);
+ prop_data_t data;
+
+ data.lbh = lbh;
+ prop_list_builder(&data);
+
+ return (data.list);
}
@@ -82,7 +89,7 @@
* TODO: should any other properties be included?
*/
static int
-prop_list_builder_cb(zfs_handle_t *zfs_hdl, void *data)
+prop_list_builder_cb(zfs_handle_t *zfs_hdl, void *data_p)
{
/*
* TODO:
@@ -90,13 +97,17 @@
* error checking
*/
+ char buf[512];
+ prop_data_t *data;
boolean_t mounted, active, nextboot;
- char buf[512];
+
+ data = (prop_data_t *)data_p;
+
nvlist_t *props;
- libbe_handle_t *lbh = (libbe_handle_t *)data;
+ libbe_handle_t *lbh = data->lbh;
nvlist_alloc(&props, NV_UNIQUE_NAME, KM_SLEEP);
@@ -158,7 +169,7 @@
/* TODO figure out how to read nextboot (set in libzfs_pool.c) */
- nvlist_add_nvlist(lbh->list, name, props);
+ nvlist_add_nvlist(data->list, name, props);
return (0);
}
@@ -170,21 +181,27 @@
* TODO: ensure that this is always consistent (run after adds, deletes,
* renames,etc
*/
-int
-prop_list_builder(libbe_handle_t *lbh)
+static int
+prop_list_builder(prop_data_t *data)
{
- if (lbh->list != NULL) {
- /* TODO: should actually call prop_list_free */
- nvlist_free(lbh->list);
- return (1);
- }
+ zfs_handle_t *root_hdl;
- if (nvlist_alloc(&lbh->list, NV_UNIQUE_NAME, KM_SLEEP) != 0) {
+ if (nvlist_alloc(&(data->list), NV_UNIQUE_NAME, KM_SLEEP) != 0) {
/* TODO: actually handle error */
return (1);
}
- zfs_iter_filesystems(lbh->be_root, prop_list_builder_cb, lbh);
+
+ if ((root_hdl =
+ zfs_open(data->lbh->lzh, data->lbh->root,
+ ZFS_TYPE_FILESYSTEM)) == NULL) {
+ return (BE_ERR_ZFSOPEN);
+ }
+
+ // TODO: some error checking here
+ zfs_iter_filesystems(root_hdl, prop_list_builder_cb, data);
+
+ zfs_close(root_hdl);
return (0);
}
@@ -194,15 +211,10 @@
* frees property list and its children
*/
void
-prop_list_free(libbe_handle_t *lbh)
+be_prop_list_free(nvlist_t *be_list)
{
- nvlist_t *be_list;
nvlist_t *prop_list;
- if ((be_list = lbh->list) == 0) {
- return;
- }
-
nvpair_t *be_pair = nvlist_next_nvpair(be_list, NULL);
if (nvpair_value_nvlist(be_pair, &prop_list) == 0) {
More information about the svn-soc-all
mailing list