From owner-svn-src-projects@freebsd.org Fri Aug 10 21:23:58 2018 Return-Path: Delivered-To: svn-src-projects@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id EE7181075429 for ; Fri, 10 Aug 2018 21:23:57 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id A44B67B8BF; Fri, 10 Aug 2018 21:23:57 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 860B45DBA; Fri, 10 Aug 2018 21:23:57 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w7ALNvEC027139; Fri, 10 Aug 2018 21:23:57 GMT (envelope-from kevans@FreeBSD.org) Received: (from kevans@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w7ALNuKm027136; Fri, 10 Aug 2018 21:23:56 GMT (envelope-from kevans@FreeBSD.org) Message-Id: <201808102123.w7ALNuKm027136@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kevans set sender to kevans@FreeBSD.org using -f From: Kyle Evans Date: Fri, 10 Aug 2018 21:23:56 +0000 (UTC) To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r337592 - projects/bectl/lib/libbe X-SVN-Group: projects X-SVN-Commit-Author: kevans X-SVN-Commit-Paths: projects/bectl/lib/libbe X-SVN-Commit-Revision: 337592 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 10 Aug 2018 21:23:58 -0000 Author: kevans Date: Fri Aug 10 21:23:56 2018 New Revision: 337592 URL: https://svnweb.freebsd.org/changeset/base/337592 Log: libbe(3): More error handling bits be_add_child functionality gets split out into separate places as a bonus. A lot of places here we'll gloss over libzfs errors, because they shouldn't be happening given the conditions that we're operating under. "Unknown error" is what I'm intending to use for the moment to indicate an exceptional circumstance- exceptional enough that we can't tell the consumer did because we're not so certain that they did anything. Modified: projects/bectl/lib/libbe/be.c projects/bectl/lib/libbe/be.h projects/bectl/lib/libbe/be_error.c Modified: projects/bectl/lib/libbe/be.c ============================================================================== --- projects/bectl/lib/libbe/be.c Fri Aug 10 21:03:17 2018 (r337591) +++ projects/bectl/lib/libbe/be.c Fri Aug 10 21:23:56 2018 (r337592) @@ -44,6 +44,10 @@ __FBSDID("$FreeBSD$"); #include "be.h" #include "be_impl.h" +static int be_create_child_noent(libbe_handle_t *lbh, const char *active, + const char *child_path); +static int be_create_child_cloned(libbe_handle_t *lbh, const char *active); + /* * Iterator function for locating the rootfs amongst the children of the * zfs_be_root set by loader(8). data is expected to be a libbe_handle_t *. @@ -721,17 +725,111 @@ be_import(libbe_handle_t *lbh, const char *bootenv, in return (be_destroy(lbh, nbuf, 0)); } +static int +be_create_child_noent(libbe_handle_t *lbh, const char *active, + const char *child_path) +{ + nvlist_t *props; + zfs_handle_t *zfs; + int err; + nvlist_alloc(&props, NV_UNIQUE_NAME, KM_SLEEP); + nvlist_add_string(props, "canmount", "noauto"); + nvlist_add_string(props, "mountpoint", child_path); + + /* Create */ + if ((err = zfs_create(lbh->lzh, active, ZFS_TYPE_DATASET, + props)) != 0) { + switch (err) { + case EZFS_EXISTS: + return (set_error(lbh, BE_ERR_EXISTS)); + case EZFS_NOENT: + return (set_error(lbh, BE_ERR_NOENT)); + case EZFS_BADTYPE: + case EZFS_BADVERSION: + return (set_error(lbh, BE_ERR_NOPOOL)); + case EZFS_BADPROP: + default: + /* We set something up wrong, probably... */ + return (set_error(lbh, BE_ERR_UNKNOWN)); + } + } + nvlist_free(props); + + if ((zfs = zfs_open(lbh->lzh, active, ZFS_TYPE_DATASET)) == NULL) + return (set_error(lbh, BE_ERR_ZFSOPEN)); + + /* Set props */ + if ((err = zfs_prop_set(zfs, "canmount", "noauto")) != 0) { + zfs_close(zfs); + /* + * Similar to other cases, this shouldn't fail unless we've + * done something wrong. This is a new dataset that shouldn't + * have been mounted anywhere between creation and now. + */ + if (err == EZFS_NOMEM) + return (set_error(lbh, BE_ERR_NOMEM)); + return (set_error(lbh, BE_ERR_UNKNOWN)); + } + zfs_close(zfs); + return (BE_ERR_SUCCESS); +} + +static int +be_create_child_cloned(libbe_handle_t *lbh, const char *active) +{ + char buf[BE_MAXPATHLEN]; + zfs_handle_t *zfs; + int err; + + /* XXX TODO ? */ + + /* + * Establish if the existing path is a zfs dataset or just + * the subdirectory of one + */ + strlcpy(buf, "/tmp/be_snap.XXXXX", sizeof(buf)); + if (mktemp(buf) == NULL) + return (set_error(lbh, BE_ERR_UNKNOWN)); + + if ((err = zfs_snapshot(lbh->lzh, buf, false, NULL)) != 0) { + switch (err) { + case EZFS_INVALIDNAME: + return (set_error(lbh, BE_ERR_INVALIDNAME)); + + default: + /* + * The other errors that zfs_ioc_snapshot might return + * shouldn't happen if we've set things up properly, so + * we'll gloss over them and call it UNKNOWN as it will + * require further triage. + */ + if (errno == ENOTSUP) + return (set_error(lbh, BE_ERR_NOPOOL)); + return (set_error(lbh, BE_ERR_UNKNOWN)); + } + } + + /* Clone */ + if ((zfs = zfs_open(lbh->lzh, buf, ZFS_TYPE_SNAPSHOT)) == NULL) + return (BE_ERR_ZFSOPEN); + + if ((err = zfs_clone(zfs, active, NULL)) != 0) + /* XXX TODO correct error */ + return (set_error(lbh, BE_ERR_UNKNOWN)); + + /* set props */ + zfs_close(zfs); + return (BE_ERR_SUCCESS); +} + int be_add_child(libbe_handle_t *lbh, const char *child_path, bool cp_if_exists) { struct stat sb; - char active[BE_MAXPATHLEN]; - char buf[BE_MAXPATHLEN]; + char active[BE_MAXPATHLEN], buf[BE_MAXPATHLEN]; nvlist_t *props; const char *s; - zfs_handle_t *zfs; - int err; /* Require absolute paths */ if (*child_path != '/') @@ -760,60 +858,12 @@ be_add_child(libbe_handle_t *lbh, const char *child_pa if (stat(child_path, &sb) != 0) { /* Verify that error is ENOENT */ if (errno != ENOENT) - return (set_error(lbh, BE_ERR_NOENT)); - - nvlist_alloc(&props, NV_UNIQUE_NAME, KM_SLEEP); - nvlist_add_string(props, "canmount", "noauto"); - nvlist_add_string(props, "mountpoint", child_path); - - /* Create */ - if ((err = - zfs_create(lbh->lzh, active, ZFS_TYPE_DATASET, props)) != 0) - /* XXX TODO handle error */ return (set_error(lbh, BE_ERR_UNKNOWN)); - nvlist_free(props); - - if ((zfs = - zfs_open(lbh->lzh, active, ZFS_TYPE_DATASET)) == NULL) - return (set_error(lbh, BE_ERR_ZFSOPEN)); - - /* Set props */ - if ((err = zfs_prop_set(zfs, "canmount", "noauto")) != 0) - /* TODO handle error */ - return (set_error(lbh, BE_ERR_UNKNOWN)); - } else if (cp_if_exists) { + return (be_create_child_noent(lbh, active, child_path)); + } else if (cp_if_exists) /* Path is already a descendent of / and should be copied */ - - /* XXX TODO ? */ - - /* - * Establish if the existing path is a zfs dataset or just - * the subdirectory of one - */ - strlcpy(buf, "/tmp/be_snap.XXXXX", sizeof(buf)); - if (mktemp(buf) == NULL) - return (set_error(lbh, BE_ERR_UNKNOWN)); - - if ((err = zfs_snapshot(lbh->lzh, buf, false, NULL)) != 0) - /* XXX TODO correct error */ - return (set_error(lbh, BE_ERR_UNKNOWN)); - - /* Clone */ - if ((zfs = - zfs_open(lbh->lzh, buf, ZFS_TYPE_SNAPSHOT)) == NULL) - return (BE_ERR_ZFSOPEN); - - if ((err = zfs_clone(zfs, active, NULL)) != 0) - /* XXX TODO correct error */ - return (set_error(lbh, BE_ERR_UNKNOWN)); - - /* set props */ - zfs_close(zfs); - } else - /* TODO: error code for exists, but not cp? */ - return (set_error(lbh, BE_ERR_EXISTS)); - - return (BE_ERR_SUCCESS); + return (be_create_child_cloned(lbh, active)); + return (set_error(lbh, BE_ERR_EXISTS)); } static int @@ -863,22 +913,24 @@ be_activate(libbe_handle_t *lbh, const char *bootenv, if (temporary) { config = zpool_get_config(lbh->active_phandle, NULL); - if (config == NULL) { - printf("no config\n"); - return (1); - } + if (config == NULL) + /* config should be fetchable... */ + return (set_error(lbh, BE_ERR_UNKNOWN)); if (nvlist_lookup_uint64(config, ZPOOL_CONFIG_POOL_GUID, &pool_guid) != 0) - return (1); + /* Similarly, it shouldn't be possible */ + return (set_error(lbh, BE_ERR_UNKNOWN)); /* Expected format according to zfsbootcfg(8) man */ strcpy(buf, "zfs:"); strcat(buf, be_path); strcat(buf, ":"); - if (nvlist_lookup_nvlist(config, ZPOOL_CONFIG_VDEV_TREE, &vdevs) != 0) - return (1); + /* We have no config tree */ + if (nvlist_lookup_nvlist(config, ZPOOL_CONFIG_VDEV_TREE, + &vdevs) != 0) + return (set_error(lbh, BE_ERR_NOPOOL)); return (be_set_nextboot(lbh, vdevs, pool_guid, buf)); } else { Modified: projects/bectl/lib/libbe/be.h ============================================================================== --- projects/bectl/lib/libbe/be.h Fri Aug 10 21:03:17 2018 (r337591) +++ projects/bectl/lib/libbe/be.h Fri Aug 10 21:23:56 2018 (r337592) @@ -57,6 +57,7 @@ typedef enum be_error { BE_ERR_ZFSCLONE, /* error when calling zfs_clone to create be */ BE_ERR_IO, /* error when doing some I/O operation */ BE_ERR_NOPOOL, /* operation not supported on this pool */ + BE_ERR_NOMEM, /* insufficient memory */ BE_ERR_UNKNOWN, /* unknown error */ } be_error_t; Modified: projects/bectl/lib/libbe/be_error.c ============================================================================== --- projects/bectl/lib/libbe/be_error.c Fri Aug 10 21:03:17 2018 (r337591) +++ projects/bectl/lib/libbe/be_error.c Fri Aug 10 21:23:56 2018 (r337592) @@ -99,6 +99,9 @@ libbe_error_description(libbe_handle_t *lbh) case BE_ERR_NOPOOL: return ("operation not supported on this pool"); + case BE_ERR_NOMEM: + return ("insufficient memory"); + case BE_ERR_UNKNOWN: return ("unknown error");