osdc/Objecter: unwatch is a mutation, not a read

This was causing librados to unblock after the ACK on unwatch, which meant
that librbd users raced and tried to delete the image before the unwatch
change was committed..and got EBUSY.  See #3958.

The watch operation has a similar problem.

Signed-off-by: Sage Weil <sage@inktank.com>
(cherry picked from commit fea77682a6cf9c7571573bc9791c03373d1d976d)

Conflicts:

	src/librados/IoCtxImpl.cc
osd: make watch OSDOp print sanely

Signed-off-by: Sage Weil <sage@inktank.com>
(cherry picked from commit de4fa95f03b99a55b5713911c364d7e2a4588679)
objecter: separate out linger_read() and linger_mutate()

A watch is a mutation, while a notify is a read.  The mutations need to
pass in a proper snap context to be fully correct.

Also, make the WRITE flag implicit so the caller doesn't need to pass it
in.

Signed-off-by: Sage Weil <sage@inktank.com>
(cherry picked from commit 6c08c7c1c6d354d090eb16df279d4b63ca7a355a)
objecter: initialize linger op snapid

Since they are write ops now, it must be CEPH_NOSNAP or the OSD
returns EINVAL.

Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
(cherry picked from commit 15bb9ba9fbb4185708399ed6deee070d888ef6d2)
common: add lockers for RWLocks

This makes them easier to use, especially instead of existing mutexes.

Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
(cherry picked from commit e0f8e5a80d6d22bd4dee79a4996ea7265d11b0c1)
librbd: use rwlocks instead of mutexes for several fields

Image metadata like snapshots, size, and parent is frequently read,
but rarely updated. During flatten, we were depending on the parent
lock to prevent the parent ImageCtx from disappearing out from under
us while we read from it. The copy-up path also needed the parent lock
to be able to read from the parent image, which lead to a deadlock.

Convert parent_lock, snap_lock, and md_lock to RWLocks, and change
their use to read instead of exclusive locks where appropriate. The
main place exclusive locks are needed is in ictx_refresh, so this is
pretty simple. This fixes the deadlock, since parent_lock is only
needed for read access in both flatten and the copy-up operation.

cache_lock and refresh_lock are only really used for exclusive access,
so leave them as regular mutexes.

One downside to this is that there's no way to assert is_locked()
for RWLocks, so we'll have to be very careful about changing code
in the future.

Fixes: #3665
Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
(cherry picked from commit 995ff0e3eaa560b242da8c019a2e11e735e854f7)
librbd: make sure racing flattens don't crash

The only way for a parent to disappear is a racing flatten completing,
or possibly in the future the image being forcibly removed. In either
case, continuing to flatten makes no sense, so stop early.

Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
(cherry picked from commit a1ae8562877d1b902918e866a1699214090c40bd)
Merge branch 'wip-4249' into wip-4249-master

Make snap_rollback() only take a read lock on snap_lock, since
it does not modify snapshot-related fields.
Conflicts:
	src/librbd/internal.cc
(cherry picked from commit db5fc2270f91aae220fc3c97b0c62e92e263527b)
librbd: fix rollback size

The duplicate calls to get_image_size() and get_snap_size() replaced
by 5806226cf0743bb44eaf7bc815897c6846d43233 uncovered this. The first
call was using the currently set snap_id instead of the snapshot being
rolled back to.

Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
(cherry picked from commit d6c126e2131fefab6df676f2b9d0addf78f7a488)
test_rbd: move flatten tests back into TestClone

They need the same setup, and it's easy enough to run specific
subtests. Making them a separate subclass accidentally duplicated
tests from TestClone.

Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
(cherry picked from commit 9c693d7e8312026f6d8d9586381b026ada35d808)
ObjectCacher: keep track of outstanding reads on an object

Reads always use C_ReadFinish as a callback (and they are the only
user of this callback). Keep an xlist of these for each object, so
they can remove themselves as they finish. To prevent racing requests
and with discard removing objects from the cache, clear the xlist in
the object destructor, so if the Object is still valid the set_item
will still be on the list.

