7745 print error if lzc_* is called before libzfs_core_init

illumos/illumos-gate@7c13517fff
7c13517fff

https://www.illumos.org/issues/7745
  The problem is that consumers of `libZFS_Core` that forget to call
  `libzfs_core_init()` before calling any other function of the library
  are having a hard time realizing their mistake. The library's internal
  file descriptor is declared as global static, which is ok, but it is not
  initialized explicitly; therefore, it defaults to 0, which is a valid
  file descriptor. If `libzfs_core_init()`, which explicitly initializes
  the correct fd, is skipped, the ioctl functions return errors that do
  not have anything to do with `libZFS_Core`, where the problem is
  actually located.
  Even though assertions for that existed within `libZFS_Core` for debug
  builds, they were never enabled because the `-DDEBUG` flag was missing
  from the compiler flags.
  This patch applies the following changes:
  1. It adds `-DDEBUG` for debug builds of `libZFS_Core` and `libzfs`,
         to enable their assertions on debug builds.
  2. It corrects an assertion within `libzfs`, where a function had
         been spelled incorrectly (`zpool_prop_unsupported()`) and nobody
         knew because the `-DDEBUG` flag was missing, and the preprocessor
         was taking that part of the code away.
  3. The library's internal fd is initialized to `-1` and `VERIFY`
         assertions have been placed to check that the fd is not equal to
         `-1` before issuing any ioctl. It is important here to note, that
         the `VERIFY` assertions exist in both debug and non-debug builds.
  4. In `libzfs_core_fini` we make sure to never increment the
         refcount of our fd below 0, and also reset the fd to `-1` when no
         one refers to it. The reason for this, is for the rare case that
         the consumer closes all references but then calls one of the
         library's functions without using `libzfs_core_init()` first, and
         in the mean time, a previous call to `open()` decided to reuse
         our previous fd. This scenario would have passed our assertion in

Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Serapheim Dimitropoulos <serapheim@delphix.com>
This commit is contained in:
Andriy Gapon 2017-04-14 18:14:02 +00:00
parent e90b7ce5ca
commit 43fd8e6d5e
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/vendor/illumos/dist/; revision=316902
3 changed files with 23 additions and 4 deletions

View File

@ -818,7 +818,7 @@ zpool_prop_get_feature(zpool_handle_t *zhp, const char *propname, char *buf,
const char *feature = strchr(propname, '@') + 1; const char *feature = strchr(propname, '@') + 1;
supported = zpool_prop_feature(propname); supported = zpool_prop_feature(propname);
ASSERT(supported || zfs_prop_unsupported(propname)); ASSERT(supported || zpool_prop_unsupported(propname));
/* /*
* Convert from feature name to feature guid. This conversion is * Convert from feature name to feature guid. This conversion is

View File

@ -266,6 +266,15 @@ cksummer(void *arg)
ofp = fdopen(dda->inputfd, "r"); ofp = fdopen(dda->inputfd, "r");
while (ssread(drr, sizeof (*drr), ofp) != 0) { while (ssread(drr, sizeof (*drr), ofp) != 0) {
/*
* kernel filled in checksum, we are going to write same
* record, but need to regenerate checksum.
*/
if (drr->drr_type != DRR_BEGIN) {
bzero(&drr->drr_u.drr_checksum.drr_checksum,
sizeof (drr->drr_u.drr_checksum.drr_checksum));
}
switch (drr->drr_type) { switch (drr->drr_type) {
case DRR_BEGIN: case DRR_BEGIN:
{ {

View File

@ -85,7 +85,7 @@
#include <sys/stat.h> #include <sys/stat.h>
#include <sys/zfs_ioctl.h> #include <sys/zfs_ioctl.h>
static int g_fd; static int g_fd = -1;
static pthread_mutex_t g_lock = PTHREAD_MUTEX_INITIALIZER; static pthread_mutex_t g_lock = PTHREAD_MUTEX_INITIALIZER;
static int g_refcount; static int g_refcount;
@ -110,9 +110,14 @@ libzfs_core_fini(void)
{ {
(void) pthread_mutex_lock(&g_lock); (void) pthread_mutex_lock(&g_lock);
ASSERT3S(g_refcount, >, 0); ASSERT3S(g_refcount, >, 0);
g_refcount--;
if (g_refcount == 0) if (g_refcount > 0)
g_refcount--;
if (g_refcount == 0 && g_fd != -1) {
(void) close(g_fd); (void) close(g_fd);
g_fd = -1;
}
(void) pthread_mutex_unlock(&g_lock); (void) pthread_mutex_unlock(&g_lock);
} }
@ -126,6 +131,7 @@ lzc_ioctl(zfs_ioc_t ioc, const char *name,
size_t size; size_t size;
ASSERT3S(g_refcount, >, 0); ASSERT3S(g_refcount, >, 0);
VERIFY3S(g_fd, !=, -1);
(void) strlcpy(zc.zc_name, name, sizeof (zc.zc_name)); (void) strlcpy(zc.zc_name, name, sizeof (zc.zc_name));
@ -328,6 +334,9 @@ lzc_exists(const char *dataset)
*/ */
zfs_cmd_t zc = { 0 }; zfs_cmd_t zc = { 0 };
ASSERT3S(g_refcount, >, 0);
VERIFY3S(g_fd, !=, -1);
(void) strlcpy(zc.zc_name, dataset, sizeof (zc.zc_name)); (void) strlcpy(zc.zc_name, dataset, sizeof (zc.zc_name));
return (ioctl(g_fd, ZFS_IOC_OBJSET_STATS, &zc) == 0); return (ioctl(g_fd, ZFS_IOC_OBJSET_STATS, &zc) == 0);
} }
@ -573,6 +582,7 @@ recv_impl(const char *snapname, nvlist_t *props, const char *origin,
int error; int error;
ASSERT3S(g_refcount, >, 0); ASSERT3S(g_refcount, >, 0);
VERIFY3S(g_fd, !=, -1);
/* zc_name is name of containing filesystem */ /* zc_name is name of containing filesystem */
(void) strlcpy(zc.zc_name, snapname, sizeof (zc.zc_name)); (void) strlcpy(zc.zc_name, snapname, sizeof (zc.zc_name));