Merge pull request #41222 from tchaikov/wip-crimson-cleanups

crimson/os: cleanups

Reviewed-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
Reviewed-by: Xuehan Xu <xxhdx1985126@gmail.com>

Merge pull request #41223 from rzarzynski/wip-crimson-alienstore-sighup

crimson/alienstore: block SIGHUP to coexist with Seastar's signal handling

Reviewed-by: Kefu Chai <kchai@redhat.com>

Merge pull request #41177 from dsavineau/cv_remove_legacy_release_check

ceph-volume: remove legacy release check

Merge pull request #41178 from dsavineau/cv_tox_py3

ceph-volume: remove duplicate py3 env

Merge pull request #40016 from neha-ojha/wip-default-mclock

use mclock_scheduler as the default scheduler

Reviewed-by: Josh Durgin <jdurgin@redhat.com>
Reviewed-by: Sridhar Seshasayee <sseshasa@redhat.com>
Reviewed-by: Samuel Just <sjust@redhat.com>
Reviewed-by: Sunny Kumar <sunkumar@redhat.com>

crimson/alienstore: block SIGHUP to coexist with Seastar's signal handling.

In `crimson/osd/main.cc` we instruct Seastar to handle `SIGHUP`.

```
        // just ignore SIGHUP, we don't reread settings
        seastar::engine().handle_signal(SIGHUP, [] {})
```

This happens using the Seastar's signal handling infrastructure
which is incompliant with the alien world.

```
void
reactor::signals::handle_signal(int signo, noncopyable_function<void ()>&& handler) {
    // ...
    struct sigaction sa;
    sa.sa_sigaction = [](int sig, siginfo_t *info, void *p) {
        engine()._backend->signal_received(sig, info, p);
    };
    // ...
}
```

```
 extern __thread reactor* local_engine;
extern __thread size_t task_quota;

inline reactor& engine() {
    return *local_engine;
}
```

The low-level signal handler above assumes `local_engine._backend`
is not null which stays true only for threads from the S*'s world.
Unfortunately, as we don't block the `SIGHUP` for alien threads,
kernel is perfectly authorized to pick up one them to run the handler
leading to weirdly-looking segfaults like this one:

```
INFO  2021-04-23 07:06:57,807 [shard 0] bluestore - stat
DEBUG 2021-04-23 07:06:58,753 [shard 0] ms - [osd.1(client) v2:172.21.15.100:6802/30478@51064 >> mgr.4105 v2:172.21.15.109:6800/29891] --> #7 === pg_stats(0 pgs seq 55834574872 v 0) v2 (87)
...
INFO  2021-04-23 07:06:58,813 [shard 0] bluestore - stat
DEBUG 2021-04-23 07:06:59,753 [shard 0] osd - AdminSocket::handle_client: incoming asok string: {"prefix": "get_command_descriptions"}
INFO  2021-04-23 07:06:59,753 [shard 0] osd - asok response length: 2947
INFO  2021-04-23 07:06:59,817 [shard 0] bluestore - stat
DEBUG 2021-04-23 07:06:59,865 [shard 0] osd - AdminSocket::handle_client: incoming asok string: {"prefix": "get_command_descriptions"}
INFO  2021-04-23 07:06:59,866 [shard 0] osd - asok response length: 2947
DEBUG 2021-04-23 07:07:00,020 [shard 0] osd - AdminSocket::handle_client: incoming asok string: {"prefix": "get_command_descriptions"}
INFO  2021-04-23 07:07:00,020 [shard 0] osd - asok response length: 2947
INFO  2021-04-23 07:07:00,820 [shard 0] bluestore - stat
...
Backtrace:
 0# 0x00005600CD0D6AAF in ceph-osd
 1# FatalSignal::signaled(int) in ceph-osd
 2# FatalSignal::install_oneshot_signal_handler<11>()::{lambda(int, siginfo_t*, void*)#1}::_FUN(int, siginfo_t*, void*) in ceph-osd
 3# 0x00007F5877C7EB20 in /lib64/libpthread.so.0
 4# 0x00005600CD830B81 in ceph-osd
 5# 0x00007F5877C7EB20 in /lib64/libpthread.so.0
 6# pthread_cond_timedwait in /lib64/libpthread.so.0
 7# crimson::os::ThreadPool::loop(std::chrono::duration<long, std::ratio<1l, 1000l> >, unsigned long) in ceph-osd
 8# 0x00007F5877999BA3 in /lib64/libstdc++.so.6
 9# 0x00007F5877C7414A in /lib64/libpthread.so.0
10# clone in /lib64/libc.so.6
daemon-helper: command crashed with signal 11
```

