svn commit: r316902 - in vendor/illumos/dist/lib: libzfs/common libzfs_core/common
Andriy Gapon
avg at FreeBSD.org
Fri Apr 14 18:14:04 UTC 2017
Author: avg
Date: Fri Apr 14 18:14:02 2017
New Revision: 316902
URL: https://svnweb.freebsd.org/changeset/base/316902
Log:
7745 print error if lzc_* is called before libzfs_core_init
illumos/illumos-gate at 7c13517fff71be473e47531ef4330160c042bedc
https://github.com/illumos/illumos-gate/commit/7c13517fff71be473e47531ef4330160c042bedc
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 at delphix.com>
Reviewed by: Matthew Ahrens <mahrens at delphix.com>
Approved by: Dan McDonald <danmcd at omniti.com>
Author: Serapheim Dimitropoulos <serapheim at delphix.com>
Modified:
vendor/illumos/dist/lib/libzfs/common/libzfs_pool.c
vendor/illumos/dist/lib/libzfs/common/libzfs_sendrecv.c
vendor/illumos/dist/lib/libzfs_core/common/libzfs_core.c
Modified: vendor/illumos/dist/lib/libzfs/common/libzfs_pool.c
==============================================================================
--- vendor/illumos/dist/lib/libzfs/common/libzfs_pool.c Fri Apr 14 18:13:33 2017 (r316901)
+++ vendor/illumos/dist/lib/libzfs/common/libzfs_pool.c Fri Apr 14 18:14:02 2017 (r316902)
@@ -818,7 +818,7 @@ zpool_prop_get_feature(zpool_handle_t *z
const char *feature = strchr(propname, '@') + 1;
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
Modified: vendor/illumos/dist/lib/libzfs/common/libzfs_sendrecv.c
==============================================================================
--- vendor/illumos/dist/lib/libzfs/common/libzfs_sendrecv.c Fri Apr 14 18:13:33 2017 (r316901)
+++ vendor/illumos/dist/lib/libzfs/common/libzfs_sendrecv.c Fri Apr 14 18:14:02 2017 (r316902)
@@ -266,6 +266,15 @@ cksummer(void *arg)
ofp = fdopen(dda->inputfd, "r");
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) {
case DRR_BEGIN:
{
Modified: vendor/illumos/dist/lib/libzfs_core/common/libzfs_core.c
==============================================================================
--- vendor/illumos/dist/lib/libzfs_core/common/libzfs_core.c Fri Apr 14 18:13:33 2017 (r316901)
+++ vendor/illumos/dist/lib/libzfs_core/common/libzfs_core.c Fri Apr 14 18:14:02 2017 (r316902)
@@ -85,7 +85,7 @@
#include <sys/stat.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 int g_refcount;
@@ -110,9 +110,14 @@ libzfs_core_fini(void)
{
(void) pthread_mutex_lock(&g_lock);
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);
+ g_fd = -1;
+ }
(void) pthread_mutex_unlock(&g_lock);
}
@@ -126,6 +131,7 @@ lzc_ioctl(zfs_ioc_t ioc, const char *nam
size_t size;
ASSERT3S(g_refcount, >, 0);
+ VERIFY3S(g_fd, !=, -1);
(void) strlcpy(zc.zc_name, name, sizeof (zc.zc_name));
@@ -328,6 +334,9 @@ lzc_exists(const char *dataset)
*/
zfs_cmd_t zc = { 0 };
+ ASSERT3S(g_refcount, >, 0);
+ VERIFY3S(g_fd, !=, -1);
+
(void) strlcpy(zc.zc_name, dataset, sizeof (zc.zc_name));
return (ioctl(g_fd, ZFS_IOC_OBJSET_STATS, &zc) == 0);
}
@@ -573,6 +582,7 @@ recv_impl(const char *snapname, nvlist_t
int error;
ASSERT3S(g_refcount, >, 0);
+ VERIFY3S(g_fd, !=, -1);
/* zc_name is name of containing filesystem */
(void) strlcpy(zc.zc_name, snapname, sizeof (zc.zc_name));
More information about the svn-src-vendor
mailing list