]> CyberLeo.Net >> Repos - FreeBSD/FreeBSD.git/log
FreeBSD/FreeBSD.git
6 years ago9280 Assertion failure while running removal_with_ganging test with 4K devices
Alexander Motin [Wed, 28 Mar 2018 23:12:03 +0000 (23:12 +0000)]
9280 Assertion failure while running removal_with_ganging test with 4K devices

illumos/illumos-gate@243952c7eeef020886e3e2e3df99a513df40584a

Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: John Kennedy <john.kennedy@delphix.com>
Approved by: Garrett D'Amore <garrett@damore.org>
Author: Matt Ahrens <Matt.Ahrens@delphix.com>

6 years ago9188 increase size of dbuf cache to reduce indirect block decompression
Alexander Motin [Wed, 28 Mar 2018 22:57:02 +0000 (22:57 +0000)]
9188 increase size of dbuf cache to reduce indirect block decompression

illumos/illumos-gate@268bbb2a2fa79c36d4695d13a595ba50a7754b76

With compressed ARC (6950) we use up to 25% of our CPU to decompress indirect
blocks, under a workload of random cached reads. To reduce this decompression
cost, we would like to increase the size of the dbuf cache so that more
indirect blocks can be stored uncompressed.

If we are caching entire large files of recordsize=8K, the indirect blocks
use 1/64th as much memory as the data blocks (assuming they have the same
compression ratio). We suggest making the dbuf cache be 1/32nd of all memory,
so that in this scenario we should be able to keep all the indirect blocks
decompressed in the dbuf cache. (We want it to be more than the 1/64th that
the indirect blocks would use because we need to cache other stuff in the
dbuf cache as well.)

In real world workloads, this won't help as dramatically as the example
above, but we think it's still worth it because the risk of decreasing
performance is low. The potential negative performance impact is that we
will be slightly reducing the size of the ARC (by ~3%).

Reviewed by: Dan Kimmel <dan.kimmel@delphix.com>
Reviewed by: Prashanth Sreenivasa <pks@delphix.com>
Reviewed by: Paul Dagnelie <pcd@delphix.com>
Reviewed by: Sanjay Nadkarni <sanjay.nadkarni@nexenta.com>
Reviewed by: Allan Jude <allanjude@freebsd.org>
Reviewed by: Igor Kozhukhov <igor@dilos.org>
Approved by: Garrett D'Amore <garrett@damore.org>
Author: George Wilson <george.wilson@delphix.com>

6 years ago9321 arc_loan_compressed_buf() can increment arc_loaned_bytes by the wrong value
Alexander Motin [Wed, 28 Mar 2018 22:43:55 +0000 (22:43 +0000)]
9321 arc_loan_compressed_buf() can increment arc_loaned_bytes by the wrong value

illumos/illumos-gate@9be12bd737714550277bd02b0c693db560976990

arc_loan_compressed_buf() increments arc_loaned_bytes by psize unconditionally
In the case of zfs_compressed_arc_enabled=0, when the buf is returned via
arc_return_buf(), if ARC_BUF_COMPRESSED(buf) is false, then arc_loaned_bytes
is decremented by lsize, not psize.

Switch to using arc_buf_size(buf), instead of psize, which will return
psize or lsize, depending on the result of ARC_BUF_COMPRESSED(buf).

Reviewed by: Matt Ahrens <matt@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Approved by: Garrett D'Amore <garrett@damore.org>
Author: Allan Jude <allanjude@freebsd.org>

6 years ago9235 rename zpool_rewind_policy_t to zpool_load_policy_t
Alexander Motin [Wed, 28 Mar 2018 22:16:51 +0000 (22:16 +0000)]
9235 rename zpool_rewind_policy_t to zpool_load_policy_t

illumos/illumos-gate@5dafeea3ebd2dd77affc802bcb90f63faf01589f

We want to be able to pass various settings during import/open of a pool,
which are not only related to rewind. Instead of adding a new policy and
duplicate a bunch of code, we should just rename rewind_policy to a more
generic term like load_policy.

For instance, we'd like to set spa->spa_import_flags from the nvlist,
rather from a flags parameter passed to spa_import as in some cases we want
those flags not only for the import case, but also for the open case. One
such flag could be ZFS_IMPORT_MISSING_LOG (as used in zdb) which would
allow zfs to open a pool when logs are missing.

Reviewed by: Matt Ahrens <matt@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Pavel Zakharov <pavel.zakharov@delphix.com>

6 years ago9191 dump vdev tree to zfs_dbgmsg when spa load fails due to missing log devices
Alexander Motin [Wed, 28 Mar 2018 22:08:57 +0000 (22:08 +0000)]
9191 dump vdev tree to zfs_dbgmsg when spa load fails due to missing log devices

illumos/illumos-gate@ccef24b493bcbd146fcd6d8946666cae081470b6

Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Prakash Surya <prakash.surya@delphix.com>
Reviewed by: Matt Ahrens <matt@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Pavel Zakharov <pavel.zakharov@delphix.com>

6 years ago9187 racing condition between vdev label and spa_last_synced_txg in vdev_validate
Alexander Motin [Wed, 28 Mar 2018 22:06:12 +0000 (22:06 +0000)]
9187 racing condition between vdev label and spa_last_synced_txg in vdev_validate

illumos/illumos-gate@d1de72cfa29ab77ff80e2bb0e668a6afa5bccaf0

ztest failed with uncorrectable IO error despite having the fix for #7163.
Both sides of the mirror have CANT_OPEN_BAD_LABEL, which also distinguishes
it from that issue.

Definitely seems like a racing condition between the vdev_validate and spa_sync:
1. Thread A (spa_sync): vdev label is updated to latest txg
2. Thread B (vdev_validate): vdev label's txg is compared to spa_last_synced_txg and is ahead.
3. Thread A (spa_sync): spa_last_synced_txg is updated to latest txg.

Solution: do not check txg in vdev_validate unless config lock is held.

Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Matt Ahrens <matthew.ahrens@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Pavel Zakharov <pavel.zakharov@delphix.com>

6 years agoAdd files missed from r331695.
Alexander Motin [Wed, 28 Mar 2018 21:00:34 +0000 (21:00 +0000)]
Add files missed from r331695.

6 years ago9166 zfs storage pool checkpoint
Alexander Motin [Wed, 28 Mar 2018 18:12:06 +0000 (18:12 +0000)]
9166 zfs storage pool checkpoint

illumos/illumos-gate@8671400134a11c848244896ca51a7db4d0f69da4

The idea of Storage Pool Checkpoint (aka zpool checkpoint) deals with
exactly that.  It can be thought of as a “pool-wide snapshot” (or a
variation of extreme rewind that doesn’t corrupt your data).  It remembers
the entire state of the pool at the point that it was taken and the user
can revert back to it later or discard it.  Its generic use case is an
administrator that is about to perform a set of destructive actions to ZFS
as part of a critical procedure.  She takes a checkpoint of the pool before
performing the actions, then rewinds back to it if one of them fails or puts
the pool into an unexpected state.  Otherwise, she discards it.  With the
assumption that no one else is making modifications to ZFS, she basically
wraps all these actions into a “high-level transaction”.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: John Kennedy <john.kennedy@delphix.com>
Reviewed by: Dan Kimmel <dan.kimmel@delphix.com>
Approved by: Richard Lowe <richlowe@richlowe.net>
Author: Serapheim Dimitropoulos <serapheim.dimitro@delphix.com>

6 years ago9213 zfs: sytem typo
Alexander Motin [Fri, 23 Mar 2018 02:28:37 +0000 (02:28 +0000)]
9213 zfs: sytem typo

illumos/illumos-gate@edc8ef7d921c96b23969898aeb766cb24960bda7

Reviewed by: C Fraire <cfraire@me.com>
Reviewed by: Andy Fiddaman <omnios@citrus-it.co.uk>
Approved by: Joshua M. Clulow <josh@sysmgr.org>
Author: Toomas Soome <tsoome@me.com>

6 years ago9084 spa_*_ashift must ignore spare devices
Alexander Motin [Fri, 23 Mar 2018 02:22:16 +0000 (02:22 +0000)]
9084 spa_*_ashift must ignore spare devices

illumos/illumos-gate@b037f3dbd69cef4a7ffd576ad33e07bfaf0b1e84

Reviewed by: Prashanth Sreenivasa <pks@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Prakash Surya <prakash.surya@delphix.com>

6 years ago8484 Implement aggregate sum and use for arc counters
Alexander Motin [Fri, 23 Mar 2018 00:20:42 +0000 (00:20 +0000)]
8484 Implement aggregate sum and use for arc counters

In pursuit of improving performance on multi-core systems, we should
implements fanned out counters and use them to improve the performance of
some of the arc statistics. These stats are updated extremely frequently,
and can consume a significant amount of CPU time.

Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Paul Dagnelie <pcd@delphix.com>

6 years ago9164 assert: newds == os->os_dsl_dataset
Andriy Gapon [Thu, 15 Mar 2018 08:46:49 +0000 (08:46 +0000)]
9164 assert: newds == os->os_dsl_dataset

illumos/illumos-gate@5f5913bb83405db87f982abee80162a479d363af
https://github.com/illumos/illumos-gate/commit/5f5913bb83405db87f982abee80162a479d363af

https://www.illumos.org/issues/9164
  This issue has been reported by Alan Somers as
  https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=225877

  dmu_objset_refresh_ownership() first disowns a dataset (and releases
  it) and then owns it again. There is an assert that the new dataset
  object is the same as the old dataset object.  When running ZFS Test
  Suite on FreeBSD we see this panic from zpool_upgrade_007_pos test:

  panic: solaris assert: newds == os->os_dsl_dataset (0xfffff80045f4c000
  == 0xfffff80021ab4800)

  I see that the old dataset has dsl_dataset_evict_async() pending in
  ds_dbu.dbu_tqent and its ds_dbuf is NULL.

Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: Don Brady <don.brady@delphix.com>
Approved by: Richard Lowe <richlowe@richlowe.net>
Author: Andriy Gapon <avg@FreeBSD.org>

6 years ago8984 fix for 6764 breaks ACL inheritance
Andriy Gapon [Wed, 7 Mar 2018 13:47:01 +0000 (13:47 +0000)]
8984 fix for 6764 breaks ACL inheritance

illumos/illumos-gate@e9bacc6d1a71ea3f7082038b2868de8c4dd98bdc
https://github.com/illumos/illumos-gate/commit/e9bacc6d1a71ea3f7082038b2868de8c4dd98bdc

https://www.illumos.org/issues/8984
  Consider a directory configured as:
  drwx-ws---+ 2 henson cpp 3 Jan 23 12:35 dropbox/
  user:henson:rwxpdDaARWcC--:f-i----:allow
  owner@:--------------:f-i----:allow
  group@:--------------:f-i----:allow
  everyone@:--------------:f-i----:allow
  owner@:rwxpdDaARWcC--:-di----:allow
  group:cpp:-wx-----------:-------:allow
  owner@:rwxpdDaARWcC--:-------:allow
  A new file created in this directory ends up looking like:
  rw-r--r-+ 1 astudent cpp 0 Jan 23 12:39 testfile
  user:henson:rw-pdDaARWcC--:------I:allow
  owner@:--------------:------I:allow
  group@:--------------:------I:allow
  everyone@:--------------:------I:allow
  owner@:rw-p--aARWcCos:-------:allow
  group@:r-----a-R-c--s:-------:allow
  everyone@:r-----a-R-c--s:-------:allow
  with extraneous group@ and everyone@ entries allowing read access that
  shouldn't exist.
  Per Albert Lee on the zfs mailing list:
  "aclinherit=passthrough/passthrough-x should still
  ignore the requested mode when an inheritable ACE for owner@ group@,
  or everyone@ is present in the parent directory.
  It appears there was an oversight in my fix for
  https://www.illumos.org/issues/6764 which made calling zfs_acl_chmod
  from zfs_acl_inherit unconditional. I think the parent ACL check for
  aclinherit=passthrough needs to be reintroduced in zfs_acl_inherit."
  We have a large number of faculty who use dropbox directories like the example
  to have students submit projects. All of these directories are now allowing

Reviewed by: Sam Zaydel <szaydel@racktopsystems.com>
Reviewed by: Paul B. Henson <henson@acm.org>
Reviewed by: Prakash Surya <prakash.surya@delphix.com>
Approved by: Matthew Ahrens <mahrens@delphix.com>
Author: Dominik Hassler <hadfl@omniosce.org>

6 years ago8940 Sending an intra-pool resumable send stream may result in EXDEV
Alexander Motin [Thu, 22 Feb 2018 04:01:05 +0000 (04:01 +0000)]
8940 Sending an intra-pool resumable send stream may result in EXDEV

illumos/illumos-gate@544132fce3fa6583f01318f9559adc46614343a7

"zfs send -t <token>" for an incremental send should be able to resume
successfully when sending to the same pool: a subtle issue in
zfs_iter_children() doesn't currently allow this.

Because resuming from a token requires "guid" -> "dataset" mapping
(guid_to_name()), we have to walk the whole hierarchy to find the right
snapshots to send.
When resuming an incremental send both source and destination live in the
same pool and have the same guid: this is where zfs_iter_children() gets
confused and picks up the wrong snapshot, so we end up trying to send an
incremental "destination@snap1 -> source@snap2" stream instead of
"source@snap1 -> source@snap2": this fails with an "Invalid cross-device
link" (EXDEV) error.

Reviewed by: Paul Dagnelie <pcd@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Hans Rosenfeld <rosenfeld@grumpf.hope-2000.org>
Author: loli10K <ezomori.nozomu@gmail.com>

6 years ago9080 recursive enter of vdev_indirect_rwlock from vdev_indirect_remap()
Alexander Motin [Thu, 22 Feb 2018 03:52:03 +0000 (03:52 +0000)]
9080 recursive enter of vdev_indirect_rwlock from vdev_indirect_remap()