Ultimately, it turned out the thread came out from a syscall (`futex`)
and started crunching the `SIGHUP` handler's code in which a nullptr
dereference happened.

This patch blocks `SIGHUP` for all threads spawned by `AlienStore`.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>

crimson/os: use this explicitly

to silence the warning from clang. it fails to figure out that this is
actually used, and complains that this is captured but not used.

Signed-off-by: Kefu Chai <kchai@redhat.com>

crimson/os/seastore: use map::merge() to merge maps

C++17's std::map allows us to merge two maps, and in this case, we can
even consume `child_result`. so map::merge() is used instead of insert()
in hope to avoid the memcpy and allocation of pair<> nodes.

Signed-off-by: Kefu Chai <kchai@redhat.com>

crimson.os/seastore: do not capture unused variables

Signed-off-by: Kefu Chai <kchai@redhat.com>

crimson/os/seastore: do not start identifier with "__"

avoid starting identifiers with two underscores, these names are
reserved for C/C++ compiler and standard library.

Signed-off-by: Kefu Chai <kchai@redhat.com>

Merge pull request #41217 from petrutlucian94/boost_url

win*.sh,cmake: Fix Windows build

Reviewed-by: Kefu Chai <kchai@redhat.com>

Merge pull request #41129 from athanatos/sjust/wip-seastore-osd

crimson: add initial osd support for seastore

Reviewed-by: Chunmei Liu <chunmei.liu@intel.com>
Reviewed-by: Yingxin Cheng <yingxin.cheng@intel.com>
Reviewed-by: Kefu Chai <kchai@redhat.com>

Merge pull request #41215 from CloudFerro/new_boost_url

cmake: Replace boost download url

Reviewed-by: Kefu Chai <kchai@redhat.com>

win*.sh,cmake: Fix Windows linking errors

The Windows build is hitting linking errors after
bumping the Boost version to 1.75. The issue is that Boost
is now setting the zlib dependecy using INTERFACE_LINK_LIBRARIES,
which means that it's no longer located using the standard
"find_package" mechanism.

In order for the linker to locate zlib, we'll add it to the
linker search path.

[1] https://github.com/boostorg/boost_install/issues/47

Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>

cmake: Replace boost download url

Boost has moved downloads to JFrog Artifactory
https://www.boost.org/users/news/boost_has_moved_downloads_to_jfr.html

Signed-off-by: Rafał Wądołowski <rwadolowski@cloudferro.com>

crimson/tools/store-nbd: fix help message for path

Signed-off-by: Samuel Just <sjust@redhat.com>

seastore: add comment to do_transaction outlining ordering TODO

Signed-off-by: Samuel Just <sjust@redhat.com>

crimson/os/seastore/segment_manager/block: create block device during mkfs

Signed-off-by: Samuel Just <sjust@redhat.com>

crimson/os/seastore: refactor segment_cleaner to init segment_manager params after mount

Signed-off-by: Samuel Just <sjust@redhat.com>

vstart.sh: add --seastore

Signed-off-by: Samuel Just <sjust@redhat.com>

crimson/os/seastore: add seastore to FuturizedStore::create

Signed-off-by: Samuel Just <sjust@redhat.com>

crimson/os/seastore/segment_manager/block: DSYNC not needed

We are expressly flushing, so this shouldn't be needed.

Signed-off-by: Samuel Just <sjust@redhat.com>

crimson/os/seastore: refactor SegmentManager reference ownership

Gives SeaStore ownership over SegmentManager and rearranges mkfs/mount.
Replaces mkfs_config_t/mount_config_t with config params.

Signed-off-by: Samuel Just <sjust@redhat.com>

test/crimson/seastore/transaction_manager_test_state: remove duplicate setup

Signed-off-by: Samuel Just <sjust@redhat.com>

