Commit Graph

2213 Commits

Author SHA1 Message Date
markj
0a9182f1aa Limit the number of entries allocated for a REPORT_ZONES command.
The DIOCGETZONE ioctl can be used to fetch the zone list of an SMR
drive, and the caller specifies the number of entries it wants to fetch.
Clamp the caller's request to a sane limit so that a user cannot attempt
large allocations. Callers already need to invoke the ioctl multiple
times to fetch the full list in general, so there's no harm in limiting
the number of entries returned.

Fix style while here.

admbug:		807
Reported by:	Ilja Van Sprundel <ivansprundel@ioactive.com>
Reviewed by:	asomers, ken
Tested by:	ken
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D19249
2019-02-19 21:33:02 +00:00
markj
467f20b505 Impose a limit on the number of GEOM_CTL arguments.
Otherwise a privileged user can trigger a memory allocation of
unbounded size, or an integer overflow in the subsequent
geom_alloc_copyin() call, leading to out-of-bounds accesses.

Hard-code a large limit to circumvent this problem.

admbug:		854
Reported by:	Anonymous of the Shellphish Grill Team
Reviewed by:	ae
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D19251
2019-02-19 21:22:22 +00:00
avos
8fde4b66d4 geom_uzip(4): set 'gp != NULL' assertion on top of the function
There was yet another access to this variable in g_trace() few
lines upper.

PR:		203499
Reported by:	cem
MFC after:	5 days
MFC with:	343473
2019-01-26 17:17:25 +00:00
avos
47d3818cdf geom_uzip(4): move NULL pointer KASSERT check before it is dereferenced
PR:		203499
Submitted by:	<chadf@triularity.org>
MFC after:	5 days
2019-01-26 14:54:06 +00:00
cem
66fb4c422a gmirror: Relocate DEVICE_FLAGS to adjacent lines
gmirror's sc_flags is shared between some on-disk state and some runtime
only state.  There's no real reason for that and they could probably be
split up.  Until they are, locate all of the flags for the same field
nearby each other in the source, for clarity.

No functional change.

Sponsored by:	Dell EMC Isilon
2019-01-23 16:44:21 +00:00
markj
b7a9839424 Use g_handleattr() to reply to GEOM::candelete queries.
g_handleattr() fills out bp->bio_completed; otherwise, g_getattr()
returns an error in response to the query.  This caused BIO_DELETE
support to not be propagated through stacked configurations, e.g.,
a gconcat of gmirror volumes would not handle BIO_DELETE even when
the gmirrors do.  g_io_getattr() was not affected by the problem.

PR:		232676
Reported and tested by:	noah.bergbauer@tum.de
MFC after:	1 week
2019-01-02 15:52:16 +00:00
mav
929bac1991 Switch from mutexes to atomics in GEOM_DEV I/O path.
Mutexes in I/O path there were used twice per I/O to atomically access
several variables to close and/or destroy the device on last request
completion.  I found the way to fit all required info into one integer,
suitable for atomic operations.  It opened race window on device close,
but addition of timeout to the msleep() there should cover it.

Profiling shows removal of significant spinning time on those mutexes
and IOPS increase from ~600K to >800K to NVMe on 72-core systems.

MFC after:	1 month
Sponsored by:	iXsystems, Inc.
2018-12-27 19:15:24 +00:00
cem
89e84f5e34 gmirror: Remove a last-minute INVARIANTS breakage in r341840
I mistakenly added a lock assertion to this routine at the last minute
without confirming it was held during g_mirror_create.  It isn't (it isn't
even initialized yet).  Mea culpa.  Access is exclusive in both callers,
just not always by that particular lock.

Reported by:	lwhsu
X-MFC-With:	r341840, r341674
2018-12-12 18:13:56 +00:00
cem
25cda747b0 gmirror: Fix a bug introduced in r341674
r341674 inadvertently introduced a bug where newer mirror components being
tasted would clear the high sc_flags that are not controlled by component
metadata, such as G_MIRROR_DEVICE_FLAG_TASTING.  This could plausibly expose
a small window of time during STARTING where device destruction might race
with mirror component addition, probably resulting in a crash.