illumos/illumos-gate@bdfded42e66b9fc1395ff2401aa2952f7c44ae34

A scenario came up where a callback executed by vdev_indirect_remap() on a vdev, calls
vdev_indirect_remap() on the same vdev and tries to reacquire vdev_indirect_rwlock that
was already acquired from the first call to vdev_indirect_remap(). The specific scenario,
is that we want to remap a block pointer that is snapshoted but its dataset's remap_deadlist
is not cached. So in order to add it we issue a read through a vdev_indirect_remap() on the
same vdev, which brings up the aforementioned issue.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Approved by: Hans Rosenfeld <rosenfeld@grumpf.hope-2000.org>
Author: Serapheim Dimitropoulos <serapheim.dimitro@delphix.com>

6 years agoMissed pieces of r329799.
Alexander Motin [Thu, 22 Feb 2018 03:23:43 +0000 (03:23 +0000)]
Missed pieces of r329799.

6 years ago9079 race condition in starting and ending condesing thread for indirect vdevs
Alexander Motin [Thu, 22 Feb 2018 03:22:27 +0000 (03:22 +0000)]
9079 race condition in starting and ending condesing thread for indirect vdevs

illumos/illumos-gate@667ec66f1b4f491d5e839644e0912cad1c9e7122

The timeline of the race condition is the following:
[1] Thread A is about to finish condesing the first vdev in spa_condense_indirect_thread(),
so it calls the spa_condense_indirect_complete_sync() sync task which sets the
spa_condensing_indirect field to NULL. Waiting for the sync task to finish, thread A
sleeps until the txg is done. When this happens, thread A will acquire spa_async_lock
and set spa_condense_thread to NULL.
[2] While thread A waits for the txg to finish, thread B which is running spa_sync() checks
whether it should condense the second vdev in vdev_indirect_should_condense() by checking
the spa_condensing_indirect field which was set to NULL by spa_condense_indirect_thread()
from thread A. So it goes on and tries to spawn a new condensing thread in
spa_condense_indirect_start_sync() and the aforementioned assertions fails because thread A
has not set spa_condense_thread to NULL (which is basically the last thing it does before
returning).

The main issue here is that we rely on both spa_condensing_indirect and spa_condense_thread to
signify whether a condensing thread is running. Ideally we would only use one throughout the
codebase. In addition, for managing spa_condense_thread we currently use spa_async_lock which
basically tights condensing to scrubing when it comes to pausing and resuming those actions
during spa export.

Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Approved by: Hans Rosenfeld <rosenfeld@grumpf.hope-2000.org>
Author: Serapheim Dimitropoulos <serapheim@delphix.com>

6 years agor329793 | mav | 2018-02-22 04:21:03 +0200 (чт, 22 февр. 2018) | 58 lines
Alexander Motin [Thu, 22 Feb 2018 02:25:09 +0000 (02:25 +0000)]
r329793 | mav | 2018-02-22 04:21:03 +0200 (чт, 22 февр. 2018) | 58 lines

9075 Improve ZFS pool import/load process and corrupted pool recovery

illumos/illumos-gate@6f7938128a2c5e23f4b970ea101137eadd1470a1

Some work has been done lately to improve the debugability of the ZFS pool
load (and import) process. This includes:

https://www.illumos.org/issues/7638: Refactor spa_load_impl into several functions
https://www.illumos.org/issues/8961: SPA load/import should tell us why it failed
https://www.illumos.org/issues/7277: zdb should be able to print zfs_dbgmsg's

To iterate on top of that, there's a few changes that were made to make the
import process more resilient and crash free. One of the first tasks during the
pool load process is to parse a config provided from userland that describes
what devices the pool is composed of. A vdev tree is generated from that config,
and then all the vdevs are opened.

The Meta Object Set (MOS) of the pool is accessed, and several metadata objects
that are necessary to load the pool are read. The exact configuration of the
pool is also stored inside the MOS. Since the configuration provided from
userland is external and might not accurately describe the vdev tree
of the pool at the txg that is being loaded, it cannot be relied upon to safely
operate the pool. For that reason, the configuration in the MOS is read early
on. In the past, the two configurations were compared together and if there was
a mismatch then the load process was aborted and an error was returned.

The latter was a good way to ensure a pool does not get corrupted, however it
made the pool load process needlessly fragile in cases where the vdev
configuration changed or the userland configuration was outdated. Since the MOS
is stored in 3 copies, the configuration provided by userland doesn't have to be
perfect in order to read its contents. Hence, a new approach has been adopted:
The pool is first opened with the untrusted userland configuration just so that
the real configuration can be read from the MOS. The trusted MOS configuration
is then used to generate a new vdev tree and the pool is re-opened.

When the pool is opened with an untrusted configuration, writes are disabled
to avoid accidentally damaging it. During reads, some sanity checks are
performed on block pointers to see if each DVA points to a known vdev;
when the configuration is untrusted, instead of panicking the system if those
checks fail we simply avoid issuing reads to the invalid DVAs.

This new two-step pool load process now allows rewinding pools accross
vdev tree changes such as device replacement, addition, etc. Loading a pool
from an external config file in a clustering environment also becomes much
safer now since the pool will import even if the config is outdated and didn't,
for instance, register a recent device addition.

With this code in place, it became relatively easy to implement a
long-sought-after feature: the ability to import a pool with missing top level
(i.e. non-redundant) devices. Note that since this almost guarantees some loss
Of data, this feature is for now restricted to a read-only import.

Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Andrew Stormont <andyjstormont@gmail.com>
Approved by: Hans Rosenfeld <rosenfeld@grumpf.hope-2000.org>
Author: Pavel Zakharov <pavel.zakharov@delphix.com>

6 years ago9075 Improve ZFS pool import/load process and corrupted pool recovery
Alexander Motin [Thu, 22 Feb 2018 02:21:03 +0000 (02:21 +0000)]
9075 Improve ZFS pool import/load process and corrupted pool recovery

illumos/illumos-gate@6f7938128a2c5e23f4b970ea101137eadd1470a1

Some work has been done lately to improve the debugability of the ZFS pool
load (and import) process. This includes:

https://www.illumos.org/issues/7638: Refactor spa_load_impl into several functions
https://www.illumos.org/issues/8961: SPA load/import should tell us why it failed
https://www.illumos.org/issues/7277: zdb should be able to print zfs_dbgmsg's

To iterate on top of that, there's a few changes that were made to make the
import process more resilient and crash free. One of the first tasks during the
pool load process is to parse a config provided from userland that describes
what devices the pool is composed of. A vdev tree is generated from that config,
and then all the vdevs are opened.

The Meta Object Set (MOS) of the pool is accessed, and several metadata objects
that are necessary to load the pool are read. The exact configuration of the
pool is also stored inside the MOS. Since the configuration provided from
userland is external and might not accurately describe the vdev tree
of the pool at the txg that is being loaded, it cannot be relied upon to safely
operate the pool. For that reason, the configuration in the MOS is read early
on. In the past, the two configurations were compared together and if there was
a mismatch then the load process was aborted and an error was returned.

The latter was a good way to ensure a pool does not get corrupted, however it
made the pool load process needlessly fragile in cases where the vdev
configuration changed or the userland configuration was outdated. Since the MOS
is stored in 3 copies, the configuration provided by userland doesn't have to be
perfect in order to read its contents. Hence, a new approach has been adopted:
The pool is first opened with the untrusted userland configuration just so that
the real configuration can be read from the MOS. The trusted MOS configuration
is then used to generate a new vdev tree and the pool is re-opened.

When the pool is opened with an untrusted configuration, writes are disabled
to avoid accidentally damaging it. During reads, some sanity checks are
performed on block pointers to see if each DVA points to a known vdev;
when the configuration is untrusted, instead of panicking the system if those
checks fail we simply avoid issuing reads to the invalid DVAs.

This new two-step pool load process now allows rewinding pools accross
vdev tree changes such as device replacement, addition, etc. Loading a pool
from an external config file in a clustering environment also becomes much
safer now since the pool will import even if the config is outdated and didn't,
for instance, register a recent device addition.

With this code in place, it became relatively easy to implement a
long-sought-after feature: the ability to import a pool with missing top level
(i.e. non-redundant) devices. Note that since this almost guarantees some loss
Of data, this feature is for now restricted to a read-only import.

Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Andrew Stormont <andyjstormont@gmail.com>
Approved by: Hans Rosenfeld <rosenfeld@grumpf.hope-2000.org>
Author: Pavel Zakharov <pavel.zakharov@delphix.com>

6 years ago8942 zfs promote .../%recv should be an error
Alexander Motin [Thu, 22 Feb 2018 01:30:03 +0000 (01:30 +0000)]
8942 zfs promote .../%recv should be an error

illumos/illumos-gate@add927f8c8d101e16c23eb9cd270be4fd7edf7d5

Reported on the ZFSonLinux https://github.com/zfsonlinux/zfs/issues/4843,
fixed by https://github.com/zfsonlinux/zfs/pull/6339:

If we are in the middle of an incremental zfs receive, the child .../%recv
will exist. If you concurrently run zfs promote .../%recv, it will "work",
but then zfs gets confused. For example, there's no obvious way to destroy
the containing filesystem (because it is now a clone of its invisible child).

Attempting to do this promote should be an error. We could fix this by
having zfs_ioc_promote() check if zc_name contains a %, similar to
zfs_ioc_rename().

Reviewed by: Paul Dagnelie <pcd@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: loli10K <ezomori.nozomu@gmail.com>

6 years ago8941 zpool add: assertion failed in get_replication() with nested interior VDEVs
Alexander Motin [Thu, 22 Feb 2018 01:17:32 +0000 (01:17 +0000)]
8941 zpool add: assertion failed in get_replication() with nested interior VDEVs

illumos/illumos-gate@ac0215f4d618163d117a40fbf77a3f944852cb7b

When replacing a faulted device which was previously handled by a spare
multiple levels of nested interior VDEVs will be present in the pool
configuration: get_replication() needs to handle this situation gracefully
to let zpool add new devices to the pool

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Andrew Stormont <andyjstormont@gmail.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: loli10K <ezomori.nozomu@gmail.com>

6 years ago8477 Assertion failed in vdev_state_dirty(): spa_writeable(spa)
Alexander Motin [Thu, 22 Feb 2018 00:59:49 +0000 (00:59 +0000)]
8477 Assertion failed in vdev_state_dirty(): spa_writeable(spa)

illumos/illumos-gate@f4c1745bd6c9829a05ecec15759ede7757100ab5

Illumos 4080 allows "zpool clear" to work on readonly pools: i don't think
this is the intended behaviour, we shouldn't be allowed to clear readonly
pools. Probably.

A fix is already in the ZFS on Linux repository to addess this issue:
https://github.com/zfsonlinux/zfs/commit/92e43c17188d47f47b69318e4884096dec380e36

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: loli10K <ezomori.nozomu@gmail.com>

6 years ago8408 dsl_props_set_sync_impl() does not handle nested nvlists correctly
Alexander Motin [Thu, 22 Feb 2018 00:53:59 +0000 (00:53 +0000)]
8408 dsl_props_set_sync_impl() does not handle nested nvlists correctly

illumos/illumos-gate@85723e5eec42f46dbfdb4c09b9e1ed66501d1ccf

When iterating over the input nvlist in dsl_props_set_sync_impl() when we
don't preserve the nvpair name before looking up ZPROP_VALUE, so when we
later go to process it nvpair_name() is always "value" instead of the actual
property name.

This results in a couple of bugs in the recv code:

 - received properties are not restored correctly when failing to receive
   an incremental send stream
 - received properties are not completely replaced by the new ones when
   successfully receiving an incremental send stream

