OpenZFS 7745 - print error if lzc_* is called before libzfs_core_init

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
       non-debug builds.

    5. Once the `ASSERTION` macros were enabled again, two tests from
       the test suite were failing in `libzfs_sendrecv.c` at a
       `ZIO_CHECKSUM_IS_ZERO` check within `dump_record()`. We now zero
       the kernel filled checksums in all `dmu_replay_record`s that we
       read in `cksummer()`, except the ones that are of type
       `DRR_BEGIN`.

I considered making all assertions available for both debug and
non-debug builds, but I figured that it would not be appropriate if, for
example, an outside consumer of `libZFS_Core` suddenly triggers an
assertion failure because they happened to call `libzfs_core_fini()`,
even if previously the reference counter was `0`. Therefore, all the
reference counter related assertions are only enabled for debug builds,
and fd related assertions are enabled for debug and non-debug builds.

Porting notes:
- `ASSERT3S(g_refcount, >, 0);` added to `recv_impl` in
  lib/libzfs_core/libzfs_core.c .

Authored by: Serapheim Dimitropoulos <[email protected]>
Reviewed by: Pavel Zakharov <[email protected]>
Reviewed by: Matthew Ahrens <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Ported-by: George Melikov <[email protected]>

OpenZFS-issue: https://www.illumos.org/issues/7745
OpenZFS-commit: https://github.com/openzfs/openzfs/commit/7e3139a
Closes #5698 

2 files changed, 24 insertions, 3 deletions

diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c
index 70488aa97..b08cd2b63 100644
--- a/lib/libzfs/libzfs_sendrecv.c
+++ b/lib/libzfs/libzfs_sendrecv.c
@@ -272,6 +272,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:
 		{
diff --git a/lib/libzfs_core/libzfs_core.c b/lib/libzfs_core/libzfs_core.c
index 5924f6762..bd2db2211 100644
--- a/lib/libzfs_core/libzfs_core.c
+++ b/lib/libzfs_core/libzfs_core.c
@@ -84,7 +84,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;
 
@@ -109,9 +109,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);
 }
 
@@ -125,6 +130,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));
 
@@ -327,6 +333,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);
 }
@@ -576,6 +585,9 @@ recv_impl(const char *snapname, nvlist_t *props, const char *origin,
 	char *atp;
 	int error;
 
+	ASSERT3S(g_refcount, >, 0);
+	VERIFY3S(g_fd, !=, -1);
+
 	/* Set 'fsname' to the name of containing filesystem */
 	(void) strlcpy(fsname, snapname, sizeof (fsname));
 	atp = strchr(fsname, '@');