Reviewed by:	markj
X-MFC-With:	r341674
Differential Revision:	https://reviews.freebsd.org/D18521
2018-12-12 05:48:27 +00:00
cem
42d84ef531 gmirror: Evaluate mirror components against newest metadata copy
Re-apply r341665 with format strings fixed.

If we happen to taste a stale mirror component first, don't reject valid,
newer components that have differing metadata from the stale component
(during STARTING).  Instead, update our view of the most recent metadata as
we taste components.

Like mediasize beforehand, remove some checks from g_mirror_check_metadata
which would evict valid components due to metadata that can change over a
mirror's lifetime.  g_mirror_check_metadata is invoked long before we check
genid/syncid and decide which component(s) are newest and whether or not we
have quorum.

Before checking if we can enter RUNNING (i.e., we have quorum) after a NEW
component is added, first remove any known stale or inconsistent disks from
the mirrorset, rather than removing them *after* deciding we have quorum.
Check if we have quorum after removing these components.

Additionally, add a knob, kern.geom.mirror.launch_mirror_before_timeout, to
force gmirrors to wait out the full timeout (kern.geom.mirror.timeout)
before transitioning from STARTING to RUNNING.  This is a kludge to help
ensure all eligible, boot-time available mirror components are tasted before
RUNNING a gmirror.

Add a basic test case for STARTING -> RUNNING startup behavior around stale
genids.

PR:		232671, 232835
Submitted by:	Cindy Yang <cyang AT isilon.com> (previous version)
Reviewed by:	markj (kernel portions)
Discussed with:	asomers, Cindy Yang
Tested by:	pho
Sponsored by:	Dell EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D18062
2018-12-07 02:44:04 +00:00
cem
a6f65b4008 Revert r341665 due to tinderbox breakage
I didn't notice that some format strings were non-portable.  Will fix and
re-commit later.
2018-12-07 00:47:05 +00:00
cem
0b3fcef470 gmirror: Evaluate mirror components against newest metadata copy
If we happen to taste a stale mirror component first, don't reject valid,
newer components that have differing metadata from the stale component
(during STARTING).  Instead, update our view of the most recent metadata as
we taste components.

Like mediasize beforehand, remove some checks from g_mirror_check_metadata
which would evict valid components due to metadata that can change over a
mirror's lifetime.  g_mirror_check_metadata is invoked long before we check
genid/syncid and decide which component(s) are newest and whether or not we
have quorum.

Before checking if we can enter RUNNING (i.e., we have quorum) after a NEW
component is added, first remove any known stale or inconsistent disks from
the mirrorset, rather than removing them *after* deciding we have quorum.
Check if we have quorum after removing these components.

Additionally, add a knob, kern.geom.mirror.launch_mirror_before_timeout, to
force gmirrors to wait out the full timeout (kern.geom.mirror.timeout)
before transitioning from STARTING to RUNNING.  This is a kludge to help
ensure all eligible, boot-time available mirror components are tasted before
RUNNING a gmirror.

When we are instructed to forget mirror components, bump the generation id
to avoid confusion with such stale components later.

Add a basic test case for STARTING -> RUNNING startup behavior around stale
genids.

PR:		232671, 232835
Submitted by:	Cindy Yang <cyang AT isilon.com> (previous version)
Reviewed by:	markj (kernel portions)
Discussed with:	asomers, Cindy Yang
Tested by:	pho
Sponsored by:	Dell EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D18062
2018-12-06 23:55:39 +00:00
mckusick
2c9178edde Normally when an attempt is made to mount a UFS/FFS filesystem whose
superblock has a check-hash error, an error message noting the
superblock check-hash failure is printed and the mount fails. The
administrator then runs fsck to repair the filesystem and when
successful, the filesystem can once again be mounted.

This approach fails if the filesystem in question is a root filesystem
from which you are trying to boot. Here, the loader fails when trying
to access the filesystem to get the kernel to boot. So it is necessary
to allow the loader to ignore the superblock check-hash error and make
a best effort to read the kernel. The filesystem may be suffiently
corrupted that the read attempt fails, but there is no harm in trying
since the loader makes no attempt to write to the filesystem.

Once the kernel is loaded and starts to run, it attempts to mount its
root filesystem. Once again, failure means that it breaks to its prompt
to ask where to get its root filesystem. Unless you have an alternate
root filesystem, you are stuck.

