HomeFreeBSD

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

Description

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_records 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 <serapheim@delphix.com>
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Ported-by: George Melikov <mail@gmelikov.ru>

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

Details

Provenance
George Melikov <mail@gmelikov.ru>Authored on Jan 31 2017, 6:48 PM
Brian Behlendorf <behlendorf1@llnl.gov>Committed on Jan 31 2017, 6:48 PM
Parents
rG005e27e3b308: OpenZFS 7019 - zfsdev_ioctl skips secpolicy when FKIOCTL is set
Branches
Unknown
Tags
Unknown

Event Timeline

Brian Behlendorf <behlendorf1@llnl.gov> committed rGe24548975ccd: OpenZFS 7745 - print error if lzc_* is called before libzfs_core_init (authored by George Melikov <mail@gmelikov.ru>).Jan 31 2017, 6:48 PM