illumos/illumos-gate@6401734d546401734d54https://www.illumos.org/issues/6418
An easy, direct means of sanitizing pool vdevs can be helpful for management
purposes.
FreeBSD has had a 'zpool labelclear' for some time, see: https://
svnweb.freebsd.org/base?view=revision&revision=224171
SpectraBSD has a slightly updated version, which I propose for inclusion.
Reviewed by: Yuri Pankov <yuri.pankov@nexenta.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Will Andrews <will@firepipe.net>
Note: the bulk of the change has been already imported, this is a follow
up that imports zpool.1m changes.
illumos/illumos-gate@e4cb59f791e4cb59f791https://www.illumos.org/issues/6781
cache
A device used to cache storage pool data. A cache device cannot
be cannot be configured as a mirror or raidz group. For more
information, see the "Cache Devices" section.
needs changed to
cache
A device used to cache storage pool data. A cache device cannot
be configured as a mirror or raidz group. For more
information, see the "Cache Devices" section.
Reviewed by: Toomas Soome <tsoome@me.com>
Reviewed by: Robert Mustacchi <rm@joyent.com>
Approved by: Matthew Ahrens <mahrens@delphix.com>
Author: Alexander Pyhalov <apyhalov@gmail.com>
illumos/illumos-gate@879bece34e879bece34ehttps://www.illumos.org/issues/2897
Found this option in some Oracle documentation and wanted to check out the
zpool manpage on it in OI. Unfortunately it seems to be missing from the
manpage, so I first thought it was unsupported. However, "# zpool split" does
print the correct usage. Testing with the "-n" switch makes me believe it is
supported (I don't actually need to split my pool).
Reviewed by: Dan McDonald <danmcd@omniti.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Hans Rosenfeld <rosenfeld@grumpf.hope-2000.org>
Author: Steven Burgess <sburgess@datto.com>
5659 in the manual page for zpool(1M), one misuse of the word 'zpool' to describe a pool
illumos/illumos-gate@c8323d4323c8323d4323https://www.illumos.org/issues/4465
zpool(1M) is able to offline cache vdevs despite man page saying that it isn't:
zpool offline [-t] pool device ...
Takes the specified physical device offline. While the device is
offline, no attempt is made to read or write to the device.
This command is not applicable to spares or cache devices.
altair:root:~# zpool create testoff c9t67d0 cache c9t71d0
altair:root:~# zpool status testoff
pool: testoff
state: ONLINE
scan: none requested
config:
NAME STATE READ WRITE CKSUM
testoff ONLINE 0 0 0
c9t67d0 ONLINE 0 0 0
cache
c9t71d0 ONLINE 0 0 0
errors: No known data errors
altair:root:~# zpool offline testoff c9t71d0
altair:root:~# zpool status testoff
pool: testoff
state: ONLINE
status: One or more devices has been taken offline by the administrator.
Sufficient replicas exist for the pool to continue functioning in a
degraded state.
action: Online the device using 'zpool online' or replace the device with
'zpool replace'.
scan: none requested
https://www.illumos.org/issues/5659
At https://github.com/illumos/illumos-gate/blob/master/usr/src/man/man1m/
zpool.1m#L931
Do not add a disk that is currently configured as a quorum device to
a zpool.
– should be:
Do not add a disk that is currently configured as a quorum device to
a pool.
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Hans Rosenfeld <rosenfeld@grumpf.hope-2000.org>
Author: Yuri Pankov <yuri.pankov@nexenta.com>
illumos/illumos-gate@40713f2b2440713f2b24https://www.illumos.org/issues/8070
Add some ZFS comments left by various developers at different times
Reviewed by: Yuri Pankov <yuri.pankov@gmail.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Alan Somers <asomers@gmail.com>
illumos/illumos-gate@ade42b557aade42b557ahttps://www.illumos.org/issues/8064
It's currently nearly impossible to trace what process places a hold on
a vnode, as the only ways holds are place is via the `VN_HOLD()` and
`VN_HOLD_CALLER()` macros, which inline the bumping of `v_count`. Adding
static DTrace probes to these macros would enable tracing of where
specific vnode references come from.
For completeness and symmetry, a similar static probe should be added to
`vn_rele()` and `vn_rele_dnlc()`.
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: Prakash Surya <prakash.surya@delphix.com>
Reviewed by: Prashanth Sreenivasa <pks@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Sebastien Roy <seb@delphix.com>
illumos/illumos-gate@b7b2590dd9b7b2590dd9https://www.illumos.org/issues/8063
A standard practice in ZFS is to keep track of "per-txg" state. Any of
the 3 active TXG's (open, quiescing, syncing) can have different values
for this state. We should assert that we do not attempt to modify other
(inactive) TXG's.
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Matthew Ahrens <mahrens@delphix.com>
illumos/illumos-gate@5f368aef865f368aef86https://www.illumos.org/issues/7786
Currently, vdev_online() will only post sysevent if previous state was
"offline". It should also post the event when the state changes from "removed"
or "faulted" to "healthy" or "degraded".
This will fix the following scenario:
- pull disk from slot A
- check that hotspare has taken its place (if available)
- insert disk into slot B
- check that hotspare moved back to "avail" state (if spare was used)
The problem here is that we don't get any ESC_ZFS_VDEV_* notification and fail
to update the vdev FRU.
Reviewed by: Matthew Ahrens mahrens@delphix.com
Reviewed by: George Wilson george.wilson@delphix.com
Approved by: Albert Lee <trisk@forkgnu.org>
Author: Yuri Pankov <yuri.pankov@nexenta.com>
illumos/illumos-gate@def4fac588def4fac588https://www.illumos.org/issues/8025
dbuf_read() creates a zio_root() to track and wait for all the zio's
that may happen as part of this call. However, if the blkptr_t for
this buffer is NULL or a hole, we will not create any more zio's, so
this zio_root() is unnecessary. This is always the case when calling
dbuf_read() on a bonus buffer, because it has no blkptr (it's part of
the containing dnode). For workloads that read a lot of bonus buffers
(e.g. file creation and removal), creating and destroying these
unnecessary zio's can decrease performance by around 3%.
Reviewed by: Dan Kimmel <dan.kimmel@delphix.com>
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: Prashanth Sreenivasa <pks@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Matthew Ahrens <mahrens@delphix.com>
illumos/illumos-gate@bde3d612a7bde3d612a7https://www.illumos.org/issues/5704
libzfs uses fopen(), at least in libzfs_init(). If there are more than 255
filedescriptors open, fopen() will fail unless you give 'F' as the last mode
character. The fix would be to give 'rF' instead of 'r' as mode to fopen().
Reviewed by: Josef 'Jeff' Sipek <josef.sipek@nexenta.com>
Reviewed by: John Kennedy <john.kennedy@delphix.com>
Approved by: Richard Lowe <richlowe@richlowe.net>
Author: Simon Klinkert <simon.klinkert@gmail.com>
illumos/illumos-gate@ed4e7a6a5ced4e7a6a5chttps://www.illumos.org/issues/7340
When -o origin=<snapshot> is specified as part of a ZFS receive, that origin
should override the automatic detection in libzfs.
Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Paul Dagnelie <pcd@delphix.com>
illumos/illumos-gate@d5f26ad812d5f26ad812https://www.illumos.org/issues/5142
the current libzfs only allows simple disk and mirror setup for boot pool, as
loader does support booting from raidz, this feature will remove raidz
restriction from boot pool setup.
Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Yuri Pankov <yuri.pankov@nexenta.com>
Reviewed by: Andrew Stormont <andyjstormont@gmail.com>
Reviewed by: Albert Lee <trisk@omniti.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Toomas Soome <tsoome@me.com>
illumos/illumos-gate@d1672efb6fd1672efb6fhttps://www.illumos.org/issues/6280
The unshare_one() in libzfs could fail with EZFS_SHARENFSFAILED at line 834
here:
831 /* make sure libshare initialized */
832 if ((err = zfs_init_libshare(hdl, SA_INIT_SHARE_API)) != SA_OK) {
833 free(mntpt); /* don't need the copy anymore */
834 return (zfs_error_fmt(hdl, EZFS_SHARENFSFAILED,
835 dgettext(TEXT_DOMAIN, "cannot unshare '%s': %s"),
836 name, _sa_errorstr(err)));
837 }
The correct error should be EZFS_UNSHARENFSFAILED instead.
Reviewed by: Toomas Soome <tsoome@me.com>
Reviewed by: Dan McDonald <danmcd@omniti.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Gordon Ross <gwr@nexenta.com>
Author: Marcel Telka <marcel.telka@nexenta.com>
illumos/illumos-gate@aab04418a7aab04418a7https://www.illumos.org/issues/6268
The zfs diff command presents a description of the changes that have occurred
to files within a filesystem between two snapshots. If a file is renamed, the
tool is capable of reporting this, e.g.:
cd /some/zfs/dataset/subdir
mv file0 file1
Will result in a diff record like:
R /some/zfs/dataset/subdir/file0 -> /some/zfs/dataset/subdir/file1
Unfortunately, it seems that rename detection only uses the base filename to
determine if a file has been renamed or simply modified. This leads to
misreporting only the original filename, omitting the more relevant destination
filename entirely. For example:
cd /some/zfs/dataset/subdir
mv file0 ../otherdir/file0
Will result in a diff entry:
M /some/zfs/dataset/subdir/file0
But it should really emit:
R /some/zfs/dataset/subdir/file0 -> /some/zfs/dataset/otherdir/file0
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Justin Gibbs <gibbs@scsiguy.com>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Joshua M. Clulow <josh@sysmgr.org>
illumos/illumos-gate@b67dde11a7b67dde11a7https://www.illumos.org/issues/5814
Lets pull in this patch from freebsd:
http://svnweb.freebsd.org/base?view=revision&revision=271781
bpobj_iterate_impl(): Close a refcount leak iterating on a sublist.
If bpobj_space() returned non-zero here, the sublist would have been
left open, along with the bonus buffer hold it requires. This call
does not invoke any calls to bpobj_close() itself.
This bug doesn't have any known vector, but was found on inspection.
MFC after: 1 week
Sponsored by: Spectra Logic
Affects: All ZFS versions starting 21 May 2010 (illumos cde58dbc)
MFSpectraBSD: r1050998 on 2014/03/26
Fix bpobj_iterate_impl() to properly call bpobj_close() if bpobj_space()
returns an error.
Reviewed by: Prakash Surya <prakash.surya@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Paul Dagnelie <paul.dagnelie@delphix.com>
Reviewed by: Simon Klinkert <simon.klinkert@gmail.com>
Approved by: Gordon Ross <gwr@nexenta.com>
Author: Will Andrews <will@freebsd.org>
illumos/illumos-gate@af868f46a5af868f46a5https://www.illumos.org/issues/6914
This change allows the kernel to use more virtual address space. This will
allow us to devote 1.5x physmem for the zio arena, and an additional 1.5x
physmem for the kernel heap.
We saw a hang when unable to find any 128K contiguous memory segments. Looking
at the core file we see many threads in stacks similar to this:
> ffffff68c9c87c00::findstack -v
stack pointer for thread ffffff68c9c87c00: ffffff02cd63d8b0
[ ffffff02cd63d8b0 _resume_from_idle+0xf4() ]
ffffff02cd63d8e0 swtch+0x141()
ffffff02cd63d920 cv_wait+0x70(ffffff6009b1b01e, ffffff6009b1b020)
ffffff02cd63da50 vmem_xalloc+0x640(ffffff6009b1b000, 20000, 1000, 0, 0, 0, 0, ffffff0200000004)
ffffff02cd63dac0 vmem_alloc+0x135(ffffff6009b1b000, 20000, 4)
ffffff02cd63db60 segkmem_xalloc+0x171(ffffff6009b1b000, 0, 20000, 4, 0, fffffffffb885fe0, fffffffffbcefa10)
ffffff02cd63dbc0 segkmem_alloc_vn+0x4a(ffffff6009b1b000, 20000, 4, fffffffffbcefa10)
ffffff02cd63dbf0 segkmem_zio_alloc+0x20(ffffff6009b1b000, 20000, 4)
ffffff02cd63dd20 vmem_xalloc+0x5b1(ffffff6009b1c000, 20000, 1000, 0, 0, 0, 0, 4)
ffffff02cd63dd90 vmem_alloc+0x135(ffffff6009b1c000, 20000, 4)
ffffff02cd63de20 kmem_slab_create+0x8d(ffffff605fd37008, 4)
ffffff02cd63de80 kmem_slab_alloc+0x11e(ffffff605fd37008, 4)
ffffff02cd63dee0 kmem_cache_alloc+0x233(ffffff605fd37008, 4)
ffffff02cd63df10 zio_data_buf_alloc+0x5b(20000)
ffffff02cd63df70 arc_get_data_buf+0x92(ffffff6265a70588, 20000, ffffff901fd796f8)
ffffff02cd63dfb0 arc_buf_alloc_impl+0x9c(ffffff6265a70588, ffffff6d233ab0b8)
Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Adam Leventhal <ahl@delphix.com>
Reviewed by: John Kennedy <john.kennedy@delphix.com>
Reviewed by: Igor Kozhukhov <ikozhukhov@gmail.com>
Reviewed by: Josef 'Jeff' Sipek <josef.sipek@nexenta.com>
Approved by: Garrett D'Amore <garrett@damore.org>
Author: Matthew Ahrens <mahrens@delphix.com>
illumos/illumos-gate@0c94e1af670c94e1af67https://www.illumos.org/issues/7256
error = dmu_sync(zio, lr->lr_common.lrc_txg,
zfs_get_done, zgd);
ASSERT(error || lr->lr_length <= zp->z_blksz);
It's possible, although extremely rare, that the zfs_get_done() callback is
executed before dmu_sync() returns.
In that case the znode's range lock is dropped and the znode is unreferenced.
Thus, the assertion can access some invalid or wrong data via the zp pointer.
size variable caches the correct value of z_blksz and can be safely used here.
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Andriy Gapon <andriy.gapon@clusterhq.com>
illumos/illumos-gate@80e10fd0d280e10fd0d2https://www.illumos.org/issues/5379
The following is based on a review of the illumos code and on a similar problem
reported for FreeBSD where the relevant code is different.
Looking at this block of code http://src.illumos.org/source/xref/illumos-gate/
usr/src/uts/common/fs/zfs/zfs_vnops.c#4187 I see code to set up an
sa_bulk_attr_t object, I see code to set up mtime and ctime values, but I do
not see code to actually apply the attributes...
I would expect there to be a call to sa_bulk_update(), there is such a call in
zfs_write() for instance.
mmap_write.c [Magnifier] - demo (1.42 KB) Andriy Gapon, 2015-11-11 01:53 PM
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Prashanth Sreenivasa <pks@delphix.com>
Reviewed by: Dan McDonald <danmcd@omniti.com>
Approved by: Gordon Ross <gordon.w.ross@gmail.com>
Author: Andriy Gapon <andriy.gapon@clusterhq.com>
illumos/illumos-gate@8a981c33568a981c3356https://www.illumos.org/issues/7955
Libshare currently initializes all available filesystems when doing any
libshare operation. This requires iterating through all the filesystem
multiple times, which is a huge performance problem for sharing and
unsharing operations.
Reviewed by: Steve Gonczi <steve.gonczi@delphix.com>
Reviewed by: Sebastien Roy <sebastien.roy@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: Yuri Pankov <yuri.pankov@gmail.com>
Approved by: Gordon Ross <gordon.w.ross@gmail.com>
Author: Daniel Hoffman <dj.hoffman@delphix.com>
illumos/illumos-gate@b127fe3c05b127fe3c05https://www.illumos.org/issues/6101
lzc_create(), or more correctly, zfs_ioc_create() does not reject an attempt to
create a filesystem as a child of a volume, instead it proceeds to a crash.
A crash stack obtained on FreeBSD:
page fault while in kernel mode
zap_leaf_lookup()
fzap_lookup()
zap_lookup_norm()
zap_lookup()
zfs_get_zplprop()
zfs_fill_zplprops_impl()
zfs_ioc_create()
zfsdev_ioctl()
devfs_ioctl_f()
kern_ioctl()
sys_ioctl()
This crash happened with a kernel without debugging assertions.
The immediate cause of crash appears to an attempt to interpret a zvol object
as a zap object.
For filesystems:
#define MASTER_NODE_OBJ 1
For zvols:
#define ZVOL_OBJ 1ULL
#define ZVOL_ZAP_OBJ 2ULL
So, I see two problems here:
1. an attempt to create a filesystem under a zvol should be rejected as
early as possible, maybe in zfs_fill_zplprops()
2. maybe zap_lookup / zap_lockdir should reject objects that are not of one
of the zap object types
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Andriy Gapon <avg@FreeBSD.org>
illumos/illumos-gate@7f0bdb42577f0bdb4257https://www.illumos.org/issues/8061
sa_find_idx_tab() is declared as taking and returning "void *" parameters.
These can be declared to be the specific types.
Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Chris Williamson <chris.williamson@delphix.com>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Matthew Ahrens <mahrens@delphix.com>
illumos/illumos-gate@6b036259816b03625981https://www.illumos.org/issues/8026
zfs_throttle_delay and zfs_throttle_resolution became disused since the new
write throttling mechanism was introduced.
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Approved by: Richard Lowe <richlowe@richlowe.net>
Author: Andriy Gapon <avg@FreeBSD.org>
illumos/illumos-gate@471a88e499471a88e499https://www.illumos.org/issues/5380
A stream created with zfs send -p -I contains properties of all snapshots of a
given dataset as opposed to only properties of snapshots in a given range.
Not only this is suboptimal but the receive code also does not filter
properties by the range. So, properties of earlier snapshots would be updated
even though the snapshots themselves are not in the stream (just their
properties).
Given that modifying the snapshot properties requires a TXG sync and that the
snapshots are updated one by one the described behavior may lead to a sever
performance penalty.
Reviewed by: Paul Dagnelie <pcd@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Andriy Gapon <avg@FreeBSD.org>
illumos/illumos-gate@313ae1e182313ae1e182https://www.illumos.org/issues/8027
dsl_pool_dirty_delta() should not wake up waiters when dp->dp_dirty_total ==
zfs_dirty_data_max, because they wait for dp_dirty_total to fall strictly below
the threshold.
It's probably very rare for that condition to occur, but it's better to have
more accurate code.
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Paul Dagnelie <pcd@delphix.com>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Andriy Gapon <avg@FreeBSD.org>
illumos/illumos-gate@c040c10cddc040c10cddhttps://www.illumos.org/issues/7885
When a member of a RAIDZ has been replaced with a device smaller than the
original, then the top level vdev can report its expand size as 16.0E.
The reduced child asize causes the RAIDZ to have a vdev_asize lower than its
vdev_max_asize which then results in an underflow during the calculation of the
parents expand size.
Also for RAIDZ vdevs the sum of their child vdev_min_asize could be smaller
than the parents vdev_min_size.
Fixed by: https://github.com/openzfs/openzfs/pull/296
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Approved by: Gordon Ross <gordon.w.ross@gmail.com>
Author: Steven Hartland <steven.hartland@multiplay.co.uk>
illumos/illumos-gate@d8584ba6fbd8584ba6fbhttps://www.illumos.org/issues/7990
The snapspec_cb() callback function in libzfs does not need to call zfs_strdup().
Reviewed by: Yuri Pankov <yuri.pankov@gmail.com>
Reviewed by: Toomas Soome <tsoome@me.com>
Approved by: Matthew Ahrens <mahrens@delphix.com>
Author: Marcel Telka <marcel@telka.sk>
illumos/illumos-gate@94c2d0eb2294c2d0eb22https://www.illumos.org/issues/7968
spa_sync() iterates over all the dirty dnodes and processes each of them by
calling dnode_sync(). If there are many dirty dnodes (e.g. because we created
or removed a lot of files), the single thread of spa_sync() calling dnode_sync
() can become a bottleneck. Additionally, if many dnodes are dirtied
concurrently in open context (e.g. due to concurrent file creation), the
os_lock will experience lock contention via dnode_setdirty().
The solution is to track dirty dnodes on a multilist_t, and for spa_sync() to
use separate threads to process each of the sublists in the multilist.
On the concurrent file creation microbenchmark, the performance improvement
from dnode_setdirty() is up to 7%. Additionally, the wall clock time spent in
spa_sync() is reduced to 15%-40% of the single-threaded case. In terms of cost/
reward, once the other bottlenecks are addressed, fixing this bug will provide
a medium-large performance gain and require a medium amount of effort to
implement.
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Saso Kiselkov <saso.kiselkov@nexenta.com>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Matthew Ahrens <mahrens@delphix.com>
illumos/illumos-gate@10fbdecb0510fbdecb05https://www.illumos.org/issues/7970
The global tunable zfs_arc_num_sublists_per_state is used by the ARC and
the dbuf cache, and other users are planned. We should change this
tunable to be common to all multilists.
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Saso Kiselkov <saso.kiselkov@nexenta.com>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Matthew Ahrens <mahrens@delphix.com>
illumos/illumos-gate@411be58a6e411be58a6ehttps://www.illumos.org/issues/7801
Add *_by_dnode() routines for accessing objects given their
dnode_t *, this is more efficient than accessing the object by
(objset_t *, uint64_t object). This change converts some but
not all of the existing consumers. As performance-sensitive
code paths are discovered they should be converted to use
these routines.
Ported from: https://github.com/zfsonlinux/zfs/commit/
0eef1bde31d67091d3deed23fe2394f5a8bf2276
Author: Matthew Ahrens <mahrens@delphix.com>
illumos/illumos-gate@b0c42cd470b0c42cd470https://www.illumos.org/issues/7801
Add *_by_dnode() routines for accessing objects given their
dnode_t *, this is more efficient than accessing the object by
(objset_t *, uint64_t object). This change converts some but
not all of the existing consumers. As performance-sensitive
code paths are discovered they should be converted to use
these routines.
Ported from: https://github.com/zfsonlinux/zfs/commit/
0eef1bde31d67091d3deed23fe2394f5a8bf2276
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: bzzz77 <bzzz.tomas@gmail.com>
illumos/illumos-gate@a3905a4592a3905a4592https://www.illumos.org/issues/7869
The issue fixed by this patch is a race condition in the deadlist code.
A thread executing an administrative command that uses
`dsl_deadlist_space_range()` holds the lock of the whole `deadlist_t` to
protect the access of all its entries that the deadlist contains in an
avl tree.
Sync threads trying to insert a new entry in the deadlist
(through `dsl_deadlist_insert()` -> `dle_enqueue()`) do not hold the
deadlist lock at that moment. If the `dle_bpobj` is the empty bpobj (our
sentinel value), we close and reopen it. Between these two operations,
it is possible for the `dsl_deadlist_space_range()` thread to dereference
that bpobj which is `NULL` during that window.
Threads should hold the a deadlist's `dl_lock` when they manipulate its
internal data so scenarios like the one above are avoided. In addition,
threads should also hold the bpobj lock whenever they are allocating the
subobj list of a bpobj, and not just when they actually insert the subobj
to the list. This way we can avoid potential memory leaks.
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: Dan Kimmel <dan.kimmel@delphix.com>
Reviewed by: Steve Gonczi <steve.gonczi@delphix.com>
Reviewed by: John Kennedy <john.kennedy@delphix.com>
Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Serapheim Dimitropoulos <serapheim@delphix.com>
illumos/illumos-gate@61e255ce7261e255ce72https://www.illumos.org/issues/7793
Background information: This assertion about tx_space_* verifies that we
are not dirtying more stuff than we thought we would. We “need” to know
how much we will dirty so that we can check if we should fail this
transaction with ENOSPC/EDQUOT, in dmu_tx_assign(). While the
transaction is open (i.e. between dmu_tx_assign() and dmu_tx_commit() —
typically less than a millisecond), we call dbuf_dirty() on the exact
blocks that will be modified. Once this happens, the temporary
accounting in tx_space_* is unnecessary, because we know exactly what
blocks are newly dirtied; we call dnode_willuse_space() to track this
more exact accounting.
The fundamental problem causing this bug is that dmu_tx_hold_*() relies
on the current state in the DMU (e.g. dn_nlevels) to predict how much
will be dirtied by this transaction, but this state can change before we
actually perform the transaction (i.e. call dbuf_dirty()).
This bug will be fixed by removing the assertion that the tx_space_*
accounting is perfectly accurate (i.e. we never dirty more than was
predicted by dmu_tx_hold_*()). By removing the requirement that this
accounting be perfectly accurate, we can also vastly simplify it, e.g.
removing most of the logic in dmu_tx_count_*().
The new tx space accounting will be very approximate, and may be more or
less than what is actually dirtied. It will still be used to determine
if this transaction will put us over quota. Transactions that are marked
by dmu_tx_mark_netfree() will be excepted from this check. We won’t make
an attempt to determine how much space will be freed by the transaction
— this was rarely accurate enough to determine if a transaction should
be permitted when we are over quota, which is why dmu_tx_mark_netfree()
was introduced in 2014.
We also won’t attempt to give “credit” when overwriting existing blocks,
if those blocks may be freed. This allows us to remove the
do_free_accounting logic in dbuf_dirty(), and associated routines. This
Reviewed by: Steve Gonczi <steve.gonczi@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Matthew Ahrens <mahrens@delphix.com>
illumos/illumos-gate@48bbca816848bbca8168https://www.illumos.org/issues/7812
This change removes all gendered language that did not refer specifically
to an individual person or pet. The convention taken was to use
variations on "they" when referring to users and/or human beings, while
using "it" when referring to code, functions, and/or libraries.
Additionally, we took the liberty to fix up any whitespace issues that
were found in any files that were already being modified.
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: Prakash Surya <prakash.surya@delphix.com>
Reviewed by: Steve Gonczi <steve.gonczi@delphix.com>
Reviewed by: Chris Williamson <chris.williamson@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Igor Kozhukhov <igor@dilos.org>
Reviewed by: Dan McDonald <danmcd@omniti.com>
Reviewed by: Robert Mustacchi <rm@joyent.com>
Approved by: Richard Lowe <richlowe@richlowe.net>
Author: Daniel Hoffman <dj.hoffman@delphix.com>
illumos/illumos-gate@46d46cd4fa46d46cd4fahttps://www.illumos.org/issues/7803
Make get_devid() from libzfs a public function in libdevid, as its pretty
usable in other places and duplicating all the logic required to get string
encoded devid from path seems counter-productive.
Reviewed by: Dan McDonald <danmcd@omniti.com>
Reviewed by: Jason King <jason.brian.king@gmail.com>
Reviewed by: Marcel Telka <marcel@telka.sk>
Reviewed by: Prakash Surya <prakash.surya@delphix.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Yuri Pankov <yuri.pankov@nexenta.com>
illumos/illumos-gate@8b65a70b768b65a70b76https://www.illumos.org/issues/7541
When calling zpool import, zpool does a few ioctls to ZFS.
zpool allocates a buffer in userland and passes it to the kernel so that ZFS
can copy info into it. ZFS will use it to put the nvlist that describes the
pool configuration.
If the allocated buffer is too small, ZFS will return ENOMEM and the call will
have to be redone. This wastes CPU time and slows down the import process. This
happens very often for the ZFS_IOC_POOL_TRYIMPORT call.
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Dan Kimmel <dan.kimmel@delphix.com>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Pavel Zakharov <pavel.zakharov@delphix.com>
illumos/illumos-gate@1c17160ac51c17160ac5https://www.illumos.org/issues/1300
Problem:
We can create invisible file in ZFS.
How to reproduce:
0. Prepare normalization formD ZFS.
# cat cat /etc/release
cat: cat: No such file or directory
OpenIndiana Development oi_151 X86 (powered by illumos)
Copyright 2011 Oracle and/or its affiliates. All rights reserved.
Use is subject to license terms.
Assembled 28 April 2011
# mkfile 100M 100M
# zpool create -O utf8only=on -O normalization=formD test_pool $( pwd )/
100M
# zpool upgrade
This system is currently running ZFS pool version 28.
All pools are formatted using this version.
# zfs get normalization test_pool
NAME PROPERTY VALUE SOURCE
test_pool normalization formD -
# chmod 777 /test_pool
1. Create a NFD file.
$ cd /test_pool/
$ cp /etc/release $( echo "\x75\xcc\x88" )
$ ls -la
total 4
drwxrwxrwx 2 root root 3 2011-07-29 08:53 .
drwxr-xr-x 25 root root 26 2011-07-29 08:53 ..
-r--r--r-- 1 test1 staff 251 2011-07-29 08:53 u?
Reviewed by: Yuri Pankov <yuri.pankov@nexenta.com>
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Kevin Crowe <kevin.crowe@nexenta.com>
illumos/illumos-gate@54207fd2e154207fd2e1https://www.illumos.org/issues/4242
From Joyent's OS-2557:
So we're basically just doing a check here that after we got a 'rename' event
for our file, the file has actually been moved out of the way.
What I've seen in bh1-stage2 is that this happens as you'd expect for all zones
but many times over the last week it's failed because when we do the fs.exists
() check here, the file that we got the rename event for, still exists.
I've confirmed what's happening using the following dtrace script:
#!/usr/sbin/dtrace -s
#pragma D option quiet
#pragma D option bufsize=256k
syscall::open:entry,
syscall::open64:entry
/copyinstr(arg0) == "/var/svc/provisioning" || (strlen(copyinstr(arg0)) == 69
&& substr(copyinstr(arg0), 48) == "/var/svc/provisioning")/
{
this->watching_open = 1;
printf("%d zone %s process %s(%d) [%s] open(%s)\\n",
timestamp,
zonename,
execname,
pid,
curpsinfo->pr_psargs,
copyinstr(arg0));
}
syscall::open:return,
syscall::open64:return
/this->watching_open == 1/
Reviewed by: Robert Mustacchi <rm@joyent.com>
Reviewed by: Marcel Telka <marcel@telka.sk>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Jerry Jelinek <jerry.jelinek@joyent.com>
illumos/illumos-gate@7de35a3ed07de35a3ed0https://www.illumos.org/issues/7740
The problem is that dbuf_findbp will return ENOENT if the block it's
trying to find is beyond the end of the file. If that happens, we assume
there is no birth time, and so we lose that information when we write
out new blkptrs. We should teach dbuf_findbp to look for things that are
beyond the current end, but not beyond the absolute end of the file.
To verify, create a large file, truncate it to a short length, and then
write beyond the end. Check with zdb to make sure that there are no
holes with birth time zero (will appear as gaps).
Reviewed by: Steve Gonczi <steve.gonczi@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Paul Dagnelie <pcd@delphix.com>
illumos/illumos-gate@b7f9f60c8eb7f9f60c8ehttps://www.illumos.org/issues/7779
zfsctl_ops_shares_dir and ZFSCTL_INO_SHARES are essentially dead code.
While there, fix the index range check in zfsctl_root_inode_cb.
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Andriy Gapon <andriy.gapon@clusterhq.com>
illumos/illumos-gate@7c13517fff7c13517fffhttps://www.illumos.org/issues/7745
The problem is that consumers of `libZFS_Core` that forget to call
`libzfs_core_init()` before calling any other function of the library
are having a hard time realizing their mistake. The library's internal
file descriptor is declared as global static, which is ok, but it is not
initialized explicitly; therefore, it defaults to 0, which is a valid
file descriptor. If `libzfs_core_init()`, which explicitly initializes
the correct fd, is skipped, the ioctl functions return errors that do
not have anything to do with `libZFS_Core`, where the problem is
actually located.
Even though assertions for that existed within `libZFS_Core` for debug
builds, they were never enabled because the `-DDEBUG` flag was missing
from the compiler flags.
This patch applies the following changes:
1. It adds `-DDEBUG` for debug builds of `libZFS_Core` and `libzfs`,
to enable their assertions on debug builds.
2. It corrects an assertion within `libzfs`, where a function had
been spelled incorrectly (`zpool_prop_unsupported()`) and nobody
knew because the `-DDEBUG` flag was missing, and the preprocessor
was taking that part of the code away.
3. The library's internal fd is initialized to `-1` and `VERIFY`
assertions have been placed to check that the fd is not equal to
`-1` before issuing any ioctl. It is important here to note, that
the `VERIFY` assertions exist in both debug and non-debug builds.
4. In `libzfs_core_fini` we make sure to never increment the
refcount of our fd below 0, and also reset the fd to `-1` when no
one refers to it. The reason for this, is for the rare case that
the consumer closes all references but then calls one of the
library's functions without using `libzfs_core_init()` first, and
in the mean time, a previous call to `open()` decided to reuse
our previous fd. This scenario would have passed our assertion in
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Serapheim Dimitropoulos <serapheim@delphix.com>
illumos/illumos-gate@555da5111b555da5111bhttps://www.illumos.org/issues/7743
When loading a pool that had been created before the existance of
per-vdev zaps, on a system that knows about per-vdev zaps, the
per-vdev zaps will not be allocated and initialized.
This appears to be because the logic that would have done so, in
spa_sync_config_object(), is not reached under normal operation. It is
only reached if spa_config_dirty_list is non-empty.
The fix is to add another `AVZ_ACTION_` enum that will allow this code
to be reached when we detect that we're loading an old pool, even when
there are no dirty configs.
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Don Brady <don.brady@intel.com>
Approved by: Robert Mustacchi <rm@joyent.com>
Author: Paul Dagnelie <pcd@delphix.com>
illumos/illumos-gate@5f145778015f14577801https://www.illumos.org/issues/7613
metaslab_t:ms_freetree[TXG_SIZE] is only used in syncing context. We should
replace it with two trees: the freeing tree (ranges that we are freeing this
syncing txg) and the freed tree (ranges which have been freed this txg).
Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Alex Reece <alex@delphix.com>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Matthew Ahrens <mahrens@delphix.com>
illumos/illumos-gate@4ba5b961634ba5b96163https://www.illumos.org/issues/7586
The #ifdef __lint in dmu.h is ugly, and it would be nice not to duplicate it if
we add other inline functions into header files in ZFS, especially since it is
difficult to make any other solution work across all compilation targets. We
should switch to disabling the lint flags that are failing instead.
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: Dan Kimmel <dan.kimmel@delphix.com>
illumos/illumos-gate@1a01181fdc1a01181fdchttps://www.illumos.org/issues/7580
We need to prevent any reader whenever we're about the zero out all the
blkptrs. To do this we need to grab the dn_struct_rwlock as writer in
dbuf_write_children_ready and free_children just prior to calling bzero.
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: Steve Gonczi <steve.gonczi@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@omniti.com>
Author: George Wilson <george.wilson@delphix.com>