Since the root filesystem is initially mounted read-only, it is
safe to make an attempt to mount the root filesystem with the failed
superblock check-hash. Thus, when asked to mount a root filesystem
with a failed superblock check-hash, the kernel prints a warning
message that the root filesystem superblock check-hash needs repair,
but notes that it is ignoring the error and proceeding. It does
mark the filesystem as needing an fsck which prevents it from being
enabled for writing until fsck has been run on it. The net effect
is that the reboot fails to single user, but at least at that point
the administrator has the tools at hand to fix the problem.

Reported by:    Rick Macklem (rmacklem@)
Discussed with: Warner Losh (imp@)
Sponsored by:   Netflix
2018-12-06 00:09:39 +00:00
sobomax
f69b381a75 Another attempt to fix issue with the DIOCGDELETE ioctl(2) not
handling slightly out-of-bound requests properly (r340187).
Perform range check here rather then rely on g_delete_data() to DTRT.

The g_delete_data() would always return success for requests
starting just the next byte after providers media boundary.

MFC after:	4 weeks
2018-12-04 21:48:56 +00:00
des
6dea58b239 Add a “skip_dsn” option to g_part's bootcode verb to prevent g_part_mbr
from setting the volume serial number.  This unbreaks older boot blocks
that don't support serial numbers, and allows boot0cfg to set the serial
number itself if requested by the user.

Submitted by:	lev@, yuripv@
MFC after:	1 week
Differential Revision:	https://reviews.freebsd.org/D17386
2018-11-27 14:58:19 +00:00
sobomax
02f48a53e7 Revert r340187, it breaks EOD (end-of-device) detection logic. Turns out,
i/o into last_sector+N is handled differently for N==1 and N>1 cases to
accomodate that, so some other approach would be needed to fix DIOCGDELETE
ioctl(2).
2018-11-07 16:28:09 +00:00
sobomax
f92b92777e Don't allow BIO_READ, BIO_WRITE or BIO_DELETE requests that are
fully beyond the end of providers media. The only exception is made
for the zero length transfers which are allowed to be just on the
boundary. Previously, any requests starting on the boundary (i.e. next
byte after the last one) have been allowed to go through.

No response from:	freebsd-geom@, phk
MFC after:		1 month
2018-11-06 15:55:41 +00:00
markj
078f83caf8 Have gconcat advertise delete support if one of its disks does.
This follows the example set by other multi-disk GEOM classes.

PR:		232676
Tested by:	noah.bergbauer@tum.de
MFC after:	1 month
2018-10-30 00:22:14 +00:00
eugen
f8a3770ab6 Extend stripeoffset and stripesize of GEOMs from u_int to off_t
GEOM's stripeoffset overflows at 4 gigabyte margin (2^32)
because of its u_int type. This leads to incorrect data in the output
generated by "sysctl kern.geom.confxml" command, "graid list" etc.
when GEOM array has volumes larger than 4G, for example.

This change does not affect ABI but changes KBI. No MFC planned.

Differential Revision:	https://reviews.freebsd.org/D13426
2018-10-27 16:14:42 +00:00
delphij
e1e4619a35 Restore backward compatibility for "attach" verb.
In r332361 and r333439, two new parameters were added to geli attach
verb using gctl_get_paraml, which requires the value to be present.
This would prevent old geli(8) binary from attaching geli(4) device
as they have no knowledge about the new parameters.

Restore backward compatibility by treating the absense of these two
values as seeing the default value supplied by userland.

PR:		232595
Reviewed by:	oshogbo
MFC after:	3 days
Differential Revision:	https://reviews.freebsd.org/D17680
2018-10-27 03:37:14 +00:00
gjb
fcf5119e83 MFH r338661 through r339200.
Sponsored by:	The FreeBSD Foundation
2018-10-05 17:53:47 +00:00
mav
d88eb6692d Fix use-after-free in RAID0 error reporting of GEOM_RAID.
PR:		231510
Submitted by:	yangx92@hotmail.com
Approved by:	re (gjb)
MFC after:	1 week
2018-09-24 16:58:55 +00:00
jkim
69a6e12782 Make geli(8) buildable. 2018-09-19 07:08:04 +00:00
cem
99ba792d73 OpenCrypto: Convert sessions to opaque handles instead of integers
Track session objects in the framework, and pass handles between the
framework (OCF), consumers, and drivers.  Avoid redundancy and complexity in
individual drivers by allocating session memory in the framework and
providing it to drivers in ::newsession().

