Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add recursive/limited -a mount & unmount | introduce -A for noauto bypass #15264

Closed
wants to merge 5 commits into from
Closed

Add recursive/limited -a mount & unmount | introduce -A for noauto bypass #15264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f476a37
This commit adds functions to libzfs to check if one dataset is related
QORTEC Oct 10, 2023
a1295cd
Added limited/recursive operation to `zfs mount`
QORTEC Oct 10, 2023
e07de81
Updated/added documentation & tests for the changes to `zfs mount -a`
QORTEC Oct 10, 2023
9246667
Added limited/recursive operation to `zfs unmount`
QORTEC Oct 10, 2023
64400c3
Updated/added documentation & tests for the changes to `zfs unmount -a`
QORTEC Oct 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 112 additions & 31 deletions cmd/zfs/zfs_main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,8 @@ get_usage(zfs_help_t idx)
"[filesystem|volume|snapshot] ...\n"));
case HELP_MOUNT:
return (gettext("\tmount\n"
"\tmount [-flvO] [-o opts] <-a | filesystem>\n"));
"\tmount [-flvO] [-o opts] "
"<-a [filesystem] | [-A] filesystem>\n"));
case HELP_PROMOTE:
return (gettext("\tpromote <clone-filesystem>\n"));
case HELP_RECEIVE:
Expand Down Expand Up @@ -346,7 +347,7 @@ get_usage(zfs_help_t idx)
"<filesystem|volume>@<snap> ...\n"));
case HELP_UNMOUNT:
return (gettext("\tunmount [-fu] "
"<-a | filesystem|mountpoint>\n"));
"<-a [filesystem] | [-A] filesystem|mountpoint>\n"));
case HELP_UNSHARE:
return (gettext("\tunshare "
"<-a [nfs|smb] | filesystem|mountpoint>\n"));
Expand Down Expand Up @@ -6731,6 +6732,7 @@ zfs_do_holds(int argc, char **argv)
typedef struct get_all_state {
boolean_t ga_verbose;
get_all_cb_t *ga_cbp;
const char *ga_parent;
} get_all_state_t;

static int
Expand Down Expand Up @@ -6769,18 +6771,29 @@ get_one_dataset(zfs_handle_t *zhp, void *data)
zfs_close(zhp);
return (0);
}

/*
* Skip any dataset that's not related to ga_parent.
*/
if (state->ga_parent != NULL &&
!zfs_dataset_related(zhp, state->ga_parent)) {
zfs_close(zhp);
return (0);
}

libzfs_add_handle(state->ga_cbp, zhp);
assert(state->ga_cbp->cb_used <= state->ga_cbp->cb_alloc);

return (0);
}

static void
get_all_datasets(get_all_cb_t *cbp, boolean_t verbose)
get_all_datasets(get_all_cb_t *cbp, boolean_t verbose, const char *parent)
{
get_all_state_t state = {
.ga_verbose = verbose,
.ga_cbp = cbp
.ga_cbp = cbp,
.ga_parent = parent
};

if (verbose)
Expand Down Expand Up @@ -6808,14 +6821,15 @@ typedef struct share_mount_state {
uint_t sm_total; /* number of filesystems to process */
uint_t sm_done; /* number of filesystems processed */
int sm_status; /* -1 if any of the share/mount operations failed */
boolean_t sm_mount_noauto; /* treat 'canmount=noauto' as 'on' */
} share_mount_state_t;

/*
* Share or mount a dataset.
*/
static int
share_mount_one(zfs_handle_t *zhp, int op, int flags, enum sa_protocol protocol,
boolean_t explicit, const char *options)
boolean_t explicit, const char *options, boolean_t mount_noauto)
{
char mountpoint[ZFS_MAXPROPLEN];
char shareopts[ZFS_MAXPROPLEN];
Expand Down Expand Up @@ -6905,13 +6919,14 @@ share_mount_one(zfs_handle_t *zhp, int op, int flags, enum sa_protocol protocol,
}

/*
* canmount explicit outcome
* on no pass through
* on yes pass through
* off no return 0
* off yes display error, return 1
* noauto no return 0
* noauto yes pass through
* canmount explicit mount_noauto outcome
* on no n/a pass through
* on yes n/a pass through
* off no n/a return 0
* off yes n/a display error, return 1
* noauto no no return 0
* noauto no yes pass through
* noauto yes yes/no pass through
*/
canmount = zfs_prop_get_int(zhp, ZFS_PROP_CANMOUNT);
if (canmount == ZFS_CANMOUNT_OFF) {
Expand All @@ -6922,11 +6937,13 @@ share_mount_one(zfs_handle_t *zhp, int op, int flags, enum sa_protocol protocol,
"'canmount' property is set to 'off'\n"), cmdname,
zfs_get_name(zhp));
return (1);
} else if (canmount == ZFS_CANMOUNT_NOAUTO && !explicit) {
} else if (canmount == ZFS_CANMOUNT_NOAUTO &&
!explicit && !mount_noauto) {
/*
* When performing a 'zfs mount -a', we skip any mounts for
* datasets that have 'noauto' set. Sharing a dataset with
* 'noauto' set is only allowed if it's mounted.
* datasets that have 'noauto' set. However, 'zfs mount -A'
* will mount datasets with 'noauto' set. Sharing a dataset
* with 'noauto' set is only allowed if it's mounted.
*/
if (op == OP_MOUNT)
return (0);
Expand Down Expand Up @@ -7082,7 +7099,7 @@ share_mount_one_cb(zfs_handle_t *zhp, void *arg)
int ret;

ret = share_mount_one(zhp, sms->sm_op, sms->sm_flags, sms->sm_proto,
B_FALSE, sms->sm_options);
B_FALSE, sms->sm_options, sms->sm_mount_noauto);