test/crimson/gtest_seastar: init config and perf counters for crimson tests

Signed-off-by: Samuel Just <sjust@redhat.com>

crimson/tools/store-nbd: don't use detailed space tracker

Intended for debugging rather than performance testing.

Signed-off-by: Samuel Just <sjust@redhat.com>

crimson/os/seastore/journal: close open segment and reset soft state in close()

Signed-off-by: Samuel Just <sjust@redhat.com>

crimson/os/journal: use SegmentManager methods for block size, etc

Otherwise, initing those values needs to be done after the SegmentManager
instance is mounted.  This is simpler for now.

Signed-off-by: Samuel Just <sjust@redhat.com>

crimson/os/seastore: only create handle in create_new_collection

FuturizedStore::create_new_collection isn't supposed to actually
create the collection.  See OSD::mkfs for a usage example.

Signed-off-by: Samuel Just <sjust@redhat.com>

crimson/os/seastore: clean up meta implementation

There's really no reason to cache the decoded representation here since
the meta keys are only accessed during startup, mkfs.  This approach is
much simpler.

Signed-off-by: Samuel Just <sjust@redhat.com>

crimson/.../transaction_manager: skip zero mappings in mount

Signed-off-by: Samuel Just <sjust@redhat.com>

crimson/os/seastore: fix read() to use onode.size for len=0

Signed-off-by: Samuel Just <sjust@redhat.com>

crimson/os/seastore: fix do_transaction -- transactions may be empty

Signed-off-by: Samuel Just <sjust@redhat.com>

crimson/os/seastore: wire up get_fsid

Signed-off-by: Samuel Just <sjust@redhat.com>

crimson/os/seastore: wire up stat

Signed-off-by: Samuel Just <sjust@redhat.com>

crimson/os/seastore/seastore.cc: update to use new debug macros

Signed-off-by: Samuel Just <sjust@redhat.com>

crimson/os/seastore: convert cache and transaction_manager to use new debugging macros

The goal here is to capture the transaction address and to standardize the
prefix format.

Signed-off-by: Samuel Just <sjust@redhat.com>

win32*.sh: ensure that the build dir exists

The Windows build scripts try to use the build dir before
actually creating it.

We'll have to move up the "mkdir" command a few lines.

Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>

Merge pull request #41214 from tchaikov/wip-crimson-clang-cleanups

crimson: clang related cleanups

Reviewed-by: Yingxin Cheng <yingxin.cheng@intel.com>

crimson/common/config_proxy: add a helper for get_val<>()

otherwise we have to put something like

local_conf().template get_val<T>(name)

which is not quite convenient or readable.

Signed-off-by: Kefu Chai <kchai@redhat.com>

crimson/os/seastore: do not redefine default argument

we should not redefine a default argument of a method of templated class.

this change also address following error from clang:

../src/crimson/os/seastore/onode_manager/staged-fltree/node.cc:621:30: error: template parameter redefines default argument
template <bool FORCE_MERGE = false>
                             ^
../src/crimson/os/seastore/onode_manager/staged-fltree/node.h:438:32: note: previous default template argument defined here
  template <bool FORCE_MERGE = false>
                               ^

Signed-off-by: Kefu Chai <kchai@redhat.com>

crimson/os/seastore: do not capture non-variables

merge_stage and merge_size are structured bindings, they are not
variables. so cannot be captured without defining variables in
capture list.

Signed-off-by: Kefu Chai <kchai@redhat.com>

crimson/os/seastore: do not capture unused variable

Signed-off-by: Kefu Chai <kchai@redhat.com>

Merge pull request #41211 from neha-ojha/wip-remove-mon-election

qa/suites/rados/standalone: remove mon_election symlink

Reviewed-by: Kefu Chai <kchai@redhat.com>
Reviewed-by: Josh Durgin <jdurgin@redhat.com>

qa/suites/rados/standalone: remove mon_election symlink

The standalone tests need parameters to be passed as ceph_args to
override defaults.

This was just doubling the number of standalone tests being run in each rados
run with no effect!

Signed-off-by: Neha Ojha <nojha@redhat.com>

Merge PR #41201 into master

* refs/pull/41201/head:
doc/releases: 16.2.3