Session handles are no longer integers with information encoded in various
high bits.  Use of the CRYPTO_SESID2FOO() macros should be replaced with the
appropriate crypto_ses2foo() function on the opaque session handle.

Convert OCF drivers (in particular, cryptosoft, as well as myriad others) to
the opaque handle interface.  Discard existing session tracking as much as
possible (quick pass).  There may be additional code ripe for deletion.

Convert OCF consumers (ipsec, geom_eli, krb5, cryptodev) to handle-style
interface.  The conversion is largely mechnical.

The change is documented in crypto.9.

Inspired by
https://lists.freebsd.org/pipermail/freebsd-arch/2018-January/018835.html .

No objection from:	ae (ipsec portion)
Reported by:	jhb
2018-07-18 00:56:25 +00:00
cem
6905c74db5 OCF: Convert consumers to the session id typedef
These were missed in the earlier r336269.

No functional change.

Sponsored by:	Dell EMC Isilon
2018-07-16 19:01:05 +00:00
oshogbo
feba2ca94d Let geli deal with lost devices without crashing.
PR:		162036
Submitted by:	Fabian Keil <fk@fabiankeil.de>
Obtained from:	ElectroBSD
Discussed with: pjd@
2018-07-15 18:03:19 +00:00
imp
7218f22343 g_eli_key_cmp is used only in the kernel, so only define it in the
kernel.
2018-07-13 18:21:38 +00:00
trociny
414774b793 geom_gate: enable resize
Reviewed By:	pjd
Approved By:	pjd
Differential Revision:	https://reviews.freebsd.org/D11531
2018-07-13 07:08:06 +00:00
emaste
01a87b42b7 gpart: add EFI alias for MBR partition scheme
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D15870
2018-06-17 20:10:48 +00:00
emaste
7896c4c978 Sort geom/part mbr/ebr/ldm alias table entries
Having the table entries in alpha order simplifies future additions.

Sponsored by:	The FreeBSD Foundation
2018-06-17 20:06:27 +00:00
oshogbo
9f099d8764 Introduce the 'n' flag for the geli attach command.
If the 'n' flag is provided the provided key number will be used to
decrypt device. This can be used combined with dryrun to verify if the key
is set correctly. This can be also used to determine which key slot we want to
change on already attached device.

Reviewed by:	allanjude
Differential Revision:	https://reviews.freebsd.org/D15309
2018-05-09 20:53:38 +00:00
markj
4476e82765 Refactor some of the MI kernel dump code in preparation for netdump.
- Add clear_dumper() to complement set_dumper().
- Drain netdump's preallocated mbuf pool when clearing the dumper.
- Don't do bounds checking for dumpers with mediasize 0.
- Add dumper callbacks for initialization for writing out headers.

Reviewed by:	sbruno
MFC after:	1 month
Sponsored by:	Dell EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D15252
2018-05-06 00:22:38 +00:00
markj
7f301bacf9 Remove a redundant assertion.
MFC after:	1 week
Sponsored by:	Dell EMC Isilon
2018-05-06 00:05:03 +00:00
markj
3579270d7e Avoid dropping the topology lock in gmirror's dumpconf implementation.
Doing so introduces races which can lead to a use-after-free when
grabbing a snapshot of the GEOM mesh.

To ensure that a mirror's disk list remains stable, change its locking
protocol: both the softc lock and the topology lock are now required
to modify the list, so either lock is sufficient for traversal.

Tested by:	pho
MFC after:	2 weeks
Sponsored by:	Dell EMC Isilon
2018-05-06 00:03:24 +00:00
emaste
b869ee6435 gpart: add fat32lba MBR partition type
FAT32 partition with LBA addressing.

Reviewed by:	marcel
MFC after:	3 days
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D15266
2018-05-04 00:34:27 +00:00
kevans
dd6f2f2c8d Annotate geom modules with MODULE_VERSION
GEOM ELI may double ask the password during boot. Once at loader time, and
once at init time.

