Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 22 Mar 2018 23:52:37 +0000 (UTC)
From:      Alexander Motin <mav@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org
Subject:   svn commit: r331393 - in stable/11/cddl/contrib/opensolaris/lib: libzfs/common libzfs_core/common
Message-ID:  <201803222352.w2MNqboK000607@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mav
Date: Thu Mar 22 23:52:37 2018
New Revision: 331393
URL: https://svnweb.freebsd.org/changeset/base/331393

Log:
  MFC r329667: MFV r316902:
  7745 print error if lzc_* is called before libzfs_core_init
  
  illumos/illumos-gate@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@delphix.com>
  Reviewed by: Matthew Ahrens <mahrens@delphix.com>
  Approved by: Dan McDonald <danmcd@omniti.com>
  Author: Serapheim Dimitropoulos <serapheim@delphix.com>

Modified:
  stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c
  stable/11/cddl/contrib/opensolaris/lib/libzfs_core/common/libzfs_core.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c
==============================================================================
--- stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c	Thu Mar 22 23:51:39 2018	(r331392)
+++ stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c	Thu Mar 22 23:52:37 2018	(r331393)
@@ -273,6 +273,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: stable/11/cddl/contrib/opensolaris/lib/libzfs_core/common/libzfs_core.c
==============================================================================
--- stable/11/cddl/contrib/opensolaris/lib/libzfs_core/common/libzfs_core.c	Thu Mar 22 23:51:39 2018	(r331392)
+++ stable/11/cddl/contrib/opensolaris/lib/libzfs_core/common/libzfs_core.c	Thu Mar 22 23:52:37 2018	(r331393)
@@ -94,7 +94,7 @@
 extern int zfs_ioctl_version;
 #endif
 
-static int g_fd;
+static int g_fd = -1;
 static pthread_mutex_t g_lock = PTHREAD_MUTEX_INITIALIZER;
 static int g_refcount;
 
@@ -120,9 +120,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);
 }
 
@@ -139,6 +144,7 @@ lzc_ioctl(zfs_ioc_t ioc, const char *name,
 	size_t size;
 
 	ASSERT3S(g_refcount, >, 0);
+	VERIFY3S(g_fd, !=, -1);
 
 	(void) strlcpy(zc.zc_name, name, sizeof (zc.zc_name));
 
@@ -411,6 +417,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);
 }
@@ -656,6 +665,7 @@ recv_impl(const char *snapname, nvlist_t *props, const
 	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));



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201803222352.w2MNqboK000607>