pthread_mutex_lock(&sms->sm_lock);
if (ret != 0)
Expand Down Expand Up @@ -7132,18 +7149,22 @@ sa_protocol_decode(const char *protocol)
static int
share_mount(int op, int argc, char **argv)
{
int do_all = 0;
boolean_t do_all = B_FALSE;
boolean_t do_noauto = B_FALSE;
boolean_t verbose = B_FALSE;
int c, ret = 0;
char *options = NULL;
int flags = 0;

/* check options */
while ((c = getopt(argc, argv, op == OP_MOUNT ? ":alvo:Of" : "al"))
while ((c = getopt(argc, argv, op == OP_MOUNT ? ":aAlvo:Of" : "al"))
!= -1) {
switch (c) {
case 'a':
do_all = 1;
do_all = B_TRUE;
break;
case 'A':
do_noauto = B_TRUE;
break;
case 'v':
verbose = B_TRUE;
Expand Down Expand Up @@ -7186,23 +7207,48 @@ share_mount(int op, int argc, char **argv)
argv += optind;

/* check number of arguments */
if (do_all) {
if (do_all || do_noauto) {
enum sa_protocol protocol = SA_NO_PROTOCOL;

if (op == OP_SHARE && argc > 0) {
protocol = sa_protocol_decode(argv[0]);
argc--;
argv++;
}

if (argc != 0) {
/* check number of arguments */
if (do_noauto && argc < 1) {
(void) fprintf(stderr, gettext("missing "
"dataset argument\n"));
usage(B_FALSE);
}
if ((op == OP_SHARE && argc != 0) || argc > 1) {
(void) fprintf(stderr, gettext("too many arguments\n"));
usage(B_FALSE);
}

/*
* Limit `-a filesystem` to mount only
*/
const char *filesystem = NULL;
if (op == OP_MOUNT)
filesystem = argv[0];
Comment on lines +7233 to +7234
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason this is limited only to mount?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes/no, this pull request was originally for mount only, and zfs share has additional args [smb|nfs] that I didn't see where it was pulled from.
I thought it best to limit the PR to zfs mount untill/unless I look into how share works.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no additional arguments documented for zfs share. The control flow is exactly the same except for the part that actually acts on each filesystem, so limiting this to mount-only seems arbitrary and causes an unnecessary split in the nature of the two recursive filesystem operations.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, I didn't see anything in the manpages, but usage differs?
Hence why I disabled it until I can investigate it properly.

Copy link
Contributor

@ahesford ahesford Sep 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the time you're consuming this argument, any nfs or smb argument has already been consumed zfs zfs share, so the syntax for zfs share becomes

zfs share [-l] <filesystem | <-a|-r> [<nfs|smb> [filesystem]]>

assuming, of course, that you don't drop the -r.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-r is not currently enabled for zfs share, I was using op == OP_MOUNT to restrict -a [filesystem].

I believe this would be the correct syntax for the -a arg;

zfs share [-l] <-a [nfs|smb] [filesystem] | filesystem>

My original reasoning for disabling -a [filesystem]

  • I didn't have time to track down how the args work to verify if my usage is correct
  • I would need to update the man pages adding the missing information [nfs|smb]
  • I haven't use zfs share (something I should do when I have spare time)


/*
* Validate filesystem is actually a valid zfs filesystem
*/
if (filesystem != NULL) {
zfs_handle_t *zhp = zfs_open(g_zfs, filesystem,
ZFS_TYPE_FILESYSTEM);
if (zhp == NULL) {
free(options);
return (1);
}
zfs_close(zhp);
}
Comment on lines +7236 to +7247
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method of validating filesystem feels a bit messy. While looking through the ZFS codebase, I haven't come across a better (acceptable) way to accomplish this.

If we only needed to validate filesystem, then we could use zfs_dataset_exists(). However, this is not acceptable for the CLI since the function only returns a simple true or false, without providing any error messages.

The best alternative I've thought of so far is to modify zfs_dataset_exists() by adding the seven lines of code necessary to enable error reporting capabilities in the function, matching those of zfs_open().

To maintain compatibility, we can call this function zfs_dataset_exists_verbose() and control the error reporting via the boolean_t error property. Next, we can make zfs_dataset_exists() a helper function that executes zfs_dataset_exists_verbose() with error reporting disabled.

Click here to see an example of the proposed code: 
/*
 * Finds whether the dataset of the given type(s) exists.
 */
boolean_t
zfs_dataset_exists_verbose(libzfs_handle_t *hdl, const char *path, zfs_type_t types, boolean_t error)
{
	zfs_handle_t *zhp;
	char errbuf[ERRBUFLEN];

	(void) snprintf(errbuf, sizeof (errbuf),
	    dgettext(TEXT_DOMAIN, "cannot open '%s'"), path);

	/*
	 * Validate the name before we even try to open it.
	 */
	if (!zfs_validate_name(hdl, path, types, B_FALSE)) {
		if (error)
			(void) zfs_error(hdl, EZFS_INVALIDNAME, errbuf);
		return (B_FALSE);
	}

	/*
	 * Try to get stats for the dataset, which will tell us if it exists.
	 */
	if ((zhp = make_dataset_handle(hdl, path)) != NULL) {
		int ds_type = zhp->zfs_type;

		zfs_close(zhp);
		if (types & ds_type)
			return (B_TRUE);
	}

	if (error)
		(void) zfs_standard_error(hdl, errno, errbuf);
	return (B_FALSE);
}

boolean_t
zfs_dataset_exists(libzfs_handle_t *hdl, const char *path, zfs_type_t types)
{
	return (zfs_dataset_exists_verbose(hdl, path, types, B_TRUE));
}

libzfs_dataset.c

_LIBZFS_H boolean_t zfs_dataset_exists_verbose(libzfs_handle_t *, const char *,
    zfs_type_t, boolean_t);

include/libzfs.h

/* share_mount(): Validate filesystem */
if (filesystem != NULL && !zfs_dataset_exists_verbose(g_zfs,
    filesystem, ZFS_TYPE_FILESYSTEM, B_TRUE)) {
	free(options);
	return (1);
}

/* unshare_unmount() Validate filesystem */
if (filesystem != NULL && !zfs_dataset_exists_verbose(g_zfs,
    filesystem, ZFS_TYPE_FILESYSTEM, B_TRUE))
	return (1);

As seen above, the validation code looks cleaner and is easier to understand. However, I'm unsure about the general usefulness of the proposed zfs_dataset_exists_verbose() function and whether this should be added to the PR.


start_progress_timer();
get_all_cb_t cb = { 0 };
get_all_datasets(&cb, verbose);
get_all_datasets(&cb, verbose, filesystem);

if (cb.cb_used == 0) {
free(options);
Expand All @@ -7216,6 +7262,7 @@ share_mount(int op, int argc, char **argv)
share_mount_state.sm_options = options;
share_mount_state.sm_proto = protocol;
share_mount_state.sm_total = cb.cb_used;
share_mount_state.sm_mount_noauto = do_noauto;
pthread_mutex_init(&share_mount_state.sm_lock, NULL);

/* For a 'zfs share -a' operation start with a clean slate. */
Expand Down Expand Up @@ -7283,7 +7330,7 @@ share_mount(int op, int argc, char **argv)
ret = 1;
} else {
ret = share_mount_one(zhp, op, flags, SA_NO_PROTOCOL,
B_TRUE, options);
B_TRUE, options, B_FALSE);
zfs_commit_shares(NULL);
zfs_close(zhp);
}
Expand Down Expand Up @@ -7441,7 +7488,8 @@ unshare_unmount_path(int op, char *path, int flags, boolean_t is_manual)
static int
unshare_unmount(int op, int argc, char **argv)
{
int do_all = 0;
boolean_t do_all = B_FALSE;
boolean_t do_noauto = B_FALSE;
int flags = 0;
int ret = 0;
int c;
Expand All @@ -7450,10 +7498,13 @@ unshare_unmount(int op, int argc, char **argv)
char sharesmb[ZFS_MAXPROPLEN];

/* check options */
while ((c = getopt(argc, argv, op == OP_SHARE ? ":a" : "afu")) != -1) {
while ((c = getopt(argc, argv, op == OP_SHARE ? ":a" : "aAfu")) != -1) {
switch (c) {
case 'a':
do_all = 1;
do_all = B_TRUE;
break;
case 'A':
do_noauto = B_TRUE;
break;
case 'f':
flags |= MS_FORCE;
Expand All @@ -7476,7 +7527,7 @@ unshare_unmount(int op, int argc, char **argv)
argc -= optind;
argv += optind;

if (do_all) {
if (do_all || do_noauto) {
/*
* We could make use of zfs_for_each() to walk all datasets in
* the system, but this would be very inefficient, especially
Expand Down Expand Up @@ -7508,11 +7559,31 @@ unshare_unmount(int op, int argc, char **argv)
argv++;
}

if (argc != 0) {
/* check number of arguments */
if (do_noauto && argc < 1) {
(void) fprintf(stderr, gettext("missing "
"dataset argument\n"));
usage(B_FALSE);
}
if ((op == OP_SHARE && argc != 0) || argc > 1) {
(void) fprintf(stderr, gettext("too many arguments\n"));
usage(B_FALSE);
}

/*
* Limit `-a filesystem` to unmount only
*/
const char *filesystem = NULL;
if (op == OP_MOUNT)
filesystem = argv[0];

/*
* Validate filesystem is actually a valid zfs filesystem
*/
if (filesystem != NULL && (zhp = zfs_open(g_zfs,
filesystem, ZFS_TYPE_FILESYSTEM)) == NULL)
return (1);

if (((pool = uu_avl_pool_create("unmount_pool",
sizeof (unshare_unmount_node_t),
offsetof(unshare_unmount_node_t, un_avlnode),
Expand Down Expand Up @@ -7574,15 +7645,25 @@ unshare_unmount(int op, int argc, char **argv)
NULL, NULL, 0, B_FALSE) == 0);
if (strcmp(nfs_mnt_prop, "legacy") == 0)
continue;
/* Ignore canmount=noauto mounts */
/*
* Ignore canmount=noauto mounts if
* 'do_noauto' is set to false (default: false)
*/
if (zfs_prop_get_int(zhp, ZFS_PROP_CANMOUNT) ==
ZFS_CANMOUNT_NOAUTO)
ZFS_CANMOUNT_NOAUTO && !do_noauto)
continue;
break;
default:
break;
}

/*
* Skip any dataset that's not related to filesystem.
*/
if (filesystem != NULL &&
!zfs_dataset_related(zhp, filesystem))
continue;

node = safe_malloc(sizeof (unshare_unmount_node_t));
node->un_zhp = zhp;
node->un_mountp = safe_strdup(entry.mnt_mountp);
Expand Down
1 change: 1 addition & 0 deletions include/libzfs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -887,6 +887,7 @@ _LIBZFS_H int zfs_name_valid(const char *, zfs_type_t);
_LIBZFS_H zfs_handle_t *zfs_path_to_zhandle(libzfs_handle_t *, const char *,
zfs_type_t);
_LIBZFS_H int zfs_parent_name(zfs_handle_t *, char *, size_t);
_LIBZFS_H boolean_t zfs_dataset_related(zfs_handle_t *, const char *);
_LIBZFS_H boolean_t zfs_dataset_exists(libzfs_handle_t *, const char *,
zfs_type_t);
_LIBZFS_H int zfs_spa_version(zfs_handle_t *, int *);
Expand Down
29 changes: 29 additions & 0 deletions lib/libzfs/libzfs_dataset.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3446,6 +3446,35 @@ is_descendant(const char *ds1, const char *ds2)
return (ds2[d1len] == '/' && (strncmp(ds1, ds2, d1len) == 0));
}

/*
* Is one dataset is related to another, including self-relation?
*
* Needs to handle these cases:
* Dataset 1 "a/foo" "a/foo" "a/foo" "a/foo"
* Dataset 2 "a/foo" "a/fo" "a/foobar" "a/foo/bar"
* Related? Yes. No. No. Yes.
*/
static boolean_t
dataset_related(const char *ds1, const char *ds2)
{
/* ds1 and ds2 are identical */
if (strcmp(ds1, ds2) == 0)
return (B_TRUE);

/* ds2 is a descendant of ds1 */
if (is_descendant(ds1, ds2))
return (B_TRUE);

/* dataset are not related. */
return (B_FALSE);
}

boolean_t
zfs_dataset_related(zfs_handle_t *zhp, const char *parent)
{
return (dataset_related(parent, zfs_get_name(zhp)));
}

/*
* Given a complete name, return just the portion that refers to the parent.
* Will return -1 if there is no parent (path is just the name of the
Expand Down
Loading
Loading