This happens due a module loading bug. By default GEOM ELI caches the
password in the kernel, but without the MODULE_VERSION annotation, the
kernel loads over the kernel module, even if the GEOM ELI was compiled into
the kernel. In this case, the newly loaded module
purges/invalidates/overwrites the GEOM ELI's password cache, which causes
the double asking.

MFC Note: There's a pc98 component to the original submission that is
omitted here due to pc98 removal in head. This part will need to be revived
upon MFC.

Reviewed by:	imp
Submitted by:	op
Obtained from:	opBSD
MFC after:	1 week
Differential Revision:	https://reviews.freebsd.org/D14992
2018-04-10 19:18:16 +00:00
oshogbo
be1fef5d07 Introduce dry run option for attaching the device.
This will allow us to verify if passphrase and key is valid without
decrypting whole device.

Reviewed by:	cem@, allanjude@
Differential Revision:	https://reviews.freebsd.org/D15000
2018-04-10 13:22:48 +00:00
kevans
04729ee082 Retire the geom_aes class
It's had a good life, but it's not really configurable and not really used.

Obtained from:	opBSD (with some changes)
Differential Revision:	https://reviews.freebsd.org/D14991
2018-04-09 17:30:30 +00:00
brooks
9d79658aab Move most of the contents of opt_compat.h to opt_global.h.
opt_compat.h is mentioned in nearly 180 files. In-progress network
driver compabibility improvements may add over 100 more so this is
closer to "just about everywhere" than "only some files" per the
guidance in sys/conf/options.