This was discovered on ZFS on Linux (fixed in
https://github.com/zfsonlinux/zfs/commit/5f1346c29997dd4e02acf4c19c875d5484f33b1e)

Reviewed by: Paul Dagnelie <pcd@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: loli10K <ezomori.nozomu@gmail.com>

6 years ago9036 zfs: duplicate 'const' declaration specifier
Alexander Motin [Thu, 22 Feb 2018 00:48:59 +0000 (00:48 +0000)]
9036 zfs: duplicate 'const' declaration specifier

illumos/illumos-gate@f02c28e434fb4d81d95122bab187fb43d4f19c2a

Reviewed by: Yuri Pankov <yuripv@yuripv.net>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Toomas Soome <tsoome@me.com>

6 years ago9035 zfs: this statement may fall through
Alexander Motin [Thu, 22 Feb 2018 00:46:24 +0000 (00:46 +0000)]
9035 zfs: this statement may fall through

illumos/illumos-gate@46ac8fdfc5a1f9d8240c79a6ae5b2889cbe83553

Reviewed by: Yuri Pankov <yuripv@yuripv.net>
Reviewed by: Andy Fiddaman <omnios@citrus-it.co.uk>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Toomas Soome <tsoome@me.com>

6 years ago8962 zdb should work on non-idle pools
Alexander Motin [Thu, 22 Feb 2018 00:09:15 +0000 (00:09 +0000)]
8962 zdb should work on non-idle pools

illumos/illumos-gate@e144c4e6c90e7d4dccaad6db660ee42b6e7ba04f

Currently `zdb` consistently fails to examine non-idle pools as it fails
during the `spa_load()` process. The main problem seems to be that
`spa_load_verify()` fails as can be seen below:

$ sudo zdb -d -G dcenter
    zdb: can't open 'dcenter': I/O error

ZFS_DBGMSG(zdb):
    spa_open_common: opening dcenter
    spa_load(dcenter): LOADING
    disk vdev '/dev/dsk/c4t11d0s0': best uberblock found for spa dcenter. txg 40824950
    spa_load(dcenter): using uberblock with txg=40824950
    spa_load(dcenter): UNLOADING
    spa_load(dcenter): RELOADING
    spa_load(dcenter): LOADING
    disk vdev '/dev/dsk/c3t10d0s0': best uberblock found for spa dcenter. txg 40824952
    spa_load(dcenter): using uberblock with txg=40824952
    spa_load(dcenter): FAILED: spa_load_verify failed [error=5]
    spa_load(dcenter): UNLOADING

This change makes `spa_load_verify()` a dryrun when ran from `zdb`. This is
done by creating a global flag in zfs and then setting it in `zdb`.

Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Andy Stormont <astormont@racktopsystems.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Pavel Zakharov <pavel.zakharov@delphix.com>

6 years ago8961 SPA load/import should tell us why it failed
Alexander Motin [Wed, 21 Feb 2018 23:42:30 +0000 (23:42 +0000)]
8961 SPA load/import should tell us why it failed

illumos/illumos-gate@3ee8c80c747c4aa3f83351a6920f30c411236e1b

When we fail to open or import a storage pool, we typically don't get any
additional diagnostic information, just "no pool found" or "can not import".

While there may be no additional user-consumable information, we should at
least make this situation easier to debug/diagnose for developers and support.
For example, we could start by using `zfs_dbgmsg()` to log each thing that we
try when importing, and which things failed. E.g. "tried uberblock of txg X
from label Y of device Z". Also, we could log each of the stages that we go
through in `spa_load_impl()`.

Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Andrew Stormont <andyjstormont@gmail.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Pavel Zakharov <pavel.zakharov@delphix.com>

6 years ago7638 Refactor spa_load_impl into several functions
Alexander Motin [Wed, 21 Feb 2018 23:25:11 +0000 (23:25 +0000)]
7638 Refactor spa_load_impl into several functions

illumos/illumos-gate@1fd3785ff6601d3e391378c2dcbf4c5f27e1fe32

spa_load_impl has grown out of proportions.  It is currently over 700
lines long and makes it very hard to follow or debug the import process
even for experienced ZFS developers.  The objective is to split it up
in a series of well commented functions.

Reviewed by: Paul Dagnelie <pcd@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Andrew Stormont <andyjstormont@gmail.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Pavel Zakharov <pavel.zakharov@delphix.com>

6 years ago9018 Replace kmem_cache_reap_now() with kmem_cache_reap_soon()
Alexander Motin [Wed, 21 Feb 2018 22:14:19 +0000 (22:14 +0000)]
9018 Replace kmem_cache_reap_now() with kmem_cache_reap_soon()

illumos/illumos-gate@36a64e62848b51ac5a9a5216e894ec723cfef14e

To prevent kmem_cache reaping from blocking other system resources, turn
kmem_cache_reap_now() (which blocks) into kmem_cache_reap_soon(). Callers
to kmem_cache_reap_soon() should use kmem_cache_reap_active(), which
exploits #9017's new taskq_empty().

Reviewed by: Bryan Cantrill <bryan@joyent.com>
Reviewed by: Dan McDonald <danmcd@joyent.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Yuri Pankov <yuripv@yuripv.net>
Author: Tim Kordas <tim.kordas@joyent.com>

6 years ago8809 libzpool should leverage work done in libfakekernel
Alexander Motin [Wed, 21 Feb 2018 21:04:46 +0000 (21:04 +0000)]
8809 libzpool should leverage work done in libfakekernel

illumos/illumos-gate@f06dce2c1f0f3af78581e7574f65bfba843ddb6e

Reviewed by: Sebastien Roy <sebastien.roy@delphix.com>
Reviewed by: Prakash Surya <prakash.surya@delphix.com>
Reviewed by: Gordon Ross <gordon.w.ross@gmail.com>
Approved by: Richard Lowe <richlowe@richlowe.net>
Author: Andrew Stormont <astormont@racktopsystems.com>

6 years ago8969 Cannot boot from RAIDZ with parity > 1
Alexander Motin [Wed, 21 Feb 2018 18:09:07 +0000 (18:09 +0000)]
8969 Cannot boot from RAIDZ with parity > 1

illumos/illumos-gate@0fb055e81fd0cda5221da8ddd98b2f8d1fc6bdbe

At present it is possible to boot from a root pool that is on RAIDZ but not
one that is on RAIDZ2 or RAIDZ3.  This is because, at the time the pool
version is checked to ensure support for dual/triple parity, the uberblock
has not yet been loaded into the SPA and therefore the code determines that
the pool version is too old and returns ENOTSUP.

Reviewed by: Igor Kozhukhov <igor@dilos.org>
Reviewed by: Andriy Gapon <avg@FreeBSD.org>
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: Andy Stormont <astormont@racktopsystems.com>
Reviewed by: Toomas Soome <tsoome@me.com>
Approved by: Gordon Ross <gwr@nexenta.com>
Author: Andy Fiddaman <omnios@citrus-it.co.uk>

6 years ago8520 7198 lzc_rollback_to should support rolling back to origin
Andriy Gapon [Wed, 21 Feb 2018 15:10:33 +0000 (15:10 +0000)]
8520 7198 lzc_rollback_to should support rolling back to origin

illumos/illumos-gate@95643f75d23914a3e332adc9661ed51749e9858d
https://github.com/illumos/illumos-gate/commit/95643f75d23914a3e332adc9661ed51749e9858d

https://www.illumos.org/issues/8520
  lzc_rollback_to() should support rolling back to a clone's origin.
  The current checks in zfs_ioc_rollback() would not allow that because the
  origin snapshot belongs to a different filesystem.
  The overly restrictive check was introduced in 7600, but it was not a
  regression as none of the existing tools provided a way to rollback to the
  origin.

https://www.illumos.org/issues/7198
  EINVAL is returned when a dataset does not have any snapshots, so there is
  nothing to roll back to.
  Although the code in zfs_do_rollback checks for that condition in advance, it's
  still possible that the snapshot(s) gets removed after the check and before the
  rollback sync task is executed.
  At the moment zfs command would crash when that happens.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Andriy Gapon <avg@FreeBSD.org>

6 years ago8997 ztest assertion failure in zil_lwb_write_issue
Andriy Gapon [Wed, 21 Feb 2018 14:33:00 +0000 (14:33 +0000)]
8997 ztest assertion failure in zil_lwb_write_issue

illumos/illumos-gate@f864f99efe57685e1762590c1a880dd16bca6da9
https://github.com/illumos/illumos-gate/commit/f864f99efe57685e1762590c1a880dd16bca6da9

https://www.illumos.org/issues/8997
  When dmu_tx_assign is called from zil_lwb_write_issue, it's possible
  for either ERESTART or EIO to be returned.
  If ERESTART is returned, this will cause an assertion to fail directly
  in zil_lwb_write_issue, where the code assumes the return value is
  EIO if dmu_tx_assign returns a non-zero value. This can occur if the
  SPA is suspended when dmu_tx_assign is called, and most often occurs
  when running zloop.
  If EIO is returned, this can cause assertions to fail elsewhere in the
  ZIL code. For example, zil_commit_waiter_timeout contains the
  following logic:
    lwb_t *nlwb = zil_lwb_write_issue(zilog, lwb);
    ASSERT3S(lwb->lwb_state, !=, LWB_STATE_OPENED);
  In this case, if dmu_tx_assign returned EIO from within
  zil_lwb_write_issue, the lwb variable passed in will not be issued
  to disk. Thus, it's lwb_state field will remain LWB_STATE_OPENED and
  this assertion will fail. zil_commit_waiter_timeout assumes that after
  it calls zil_lwb_write_issue, the lwb will be issued to disk, and
  doesn't handle the case where this is not true; i.e. it doesn't handle
  the case where dmu_tx_assign returns EIO.

Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: Andriy Gapon <avg@FreeBSD.org>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Prakash Surya <prakash.surya@delphix.com>

6 years ago8731 ASSERT3U(nui64s, <=, UINT16_MAX) fails for large blocks
Andriy Gapon [Wed, 21 Feb 2018 14:30:34 +0000 (14:30 +0000)]
8731 ASSERT3U(nui64s, <=, UINT16_MAX) fails for large blocks

illumos/illumos-gate@a6c1eb3c08094a6db69aa1dc6315bc814e82e79c
https://github.com/illumos/illumos-gate/commit/a6c1eb3c08094a6db69aa1dc6315bc814e82e79c

https://www.illumos.org/issues/8731
  annotate_ecksum() asserts that nui64s, calculated as nui64s = size / sizeof
  (uint64_t), is not greater than UINT16_MAX.
  This restriction is needed because histograms of incorrectly set and cleared
  bits have 16 bit counters and if the buffer consists of too many 64-bit words,
  then a counter can potentially overflow producing an incorrect result.
  When the largest buffer size was 128KB the greatest value of nui64s was 16K,
  well within the limit.
  But now we have support for large buffers and for buffer sizes of 512KB and
  above the restriction is violated.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Andriy Gapon <avg@FreeBSD.org>

6 years ago8966 Source file zfs_acl.c, function zfs_aclset_common contains a use after end of...
Andriy Gapon [Wed, 21 Feb 2018 14:12:29 +0000 (14:12 +0000)]
8966 Source file zfs_acl.c, function zfs_aclset_common contains a use after end of the lifetime of a local variable

illumos/illumos-gate@82693e09cc02331fa1b3b73b54b1060e73507a8d
https://github.com/illumos/illumos-gate/commit/82693e09cc02331fa1b3b73b54b1060e73507a8d

https://www.illumos.org/issues/8966

Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: Andriy Gapon <avg@FreeBSD.org>
Approved by: Richard Lowe <richlowe@richlowe.net>
Author: WHR <msl0000023508@gmail.com>

6 years ago7614 zfs device evacuation/removal
Alexander Motin [Sun, 18 Feb 2018 01:21:52 +0000 (01:21 +0000)]
7614 zfs device evacuation/removal

illumos/illumos-gate@5cabbc6b49070407fb9610cfe73d4c0e0dea3e77

https://www.illumos.org/issues/7614:
This project allows top-level vdevs to be removed from the storage pool with
“zpool remove”, reducing the total amount of storage in the pool. This
operation copies all allocated regions of the device to be removed onto other
devices, recording the mapping from old to new location. After the removal is
complete, read and free operations to the removed (now “indirect”) vdev must
be remapped and performed at the new location on disk. The indirect mapping
table is kept in memory whenever the pool is loaded, so there is minimal
performance overhead when doing operations on the indirect vdev.

The size of the in-memory mapping table will be reduced when its entries
become “obsolete” because they are no longer used by any block pointers in
the pool. An entry becomes obsolete when all the blocks that use it are
freed. An entry can also become obsolete when all the snapshots that
reference it are deleted, and the block pointers that reference it have been
“remapped” in all filesystems/zvols (and clones). Whenever an indirect block
is written, all the block pointers in it will be “remapped” to their new
(concrete) locations if possible. This process can be accelerated by using
the “zfs remap” command to proactively rewrite all indirect blocks that
reference indirect (removed) vdevs.

Note that when a device is removed, we do not verify the checksum of the data
that is copied. This makes the process much faster, but if it were used on
redundant vdevs (i.e. mirror or raidz vdevs), it would be possible to copy
the wrong data, when we have the correct data on e.g. the other side of the
mirror. Therefore, mirror and raidz devices can not be removed.

Reviewed by: Alex Reece <alex@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: John Kennedy <john.kennedy@delphix.com>
Reviewed by: Prakash Surya <prakash.surya@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Richard Laager <rlaager@wiktel.com>
Reviewed by: Tim Chase <tim@chase2k.com>
Approved by: Garrett D'Amore <garrett@damore.org>
Author: Prashanth Sreenivasa <pks@delphix.com>

6 years ago8857 zio_remove_child() panic due to already destroyed parent zio
Andriy Gapon [Thu, 15 Feb 2018 14:34:18 +0000 (14:34 +0000)]
8857 zio_remove_child() panic due to already destroyed parent zio

illumos/illumos-gate@d6e1c446d7897003fd9fd36ef5aa7da350b7f6af
https://github.com/illumos/illumos-gate/commit/d6e1c446d7897003fd9fd36ef5aa7da350b7f6af

https://www.illumos.org/issues/8857
  I had an OS panic on one of our servers:

  ffffff01809128c0 vpanic()
  ffffff01809128e0 mutex_panic+0x58(fffffffffb94c904ffffff597dde7f80)
  ffffff0180912950 mutex_vector_enter+0x347(ffffff597dde7f80)
  ffffff01809129b0 zio_remove_child+0x50(ffffff597dde7c58ffffff32bd901ac0,
  ffffff3373370908)
  ffffff0180912a40 zio_done+0x390(ffffff32bd901ac0)
  ffffff0180912a70 zio_execute+0x78(ffffff32bd901ac0)
  ffffff0180912b30 taskq_thread+0x2d0(ffffff33bae44140)
  ffffff0180912b40 thread_start+8()

  It panicked here:
  http://src.illumos.org/source/xref/illumos-gate/usr/src/uts/common/fs/zfs/
  zio.c#430

  pio->io_lock is DEAD, thus a panic. Further analysis shows the "pio"
  (parent zio of "cio") has already been destroyed.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Andriy Gapon <avg@FreeBSD.org>
Reviewed by: Youzhong Yang <youzhong@gmail.com>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: George Wilson <george.wilson@delphix.com>

6 years ago8972 zfs holds: In scripted mode, do not pad columns with spaces
Alexander Motin [Mon, 22 Jan 2018 05:59:48 +0000 (05:59 +0000)]
8972 zfs holds: In scripted mode, do not pad columns with spaces

illumos/illumos-gate@e9b7d6e7f7a6477679a35b73eb3934b096b3dd39

https://www.illumos.org/issues/8972:
'zfs holds -H' does not properly output content in scripted mode. It uses a
tab instead of two spaces, but it still pads column widths with spaces when
it should not.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Allan Jude <allanjude@freebsd.org>

6 years ago8835 Speculative prefetch in ZFS not working for misaligned reads
Alexander Motin [Mon, 22 Jan 2018 05:55:43 +0000 (05:55 +0000)]
8835 Speculative prefetch in ZFS not working for misaligned reads

illumos/illumos-gate@5cb8d943bc8513c6230589aad5a409d58b0297cb

https://www.illumos.org/issues/8835:
Sequential reads not aligned to block size are not detected by ZFS
prefetcher as sequential, killing prefetch and severely hurting
performance.  It is caused by dmu_zfetch() in case of misaligned
sequential accesses being called with overlap of one block.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Allan Jude <allanjude@freebsd.org>
Approved by: Gordon Ross <gwr@nexenta.com>
Author: Alexander Motin <mav@FreeBSD.org>

6 years ago8652 Tautological comparisons with ZPROP_INVAL
Alexander Motin [Mon, 22 Jan 2018 04:48:14 +0000 (04:48 +0000)]
8652 Tautological comparisons with ZPROP_INVAL

illumos/illumos-gate@4ae5f5f06c6c2d1db8167480f7d9e3b5378ba2f2

https://www.illumos.org/issues/8652:
Clang and GCC prefer to use unsigned ints to store enums. With Clang, that
causes tautological comparison warnings when comparing a zfs_prop_t or
zpool_prop_t variable to the macro ZPROP_INVAL. It's likely that error
handling code is being silently removed as a result.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Igor Kozhukhov <igor@dilos.org>
Approved by: Gordon Ross <gwr@nexenta.com>
Author: Alan Somers <asomers@gmail.com>

6 years ago8641 "zpool clear" and "zinject" don't work on "spare" or "replacing" vdevs
Alexander Motin [Mon, 22 Jan 2018 04:35:17 +0000 (04:35 +0000)]
8641 "zpool clear" and "zinject" don't work on "spare" or "replacing" vdevs

illumos/illumos-gate@2ba5f978a4f9b02da9db1b8cdd9ea5498eb00ad9

https://www.illumos.org/issues/8641:
"zpool clear" and "zinject -d" can both operate on specific vdevs, either
leaf or interior. However, due to an oversight, neither works on a "spare"
or "replacing" vdev. For example:

sudo zpool create foo raidz1 c1t5000CCA000081D61d0 c1t5000CCA000186235d0 spare c1t5000CCA000094115d0
sudo zpool replace foo c1t5000CCA000186235d0 c1t5000CCA000094115d0
$ zpool status foo pool: foo
state: ONLINE
scan: resilvered 81.5K in 0h0m with 0 errors on Fri Sep 8 10:53:03 2017
config:

NAME                         STATE     READ WRITE CKSUM
        foo                          ONLINE       0     0     0
          raidz1-0                   ONLINE       0     0     0
            c1t5000CCA000081D61d0    ONLINE       0     0     0
            spare-1                  ONLINE       0     0     0
              c1t5000CCA000186235d0  ONLINE       0     0     0
              c1t5000CCA000094115d0  ONLINE       0     0     0
        spares
          c1t5000CCA000094115d0      INUSE     currently in use
$ sudo zinject -d spare-1 -A degrade foo
cannot find device 'spare-1' in pool 'foo'
$ sudo zpool clear foo spare-1
cannot clear errors for spare-1: no such device in pool

Even though there was nothing to clear, those commands shouldn't have
reported an error. by contrast, trying to clear "raidz1-0" works just fine:
$ sudo zpool clear foo raidz1-0

6 years ago8959 Add notifications when a scrub is paused or resumed
Alexander Motin [Mon, 22 Jan 2018 04:27:05 +0000 (04:27 +0000)]
8959 Add notifications when a scrub is paused or resumed

illumos/illumos-gate@301fd1d6f25595cd8c6d6795f39c72d97aff8cd9

Reviewed by: Alek Pinchuk <pinchuk.alek@gmail.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Gordon Ross <gwr@nexenta.com>
Author: Sean Eric Fagan <sef@ixsystems.com>

6 years ago8856 arc_cksum_is_equal() doesn't take into account ABD-logic
Alexander Motin [Mon, 22 Jan 2018 04:21:55 +0000 (04:21 +0000)]
8856 arc_cksum_is_equal() doesn't take into account ABD-logic

illumos/illumos-gate@01a059ee0cdece49f47fd4d70086dd5bc7d0b0ff

https://www.illumos.org/issues/8856:
arc_cksum_is_equal() calls zio_push_transform() that requires abd_t*
(second arg), but a void* is passed.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Approved by: Gordon Ross <gwr@nexenta.com>
Author: Roman Strashkin <roman.strashkin@nexenta.com>

6 years ago8898 creating fs with checksum=skein on the boot pools fails ungracefully
Alexander Motin [Sun, 21 Jan 2018 23:57:41 +0000 (23:57 +0000)]
8898 creating fs with checksum=skein on the boot pools fails ungracefully

illumos/illumos-gate@9fa2266d9a78b8366e1cd2d5f050e8b5e37d558c

https://www.illumos.org/issues/8898:
# zfs create -o checksum=skein rpool/test
internal error: Result too large
Abort (core dumped)

Not a big deal per se, but should be handled correctly.

Also reported as: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=222199

Reviewed by: Toomas Soome <tsoome@me.com>
Reviewed by: Andy Stormont <astormont@racktopsystems.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Yuri Pankov <yuri.pankov@nexenta.com>

6 years ago8897 zpool online -e fails assertion when run on non-leaf vdevs
Alexander Motin [Sun, 21 Jan 2018 23:52:37 +0000 (23:52 +0000)]
8897 zpool online -e fails assertion when run on non-leaf vdevs

illumos/illumos-gate@9a551dd645b478816cb11251b19f5034d885bf01

https://www.illumos.org/issues/8897:
# zpool online -e test mirror-1
Assertion failed: nvlist_lookup_string(tgt, "path", &pathname) == 0, file ../common/libzfs_pool.c, line 2558, function zpool_vdev_online
Abort (core dumped)

Not a big deal per se, but should be handled gracefully, same way as 'offline' and 'online' without '-e'.

Also reported as: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=221408

Reviewed by: Toomas Soome <tsoome@me.com>
Reviewed by: Igor Kozhukhov <igor@dilos.org>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Yuri Pankov <yuri.pankov@nexenta.com>

6 years ago8930 zfs_zinactive: do not remove the node if the filesystem is readonly
Alexander Motin [Sun, 21 Jan 2018 23:42:45 +0000 (23:42 +0000)]
8930 zfs_zinactive: do not remove the node if the filesystem is readonly

illumos/illumos-gate@93c618e0f4932dc0bb9a9c90d8c4a5d029de5797

https://www.illumos.org/issues/8930:
We normally remove an unlinked node when its last user goes away and the
node becomes inactive. However, we should not do that if the filesystem
is mounted read-only including the case where it has its readonly
property set. The node will remain on the unlinked queue, so it will
not be leaked.

One particular scenario is when we receive an incremental stream into a
mounted read-only filesystem and that stream contains an unlinked file
(still on the unlinked queue). If that file is opened before the
receive and some time later after the receive it becomes inactive we
would remove it and, thus, modify the read-only filesystem. As a
result, the filesystem would diverge from its source and further
incremental receives would not be possible (without forcing a rollback).

Another related scenario, that may or may not be possible depending on an
OS / VFS policy, is when an open file is unlinked, then the filesystem is
remounted read-only, and then the file is closed.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Gordon Ross <gwr@nexenta.com>
Author: Andriy Gapon <avg@FreeBSD.org>

6 years ago8909 8585 can cause a use-after-free kernel panic
Alexander Motin [Sun, 21 Jan 2018 23:12:38 +0000 (23:12 +0000)]
8909 8585 can cause a use-after-free kernel panic

illumos/illumos-gate@94ddd0900a8838f62bba15e270649a42f4ef9f81

https://www.illumos.org/issues/8909:
There's a race condition that exists if `zil_free_lwb` races with either
`zil_commit_waiter_timeout` and/or `zil_lwb_flush_vdevs_done`.

Here's an example panic due to this bug:

> ::status
    debugging crash dump vmcore.0 (64-bit) from ip-10-110-205-40
    operating system: 5.11 dlpx-5.2.2.0_2017-12-04-17-28-32b6ba51fb (i86pc)
    image uuid: 4af0edfb-e58e-6ed8-cafc-d3e9167c7513
    panic message:
    BAD TRAP: type=e (#pf Page fault) rp=ffffff0010555970 addr=60 occurred in module "zfs" due to a NULL pointer dereference
    dump content: kernel pages only

> $c
    zio_shrink+0x12()
    zil_lwb_write_issue+0x30d(ffffff03dcd15cc0ffffff03e0730e20)
    zil_commit_waiter_timeout+0xa2(ffffff03dcd15cc0ffffff03d97ffcf8)
    zil_commit_waiter+0xf3(ffffff03dcd15cc0ffffff03d97ffcf8)
    zil_commit+0x80(ffffff03dcd15cc0, 9a9)
    zfs_write+0xc34(ffffff03dc38b140ffffff0010555e60, 40, ffffff03e00fb758, 0)
    fop_write+0x5b(ffffff03dc38b140ffffff0010555e60, 40, ffffff03e00fb758, 0)
    write+0x250(42, fffffd7ff4832000, 2000)
    sys_syscall+0x177()

If there's an outstanding lwb that's in `zil_commit_waiter_timeout`
waiting to timeout, waiting on it's waiter's CV, we must be sure not to
call `zil_free_lwb`. If we end up calling `zil_free_lwb`, then that LWB
may be freed and can result in a use-after-free situation where the
stale lwb pointer stored in the `zil_commit_waiter_t` structure of the
thread waiting on the waiter's CV is used.

A similar situation can occur if an lwb is issued to disk, and thus in
the `LWB_STATE_ISSUED` state, and `zil_free_lwb` is called while the
disk is servicing that lwb. In this situation, the lwb will be freed by
`zil_free_lwb`, which will result in a use-after-free situation when the
lwb's zio completes, and `zil_lwb_flush_vdevs_done` is called.

This race condition is prevented in `zil_close` by calling `zil_commit`
before `zil_free_lwb` is called, which will ensure all outstanding (i.e.
all lwb's in the `LWB_STATE_OPEN` and/or `LWB_STATE_ISSUED` states)
reach the `LWB_STATE_DONE` state before the lwb's are freed
(`zil_commit` will not return untill all the lwb's are
`LWB_STATE_DONE`).

Further, this race condition is prevented in `zil_sync` by only calling
`zil_free_lwb` for lwb's that do not have their `lwb_buf` pointer set.
All lwb's not in the `LWB_STATE_DONE` state will have a non-null value
for this pointer; the pointer is only cleared in
`zil_lwb_flush_vdevs_done`, at which point the lwb's state will be
changed to `LWB_STATE_DONE`.

This race is present in `zil_suspend`, leading to this bug.

At first glance, it would appear as though this would not be true
because `zil_suspend` will call `zil_commit`, just like `zil_close`, but
the problem is that `zil_suspend` will set the zilog's `zl_suspend`
field prior to calling `zil_commit`. Further, in `zil_commit`, if
`zl_suspend` is set, `zil_commit` will take a special branch of logic
and use `txg_wait_synced` instead of performing the normal `zil_commit`
logic.

This call to `txg_wait_synced` might be good enough for the data to
reach disk safely before it returns, but it does not ensure that all
outstanding lwb's reach the `LWB_STATE_DONE` state before it returns.
This is because, if there's an lwb "stuck" in
`zil_commit_waiter_timeout`, waiting for it's lwb to timeout, it will
maintain a non-null value for it's `lwb_buf` field and thus `zil_sync`
will not free that lwb. Thus, even though the lwb's data is already on
disk, the lwb will be left lingering, waiting on the CV, and will
eventually timeout and be issued to disk even though the write is
unnesseary.

So, after `zil_commit` is called from `zil_suspend`, we incorrectly
assume that there are not outstanding lwb's, and proceed to free all
lwb's found on the zilog's lwb list. As a result, we free the lwb that
will later be used `zil_commit_waiter_timeout`.

Reviewed by: John Kennedy <jwk404@gmail.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Igor Kozhukhov <igor@dilos.org>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Prakash Surya <prakash.surya@delphix.com>

6 years ago8603 rename zilog's "zl_writer_lock" to "zl_issuer_lock"
Alexander Motin [Sun, 21 Jan 2018 23:04:25 +0000 (23:04 +0000)]
8603 rename zilog's "zl_writer_lock" to "zl_issuer_lock"

illumos/illumos-gate@cf07d3da9915c0d22da8f59e991639f819463cef

https://www.illumos.org/issues/8603:
  To help make the ZIL's code more understandable, it was suggested that
  the zilog_t's "zl_writer_lock" field should be renamed to "zl_issuer_lock".

6 years ago8677 Open-Context Channel Programs
Alexander Motin [Sun, 21 Jan 2018 19:26:38 +0000 (19:26 +0000)]
8677 Open-Context Channel Programs

illumos/illumos-gate@a3b2868063897ff0083dea538f55f9873eec981f

https://www.illumos.org/issues/8677
  We want to be able to run channel programs outside of synching context.
  This would greatly improve performance of channel program that just gather
  information, as we won't have to wait for synching context anymore.

  This feature should introduce the following:
  - A new command line flag in "zfs program" to specify our intention to
  run in open context.
  - A new flag/option within the channel program ioctl which selects the
  context.
  - Appropriate error handling whenever we try a channel program in
  open-context that contains zfs.sync* expressions.
  - Documentation for the new feature in the manual pages.

Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: Chris Williamson <chris.williamson@delphix.com>
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Serapheim Dimitropoulos <serapheim@delphix.com>

6 years ago8880 improve DTrace error checking
Mark Johnston [Tue, 12 Dec 2017 00:51:39 +0000 (00:51 +0000)]
8880 improve DTrace error checking

illumos/illumos-gate@2cf374268f3e1c9e9be6367466b183d27632583a
https://github.com/illumos/illumos-gate/commit/2cf374268f3e1c9e9be6367466b183d27632583a

https://www.illumos.org/issues/8880

Reviewed by: Tim Kordas <tim.kordas@joyent.com>
Reviewed by: Bryan Cantrill <bryan@joyent.com>
Reviewed by: Richard Lowe <richlowe@richlowe.net>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Jerry Jelinek <jerry.jelinek@joyent.com>

6 years ago7531 Assign correct flags to prefetched buffers
Andriy Gapon [Thu, 9 Nov 2017 18:21:17 +0000 (18:21 +0000)]
7531 Assign correct flags to prefetched buffers

illumos/illumos-gate@272952165423c254ad7708f1b3fe2ff0a6ce408b
https://github.com/illumos/illumos-gate/commit/272952165423c254ad7708f1b3fe2ff0a6ce408b

https://www.illumos.org/issues/7531
  I found that some buffers that could be L2ARC eligible are not flagged
  such, leading to some performance impact.  As a test I ran the same IO
  workload 10 times in a raw.  It is a metadata only workload (files
  listing).  l2arc_noprefetch=0.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Prakash Surya <prakash.surya@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: benrubson <ben.rubson@gmail.com>

MFC after: 8 days

6 years ago8607 zfs: variable set but not used
Andriy Gapon [Thu, 9 Nov 2017 18:13:26 +0000 (18:13 +0000)]
8607 zfs: variable set but not used

illumos/illumos-gate@b852c2f54326f8ac1daa372a88bfe951dd7e20ed
https://github.com/illumos/illumos-gate/commit/b852c2f54326f8ac1daa372a88bfe951dd7e20ed

https://www.illumos.org/issues/8607

Reviewed by: Yuri Pankov <yuripv@gmx.com>
Reviewed by: Igor Kozhukhov <igor@dilos.org>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Toomas Soome <tsoome@me.com>

6 years ago8713 Buffer overflow in dsl_dataset_name()
Andriy Gapon [Thu, 9 Nov 2017 18:06:18 +0000 (18:06 +0000)]
8713 Buffer overflow in dsl_dataset_name()

illumos/illumos-gate@f37ae9a714b97eca91c74c680c20c750c7cf5c02
https://github.com/illumos/illumos-gate/commit/f37ae9a714b97eca91c74c680c20c750c7cf5c02

https://www.illumos.org/issues/8713
  If we're creating a pool with version >= SPA_VERSION_DSL_SCRUB (v11) we need to
  account for additional space needed by the origin dataset which will also be
  snapshotted: "poolname"+"/"+"$ORIGIN"+"@"+"$ORIGIN".
  Enforce this limit in pool_namecheck().

Reviewed by: Prakash Surya <prakash.surya@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: loli10K <ezomori.nozomu@gmail.com>

6 years agofollow up to r325013, add libcmdutils.h to the vendor area
Andriy Gapon [Fri, 27 Oct 2017 11:26:36 +0000 (11:26 +0000)]
follow up to r325013, add libcmdutils.h to the vendor area

6 years ago640 number_to_scaled_string is duplicated in several commands
Andriy Gapon [Thu, 26 Oct 2017 16:20:47 +0000 (16:20 +0000)]
640 number_to_scaled_string is duplicated in several commands

illumos/illumos-gate@0a0551200ecbcd4f1b17560acaeeb7b6c8b0090e
https://github.com/illumos/illumos-gate/commit/0a0551200ecbcd4f1b17560acaeeb7b6c8b0090e

https://www.illumos.org/issues/640
  du(1), df(1m), ls(1), and swap(1m) all include a copy (it appears literally
  copied) of the 'number_to_scaled_string' function in their source. This should
  be moved to a shared library and all 4 commands should use this instead.

Reviewed by: Sebastian Wiedenroth <wiedi@frubar.net>
Reviewed by: Robert Mustacchi <rm@joyent.com>
Reviewed by: Yuri Pankov <yuripv@gmx.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Jason King <jason.brian.king@gmail.com>

6 years ago8081 Compiler warnings in zdb
Andriy Gapon [Mon, 2 Oct 2017 11:55:11 +0000 (11:55 +0000)]
8081 Compiler warnings in zdb

illumos/illumos-gate@3f7978d02b206a6ebc5652c91aa9f42da6fbe00c
https://github.com/illumos/illumos-gate/commit/3f7978d02b206a6ebc5652c91aa9f42da6fbe00c

https://www.illumos.org/issues/8081
  zdb(8) is full of minor problems that generate compiler warnings. On FreeBSD,
  which uses -WError, the only way to build it is to disable all compiler
  warnings. This makes it much harder to detect newly introduced bugs. We should
  cleanup all the warnings.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Prakash Surya <prakash.surya@delphix.com>
Approved by: Richard Lowe <richlowe@richlowe.net>
Author: Alan Somers <asomers@gmail.com>

6 years ago8648 Fix range locking in ZIL commit codepath
Andriy Gapon [Fri, 22 Sep 2017 08:23:24 +0000 (08:23 +0000)]
8648 Fix range locking in ZIL commit codepath

illumos/illumos-gate@42b14111721da2ebd5159e7b45012a3eb0e3384c
https://github.com/illumos/illumos-gate/commit/42b14111721da2ebd5159e7b45012a3eb0e3384c

https://www.illumos.org/issues/8648
  I'm opening this bug to track integration of the following ZFS on Linux
  commit into illumos:

  commit f763c3d1df569a8d6b60bcb5e95cf07aa7a189e6
  Author: LOLi <loli10K@users.noreply.github.com>
  Date:   Mon Aug 21 17:59:48 2017 +0200

      Fix range locking in ZIL commit codepath

      Since OpenZFS 7578 (1b7c1e5) if we have a ZVOL with logbias=throughput
      we will force WR_INDIRECT itxs in zvol_log_write() setting itx->itx_lr
      offset and length to the offset and length of the BIO from
      zvol_write()->zvol_log_write(): these offset and length are later used
      to take a range lock in zillog->zl_get_data function: zvol_get_data().

      Now suppose we have a ZVOL with blocksize=8K and push 4K writes to
      offset 0: we will only be range-locking 0-4096. This means the
      ASSERTion we make in dbuf_unoverride() is no longer valid because now
      dmu_sync() is called from zilog's get_data functions holding a partial
      lock on the dbuf.

      Fix this by taking a range lock on the whole block in zvol_get_data().

Reviewed-by: Chunwei Chen <tuxoko@gmail.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: loli10K <ezomori.nozomu@gmail.com>
Reviewed by: Igor Kozhukhov <igor@dilos.org>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: Andriy Gapon <avg@FreeBSD.org>
Reviewed by: Alexander Motin <mav@FreeBSD.org>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: LOLi <loli10K@users.noreply.github.com>

6 years ago8661 remove "zil-cw2" dtrace probe
Andriy Gapon [Fri, 22 Sep 2017 08:18:49 +0000 (08:18 +0000)]
8661 remove "zil-cw2" dtrace probe

illumos/illumos-gate@bd9d3f904625846bdc61af8897a1072029c7aeb7
https://github.com/illumos/illumos-gate/commit/bd9d3f904625846bdc61af8897a1072029c7aeb7

https://www.illumos.org/issues/8661
  The "zil-cw1" dtrace probe was previously removed in 8558, and the "zil-cw2"
  probe should have been removed in that patch as well. Unfortunately, the "zil-
  cw2" was not removed in 8558, so this bug is to track it's removal.

Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Igor Kozhukhov <igor@dilos.org>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Prakash Surya <prakash.surya@delphix.com>

6 years ago8600 ZFS channel programs - snapshot
Andriy Gapon [Fri, 22 Sep 2017 08:18:05 +0000 (08:18 +0000)]
8600 ZFS channel programs - snapshot

illumos/illumos-gate@2840dce1a029098fb784afd951d5f98089f850d8
https://github.com/illumos/illumos-gate/commit/2840dce1a029098fb784afd951d5f98089f850d8

https://www.illumos.org/issues/8600
  ZFS channel programs should be able to create snapshots.
  In addition to the base snapshot functionality, this will likely entail adding
  extra logic to handle edge cases which were formerly not possible, such as
  creating then destroying a snapshot in the same transaction sync.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: John Kennedy <john.kennedy@delphix.com>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Chris Williamson <chris.williamson@delphix.com>

6 years ago8592 ZFS channel programs - rollback
Andriy Gapon [Fri, 22 Sep 2017 08:15:35 +0000 (08:15 +0000)]
8592 ZFS channel programs - rollback

illumos/illumos-gate@000cce6b6fad4a8b0eecef6e1251f6aca1719c55
https://github.com/illumos/illumos-gate/commit/000cce6b6fad4a8b0eecef6e1251f6aca1719c55

https://www.illumos.org/issues/8592
  ZFS channel programs should be able to perform a rollback. This logic will
  probably look pretty similar to zfs.sync.destroy().

Reviewed by: Chris Williamson <chris.williamson@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Brad Lewis <brad.lewis@delphix.com>

6 years ago8502 illumos#7955 broke delegated datasets when libshare is not present
Andriy Gapon [Fri, 22 Sep 2017 08:13:09 +0000 (08:13 +0000)]
8502 illumos#7955 broke delegated datasets when libshare is not present

illumos/illumos-gate@1c18e8fbd8db41a9fb39bd3ef7a18ee71ece20da
https://github.com/illumos/illumos-gate/commit/1c18e8fbd8db41a9fb39bd3ef7a18ee71ece20da

https://www.illumos.org/issues/8502
  The code in lib/libzfs/common/libzfs_mount.c already basically handles
  the case when libshare is not installed. We just need to not fail in
  zfs_init_libshare_impl.  I tested this in lx and things work as
  expected. I also tested there trying to set sharenfs and sharesmb on
  the delegated dataset. Neither is allowed from within a zone.  The
  spew of msgs from a native zone is not ZFS specific. I see the same
  spew simply running the share command.

Reviewed by: Robert Mustacchi <rm@joyent.com>
Reviewed by: Yuri Pankov <yuripv@gmx.com>
Approved by: Richard Lowe <richlowe@richlowe.net>
Author: Jerry Jelinek <jerry.jelinek@joyent.com>

6 years ago8604 Avoid unnecessary work search in VFS when unmounting snapshots
Andriy Gapon [Wed, 20 Sep 2017 07:28:18 +0000 (07:28 +0000)]
8604 Avoid unnecessary work search in VFS when unmounting snapshots

illumos/illumos-gate@ed992b0aac4e5b70dc1273b1d055c0d471fbb4b1
https://github.com/illumos/illumos-gate/commit/ed992b0aac4e5b70dc1273b1d055c0d471fbb4b1

https://www.illumos.org/issues/8604
  Every time we want to unmount a snapshot (happens during snapshot deletion or
  renaming) we unnecessarily iterate through all the mountpoints in the VFS layer
  (see zfs_get_vfs).
  Ideally we would just put a hold on the snapshot and access its respective VFS
  resource directly.
  gwilson_snap_unmount.svg - Flamegraph indicating the issue discussed (138 KB)
  Serapheim Dimitropoulos, 2017-09-14 06:36 PM

Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Andy Stormont <astormont@racktopsystems.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Serapheim Dimitropoulos <serapheim@delphix.com>

6 years ago8605 zfs channel programs: zfs.exists undocumented and non-working
Andriy Gapon [Wed, 20 Sep 2017 07:27:45 +0000 (07:27 +0000)]
8605 zfs channel programs: zfs.exists undocumented and non-working

illumos/illumos-gate@5f39f884e2035d671ec02148fc4d8420c670bcb4
https://github.com/illumos/illumos-gate/commit/5f39f884e2035d671ec02148fc4d8420c670bcb4

https://www.illumos.org/issues/8605
  zfs.exists() in channel programs doesn't return any result, and should have a
  man page entry.

Reviewed by: Paul Dagnelie <pcd@delphix.com>
Reviewed by: Dan Kimmel <dan.kimmel@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Chris Williamson <chris.williamson@delphix.com>

6 years ago8602 remove unused "dp_early_sync_tasks" field from "dsl_pool" structure
Andriy Gapon [Wed, 20 Sep 2017 07:24:57 +0000 (07:24 +0000)]
8602 remove unused "dp_early_sync_tasks" field from "dsl_pool" structure

illumos/illumos-gate@2bcb5458541cc6e8bf7dc541303da29297b82e8b
https://github.com/illumos/illumos-gate/commit/2bcb5458541cc6e8bf7dc541303da29297b82e8b

https://www.illumos.org/issues/8602
  When I landed the fix for 8558, I incorrectly added the "dp_early_sync_tasks"
  field to the "dsl_pool" structure. This field is used in DelphixOS, but not in
  illumos. It was incorrectly pulled into illumos, so this bug is to remove it
  from the structure.

Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Prakash Surya <prakash.surya@delphix.com>

6 years ago8567 Inconsistent return value in zpool_read_label
Andriy Gapon [Wed, 20 Sep 2017 07:18:09 +0000 (07:18 +0000)]
8567 Inconsistent return value in zpool_read_label

illumos/illumos-gate@c861bfbd77c4ae780a0341e9cb6926d8b74341cf
https://github.com/illumos/illumos-gate/commit/c861bfbd77c4ae780a0341e9cb6926d8b74341cf

https://www.illumos.org/issues/8567
  If fstat64 fails, pread64 fails, or the label is unintelligible,
  zpool_read_label will return 0. But if malloc fails, it will return -1. For
  consistency, it should always return -1 on failure or 0 on success.

Reviewed by: Prakash Surya <prakash.surya@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Alan Somers <asomers@gmail.com>

6 years ago8473 scrub does not detect errors on active spares
Andriy Gapon [Wed, 20 Sep 2017 06:34:48 +0000 (06:34 +0000)]
8473 scrub does not detect errors on active spares

illumos/illumos-gate@554675eee75dd2d7398d960aa5c81083ceb8505a
https://github.com/illumos/illumos-gate/commit/554675eee75dd2d7398d960aa5c81083ceb8505a

https://www.illumos.org/issues/8473
  Scrubbing is supposed to detect and repair all errors in the pool. However, it
  wrongly ignores active spare devices. The problem can easily be reproduced in
  OpenZFS at git rev 0ef125d with these commands:

  truncate -s 64m /tmp/a /tmp/b /tmp/c
  sudo zpool create testpool mirror /tmp/a /tmp/b spare /tmp/c
  sudo zpool replace testpool /tmp/a /tmp/c
  /bin/dd if=/dev/zero bs=1024k count=63 oseek=1 conv=notrunc of=/tmp/c
  sync
  sudo zpool scrub testpool
  zpool status testpool # Will show 0 errors, which is wrong
  sudo zpool offline testpool /tmp/a
  sudo zpool scrub testpool
  zpool status testpool # Will show errors on /tmp/c, which should've already been fixed

  FreeBSD head is partially affected: the first scrub will detect some errors,
  but the second scrub will detect more.

Reviewed by: Andy Stormont <astormont@racktopsystems.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Approved by: Richard Lowe <richlowe@richlowe.net>
Author: Alan Somers <asomers@gmail.com>

6 years ago8585 improve batching done in zil_commit()
Andriy Gapon [Wed, 13 Sep 2017 10:59:36 +0000 (10:59 +0000)]
8585 improve batching done in zil_commit()

illumos/illumos-gate@1271e4b10dfaaed576c08a812f466f6e81370e5e
https://github.com/illumos/illumos-gate/commit/1271e4b10dfaaed576c08a812f466f6e81370e5e

https://www.illumos.org/issues/8585
  The current implementation of zil_commit() can introduce significant
  latency, beyond what is inherent due to the latency of the underlying
  storage. The additional latency comes from two main problems:
  1. When there's outstanding ZIL blocks being written (i.e. there's
      already a "writer thread" in progress), then any new calls to
      zil_commit() will block waiting for the currently oustanding ZIL
      blocks to complete. The blocks written for each "writer thread" is
      coined a "batch", and there can only ever be a single "batch" being
      written at a time. When a batch is being written, any new ZIL
      transactions will have to wait for the next batch to be written,
      which won't occur until the current batch finishes.
  As a result, the underlying storage may not be used as efficiently
      as possible. While "new" threads enter zil_commit() and are blocked
      waiting for the next batch, it's possible that the underlying
      storage isn't fully utilized by the current batch of ZIL blocks. In
      that case, it'd be better to allow these new threads to generate
      (and issue) a new ZIL block, such that it could be serviced by the
      underlying storage concurrently with the other ZIL blocks that are
      being serviced.
  2. Any call to zil_commit() must wait for all ZIL blocks in its "batch"
      to complete, prior to zil_commit() returning. The size of any given
      batch is proportional to the number of ZIL transaction in the queue
      at the time that the batch starts processing the queue; which
      doesn't occur until the previous batch completes. Thus, if there's a
      lot of transactions in the queue, the batch could be composed of
      many ZIL blocks, and each call to zil_commit() will have to wait for
      all of these writes to complete (even if the thread calling
      zil_commit() only cared about one of the transactions in the batch).

Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Prakash Surya <prakash.surya@delphix.com>

6 years ago8590 memory leak in dsl_destroy_snapshots_nvl()
Andriy Gapon [Wed, 13 Sep 2017 10:57:52 +0000 (10:57 +0000)]
8590 memory leak in dsl_destroy_snapshots_nvl()

illumos/illumos-gate@e6ab4525d156c82445c116ecf6b2b874d5e9009d
https://github.com/illumos/illumos-gate/commit/e6ab4525d156c82445c116ecf6b2b874d5e9009d

https://www.illumos.org/issues/8590
  In dsl_destroy_snapshots_nvl(), "snaps_normalized" is not freed after it is
  added to "arg".

Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: Steve Gonczi <steve.gonczi@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Matthew Ahrens <mahrens@delphix.com>

6 years ago8552 ZFS LUA code uses floating point math
Andriy Gapon [Wed, 13 Sep 2017 10:56:19 +0000 (10:56 +0000)]
8552 ZFS LUA code uses floating point math

illumos/illumos-gate@916c8d881190bd2c3ca20d9fca919aecff504435
https://github.com/illumos/illumos-gate/commit/916c8d881190bd2c3ca20d9fca919aecff504435

https://www.illumos.org/issues/8552
  In the LUA interpreter used by "zfs program", the lua format() function
  accidentally includes support for '%f' and friends, which can cause compilation
  problems when building on platforms that don't support floating-point math in
  the kernel (e.g. sparc). Support for '%f' friends (%f %e %E %g %G) should be
  removed, since there's no way to supply a floating-point value anyway (all
  numbers in ZFS LUA are int64_t's).

Reviewed by: Yuri Pankov <yuripv@gmx.com>
Reviewed by: Igor Kozhukhov <igor@dilos.org>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Matthew Ahrens <mahrens@delphix.com>

6 years ago8521 nvlist memory leak in get_clones_stat() and spa_load_best()
Andriy Gapon [Wed, 13 Sep 2017 10:54:56 +0000 (10:54 +0000)]
8521 nvlist memory leak in get_clones_stat() and spa_load_best()

illumos/illumos-gate@7d3000f774e20097a1ee45cbd06d0e38065ddd5a
https://github.com/illumos/illumos-gate/commit/7d3000f774e20097a1ee45cbd06d0e38065ddd5a

https://www.illumos.org/issues/8521
  Yuri reported this to the mailing list:
  doing a `reboot -d` on current illumos-gate HEAD gives the following "::
  findleaks -dv" output:
  findleaks: maximum buffers => 301061
  findleaks: actual buffers => 297587
  findleaks:
  findleaks: potential pointers => 29289774
  findleaks: dismissals => 26242305 (89.5%)
  findleaks: misses => 331153 ( 1.1%)
  findleaks: dups => 2419681 ( 8.2%)
  findleaks: follows => 296635 ( 1.0%)
  findleaks:
  findleaks: peak memory usage => 7353 kB
  findleaks: elapsed CPU time => 1.5 seconds
  findleaks: elapsed wall time => 2.0 seconds
  findleaks:
  CACHE LEAKED BUFCTL CALLER
  ffffff03d222b008 120 ffffff03ef7ceb78 nv_alloc_sys+0x1f
  ffffff03d222a448 123 ffffff03f4150cc8 nv_alloc_sys+0x1f
  ffffff03d222b448 5 ffffff03f28bd598 nv_alloc_sys+0x1f
  ffffff03d222b888 87 ffffff03f28c10f0 nv_alloc_sys+0x1f
  ffffff03d222c008 21 ffffff03f4139310 nv_alloc_sys+0x1f
  ffffff03d222b888 43 ffffff040ef3f3e8 nv_alloc_sys+0x1f
  ffffff03d222c008 120 ffffff03f4591e58 nv_alloc_sys+0x1f
  ffffff03d222b008 121 ffffff03f352c068 nv_alloc_sys+0x1f
  ffffff03d222a448 112 ffffff03f414e5f8 nv_alloc_sys+0x1f
  ffffff03d222b008 119 ffffff03ee92fdc0 nv_alloc_sys+0x1f
  ffffff03d222b888 46 ffffff03f28c1378 nv_alloc_sys+0x1f
  ffffff03d222b448 4 ffffff03f28c7708 nv_alloc_sys+0x1f
  ffffff03d222c008 20 ffffff03f2a6e7e8 nv_alloc_sys+0x1f

Reviewed by: Steve Gonczi <steve.gonczi@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Yuri Pankov <yuripv@gmx.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Pavel Zakharov <pavel.zakharov@delphix.com>

6 years ago7431 ZFS Channel Programs
Andriy Gapon [Wed, 13 Sep 2017 10:45:49 +0000 (10:45 +0000)]
7431 ZFS Channel Programs

illumos/illumos-gate@dfc115332c94a2f62058ac7f2bce7631fbd20b3d
https://github.com/illumos/illumos-gate/commit/dfc115332c94a2f62058ac7f2bce7631fbd20b3d

https://www.illumos.org/issues/7431
  ZFS channel programs (ZCP) adds support for performing compound ZFS
  administrative actions via Lua scripts in a sandboxed environment (with time
  and memory limits).
  This initial commit includes both base support for running ZCP scripts, and a
  small initial library of API calls which support getting properties and
  listing, destroying, and promoting datasets.
  Testing: in addition to the included unit tests, channel programs have been in
  use at Delphix for several months for batch destroying filesystems. The
  dsl_destroy_snaps_nvl() call has also been replaced with

  For reference, the new zfs-program manpage is included below.
  ZFS-PROGRAM(1M)                       1M                       ZFS-PROGRAM(1M)

  NAME
       zfs program – executes ZFS channel programs

  SYNOPSIS
       zfs program [-t timeout] [-m memory-limit] pool script

  DESCRIPTION
       The ZFS channel program interface allows ZFS administrative operations to
       be run programmatically as a Lua script. The entire script is executed
       atomically, with no other administrative operations taking effect
       concurrently. A library of ZFS calls is made available to channel program
       scripts. Channel programs may only be run with root privileges.

       A modified version of the Lua 5.2 interpreter is used to run channel
       program scripts. The Lua 5.2 manual can be found at:

             http://www.lua.org/manual/5.2/
  ...

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: John Kennedy <john.kennedy@delphix.com>
Reviewed by: Dan Kimmel <dan.kimmel@delphix.com>
Approved by: Garrett D'Amore <garrett@damore.org>
Author: Chris Williamson <chris.williamson@delphix.com>

6 years ago5115 Want Intel 40GbE NIC driver for illumos
Andriy Gapon [Wed, 13 Sep 2017 10:41:47 +0000 (10:41 +0000)]
5115 Want Intel 40GbE NIC driver for illumos

illumos/illumos-gate@9d26e4fc021e249c93c2861629cc665e4f5bd4d6
https://github.com/illumos/illumos-gate/commit/9d26e4fc021e249c93c2861629cc665e4f5bd4d6

https://www.illumos.org/issues/5115
  Intel's NICs based on the XL710 chipset exist 1 . There exist drivers for Linux
  and FreeBSD 2 .
  It does not appear to be derived from the ixgbe driver source, so it would
  probably require porting the i40e source from FBSD to Illumos, unless a driver
  exists for a GLDv3-like platform under CDDL or similar license (none are known
  to currently be available or being worked on).

Reviewed by: Dan McDonald <danmcd@omniti.com>
Reviewed by: Hans Rosenfeld <rosenfeld@grumpf.hope-2000.org>
Approved by: Richard Lowe <richlowe@richlowe.net>
Author: Robert Mustacchi <rm@joyent.com>

Note: this change has nothing to do with FreeBSD.

6 years ago5815 libzpool's panic function doesn't set global panicstr, ::status not as useful
Andriy Gapon [Wed, 13 Sep 2017 10:33:09 +0000 (10:33 +0000)]
5815 libzpool's panic function doesn't set global panicstr, ::status not as useful

illumos/illumos-gate@fae6347731c9d3f46b26338313b0422927f29cf6
https://github.com/illumos/illumos-gate/commit/fae6347731c9d3f46b26338313b0422927f29cf6

https://www.illumos.org/issues/5815
  When panic() is called from within ztest, the mdb ::status command isn't as
  useful as it could be since the global panicstr variable isn't updated. We
  should modify the function to make sure panicstr is set, so ::status can
  present the error message just like it does on a failed assertion.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Sebastien Roy <sebastien.roy@delphix.com>
Reviewed by: Gordon Ross <gordon.ross@nexenta.com>
Reviewed by: Rich Lowe <richlowe@richlowe.net>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Prakash Surya <prakash.surya@delphix.com>

6 years ago8376 cached v_path should be kept fresh
Andriy Gapon [Wed, 13 Sep 2017 10:25:44 +0000 (10:25 +0000)]
8376 cached v_path should be kept fresh

illumos/illumos-gate@e2fc3408efa6cdfc5e33c73c3567efc8c7592707
https://github.com/illumos/illumos-gate/commit/e2fc3408efa6cdfc5e33c73c3567efc8c7592707

https://www.illumos.org/issues/8376
  The logic for generating and maintaining the cached v_path value on vnodes
  could stand to be improved. If vnodes were purely ephemeral, then freshly
  calculating v_path at the time of lookup() would result in correct values (at a
  performance cost). When they persist, either as referenced by other structures
  (such as open files, process cwd, dnlc entries, etc), the opportunity for the
  v_path to become stale arises. This is exacerbated by the current behavior
  that, when v_path is found to be invalid (during a vnodetopath operation) will
  strive to recalculate it, but not preserve the result. The overall situation
  leads to both performance and correctness (due to lack of results) problems
  relating to v_path.
  This has been addressed in SmartOS through a series of changes. Firstly, to do
  proper invalidation of v_path when it's found to be stale:
  - OS-3891 stale v_path slows vfs lookups
  OS-3891 revealed that some logic made assumptions about v_path never
  transitioning from non-NULL to NULL. It was addressed here:
  - OS-4317 v_path accesses can race
  While the pathological stale v_path behavior had been addressed, there are
  still cases where the absence of valid v_path information was causing problems.
  The largest patch in this series addressed it by performing v_path checking and
  updates during vnode lookups/updates, when it is most convenient:
  - OS-5167 cached v_path should be kept fresh
  Two smaller updates are included too, to prevent erroneous behavior introduced
  by the prior changes:
  - OS-5846 procfs should follow VFS rules
  - OS-6134 vn_reinit balks on zeroed vnodes

Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
Reviewed by: Robert Mustacchi <rm@joyent.com>
Approved by: Gordon Ross <gwr@nexenta.com>
Author: Patrick Mooney <pmooney@pfmooney.com>

6 years ago8331 zfs_unshare returns wrong error code for smb unshare failure
Andriy Gapon [Wed, 13 Sep 2017 10:17:14 +0000 (10:17 +0000)]
8331 zfs_unshare returns wrong error code for smb unshare failure

illumos/illumos-gate@4f4378cc54b7deec3a35c529dc397dbdc325b4bb
https://github.com/illumos/illumos-gate/commit/4f4378cc54b7deec3a35c529dc397dbdc325b4bb

https://www.illumos.org/issues/8331
  zfs_unshare returns EZFS_UNSHARENFSFAILED on error for all share types.

Reviewed by: Marcel Telka <marcel@telka.sk>
Reviewed by: Toomas Soome <tsoome@me.com>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Andrew Stormont <astormont@racktopsystems.com>

7 years ago8569 problem with inline functions in abd.h
Andriy Gapon [Fri, 1 Sep 2017 18:02:53 +0000 (18:02 +0000)]
8569 problem with inline functions in abd.h

illumos/illumos-gate@37e84ab74e939caf52150fc3352081786ecc0c29
https://github.com/illumos/illumos-gate/commit/37e84ab74e939caf52150fc3352081786ecc0c29

https://www.illumos.org/issues/8569
  C [C99] has peculiar rules for inline functions that are different from the
  C++ rules.  Unlike C++ where inline is "fire and forget", in C a programmer
  must pay attention to the function's storage class / visibility.  The main
  problem is with the case where a compiler decides to not inline a call to the
  function declared as inline.
  Some relevant links:
  - http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka15831.html
  - http://www.drdobbs.com/the-new-c-inline-functions/184401540
  The summary is that either the inline functions should be declared 'static
  inline' or one of the compilation units (.c files) must provide a callable
  externally visible function definition.  In the former case, the compiler would
  automatically create a local non-inlined function instance in every compilation
  unit where it's needed.  In the latter case the single external definition is
  used to satisfy any non-inlined calls in all compilation units.  As things
  stand right now, we can get an undefined reference error under certain
  combinations of compilers and compiler options.  For example, this is what I
  get on FreeBSD when compiling with clang 4.0.0 and -O1:
    In function `abd_free': /usr/src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/abd.c:385:
    undefined reference to `abd_is_linear'
  So, there are two alternatives. Either to qualify each inline function in
  abd.h with static storage class.  Or to add declarations like the following to
  abd.c: extern inline boolean_t abd_is_linear(abd_t *abd); Both work. I am not
  sure which one would be more consistent with the illumos development rules.

Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Andriy Gapon <avg@FreeBSD.org>

7 years ago8558 lwp_create() returns EAGAIN on system with more than 80K ZFS filesystems
Andriy Gapon [Fri, 1 Sep 2017 17:57:51 +0000 (17:57 +0000)]
8558 lwp_create() returns EAGAIN on system with more than 80K ZFS filesystems

illumos/illumos-gate@216d7723a1a58124cf95c4950d51d5f99d3f4128
https://github.com/illumos/illumos-gate/commit/216d7723a1a58124cf95c4950d51d5f99d3f4128

https://www.illumos.org/issues/8558
  On a system with more than 80K ZFS filesystems, we've seen cases where
  lwp_create() will start to fail by returning EAGAIN. The problem being,
  for each of those 80K ZFS filesystems, a taskq will be created for each
  dataset as part of the ZIL for each dataset.
  For each of these taskq's, a kernel thread will be created which results
  in 24KB being allocated for each thread. With enough of these 24KB
  allocations, we eventually exhaust the memory region set aside for these
  allocations. Currently, segkpsize is set to a value of 2GB, which means
  we can only support about 80K filesystems; 2GB / 24KB = ~80K.
  The lwp_create() failure comes into play due to the fact that LWP
  creation also allocates 24KB from this same region of memory. Thus, if
  we've exhausted this region of memory due to the number of ZIL taskq's,
  there won't be any memory avaible to allow the call to lwp_create() to
  succeed.

Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Sebastien Roy <sebastien.roy@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Prakash Surya <prakash.surya@delphix.com>

7 years ago8435 zpool.1m and zfs.1m: minor cleanup
Andriy Gapon [Fri, 1 Sep 2017 17:56:34 +0000 (17:56 +0000)]
8435 zpool.1m and zfs.1m: minor cleanup

illumos/illumos-gate@a058d1cc571af5fbcfe7f1d719df1abbfdb722f3
https://github.com/illumos/illumos-gate/commit/a058d1cc571af5fbcfe7f1d719df1abbfdb722f3

https://www.illumos.org/issues/8435
  This commit is a result of re-read while porting zpool.1m to ZFSonLinux.
  See https://github.com/zfsonlinux/zfs/commit/
  cda0317e4d2a1277b328e4fc42ee3699bbe46c12

https://www.illumos.org/issues/3796
  The listsnapshots property is not documented in the zpool man page.

3796 listsnapshots not documented in zpool man page
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: Yuri Pankov <yuripv@gmx.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: George Melikov <mail@gmelikov.ru>

7 years ago8414 Implemented zpool scrub pause/resume
Andriy Gapon [Fri, 1 Sep 2017 17:43:08 +0000 (17:43 +0000)]
8414 Implemented zpool scrub pause/resume

illumos/illumos-gate@1702cce751c5cb7ead878d0205a6c90b027e3de8
https://github.com/illumos/illumos-gate/commit/1702cce751c5cb7ead878d0205a6c90b027e3de8

https://www.illumos.org/issues/8414
  This issue tracks the port of scrub pause from ZoL: https://github.com/zfsonlinux/zfs/pull/6167
  Currently, there is no way to pause a scrub. Pausing may be useful when
 the pool is busy with other I/O to preserve bandwidth.

  Description

  This patch adds the ability to pause and resume scrubbing.  This is achieved
  by maintaining a persistent on-disk scrub state.  While the state is 'paused'
  we do not scrub any more blocks.  We do however perform regular scan
  housekeeping such as freeing async destroyed and deadlist blocks while paused.

  If you're testing this change, you probably want to include the patch from #6164

  Motivation and Context

  Scrub pausing can be an I/O intensive operation and people have been asking
  for the ability to pause a scrub for a while. This allows one to preserve scrub
  progress while freeing up bandwidth for other I/O.

  How Has This Been Tested?

  Unit testing and zfs-tests.  to the pool.  This patch will also include the
  patch from https://github.com/zfsonlinux/zfs/ pull/6164 In certain cases
  (dsl_scan_sync() is one), we may end up calling

Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Alek Pinchuk <apinchuk@datto.com>

7 years ago8547 update mandoc to 1.14.3
Andriy Gapon [Fri, 1 Sep 2017 17:38:49 +0000 (17:38 +0000)]
8547 update mandoc to 1.14.3

illumos/illumos-gate@c66b8046543352459a11a51501b628d1c98a8c44
https://github.com/illumos/illumos-gate/commit/c66b8046543352459a11a51501b628d1c98a8c44

https://www.illumos.org/issues/8547
  update mandoc (now it's the official name) suite to new upstream version, which
  among a lot of fixes, brings in much improved eqn(5)/tbl(5) support.

Reviewed by: Robert Mustacchi <rm@joyent.com>
Approved by: Richard Lowe <richlowe@richlowe.net>
Author: Yuri Pankov <yuri.pankov@nexenta.com>

7 years ago8300 fix man page issues found by mandoc 1.14.1
Andriy Gapon [Fri, 1 Sep 2017 17:37:10 +0000 (17:37 +0000)]
8300 fix man page issues found by mandoc 1.14.1

illumos/illumos-gate@72d3dbb9ab4481606cb93caca98ba3b3a8eb6ce2
https://github.com/illumos/illumos-gate/commit/72d3dbb9ab4481606cb93caca98ba3b3a8eb6ce2

https://www.illumos.org/issues/8300
  Prior to integrating the mdocml update to 1.14.1, fix issues found by new
  version, especially the "new sentence, new line" style rule.

Reviewed by: Robert Mustacchi <rm@joyent.com>
Reviewed by: Toomas Soome <tsoome@me.com>
Approved by: Gordon Ross <gwr@nexenta.com>
Author: Yuri Pankov <yuri.pankov@nexenta.com>

7 years ago8138 Improve manpage spelling
Andriy Gapon [Fri, 1 Sep 2017 17:35:27 +0000 (17:35 +0000)]
8138 Improve manpage spelling

illumos/illumos-gate@bccbd30bb6d0c20635d3f23e8d63f3f8170d3c46
https://github.com/illumos/illumos-gate/commit/bccbd30bb6d0c20635d3f23e8d63f3f8170d3c46

https://www.illumos.org/issues/8138
  While reading man pages, I've noticed a number of spelling mistakes
  and simple typos we should fix.

Reviewed by: Toomas Soome <tsoome@me.com>
Reviewed by: Cody Mello <melloc@joyent.com>
Reviewed by: Patrick Mooney <patrick.mooney@joyent.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Peter Tribble <peter.tribble@gmail.com>

7 years ago8508 Mounting a zpool on 32-bit platforms panics
Andriy Gapon [Tue, 8 Aug 2017 11:27:19 +0000 (11:27 +0000)]
8508 Mounting a zpool on 32-bit platforms panics

illumos/illumos-gate@b11fe8c01471a5bff68e83e1fe5f809ad16b3be8
https://github.com/illumos/illumos-gate/commit/b11fe8c01471a5bff68e83e1fe5f809ad16b3be8

https://www.illumos.org/issues/8508
  Mounting a zpool on a 32-bit system triggers a panic in spa_history_log_version
  () due to a type format mismatch for ZPL_VERSION. ZPL_VERSION is a unsigned
  long long, but the format expects an integer. On 64-bit platforms this may not
  be an issue due to word size and alignment. On 32-bit platforms a word size is
  half that of a long long, causing the second word of the long long to be seen
  as the string pointer for utsname.nodename.

Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Justin Hibbits <chmeeedalf@gmail.com>

7 years ago8373 TXG_WAIT in ZIL commit path
Andriy Gapon [Tue, 8 Aug 2017 11:24:13 +0000 (11:24 +0000)]
8373 TXG_WAIT in ZIL commit path

illumos/illumos-gate@d28671a3b094af696bea87f52272d4c4d89321c7
https://github.com/illumos/illumos-gate/commit/d28671a3b094af696bea87f52272d4c4d89321c7

https://www.illumos.org/issues/8373
  The code that writes ZIL blocks uses dmu_tx_assign(TXG_WAIT) to assign a
  transaction to a transaction group.
  That seems to be logically incorrect as writing of the ZIL block does not
  introduce any new dirty data.
  Also, when there is a lot of dirty data, the call can introduce significant
  delays into the ZIL commit path,
  thus affecting all synchronous writes. Additionally, ARC throttling may affect
  the ZIL writing.
  We probably need a new mechanism similar to dmu_tx_create_assigned to assign
  ZIL transactions.
  (Ab)using TXG_WAITED does not seem to be sufficient.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Prakash Surya <prakash.surya@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Andriy Gapon <avg@FreeBSD.org>

7 years ago8491 uberblock on-disk padding to reserve space for smoothly merging zpool checkpoint...
Andriy Gapon [Tue, 8 Aug 2017 11:19:56 +0000 (11:19 +0000)]
8491 uberblock on-disk padding to reserve space for smoothly merging zpool checkpoint & MMP in ZFS

illumos/illumos-gate@79c2b812ee2010ebf20fdd92dc5f06b59000a94c
https://github.com/illumos/illumos-gate/commit/79c2b812ee2010ebf20fdd92dc5f06b59000a94c

https://www.illumos.org/issues/8491
  The zpool checkpoint feature in DxOS added a new field in the uberblock.
  The Multi-Modifier Protection Pull Request from ZoL adds two new fields in the
  uberblock (Reference: https://github.com/zfsonlinux/zfs/pull/6279).
  As these two changes come from two different sources and once upstreamed and
  deployed will introduce an incompatibility with each other we want
  to upstream a change that will reserve the padding for both of them so
  integration goes smoothly and everyone gets both features.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Olaf Faaland <faaland1@llnl.gov>
Approved by: Gordon Ross <gwr@nexenta.com>
Author: Serapheim Dimitropoulos <serapheim@delphix.com>

7 years ago7915 checks in l2arc_evict could use some cleaning up
Andriy Gapon [Tue, 8 Aug 2017 11:15:36 +0000 (11:15 +0000)]
7915 checks in l2arc_evict could use some cleaning up

illumos/illumos-gate@267ae6c3a88d2fc39276af66caafa978b0935b82
https://github.com/illumos/illumos-gate/commit/267ae6c3a88d2fc39276af66caafa978b0935b82

https://www.illumos.org/issues/7915
  l2arc_evict() is strictly serialized with respect to l2arc_write_buffers() and
  l2arc_write_done().
  Normally, l2arc_evict() and l2arc_write_buffers() are called from the same
  thread, so they can not be concurrent.
  Also, l2arc_write_buffers() uses zio_wait() on the parent zio of all cache zio-
  s.
  That ensures that l2arc_write_done() is completed before l2arc_write_buffers()
  returns.
  Finally, if a cache device is removed, then l2arc_evict() is called under
  SCL_ALL in the exclusive mode.
  That ensures that it can not be concurrent with the normal L2ARC accesses to
  the device (including writing and evicting buffers).
  Given the above, some checks and actions in l2arc_evict() do not make sense.
  For instance, it must never encounter the write head header let alone remove it
  from the buffer list.

Reviewed by: Dan Kimmel <dan.kimmel@delphix.com>
Reviewed by: Prakash Surya <prakash.surya@delphix.com>
Approved by: Matthew Ahrens <mahrens@delphix.com>
Author: Andriy Gapon <avg@FreeBSD.org>

7 years ago8126 ztest assertion failed in dbuf_dirty due to dn_nlevels changing
Andriy Gapon [Tue, 8 Aug 2017 11:13:27 +0000 (11:13 +0000)]
8126 ztest assertion failed in dbuf_dirty due to dn_nlevels changing

illumos/illumos-gate@dcb6872c565819ac88acbc2ece999ef241c8b982
https://github.com/illumos/illumos-gate/commit/dcb6872c565819ac88acbc2ece999ef241c8b982

https://www.illumos.org/issues/8126
  The sync thread is concurrently modifying dn_phys->dn_nlevels
  while dbuf_dirty() is trying to assert something about it, without
  holding the necessary lock. We need to move this assertion further down
  in the function, after we have acquired the dn_struct_rwlock.

Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Matthew Ahrens <mahrens@delphix.com>

7 years ago8067 zdb should be able to dump literal embedded block pointer
Andriy Gapon [Tue, 8 Aug 2017 11:10:37 +0000 (11:10 +0000)]
8067 zdb should be able to dump literal embedded block pointer

illumos/illumos-gate@4923c69fddc0887da5604a262585af3efd82ee20
https://github.com/illumos/illumos-gate/commit/4923c69fddc0887da5604a262585af3efd82ee20

https://www.illumos.org/issues/8067
  Add an option to zdb to print a literal embedded block pointer supplied on the
  command line:
  zdb -E [-A] word0:word1:...:word15

Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Alex Reece <alex@delphix.com>
Reviewed by: Yuri Pankov <yuri.pankov@gmail.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Matthew Ahrens <mahrens@delphix.com>

7 years ago8426 mark immutable buffer arguments as such in abd.h
Andriy Gapon [Tue, 8 Aug 2017 10:58:01 +0000 (10:58 +0000)]
8426 mark immutable buffer arguments as such in abd.h

illumos/illumos-gate@9b195260e22529ac0e2580faaf89402420589c1c
https://github.com/illumos/illumos-gate/commit/9b195260e22529ac0e2580faaf89402420589c1c

https://www.illumos.org/issues/8426
  abd_copy_from_buf and abd_cmp_buf do not modify their void *buf arguments, so
  qualify them with const.
  abd_copy_from_buf_off and abd_cmp_buf_off already had that type for the
  corresponding arguments.

Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Andriy Gapon <avg@FreeBSD.org>

7 years ago8430 dir_is_empty_readdir() doesn't properly handle error from fdopendir()
Andriy Gapon [Tue, 8 Aug 2017 10:55:42 +0000 (10:55 +0000)]
8430 dir_is_empty_readdir() doesn't properly handle error from fdopendir()

illumos/illumos-gate@ba6e7e6505150388de6dc6a88741164118a421bf
https://github.com/illumos/illumos-gate/commit/ba6e7e6505150388de6dc6a88741164118a421bf

https://www.illumos.org/issues/8430
  we should close dirfd if fdopendir() fails.

Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Dan Kimmel <dan.kimmel@delphix.com>
Reviewed by: Yuri Pankov <yuri.pankov@nexenta.com>
Reviewed by: Igor Kozhukhov <igor@dilos.org>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Sowrabha Gopal <sowrabha.gopal@delphix.com>

7 years ago7600 zfs rollback should pass target snapshot to kernel
Andriy Gapon [Tue, 8 Aug 2017 10:49:56 +0000 (10:49 +0000)]
7600 zfs rollback should pass target snapshot to kernel

illumos/illumos-gate@77b171372ed21642e04c873ef1e87fe2365520df
https://github.com/illumos/illumos-gate/commit/77b171372ed21642e04c873ef1e87fe2365520df

https://www.illumos.org/issues/7600
  At present, the kernel side code seems to blindly rollback to whatever happens
  to be the latest snapshot at the time when the rollback task is processed.
  The expected target's name should be passed to the kernel driver and the sync
  task should validate that the target exists and that it is the latest snapshot
  indeed.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Andriy Gapon <avg@FreeBSD.org>

7 years ago8377 Panic in bookmark deletion
Andriy Gapon [Tue, 8 Aug 2017 10:47:56 +0000 (10:47 +0000)]
8377 Panic in bookmark deletion

illumos/illumos-gate@42418f9e73f0d007aa87675ecc206c26fc8e073e
https://github.com/illumos/illumos-gate/commit/42418f9e73f0d007aa87675ecc206c26fc8e073e

https://www.illumos.org/issues/8377
  The problem is that when dsl_bookmark_destroy_check() is executed from open
  context (the pre-check), it fills in dbda_success based on the existence of the
  bookmark.
  But the bookmark (or containing filesystem as in this case) can be destroyed
  before we get to syncing context. When we re-run dsl_bookmark_destroy_check()
  in syncing
  context, it will not add the deleted bookmark to dbda_success, intending for
  dsl_bookmark_destroy_sync() to not process it. But because the bookmark is
  still in dbda_success
  from the open-context call, we do try to destroy it.
  The fix is that dsl_bookmark_destroy_check() should not modify dbda_success
  when called from open context.

Reviewed by: Paul Dagnelie <pcd@delphix.com>
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Matthew Ahrens <mahrens@delphix.com>

7 years ago8378 crash due to bp in-memory modification of nopwrite block
Andriy Gapon [Tue, 8 Aug 2017 10:44:48 +0000 (10:44 +0000)]
8378 crash due to bp in-memory modification of nopwrite block

illumos/illumos-gate@b7edcb940884114e61382937505433c4c38c0278
https://github.com/illumos/illumos-gate/commit/b7edcb940884114e61382937505433c4c38c0278

https://www.illumos.org/issues/8378
  The problem is that zfs_get_data() supplies a stale zgd_bp to dmu_sync(), which
  we then nopwrite against.
  zfs_get_data() doesn't hold any DMU-related locks, so after it copies db_blkptr
  to zgd_bp, dbuf_write_ready()
  could change db_blkptr, and dbuf_write_done() could remove the dirty record.
  dmu_sync() then sees the stale
  BP and that the dbuf it not dirty, so it is eligible for nop-writing.
  The fix is for dmu_sync() to copy db_blkptr to zgd_bp after acquiring the
  db_mtx. We could still see a stale
  db_blkptr, but if it is stale then the dirty record will still exist and thus
  we won't attempt to nopwrite.

Reviewed by: Prakash Surya <prakash.surya@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Matthew Ahrens <mahrens@delphix.com>

7 years ago7910 l2arc_write_buffers() may write beyond target_sz
Andriy Gapon [Tue, 8 Aug 2017 10:37:03 +0000 (10:37 +0000)]
7910 l2arc_write_buffers() may write beyond target_sz

illumos/illumos-gate@16a7e5ac116c85d965007a5f201104b564e82210
https://github.com/illumos/illumos-gate/commit/16a7e5ac116c85d965007a5f201104b564e82210

https://www.illumos.org/issues/7910
  It seems that the change in issue #6950 resurrected the problem that was
  earlier fixed by the change in issue #5219.
  Please also see the following FreeBSD bug report: https://bugs.freebsd.org/
  bugzilla/show_bug.cgi?id=216178

Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Dan Kimmel <dan.kimmel@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Andriy Gapon <avg@FreeBSD.org>

7 years ago8416 abd.h is not C++ friendly
Andriy Gapon [Tue, 8 Aug 2017 10:31:42 +0000 (10:31 +0000)]
8416 abd.h is not C++ friendly

illumos/illumos-gate@5e2a074725cb7c16ea1c6554da11ab4d6b4e7aee
https://github.com/illumos/illumos-gate/commit/5e2a074725cb7c16ea1c6554da11ab4d6b4e7aee

https://www.illumos.org/issues/8416
  A C++ compiler fails to compile abd_is_linear(), which is an inline function
  defined in abd.h, with the following error:
       error: cannot initialize return object of type 'boolean_t' with an
       rvalue of type 'bool'
  That happens because a bool can not be converted to an enum in C++.
  That's a problem because abd.h can be visible through other header files that a
  C++ program that works with ZFS can include.

Reviewed by: Igor Kozhukhov <igor@dilos.org>
Reviewed by: Dan Kimmel <dan.kimmel@delphix.com>
Reviewed by: Alek Pinchuk <pinchuk.alek@gmail.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Andriy Gapon <avg@FreeBSD.org>

7 years ago8418 zfs_prop_get_table() call in zfs_validate_name() is a no-op
Andriy Gapon [Tue, 8 Aug 2017 10:28:01 +0000 (10:28 +0000)]
8418 zfs_prop_get_table() call in zfs_validate_name() is a no-op

illumos/illumos-gate@e09ba01dcda5e24964b8632718777b39166d86e4
https://github.com/illumos/illumos-gate/commit/e09ba01dcda5e24964b8632718777b39166d86e4

https://www.illumos.org/issues/8418
  The following line in zfs_validate_name() is just a no-op and it should be
  removed:
  108    (void) zfs_prop_get_table();

Reviewed by: Vitaliy Gusev <gusev.vitaliy@icloud.com>
Approved by: Matthew Ahrens <mahrens@delphix.com>
Author: Marcel Telka <marcel@telka.sk>

7 years ago8311 ZFS_READONLY is a little too strict
Andriy Gapon [Wed, 14 Jun 2017 16:46:49 +0000 (16:46 +0000)]
8311 ZFS_READONLY is a little too strict

illumos/illumos-gate@2889ec41c05e9ffe1890b529b3111354da325aeb
https://github.com/illumos/illumos-gate/commit/2889ec41c05e9ffe1890b529b3111354da325aeb

https://www.illumos.org/issues/8311
  Description:
  There was a misunderstanding about the enforcement details of the "Read-only"
  flag introduced for SMB/CIFS compatibility, way back in 2007 in the Sun PSARC
  2007/315 case.
  The original authors thought enforcement of the READONLY flag should work
  similarly as the IMMUTABLE flag. Unfortunately, that enforcement is
  incompatible with the expectations of Windows applications using this feature
  through the SMB service. Applications assume (and the MS File System Algorithms
  MS-FSA confirms they should) that an SMB client can:
  (a) Open an SMB handle on a file with read/write access,
  (b) Set the DOS attributes to include the READONLY flag,
  (c) continue to have write access via that handle.
  This access model is essentially the same as a Unix/POSIX application that
  creates a file (with read/write access), uses fchmod() to change the file mode
  to something not granting write access (i.e. 0444), and then continues to write
  that file using the open handle it got before the mode change.
  Currently, the SMB server works-around this problem in a way that will become
  difficult to maintain as we implement support for SMB3 persistent handles, so
  SMB depends on this fix.
  I've written a test program that can be used to demonstrate this problem, and
  added it to zfs-tests (tests/functional/acl/cifs/cifs_attr_004_pos).
  It currently fails, but will pass when this problem fixed.
  Steps to Reproduce:
    Run the test program on a ZFS file system.
  Expected Results:
    Pass
  Actual Results:
    Fail.

Reviewed by: Sanjay Nadkarni <sanjay.nadkarni@nexenta.com>
Reviewed by: Yuri Pankov <yuri.pankov@nexenta.com>
Reviewed by: Andrew Stormont <andyjstormont@gmail.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: John Kennedy <john.kennedy@delphix.com>
Approved by: Prakash Surya <prakash.surya@delphix.com>
Author: Gordon Ross <gwr@nexenta.com>

7 years ago5220 L2ARC does not support devices that do not provide 512B access
Andriy Gapon [Wed, 14 Jun 2017 16:44:10 +0000 (16:44 +0000)]
5220 L2ARC does not support devices that do not provide 512B access

illumos/illumos-gate@403a8da73c64ff9dfb6230ba045c765a242213fb
https://github.com/illumos/illumos-gate/commit/403a8da73c64ff9dfb6230ba045c765a242213fb

https://www.illumos.org/issues/5220
  There are disk devices that have logical sector size larger than 512B, for
  example 4KB. That is, their physical sector size is larger than 512B and they
  do not provide emulation for 512B sector sizes. For such devices both a data
  offset and a data size must be properly aligned. L2ARC should arrange that
  because it uses physical I/O.
  zio_vdev_io_start() performs a necessary transformation if io_size is not
  aligned to vdev_ashift, but that is done only for logical I/O. Something
  similar should be done in L2ARC code.
      * a temporary write buffer should be allocated if the original buffer is
        not going to be compressed and its size is not aligned
      * size of a temporary compression buffer should be ashift aligned
      * for the reads, if a size of a target buffer is not sufficiently large and
        it is not aligned then a temporary read buffer should be allocated

Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Dan Kimmel <dan.kimmel@delphix.com>
Reviewed by: Saso Kiselkov <saso.kiselkov@nexenta.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Andriy Gapon <avg@FreeBSD.org>

7 years ago5428 provide fts(), reallocarray(), and strtonum()
Andriy Gapon [Wed, 14 Jun 2017 16:36:01 +0000 (16:36 +0000)]
5428 provide fts(), reallocarray(), and strtonum()

illumos/illumos-gate@4585130b259133a26efae68275dbe56b08366deb
https://github.com/illumos/illumos-gate/commit/4585130b259133a26efae68275dbe56b08366deb

https://www.illumos.org/issues/5428

Reviewed by: Robert Mustacchi <rm@joyent.com>
Approved by: Joshua M. Clulow <josh@sysmgr.org>
Author: Yuri Pankov <yuri.pankov@nexenta.com>

7 years ago8264 want support for promoting datasets in libzfs_core
Andriy Gapon [Wed, 14 Jun 2017 16:27:54 +0000 (16:27 +0000)]
8264 want support for promoting datasets in libzfs_core

illumos/illumos-gate@a4b8c9aa65a0a735aba318024a424a90d7b06c37
https://github.com/illumos/illumos-gate/commit/a4b8c9aa65a0a735aba318024a424a90d7b06c37

https://www.illumos.org/issues/8264
  Oddly there is a lzc_clone function, but no lzc_promote function.

Reviewed by: Andriy Gapon <avg@FreeBSD.org>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Dan McDonald <danmcd@kebe.com>
Approved by: Dan McDonald <danmcd@kebe.com>
Author: Andrew Stormont <astormont@racktopsystems.com>