rbd: fix leak of snapshots during initial probe

When an rbd image is initially mapped, its snapshot context is
collected, and then a list of snapshot entries representing the
snapshots in that context is created.  The list is created using
rbd_dev_snaps_update().  (This function also supports updating an
existing snapshot list based on a new snapshot context.)

If an error occurs, updating the list is aborted, and the list is
currently left as-is, in an inconsistent state.  At that point,
there may be a partially-constructed list, but the calling functions
(rbd_dev_probe_finish() from rbd_dev_probe() from rbd_add()) never
clean them up.  So this constitutes a leak.

A snapshot list that is inconsistent with the current snapshot
context is of no use, and might even be actively bad.  So rather
than just having the caller clean it up, have rbd_dev_snaps_update()
just clear out the entire snapshot list in the event an error
occurs.

The other place rbd_dev_snaps_update() is used is when a refresh is
triggered, either because of a watch callback or via a write to the
/sys/bus/rbd/devices/<id>/refresh interface.  An error while
updating the snapshots has no substantive effect in either of those
cases, but one of them issues a warning.  Move that warning to the
common rbd_dev_refresh() function so it gets issued regardless of
how it got initiated.

This is part of:
    http://tracker.ceph.com/issues/4803

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: make snap_size order parameter optional

Only one of the two callers of _rbd_dev_v2_snap_size() needs the
order value returned.  So make that an optional argument--a null
pointer if the caller doesn't need it.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: only update values on snap_info success

Change rbd_dev_v2_snap_info() so it only ever sets values of the
size and features parameters if looking up the snapshot name was
successful.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: rename __rbd_add_snap_dev()

Rename __rbd_add_snap_dev() to be rbd_snap_create().  We no longer
have devices for non-mapped snapshots, and we're not actually
"adding" it to the list in this function, just creating it.

Rename rbd_remove_snap_dev() to be rbd_snap_destroy() for reasons
similar to the above.  Stop having this function delete the snapshot
from its list (to be symmetrical with its create counterpart) and do
that in the caller instead.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: fix leak of format 2 snapshot names

When the snapshot context for an rbd device gets updated (or the
initial one is recorded) a a list of snapshot structures is created
to represent them, one entry per snapshot.  Each entry includes a
dynamically-allocated copy of the snapshot name.

Currently the name is allocated in rbd_snap_create(), as a duplicate
of the passed-in name.

For format 1 images, the snapshot name provided is just a pointer to
an existing name.  But for format 2 images, the passed-in name is
already dynamically allocated, and in the the process of duplicating
it here we are leaking the passed-in name.

Fix this by dynamically allocating the name for format 1 snapshots
also, and then stop allocating a duplicate in rbd_snap_create().

Change rbd_dev_v1_snap_info() so none of its parameters is
side-effected unless it's going to return success.

This is part of:
    http://tracker.ceph.com/issues/4803

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: use rbd_obj_method_sync() return value

Now that rbd_obj_method_sync() returns the number of bytes
returned by the method call, that value should be used by
callers to ensure we don't overrun the valid portion of the
buffer.

Fix the two spots that remained that weren't doing that,
rbd_dev_image_name() and rbd_dev_v2_snap_name().

Rearrange the error path slightly in rbd_dev_v2_snap_name().

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: avoid dropping extra reference in rbd_free_disk()

I found during some failure injection testing that the call to
rbd_free_disk() in the error path of rbd_dev_probe_finish() was
dropping an extra reference to the disk queue.  The problem
occurred when put_disk tried to drop a reference to the disk's
queue.  A call to blk_cleanup_queue() just prior to that will have
also dropped a reference to the queue.

The problem is that the reference dropped by put_disk() is assumed
to have been taken by add_disk().  Our code has error paths that can
occur after the disk and its queue are initialized, but before the
call to add_disk(), and in those paths we won't have that extra
reference.

The fix is easy though.  In rbd_free_disk() we're already checking
the disk's GENHD_FL_UP flag.  That flag is an indication that
add_disk() has been called, so just call blk_cleanup_queue()
conditional on that flag being set.

This resolves:
    http://tracker.ceph.com/issues/4800

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: have rbd_dev_image_id() set format 1 image id