Keep COMPAT_LINUX32 in opt_compat.h as it is confined to a subset of
sys/compat/linux/*.c.  A fake _COMPAT_LINUX option ensure opt_compat.h
is created on all architectures.

Move COMPAT_LINUXKPI to opt_dontuse.h as it is only used to control the
set of compiled files.

Reviewed by:	kib, cem, jhb, jtl
Sponsored by:	DARPA, AFRL
Differential Revision:	https://reviews.freebsd.org/D14941
2018-04-06 17:35:35 +00:00
sbruno
2e02693803 Squash error from geom by sizing ident strings to DISK_IDENT_SIZE.
Display attribute in future error strings and differentiate g_handleattr()
error messages for ease of debugging in the future.

"g_handleattr: md1 bio_length 24 strlen 31 -> EFAULT"

Reported by:	swills
Reviewed by:	imp cem avg
Sponsored by:	Limelight Networks
Differential Revision:	https://reviews.freebsd.org/D14962
2018-04-05 13:56:40 +00:00
mckusick
44f0dfb9b7 When freeing a superblock returned by ffs_sbget, be sure to also
free the superblock summary information.

Reported by: Peter Holm (pho@)
Tested by: Peter Holm (pho@)
2018-03-24 15:36:25 +00:00
oshogbo
c4d76125e1 Remove unneeded variable which was introduced in r328472.
Pointed out by:	pjd@
2018-03-18 15:09:55 +00:00
avg
ee6ae8018d g_access: deal with races created by geoms that drop the topology lock
The problem is that g_access() must be called with the GEOM topology
lock held.  And that gives a false impression that the lock is indeed
held across the call.  But this isn't always true because many classes,
ZVOL being one of the many, need to drop the lock.  It's either to
perform an I/O on the first open or to acquire a different lock (like in
g_mirror_access).

That, of course, can break many assumptions.  For example,
g_slice_access() adds an extra exclusive count on the first open. As
described above, an underlying geom may drop the topology lock and that
would open a race with another thread that would also request another
extra exclusive count.  In general, two consumers may be granted
incompatible accesses.

To avoid this problem the code is changed to mark a geom with special
flag before calling its access method and clear the flag afterwards.  If
another thread sees that flag, then it means that the topology lock has
been dropped (either by the geom in question or downstream from it), so
it is not safe to make another access call.  So, the second thread would
use g_topology_sleep() to wait until the flag is cleared and only then
would it proceed with the access.

Also see http://docs.freebsd.org/cgi/mid.cgi?809d9254-ee56-59d8-69a4-08838e985cea

PR:		225960
Reported by:	asomers
Reviewed by:	markj, mav
MFC after:	3 weeks
Differential Revision: https://reviews.freebsd.org/D14533
2018-03-15 09:16:10 +00:00
cem
b3df8f9162 g_part_gpt: Fix memory leak in error path
If g_part_gpt_read() encountered a disk with bad primary and secondary
tables, it could leak memory.

Reported by:	Coverity
Sponsored by:	Dell EMC Isilon
2018-03-07 01:55:50 +00:00
cem
cb133ff0b2 g_label_ufs: Fix typo from r330264
Reported by:	O. Hartmann <o.hartmann AT walstatt.org>
Sponsored by:	Dell EMC Isilon
2018-03-02 06:02:54 +00:00
mckusick
ea7d671565 This change is some refactoring of Mark Johnston's changes in r329375
to fix the memory leak that I introduced in r328426. Instead of
trying to clear up the possible memory leak in all the clients, I
ensure that it gets cleaned up in the source (e.g., ffs_sbget ensures
that memory is always freed if it returns an error).

The original change in r328426 was a bit sparse in its description.
So I am expanding on its description here (thanks cem@ and rgrimes@
for your encouragement for my longer commit messages).

In preparation for adding check hashing to superblocks, r328426 is
a refactoring of the code to get the reading/writing of the superblock
into one place. Unlike the cylinder group reading/writing which
ends up in two places (ffs_getcg/ffs_geom_strategy in the kernel
and cgget/cgput in libufs), I have the core superblock functions
just in the kernel (ffs_sbfetch/ffs_sbput in ffs_subr.c which is
already imported into utilities like fsck_ffs as well as libufs to
implement sbget/sbput). The ffs_sbfetch and ffs_sbput functions
take a function pointer to do the actual I/O for which there are
four variants:

    ffs_use_bread / ffs_use_bwrite for the in-kernel filesystem

    g_use_g_read_data / g_use_g_write_data for kernel geom clients

    ufs_use_sa_read for the standalone code (stand/libsa/ufs.c
	but not stand/libsa/ufsread.c which is size constrained)

    use_pread / use_pwrite for libufs

Uses of these interfaces are in the UFS filesystem, geoms journal &
label, libsa changes, and libufs. They also permeate out into the
filesystem utilities fsck_ffs, newfs, growfs, clri, dump, quotacheck,
fsirand, fstyp, and quot. Some of these utilities should probably be
converted to directly use libufs (like dumpfs was for example), but
there does not seem to be much win in doing so.

Tested by: Peter Holm (pho@)
2018-03-02 04:34:53 +00:00
markj
65a57d392f Fix a memory leak introduced in r328426.
ffs_sbget() may return a superblock buffer even if it fails, so the
caller must be prepared to free it in this case. Moreover, when tasting
alternate superblock locations in a loop, ffs_sbget()'s readfunc
callback must free the previously allocated buffer.

Reported and tested by:	pho
Reviewed by:		kib (previous version)
Differential Revision:	https://reviews.freebsd.org/D14390
2018-02-16 15:41:03 +00:00
asomers
605b141a34 gpart: append partition name to the underlying provider's physical path
If the underlying provider's physical path is null, then the gpart device's
physical path will be, too. Otherwise, it will append the partition name,
such as "/p1" or "/s1/a". This will make gpart work better with zfsd(8).

PR:		224965
MFC after:	3 weeks
Differential Revision:	https://reviews.freebsd.org/D14010
2018-02-14 20:26:09 +00:00
asomers
d2bcc1e2c3 geli: append "/eli" to the underlying provider's physical path
If the underlying provider's physical path is null, then the geli device's
physical path will be, too. Otherwise, it will append "/eli".  This will make
geli work better with zfsd(8).

PR:		224962
MFC after:	3 weeks
Differential Revision:	https://reviews.freebsd.org/D13979
2018-02-14 20:15:32 +00:00
jhibbits
eaa64c96ea Fix a panic introduced in r329225
Some GEOM partition tables may be destroyed with incomplete partition
entries.  Guard against this with NULL checks.

Reported by:	pholm,others
Reviewed by:	markj
Tested by:	pholm
2018-02-14 15:12:09 +00:00