Reviewed-by: Neha Ojha <nojha@redhat.com>
Reviewed-by: David Galloway <dgallowa@redhat.com>

Merge PR #41179 into master

* refs/pull/41179/head:
qa/tasks/cephadm_cases: longer wait for osd to start

Reviewed-by: Sebastian Wagner <swagner@suse.com>

doc/releases: 16.2.3

Signed-off-by: Sage Weil <sage@newdream.net>

qa/suites/rados/mgr/tasks/progress: use high_recovery_ops for faster recovery

Signed-off-by: Sridhar Seshasayee <sseshasa@redhat.com>

PendingReleaseNotes: mclock_scheduler is the default scheduler for quincy

Signed-off-by: Neha Ojha <nojha@redhat.com>

qa/standalone: use osd op queue = wpq

mclock_scheduler is now the default and some of these tests need to be modified
to run well with it. Continue using wpq until
https://tracker.ceph.com/issues/50574 is addressed.

Signed-off-by: Neha Ojha <nojha@redhat.com>

common/options/global.yaml.in: use mclock_scheduler as the default scheduler

The aim is to default to mclock_scheduler in quincy, so start early and
get more testing.

Signed-off-by: Neha Ojha <nojha@redhat.com>

Merge pull request #41192 from petrutlucian94/boost_url

win32*.sh: Update boost URL, fix windows build

Reviewed-by: Kefu Chai <kchai@redhat.com>

Merge pull request #41198 from tchaikov/wip-doc-mgr-options

doc/_ext: load options defined by mgr modules

Reviewed-by: Josh Durgin <jdurgin@redhat.com>

Merge PR #41163 into master

* refs/pull/41163/head:
doc/releases/releases.yml: add 16.2.2
doc/releases/pacific: note cepahdm ingress+rgw support
doc: 16.2.2 Release Notes

Reviewed-by: Josh Durgin <jdurgin@redhat.com>
Reviewed-by: Neha Ojha <nojha@redhat.com>
Reviewed-by: Sage Weil <sage@redhat.com>
Reviewed-by: Patrick Donnelly <pdonnell@redhat.com>

Merge PR #41183 into master

* refs/pull/41183/head:
cephadm: normalize image digest in 'ls' output too

Reviewed-by: Juan Miguel Olmo <jolmomar@redhat.com>
Reviewed-by: Michael Fritch <mfritch@suse.com>
Reviewed-by: Sebastian Wagner <swagner@suse.com>

Merge pull request #40311 from rhcs-dashboard/refactor-crushmap-component

mgr/dashboard: avoid data processing in crush-map component

Reviewed-by: Aashish Sharma <aasharma@redhat.com>
Reviewed-by: Alfonso Martínez <almartin@redhat.com>
Reviewed-by: Ernesto Puerta <epuertat@redhat.com>

Merge PR #41107 into master

* refs/pull/41107/head:
mgr/cephadm: apply hostname/addr checks to 'orch host set-addr' too
mgr/cephadm: make 'host add' idempotent
mgr/cephadm: set host crush location based on HostSpec
python-common: add location property to HostSpec, + tests

Reviewed-by: Juan Miguel Olmo <jolmomar@redhat.com>

doc/_ext: rewrite directive using ObjectDescription

which allows us to use different scheme when defining an option,

without this change, if two options in different mgr module share the
same name we cannot differentiate them, after this change, their id
would prefixed with the module name.

Signed-off-by: Kefu Chai <kchai@redhat.com>

doc/rados/configuration/general-config-ref: use describe to define non-options

max_open_files is not generated from yaml files, so define it using
"describe" directive instead.

Signed-off-by: Kefu Chai <kchai@redhat.com>

doc/_ext: load options defined by mgr modules

Signed-off-by: Kefu Chai <kchai@redhat.com>

win32*.sh: bump boost version to 1.75

We're updating the Windows build scripts to use Boost 1.75, the same
as the Linux build.

We can now drop the custom Boost patches that were needed in order
to use mingw. Those have been merged upstream in the meantime.

Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>

cephadm: normalize image digest in 'ls' output too

We were previously only normalizing the 'inspect' output.

Signed-off-by: Sage Weil <sage@newdream.net>

Merge pull request #41186 from zdover23/wip-doc-cephadm-upgrade-starting-the-upgrade-2021-05-06

