socsvn commit: r325002 - soc2017/kneitinger/libbe-head/lib/libbe
kneitinger at FreeBSD.org
kneitinger at FreeBSD.org
Sun Jul 23 04:20:00 UTC 2017
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));
}
More information about the svn-soc-all
mailing list