Make the ObjectCacher constructor take an Object* instead of the pool
and object id, which are derived from the Object* anyway.

Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
(cherry picked from commit f6f876fe51e40570596c25ac84ba3689f72776c2)
ObjectCacher: add a method to clear -ENOENT caching

Clear the exists and complete flags for any objects that have exists
set to false, and force any in-flight reads to retry if they get
-ENOENT instead of generating zeros.

This is useful for getting the cache into a consistent state for rbd
after an image has been flattened, since many objects which previously
did not exist and went up to the parent to retrieve data may now exist
in the child.

Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
(cherry picked from commit f2a23dc0b092c5ac081893e8f28c6d4bcabd0c2e)
librbd: invalidate cache when flattening

The cache stores which objects don't exist. Flatten bypasses the cache
when doing its copyups, so when it is done the -ENOENT from the cache
is treated as zeroes instead of 'need to read from parent'.

Clients that have the image open need to forgot about the cached
non-existent objects as well. Do this during ictx_refresh, while the
parent_lock is held exclusively so no new reads from the parent can
happen until the updated parent metadata is visible, so no new reads
from the parent will occur.

Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
(cherry picked from commit 46e8fc00b2dc8eb17d8777b6ef5ad1cfcc389cea)
librbd: optionally wait for a flush before enabling writeback

Older guests may not send flushes properly (i.e. never), so if this is
enabled, rbd_cache=true is safe for them transparently.

Disable by default, since it will unnecessarily slow down newer guest
boot, and prevent writeback caching for things that don't need to send
flushes, like the command line tool.

Refs: #3817
Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
(cherry picked from commit 1597b3e3a1d776b56e05c57d7c3de396f4f2b5b2)
librbd: flush cache when set_snap() is called

If there are writes pending, they should be sent while the image
is still writeable. If the image becomes read-only, flushing the
cache will just mark everything dirty again due to -EROFS.

Fixes: #4525
Backport: bobtail
Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
(cherry picked from commit 613b7085bb48cde1e464b7a97c00b8751e0e917f)
ObjectCacher: optionally make writex always non-blocking

Add a callback argument to writex, and a finisher to run the
callbacks. Move the check for dirty+tx > max_dirty into a helper that
can be called from a wrapper around the callbacks from writex, or from
the current place in _wait_for_write().

Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
(cherry picked from commit c21250406eced8e5c467f492a2148c57978634f4)
librbd: make aio_writes to the cache always non-blocking by default

When the ObjectCacher's writex blocks, it affects the thread requesting
the aio, which can cause starvation for other I/O when used by QEMU.

Preserve the old behavior via a config option in case this has any
bad side-effects, like too much memory usage under heavy write loads.

Fixes: #4091
Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
(cherry picked from commit 03ac01fa6a94fa7a66ede057e9267e0a562c3cdb)
objectcacher: Remove commit_set, use flush_set

commit_set() and flush_set() are identical in functionality,
so use flush_set everywhere and remove commit_set from
the code.

Also fixes a bug in flush_set where the finisher context was
getting freed twice if no objects needed to be flushed.

Signed-off-by: Sam Lang <sam.lang@inktank.com>
(cherry picked from commit 72147fd3a1da8ecbcb31ddf6b66a158d71933909)
ObjectCacher: fix flush_set when no flushing is needed

C_GatherBuilder takes ownership of the Context we pass it. Deleting it
in flush_set after constructing the C_GatherBuilder results in a
double delete.

Fixes: #3946
Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
Reviewed-by: Sam Lang <sam.lang@inktank.com>
(cherry picked from commit 3bc21143552b35698c9916c67494336de8964d2a)
ObjectCacher: always complete flush_set() callback

This removes the last remnants of
b5e9995f59d363ba00d9cac413d9b754ee44e370. If there's nothing to flush,
immediately call the callback instead of deleting it. Callers were
assuming they were responsible for completing the callback whenever
flush_set() returned true, and always called complete(0) in this
case. Simplify the interface and just do this in flush_set(), so that
it always calls the callback.

Since C_GatherBuilder deletes its finisher if there are no subs,
only set its finisher when subs are present. This way we can still
call ->complete() for the callback.

Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
(cherry picked from commit 92db06c05dc2cad8ed31648cb08866781aee2855)

Conflicts:

	src/client/Client.cc