doc/cephadm: rewriting "starting the upgrade"

Reviewed-by: Juan Miguel Olmo Martínez <jolmomar@redhat.com>

Merge pull request #41187 from zdover23/wip-doc-cephadm-upgrade-monitoring-the-upgrade-2021-05-06

doc/cephadm: rewrite "monitoring the upgrade"

Reviewed-by: Juan Miguel Olmo Martínez <jolmomar@redhat.com>

Merge pull request #41188 from zdover23/wip-doc-cephadm-upgrade-cancelling-the-upgrade-2021-05-06

doc/cephadm: rewrite "cancelling the upgrade"

Reviewed-by: Sebastian Wagner <sewagner@redhat.com>

Merge pull request #41189 from zdover23/wip-doc-cephadm-upgrade-potential-problems-2021-05-06

doc/cephadm: rewrite "potential problems"

Reviewed-by: Juan Miguel Olmo Martínez <jolmomar@redhat.com>

Merge pull request #41190 from zdover23/wip-doc-cephadm-using-custom-containers-2021-05-06

doc/cephadm: rewrite "using customized con..."

Reviewed-by: Juan Miguel Olmo Martínez <jolmomar@redhat.com>

Merge pull request #41050 from rhcs-dashboard/fix-bucket-versioning-edit

mgr/dashboard: fix bucket versioning when locking is enabled

Reviewed-by: Aashish Sharma <aasharma@redhat.com>
Reviewed-by: Alfonso Martínez <almartin@redhat.com>

Merge pull request #41126 from idryomov/wip-rbd-qemu-iotests-focal

qa/workunits/rbd: use bionic version of qemu-iotests for focal

Reviewed-by: Sunny Kumar <sunkumar@redhat.com>

mgr/dashboard: avoid data processing in crush-map component

Fixes: https://tracker.ceph.com/issues/49236
Signed-off-by: Avan Thakkar <athakkar@redhat.com>

win32*.sh: improve logging

We'll use a log file when building the Ceph dependencies. Also, we'll
log some details about the operation that's about to be performed.

Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>

win32*.sh: fix boost url

The boost download location has changed, breaking the Windows build.
Ths commit updates the boost download url.

Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>

doc/mgr/modules: should use Option class for defining options

it is the encouraged way to define options nowadays.

Signed-off-by: Kefu Chai <kchai@redhat.com>

doc/cephadm: rewrite "using customized con..."

This PR rewrites the text in "Using Customized
Container Images" so that it is just a bit
clearer, and it also formats the prompt in the
text correctly.

Signed-off-by: Zac Dover <zac.dover@gmail.com>

doc/cephadm: rewrite "potential problems"

This PR adds correct prompts and slightly
reduces the cognitive load of the reader.

Signed-off-by: Zac Dover <zac.dover@gmail.com>

doc/cephadm: rewrite "cancelling the upgrade"

This PR corrects a single prompt in the section
"Cancelling the Upgrade".

Signed-off-by: Zac Dover <zac.dover@gmail.com>

doc/cephadm: rewrite "monitoring the upgrade"

This PR adds correctly-formatted prompts and
makes slight alterations to the readability
of the text.

Signed-off-by: Zac Dover <zac.dover@gmail.com>

doc/cephadm: rewriting "starting the upgrade"

This PR fixes the prompts in this section and
makes the sentences a tiny bit less onerous for
the reader.

Signed-off-by: Zac Dover <zac.dover@gmail.com>

doc/releases/releases.yml: add 16.2.2

Signed-off-by: Sage Weil <sage@newdream.net>

Merge pull request #40937 from MrFreezeex/snap-remove-fix

librbd/mirror/snapshot: avoid UnlinkPeerRequest with a unlinked peer

Reviewed-by: Mykola Golub <mgolub@suse.com>
Reviewed-by: Ilya Dryomov <idryomov@gmail.com>

Merge PR #41148 into master

* refs/pull/41148/head:
Add Rocky Linux to supported DISTRO_NAMES

Reviewed-by: Sage Weil <sage@redhat.com>

Merge PR #41147 into master

* refs/pull/41147/head:
ceph_test_rados_api_service: stop threads before asserting
ceph_test_rados_api_service: more retries for servicemkap

