Skip to content

Commit d107980

Browse files
committed
Wait in libzfs_init() for the /dev/zfs device
While module loading itself is synchronous the creation of the /dev/zfs device is not. This is because /dev/zfs is typically created by a udev rule after the module is registered and presented to user space through sysfs. This small window between module loading and device creation can result in spurious failures of libzfs_init(). This patch closes that race by extending libzfs_init() so it can detect that the modules are loaded and only if required wait for the /dev/zfs device to be created. This allows scripts to reliably use the following shell construct without the need for additional error handling. $ /sbin/modprobe zfs && /sbin/zpool import -a To minimize the potential time waiting in libzfs_init() a strategy similar to adaptive mutexes is employed. The function will busy-wait for up to 10ms based on the expectation that the modules were just loaded and therefore the /dev/zfs will be created imminently. If it takes longer than this it will fall back to polling for up to 10 seconds. This behavior can be customized to some degree by setting the following new environment variables. This functionality is provided for backwards compatibility with existing scripts which depend on the module auto-load behavior. By default module auto-loading is now disabled. * ZFS_MODULE_LOADING="YES|yes|ON|on" - Attempt to load modules. * ZFS_MODULE_TIMEOUT="<seconds>" - Seconds to wait for /dev/zfs The additional small changes were also made. * In libzfs_run_process() 'rc' variables was renamed to 'error' for consistency with the rest of the code base. * All fprintf() error messages were moved out of the libzfs_init() library function where they never belonged in the first place. A libzfs_error_init() function was added to provide useful error messages for the most common causes of failure. * The zfs-import-* systemd service files have been updated to call '/sbin/modprobe zfs' so they no longer rely on the legacy auto-loading behavior. Signed-off-by: Brian Behlendorf <[email protected]> Issue openzfs#2556
1 parent 141b638 commit d107980

File tree

9 files changed

+113
-28
lines changed

9 files changed

+113
-28
lines changed

cmd/mount_zfs/mount_zfs.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -473,8 +473,10 @@ main(int argc, char **argv)
473473
if (zfsflags & ZS_ZFSUTIL)
474474
zfsutil = 1;
475475

476-
if ((g_zfs = libzfs_init()) == NULL)
476+
if ((g_zfs = libzfs_init()) == NULL) {
477+
(void) fprintf(stderr, "%s", libzfs_error_init(errno));
477478
return (MOUNT_SYSERR);
479+
}
478480