Currently, rbd_dev_probe() assumes that any error returned by
rbd_dev_image_id() is most likely -ENOENT, and responds by
calling the format 1 probe routine, rbd_dev_v1_probe().  Then,
at the top of rbd_dev_v1_probe(), an empty string is allocated
for the image id.

This is sort of unbalanced.  Fix this by having rbd_dev_image_id()
look for -ENOENT from its "get_id" method call.  If that is seen,
have it allocate the empty string there rather than depending on
rbd_dev_v1_probe() to do it.

Given that this is effectively defining the format of the image,
set rbd_dev->image_format inside rbd_dev_image_id() rather than in
the format-specific probe routines.

Also drop a redundant hunk of code in rbd_dev_image_id().

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: fix image id leak in initial probe

If a format 2 image id is found for an image being mapped, but the
subsequent probe of the image fails, rbd_dev_probe() quits without
freeing the image id.  Fix that.

Also drop a redundant hunk of code in rbd_dev_image_id().

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: have snap_by_name() return a snapshot

A function called snap_by_name() ought to just look up a snapshot by
name.  It does that, but then it assigns some stuff to the rbd
device structure as well.

Change the function to do just the lookup, and have the caller do
the assignments that follow.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: set snapshot id in rbd_dev_probe_update_spec()

Set the rbd spec's snapshot id for an image getting mapped in
rbd_dev_probe_update_spec() rather than rbd_dev_set_mapping().
This is the more logical place for that to happen (even though
it means we might look up the snapshot by name twice).

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: make rbd spec names pointer to const

Make the names and image id in an rbd_spec be pointers to constant
data.  This required the use of a local variable to hold the
snapshot name in rbd_add_parse_args() to avoid a warning.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: move stripe_unit and stripe_count into header

This commit added fetching if fancy striping parameters:
    09186ddb rbd: get and check striping parameters

They are almost unused, but the two fields storing the information
really belonged in the rbd_image_header structure.

This patch moves them there.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: use rbd_warn(), not WARN_ON()

Change some calls to WARN_ON() so they use rbd_warn() instead, so we
get consistent messaging.  A few remain but they can probably just
go away eventually.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: define rbd snap context routines

Encapsulate the creation of a snapshot context for rbd in a new
function rbd_snap_context_create().  Define rbd wrappers for getting
and dropping references to them once they're created.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: make rbd_dev_destroy() match rbd_dev_create()

Currently, rbd_dev_destroy() does more than just the inverse of what
rbd_dev_create() does.  Stop doing that, and move the two extra
things it does into the three call sites.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: rename rbd_dev_probe()

Rename rbd_dev_probe() to be rbd_dev_image_probe().  Its purpose
will eventually be to probe for the existence of a valid rbd image
for the rbd device--focusing only on the ceph side and not the Linux
device side of initialization.

For now the two "sides" are not fully separated, and this function
is still the entry point for initializing the full rbd device.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: refactor rbd_dev_probe_update_spec()

Fairly straightforward refactoring of rbd_dev_probe_update_spec().
The name is changed to rbd_dev_spec_update().

Rearrange it so nothing gets assigned to the spec until all of the
names have been successfully acquired.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: fix a bug in resizing a mapping

When a snapshot context update occurs, rbd_update_mapping_size() is
called to set the capacity of the disk to record the updated
size of the image in case it has changed.

There's a bug though.  The mapping size is in units of *bytes*.  The
code that updates the mapping size field is assigning a value that
has been scaled down to *sectors*.

Fix that.  Also, check to see if the size has actually changed, and
don't bother updating things (specifically, calling set_capacity())
if it has not.

This resolves:
    http://tracker.ceph.com/issues/4833

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
rbd: fix up some sysfs stuff

This just tweaks a few things in the routines that implement
rbd sysfs files.

All of the entries for an rbd device in /sys/bus/rbd/devices/<id>/
will represent information whose valid values are known by the time
they are accessible.

Right now we get the size of the mapped image by a call to
get_capacity().  There's no need to do this, because that will
return what we last set the capacity to, which is just the size
recorded for the mapping.  So just show that value instead.

We also get this under protection of the header semaphore, in order
to provide a precisely correct value.  This isn't really necessary;
these files are really informational only and it's not necessary to
be so careful.

Finally, print a special value in case the major device number is
not recorded.  Right now that won't matter much but soon the parent
images won't have devices associated with them.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>