Reviewed-by: Deepika Upadhyay <dupadhya@redhat.com>
Reviewed-by: Neha Ojha <nojha@redhat.com>

Merge pull request #40551 from alimaredia/wip-rgw-dpp-update-master

rgw,test: Add even more DoutPrefixProvider's to the RGW codebase

Reviewed-by: Daniel Gryniewicz <dang@redhat.com>
Reviewed-by: J. Eric Ivancich <ivancich@redhat.com>

Merge pull request #40063 from rhcs-dashboard/cyclomatic-complexity

mgr/dashboard: reduce cyclomatic complexity

Reviewed-by: Avan Thakkar <athakkar@redhat.com>
Reviewed-by: Ernesto Puerta <epuertat@redhat.com>

Merge pull request #40715 from pcuzner/pool-overview-enhancement

mgr/dashboard:include compression stats on pool dashboard

Reviewed-by: Avan Thakkar <athakkar@redhat.com>
Reviewed-by: Ernesto Puerta <epuertat@redhat.com>
Reviewed-by: Nizamudeen A <nia@redhat.com>

qa/tasks/cephadm_cases: longer wait for osd to start

Signed-off-by: Sage Weil <sage@newdream.net>

Merge PR #41149 into master

* refs/pull/41149/head:
mgr/cephadm: remove haproxy+keepalived container image from service spec

Reviewed-by: Adam King <adking@redhat.com>

Merge pull request #41081 from votdev/issue_49718_disable_nfs_v3

mgr/dashboard: disable NFSv3 support in dashboard

Reviewed-by: Alfonso Martínez <almartin@redhat.com>
Reviewed-by: Alexandra Settle <asettle@suse.com>
Reviewed-by: Ernesto Puerta <epuertat@redhat.com>
Reviewed-by: Michael Fritch <mfritch@suse.com>

ceph-volume: remove legacy release check

Those conditions don't make sense anymore so we can remove them.

Signed-off-by: Dimitri Savineau <dsavinea@redhat.com>

ceph-volume: remove duplicate py3 env

We don't need to have py36 and py3 env in the tox configuration.
Currently we're running pytest twice on the same python 3 release.
Using py3 only is more generic and reflects was is defined in the
other tox.ini file for other components.

Signed-off-by: Dimitri Savineau <dsavinea@redhat.com>

Merge pull request #41133 from nh2/patch-1

cephfs.pyx: Fix docstring of get_layout

Reviewed-by: Kefu Chai <kchai@redhat.com>

doc/releases/pacific: note cepahdm ingress+rgw support

Signed-off-by: Sage Weil <sage@newdream.net>

mgr/dashboard: fix bucket versioning when locking is enabled

Fixes: https://tracker.ceph.com/issues/50545
Signed-off-by: Avan Thakkar <athakkar@redhat.com>

pybind/cephfs: Fix docstring of get_layout

It was copy-pasted from another function.
Fixed by copy-pasting the docstring from the C docs.

Signed-off-by: Niklas Hambuechen <mail@nh2.me>

Merge pull request #41165 from tchaikov/wip-cephfs-shell-manpage

doc/man: build and install cephfs-shell manpage

Reviewed-by: Varsha Rao <varao@redhat.com>
Reviewed-by: Patrick Donnelly <pdonnell@redhat.com>

Merge pull request #40977 from xxhdx1985126/wip-seastore-xattr

crimson/os/seastore: formal extended attributes support

Reviewed-by: Samuel Just <sjust@redhat.com>
Reviewed-by: Kefu Chai <kchai@redhat.com>

Merge pull request #41109 from markhpc/wip-bluefs_buffered_io-desc

src/common: Update bluefs_buffered_io descrption.

Reviewed-by: Neha Ojha <nojha@redhat.com>
Reviewed-by: Josh Durgin <jdurgin@redhat.com>

doc/man: build and install cephfs-shell manpage

otherwise the manpage of cephfs-shell is not built and installed.

Signed-off-by: Kefu Chai <kchai@redhat.com>

Merge pull request #41142 from tchaikov/wip-cmake-global

cmake: let libglobal_obj depend on legacy-option-headers

Reviewed-by: Patrick Donnelly <pdonnell@redhat.com>