Date: Sun, 23 Jul 2017 04:19:58 GMT From: kneitinger@FreeBSD.org To: svn-soc-all@FreeBSD.org Subject: socsvn commit: r325002 - soc2017/kneitinger/libbe-head/lib/libbe Message-ID: <201707230419.v6N4Jwd9090664@socsvn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: kneitinger Date: Sun Jul 23 04:19:58 2017 New Revision: 325002 URL: http://svnweb.FreeBSD.org/socsvn/?view=rev&rev=325002 Log: libbe(3): refactor be_create[_from_existing] to avoid redundant checks and return more appropriate errors Modified: soc2017/kneitinger/libbe-head/lib/libbe/be.c soc2017/kneitinger/libbe-head/lib/libbe/be.h soc2017/kneitinger/libbe-head/lib/libbe/be_error.c Modified: soc2017/kneitinger/libbe-head/lib/libbe/be.c ============================================================================== --- soc2017/kneitinger/libbe-head/lib/libbe/be.c Sat Jul 22 21:29:44 2017 (r325001) +++ soc2017/kneitinger/libbe-head/lib/libbe/be.c Sun Jul 23 04:19:58 2017 (r325002) @@ -167,15 +167,13 @@ zfs_handle_t *snap_hdl; if (be_validate_name(lbh, name)) { - err = set_error(lbh, BE_ERR_INVALIDNAME); - return (err); + return (set_error(lbh, BE_ERR_INVALIDNAME)); } if (err = be_root_concat(lbh, name, be_path)) { - return (err); + return (set_error(lbh, err)); } - // TODO: be_root_concat pos = snprintf(be_path, BE_MAXPATHLEN, "%s/%s", be_root_path(lbh), name); @@ -194,23 +192,38 @@ strftime(snap_name + pos, BE_MAXPATHLEN - pos, "@%F-%T", localtime(&rawtime)); - - // TODO: should any props be in the last arg (nvlist)? - if (err = zfs_snapshot(lbh->lzh, snap_name, true, NULL)) { - // TODO: switch on err to determine correct BE_ERR_* to return - return (-1); + if (err = zfs_snapshot(lbh->lzh, snap_name, false, NULL)) { + switch (err) { + case EZFS_SUCCESS: + break; + case EZFS_INVALIDNAME: + return (set_error(lbh, BE_ERR_INVALIDNAME)); + + default: + // TODO: elaborate return codes + return (set_error(lbh, BE_ERR_UNKNOWN)); + } } - // TODO: verify not null!! - snap_hdl = zfs_open(lbh->lzh, snap_name, ZFS_TYPE_SNAPSHOT); + if ((snap_hdl = zfs_open(lbh->lzh, snap_name, ZFS_TYPE_SNAPSHOT)) + == NULL) { + return (set_error(lbh, BE_ERR_ZFSOPEN)); + } - // TODO: should any props be in the last arg (nvlist)? if (err = zfs_clone(snap_hdl, be_path, NULL)) { - // TODO: switch on err to determine correct BE_ERR_* to return - return (-1); + switch (err) { + case EZFS_SUCCESS: + err = BE_ERR_SUCCESS; + break; + default: + err = BE_ERR_ZFSCLONE; + break; + } } - return (err); + zfs_close(snap_hdl); + + return (set_error(lbh, err)); } @@ -226,52 +239,44 @@ time_t rawtime; zfs_handle_t *snap_hdl; - - if (be_validate_name(lbh, name)) { - err = set_error(lbh, BE_ERR_INVALIDNAME); - return (err); + if (err = be_validate_name(lbh, name)) { + return (set_error(lbh, err)); } - - /* Ensure snap exists and it's parent dataset has mountpoint of '/' */ if (err = be_validate_snap(lbh, snap)) { - // set correct errs - return (-1); + return (set_error(lbh, err)); } - if (err = be_root_concat(lbh, name, be_path)) { - return (err); - } - - pos = - snprintf(be_path, BE_MAXPATHLEN, "%s/%s", be_root_path(lbh), name); - - if ((pos < 0) || (pos >= BE_MAXPATHLEN)) { - err = set_error(lbh, BE_ERR_PATHLEN); + return (set_error(lbh, err)); } if (zfs_dataset_exists(lbh->lzh, be_path, ZFS_TYPE_DATASET)) { - err = set_error(lbh, BE_ERR_EXISTS); - return (err); + return (set_error(lbh, BE_ERR_EXISTS)); } snap_hdl = zfs_open(lbh->lzh, snap, ZFS_TYPE_SNAPSHOT); - // TODO: should any props be in the last arg (nvlist)? if (err = zfs_clone(snap_hdl, be_path, NULL)) { - // TODO: switch on err to determine correct BE_ERR_* to return - return (-1); + switch (err) { + case EZFS_SUCCESS: + err = BE_ERR_SUCCESS; + break; + default: + err = BE_ERR_ZFSCLONE; + break; + } } zfs_close(snap_hdl); - return (err); + return (set_error(lbh, err)); } -/* Verifies that a snapshot has a valid name, exists, and has a mountpoint of +/* + * Verifies that a snapshot has a valid name, exists, and has a mountpoint of * '/'. Returns BE_ERR_SUCCESS (0), upon success, or the relevant BE_ERR_* upon * failure. Does not set the internal library error state. */ @@ -280,7 +285,7 @@ { zfs_handle_t *zfs_hdl; char buf[BE_MAXPATHLEN]; - char *snap_delim; + char *delim_pos; char *mountpoint; int err = 0; @@ -288,23 +293,27 @@ return (BE_ERR_PATHLEN); } - if (!zfs_dataset_exists(lbh->lzh, snap_name, ZFS_TYPE_SNAPSHOT)) { + if (!zfs_dataset_exists(lbh->lzh, snap_name, + ZFS_TYPE_SNAPSHOT)) { return (BE_ERR_NOENT); } strncpy(buf, snap_name, BE_MAXPATHLEN); - if ((snap_delim = strchr(buf, '@')) == NULL) { + /* Find the base filesystem of the snapshot */ + if ((delim_pos = strchr(buf, '@')) == NULL) { return (BE_ERR_INVALIDNAME); } + *delim_pos = '\0'; - *snap_delim = '\0'; - - if ((zfs_hdl = zfs_open(lbh->lzh, buf, ZFS_TYPE_DATASET)) == NULL) { + if ((zfs_hdl = + zfs_open(lbh->lzh, buf, ZFS_TYPE_DATASET)) == NULL) { return (BE_ERR_NOORIGIN); } - if (err = zfs_prop_get(zfs_hdl, ZFS_PROP_MOUNTPOINT, buf, BE_MAXPATHLEN, + if (err = + zfs_prop_get(zfs_hdl, ZFS_PROP_MOUNTPOINT, buf, + BE_MAXPATHLEN, NULL, NULL, 0, 1)) { err = BE_ERR_INVORIGIN; } @@ -333,6 +342,7 @@ size_t name_len, root_len; name_len = strlen(name); + root_len = strlen(lbh->root); /* Act idempotently; return be name if it is already a full path */ if (strrchr(name, '/') != NULL) { @@ -347,8 +357,8 @@ 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); + snprintf(result, BE_MAXPATHLEN, "%s/%s", lbh->root, + name); return (BE_ERR_SUCCESS); } @@ -357,21 +367,20 @@ /* - * Verifies the validity of a boot environment name (A-Za-z0-9-_.,). Returns 0 - * if the name is valid, otherwise, returns the position of the first offending - * character. Does not set internal library error state. + * Verifies the validity of a boot environment name (A-Za-z0-9-_.). Returns + * BE_ERR_SUCCESS (0) if name is valid, otherwise returns BE_ERR_INVALIDNAME. + * Does not set internal library error state. */ int be_validate_name(libbe_handle_t *lbh, char *name) { for (int i = 0; *name; i++) { char c = *(name++); - // TODO: beadm allows commas...they seem like a bad idea though - if (isalnum(c) || (c == '-') || (c == '_') || (c == '.') || - (c == ',')) { + if (isalnum(c) || (c == '-') || (c == '_') || + (c == '.')) { continue; } - return (name[i]); + return (BE_ERR_INVALIDNAME); } return (BE_ERR_SUCCESS); Modified: soc2017/kneitinger/libbe-head/lib/libbe/be.h ============================================================================== --- soc2017/kneitinger/libbe-head/lib/libbe/be.h Sat Jul 22 21:29:44 2017 (r325001) +++ soc2017/kneitinger/libbe-head/lib/libbe/be.h Sun Jul 23 04:19:58 2017 (r325002) @@ -29,7 +29,6 @@ #ifndef _LIBBE_H #define _LIBBE_H -#include <libzfs.h> #include <stdbool.h> #define BE_MAXPATHLEN ZFS_MAX_DATASET_NAME_LEN @@ -50,6 +49,7 @@ BE_ERR_MOUNTED, /* boot environment is already mounted */ BE_ERR_NOMOUNT, /* boot environment is not mounted */ BE_ERR_ZFSOPEN, /* calling zfs_open() failed */ + BE_ERR_ZFSCLONE, /* error when calling zfs_clone to create be */ BE_ERR_UNKNOWN, /* unknown error */ } be_error_t; @@ -62,7 +62,8 @@ const char *be_active_name(libbe_handle_t *); const char *be_active_path(libbe_handle_t *); const char *be_root_path(libbe_handle_t *); -nvlist_t *be_get_bootenv_props(libbe_handle_t *); + +/* nvlist_t *be_get_bootenv_props(libbe_handle_t *); */ /* Bootenv creation functions */ int be_create(libbe_handle_t *, char *); Modified: soc2017/kneitinger/libbe-head/lib/libbe/be_error.c ============================================================================== --- soc2017/kneitinger/libbe-head/lib/libbe/be_error.c Sat Jul 22 21:29:44 2017 (r325001) +++ soc2017/kneitinger/libbe-head/lib/libbe/be_error.c Sun Jul 23 04:19:58 2017 (r325002) @@ -79,6 +79,9 @@ case BE_ERR_ZFSOPEN: return ("calling zfs_open() failed"); + case BE_ERR_ZFSCLONE: + return ("error when calling zfs_clone() to create boot env"); + case BE_ERR_UNKNOWN: return ("unknown error"); @@ -104,7 +107,7 @@ lbh->error = err; - if (lbh->print_on_err) { + if (lbh->print_on_err && (err != BE_ERR_SUCCESS)) { fprintf(stderr, "%s\n", libbe_error_description(lbh)); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201707230419.v6N4Jwd9090664>