479481
/* try to open the dataset to access the mount point */
480482
if ((zhp = zfs_open(g_zfs, dataset,

cmd/zdb/zdb.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -3699,8 +3699,10 @@ main(int argc, char **argv)
36993699
zfs_vdev_async_read_max_active = 10;
37003700

37013701
kernel_init(FREAD);
3702-
if ((g_zfs = libzfs_init()) == NULL)
3702+
if ((g_zfs = libzfs_init()) == NULL) {
3703+
(void) fprintf(stderr, "%s", libzfs_error_init(errno));
37033704
return (1);
3705+
}
37043706

37053707
if (dump_all)
37063708
verbose = MAX(verbose, 1);

cmd/zfs/zfs_main.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -6708,8 +6708,10 @@ main(int argc, char **argv)
67086708
(strcmp(cmdname, "--help") == 0))
67096709
usage(B_TRUE);
67106710

6711-
if ((g_zfs = libzfs_init()) == NULL)
6711+
if ((g_zfs = libzfs_init()) == NULL) {
6712+
(void) fprintf(stderr, "%s", libzfs_error_init(errno));
67126713
return (1);
6714+
}
67136715

67146716
mnttab_file = g_zfs->libzfs_mnttab;
67156717

cmd/zinject/zinject.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -572,8 +572,10 @@ main(int argc, char **argv)
572572
int ret;
573573
int flags = 0;
574574

575-
if ((g_zfs = libzfs_init()) == NULL)
575+
if ((g_zfs = libzfs_init()) == NULL) {
576+
(void) fprintf(stderr, "%s", libzfs_error_init(errno));
576577
return (1);
578+
}
577579

578580
libzfs_print_on_error(g_zfs, B_TRUE);
579581

cmd/zpool/zpool_main.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -5919,8 +5919,10 @@ main(int argc, char **argv)
59195919
if ((strcmp(cmdname, "-?") == 0) || strcmp(cmdname, "--help") == 0)
59205920
usage(B_TRUE);
59215921

5922-
if ((g_zfs = libzfs_init()) == NULL)
5922+
if ((g_zfs = libzfs_init()) == NULL) {
5923+
(void) fprintf(stderr, "%s", libzfs_error_init(errno));
59235924
return (1);
5925+
}
59245926

59255927
libzfs_print_on_error(g_zfs, B_TRUE);
59265928

etc/systemd/system/zfs-import-cache.service.in

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,4 @@ ConditionPathExists=@sysconfdir@/zfs/zpool.cache
99
[Service]
1010
Type=oneshot
1111
RemainAfterExit=yes
12-
ExecStart=@sbindir@/zpool import -c @sysconfdir@/zfs/zpool.cache -aN
12+
ExecStart=/sbin/modprobe zfs && @sbindir@/zpool import -c @sysconfdir@/zfs/zpool.cache -aN

etc/systemd/system/zfs-import-scan.service.in

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,4 @@ ConditionPathExists=!@sysconfdir@/zfs/zpool.cache
99
[Service]
1010
Type=oneshot
1111
RemainAfterExit=yes
12-
ExecStart=@sbindir@/zpool import -d /dev/disk/by-id -aN
12+
ExecStart=/sbin/modprobe zfs && @sbindir@/zpool import -d /dev/disk/by-id -aN

include/libzfs.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ extern void zfs_save_arguments(int argc, char **, char *, int);
203203
extern int zpool_log_history(libzfs_handle_t *, const char *);
204204

205205
extern int libzfs_errno(libzfs_handle_t *);
206+
extern const char *libzfs_error_init(int);
206207
extern const char *libzfs_error_action(libzfs_handle_t *);
207208
extern const char *libzfs_error_description(libzfs_handle_t *);
208209
extern int zfs_standard_error(libzfs_handle_t *, int, const char *);
@@ -749,7 +750,6 @@ extern int zfs_nicestrtonum(libzfs_handle_t *, const char *, uint64_t *);
749750
#define STDERR_VERBOSE 0x02
750751

751752
int libzfs_run_process(const char *, char **, int flags);
752-
int libzfs_load_module(const char *);
753753

754754
/*
755755
* Given a device or file, determine if it is part of a pool.

lib/libzfs/libzfs_util.c

+95-20
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,31 @@ libzfs_errno(libzfs_handle_t *hdl)
5858
return (hdl->libzfs_error);
5959
}
6060

61+
const char *
62+
libzfs_error_init(int error)
63+
{
64+
switch (error) {
65+
case ENXIO:
66+
return (dgettext(TEXT_DOMAIN, "The ZFS modules are not "
67+
"loaded.\nTry running '/sbin/modprobe zfs' as root "
68+
"to load them.\n"));
69+
case ENOENT:
70+
return (dgettext(TEXT_DOMAIN, "The /dev/zfs device is "
71+
"missing and must be created.\nTry running 'udevadm "
72+
"trigger' as root to create it.\n"));
73+
case ENOEXEC:
74+
return (dgettext(TEXT_DOMAIN, "The ZFS modules cannot be "
75+
"auto-loaded.\nTry running '/sbin/modprobe zfs' as "
76+
"root to manually load them.\n"));
77+
case EACCES:
78+
return (dgettext(TEXT_DOMAIN, "Permission denied the "
79+
"ZFS utilities must be run as root.\n"));
80+
default:
81+
return (dgettext(TEXT_DOMAIN, "Failed to initialize the "
82+
"libzfs library.\n"));
83+
}
84+
}
85+
6186
const char *
6287
libzfs_error_action(libzfs_handle_t *hdl)
6388
{
@@ -628,7 +653,7 @@ int
628653
libzfs_run_process(const char *path, char *argv[], int flags)
629654
{
630655
pid_t pid;
631-
int rc, devnull_fd;
656+
int error, devnull_fd;
632657

633658
pid = vfork();
634659
if (pid == 0) {
@@ -650,9 +675,9 @@ libzfs_run_process(const char *path, char *argv[], int flags)
650675
} else if (pid > 0) {
651676
int status;
652677

653-
while ((rc = waitpid(pid, &status, 0)) == -1 &&
678+
while ((error = waitpid(pid, &status, 0)) == -1 &&
654679
errno == EINTR);
655-
if (rc < 0 || !WIFEXITED(status))
680+
if (error < 0 || !WIFEXITED(status))
656681
return (-1);
657682

658683
return (WEXITSTATUS(status));
@@ -661,26 +686,85 @@ libzfs_run_process(const char *path, char *argv[], int flags)
661686
return (-1);
662687
}
663688

664-
int
689+
/*
690+
* Verify the required ZFS_DEV device is available and optionally attempt
691+
* to load the ZFS modules. Under normal circumstances the modules
692+
* should already have been loaded by some external mechanism.
693+
*
694+
* Environment variables:
695+
* - ZFS_MODULE_LOADING="YES|yes|ON|on" - Attempt to load modules.
696+
* - ZFS_MODULE_TIMEOUT="<seconds>" - Seconds to wait for ZFS_DEV
697+
*/
698+
static int
665699
libzfs_load_module(const char *module)
666700
{
667701
char *argv[4] = {"/sbin/modprobe", "-q", (char *)module, (char *)0};
702+
char *load_str, *timeout_str;
703+
long timeout = 10; /* seconds */
704+
long busy_timeout = 10; /* milliseconds */
705+
int load = 0, fd;
706+
hrtime_t start;
707+
708+
/* Optionally request module loading */
709+
if (!libzfs_module_loaded(module)) {
710+
load_str = getenv("ZFS_MODULE_LOADING");
711+
if (load_str) {
712+
if (!strncasecmp(load_str, "YES", strlen("YES")) ||
713+
!strncasecmp(load_str, "ON", strlen("ON")))
714+
load = 1;
715+
else
716+
load = 0;
717+
}
668718

669-
if (libzfs_module_loaded(module))
670-
return (0);
719+
if (load && libzfs_run_process("/sbin/modprobe", argv, 0))
720+
return (ENOEXEC);
721+
}
722+
723+
/* Module loading is synchronous it must be available */
724+
if (!libzfs_module_loaded(module))
725+
return (ENXIO);
726+
727+
/*
728+
* Device creation by udev is asynchronous and waiting may be
729+
* required. Busy wait for 10ms and then fall back to polling every
730+
* 10ms for the allowed timeout (default 10s, max 10m). This is
731+
* done to optimize for the common case where the device is
732+
* immediately available and to avoid penalizing the possible
733+
* case where udev is slow or unable to create the device.
734+
*/
735+
timeout_str = getenv("ZFS_MODULE_TIMEOUT");
736+
if (timeout_str) {
737+
timeout = strtol(timeout_str, NULL, 0);
738+
timeout = MAX(MIN(timeout, (10 * 60)), 0); /* 0 <= N <= 600 */
739+
}
671740

672-
return (libzfs_run_process("/sbin/modprobe", argv, 0));
741+
start = gethrtime();
742+
do {
743+
fd = open(ZFS_DEV, O_RDWR);
744+
if (fd >= 0) {
745+
(void) close(fd);
746+
return (0);
747+
} else if (errno != ENOENT) {
748+
return (errno);
749+
} else if (NSEC2MSEC(gethrtime() - start) < busy_timeout) {
750+
sched_yield();
751+
} else {
752+
usleep(10 * MILLISEC);
753+
}
754+
} while (NSEC2MSEC(gethrtime() - start) < (timeout * MILLISEC));
755+
756+
return (ENOENT);
673757
}
674758

675759
libzfs_handle_t *
676760
libzfs_init(void)
677761
{
678762
libzfs_handle_t *hdl;
763+
int error;
679764

680-
if (libzfs_load_module("zfs") != 0) {
681-
(void) fprintf(stderr, gettext("Failed to load ZFS module "
682-
"stack.\nLoad the module manually by running "
683-
"'insmod <location>/zfs.ko' as root.\n"));
765+
error = libzfs_load_module(ZFS_DRIVER);
766+
if (error) {
767+
errno = error;
684768
return (NULL);
685769
}
686770

@@ -689,13 +773,6 @@ libzfs_init(void)
689773
}
690774

691775
if ((hdl->libzfs_fd = open(ZFS_DEV, O_RDWR)) < 0) {
692-
(void) fprintf(stderr, gettext("Unable to open %s: %s.\n"),
693-
ZFS_DEV, strerror(errno));
694-
if (errno == ENOENT)
695-
(void) fprintf(stderr,
696-
gettext("Verify the ZFS module stack is "
697-
"loaded by running '/sbin/modprobe zfs'.\n"));
698-
699776
free(hdl);
700777
return (NULL);
701778
}
@@ -706,8 +783,6 @@ libzfs_init(void)
706783
if ((hdl->libzfs_mnttab = fopen(MNTTAB, "r")) == NULL) {
707784
#endif
708785
(void) close(hdl->libzfs_fd);
709-
(void) fprintf(stderr,
710-
gettext("mtab is not present at %s.\n"), MNTTAB);
711786
free(hdl);
712787
return (NULL);
713788
}

0 commit comments

Comments
 (0)