kernel/kernel-generic.git
11 years agorbd: get rid of some version parameters
Alex Elder [Tue, 30 Apr 2013 05:44:33 +0000 (00:44 -0500)]
rbd: get rid of some version parameters

Several functions in rbd have parameters meant to allow the version
of an object to be passed in or out.  The purpose of those was to
allow the version of a header object to be maintained, but we no
longer do that.  As a result, these parameters are never actually
needed or used, so get rid of them.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: stop tracking header object version
Alex Elder [Tue, 30 Apr 2013 05:44:32 +0000 (00:44 -0500)]
rbd: stop tracking header object version

The rbd code takes care to maintain the version of the header
object.  This was done in hopes of using it to detect a change in
the object between reading it and setting up a watch request to
be notified of changes.

The mechanism was never fully implemented, however.  And we now
avoid the original problem by setting up the watch request before
ever reading the content of the header.

The osd doesn't interpret the object version supplied with a WATCH
osd op, nor does it use the version supplied with a NOTIFY_ACK op
(we can just supply 0 for both).  There is therefore no need to
maintain the header's object version any more, so stop doing so.

We'll be able to simplify some more rbd code in the next few patches
as a result of this.

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: snap names are pointer to constant data
Alex Elder [Tue, 30 Apr 2013 05:44:33 +0000 (00:44 -0500)]
rbd: snap names are pointer to constant data

Make explicit that snapshot names don't change by making functions
return and take parameters that that point to const qualified data.

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: don't revalidate so much
Alex Elder [Tue, 30 Apr 2013 05:44:32 +0000 (00:44 -0500)]
rbd: don't revalidate so much

Whenever a header object event causes a mapped rbd image to refresh
its header information, revalidate_disk() is being called.  This was
done in rbd_dev_refresh() outside the control mutex in order to
avoid a lock inversion.  Although a an event like this *might*
indicate the image has changed size, most of the time it does not.

Record the image size before and after the refresh, and only
call revalidate_disk() if it changes.

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: fix up the layering warning message
Alex Elder [Tue, 30 Apr 2013 05:44:32 +0000 (00:44 -0500)]
rbd: fix up the layering warning message

A warning gets spewed for any image being probed, including parent
images.  Set up a condition such that the warning message only gets
printed for the image being mapped, not any of its parents.

Also, I didn't like the way the warning ended up being so long.
Make it a terse warning instead.  People experimenting with layering
will know what the message means.

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agoceph: use ceph_create_snap_context()
Alex Elder [Tue, 30 Apr 2013 05:44:32 +0000 (00:44 -0500)]
ceph: use ceph_create_snap_context()

Now that we have a library routine to create snap contexts, use it.

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: create source file "net/ceph/snapshot.c"
Alex Elder [Tue, 30 Apr 2013 05:44:32 +0000 (00:44 -0500)]
libceph: create source file "net/ceph/snapshot.c"

This creates a new source file "net/ceph/snapshot.c" to contain
utility routines related to ceph snapshot contexts.  The main
motivation was to define ceph_create_snap_context() as a common way
to create these structures, but I've moved the definitions of
ceph_get_snap_context() and ceph_put_snap_context() there too.
(The benefit of inlining those is very small, and I'd rather
keep this collection of functions together.)

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: set up devices only for mapped images
Alex Elder [Mon, 29 Apr 2013 04:32:34 +0000 (23:32 -0500)]
rbd: set up devices only for mapped images

Stop setting up Linux devices during the image probe operation.
Instead, set up the devices as a separate step after the image
probe, in rbd_add().

A consequence of this is that only mapped images get devices
assigned to them, which is pretty sweet.

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: don't have device release destroy rbd_dev
Alex Elder [Mon, 29 Apr 2013 04:32:34 +0000 (23:32 -0500)]
rbd: don't have device release destroy rbd_dev

Currently an rbd_device structure gets destroyed from the release
routine for the device embedded within it.  Stop doing that, instead
calling rbd_dev_image_release() right after rbd_bus_del_dev()
wherever the latter is called.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: define rbd_dev_unprobe()
Alex Elder [Mon, 29 Apr 2013 04:32:34 +0000 (23:32 -0500)]
rbd: define rbd_dev_unprobe()

Define a new function rbd_dev_unprobe() which undoes state changes
that occur from calling rbd_dev_v1_probe() or rbd_dev_v2_probe().
Note that this is a superset of rbd_header_free(), which is now
getting removed (it seems to have been used improperly anyway).

Flesh out rbd_dev_image_release() so it undoes exactly what
rbd_dev_image_probe() does.

This means that:
    - rbd_dev_device_release() gets called when the last device
      reference gets dropped;
    - that undoes everything done by the rbd_dev_device_setup() call
      at the end of rbd_dev_image_probe() (and nothing more), ending
      by calling rbd_dev_image_release(); and
    - rbd_dev_image_release() undoes everything else done by
      rbd_dev_image_probe() (and this includes a call to
      rbd_dev_unprobe().

This means the image and device portions of an rbd device are fairly
cleanly separated now, so error paths should be a little easier to
verify than they used to be.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: don't destroy rbd_dev in device release function
Alex Elder [Mon, 29 Apr 2013 04:32:34 +0000 (23:32 -0500)]
rbd: don't destroy rbd_dev in device release function

Rename rbd_dev_probe_finish() to be rbd_dev_device_setup().  Its
purpose is to set up the Linux side of an rbd device mapping.
Rename rbd_dev_release() to be rbd_dev_device_release(), making
it more obvious it serves as the inverse of the setup function
(or it will).

Encapsulate some of what was done in rbd_dev_release() into a new
function rbd_dev_image_release(), which serves as the inverse of
setting up the ceph side of the mapped rbd image.

Define a new helper rbd_dev_clear_mapping() to simply zero out the
fields of a mapping structure--the inverse of rbd_dev_set_mapping().

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: drop module later
Alex Elder [Mon, 29 Apr 2013 04:32:34 +0000 (23:32 -0500)]
rbd: drop module later

Drop the module reference at the end of rbd_remove() for symmetry
with adding a reference at the top of rbd_add().

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: set up watch in rbd_dev_image_probe()
Alex Elder [Sat, 27 Apr 2013 14:59:31 +0000 (09:59 -0500)]
rbd: set up watch in rbd_dev_image_probe()

Move setting up the watch request for an image so it's done in
rbd_dev_image_probe() rather than rbd_dev_probe_finish().  Move
it all the way up to before doing the initial probe.  This avoids
a potential race condition, in which we get (and use) the initial
snapshot context for an image, and it gets changed between that
time and the time we get the watch set up.

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: don't bother checking whether order changes
Alex Elder [Sat, 27 Apr 2013 14:59:31 +0000 (09:59 -0500)]
rbd: don't bother checking whether order changes

When a format 2 image is refreshed, code is in place to verify that
the object order never changes from what it was originally.  This
relies on the fact that the refresh will occur *after* an initial
load of information about the image.

An upcoming patch makes it possible for the refresh to occur first,
so we can no longer make this order check.  The order really can't
ever change anyway--this was just a sanity check.  So get rid of it.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: don't clean up watch in device release function
Alex Elder [Sat, 27 Apr 2013 14:59:30 +0000 (09:59 -0500)]
rbd: don't clean up watch in device release function

Currently, a watch on an rbd device header object gets torn down
when its final Linux device reference gets dropped.  Instead, tear
it down when removing the device.  If an error occurs cleaning up
the watch event when unmapping, abort the unmap request.

All images (including parents) still get watch requests set up, so
tear these down also, in rbd_dev_remove_parent().  For now, ignore
any errors that occur in this case.

Get rid of local variable "rc" in rbd_remove(); use "ret" instead
(they both somehow ended up defined in the function and only one is
needed).

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: define rbd_header_name()
Alex Elder [Sat, 27 Apr 2013 14:59:30 +0000 (09:59 -0500)]
rbd: define rbd_header_name()

Define a new function rbd_header_name(), which allocates and formats
the name of the header object for the rbd device.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: move more initialization into rbd_dev_image_probe()
Alex Elder [Sat, 27 Apr 2013 14:59:30 +0000 (09:59 -0500)]
rbd: move more initialization into rbd_dev_image_probe()

Move a block of initialization related to the "ceph-side" of an rbd
image out of rbd_dev_probe_finish() and into rbd_dev_image_probe().

Add appropriate error handling to clean things up in the event any
of these new functions return an error.

We know that rbd_dev_snaps_update(), rbd_dev_spec_update(), and
rbd_dev_probe_parent() all clean up after themselves before they
return an error, so no special cleanup is required except when an
earlier call succeeds.  Since rbd_dev_spec_update() only updates the
spec field (whose cleanup will be handled by dropping the last
reference to the spec) there is no cleanup action associatied with
that.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: probe for the parent earlier
Alex Elder [Fri, 26 Apr 2013 20:44:37 +0000 (15:44 -0500)]
rbd: probe for the parent earlier

Probe for a parent device earlier in rbd_dev_probe_finish(), before
starting to set up the Linux side of the rbd device.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: remove parent devices on probe error
Alex Elder [Fri, 26 Apr 2013 20:44:36 +0000 (15:44 -0500)]
rbd: remove parent devices on probe error

When an error occurs while finishing probing a device it is assumed
that parent devices get cleaned up when deleting a device.  They
don't.  Add a call to clean them up.  Note that this means the
parent spec will already be cleaned up so it doesn't have to be
in one of the rbd_add() error paths.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: fix rbd_dev_remove_parent()
Alex Elder [Fri, 26 Apr 2013 20:44:36 +0000 (15:44 -0500)]
rbd: fix rbd_dev_remove_parent()

In certain error paths, it is possible for an rbd device to have a
parent spec but no parent rbd_dev.  In rbd_dev_remove_parent() use
the parent field rather than parent_spec in determining whether to
try to remove any parent devices.  Use assertions to indicate that
any non-null parent pointer has parent_spec associated with it.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: kill __rbd_remove()
Alex Elder [Fri, 26 Apr 2013 20:44:36 +0000 (15:44 -0500)]
rbd: kill __rbd_remove()

The function __rbd_remove() is used in two spots, and it's fairly
simple.  It combines cleanup of part of the ceph-side state as well
as cleaning up the Linux-side state.  Just open code it in the two
callers and eliminate the function.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: set mapping info earlier
Alex Elder [Sat, 27 Apr 2013 14:59:30 +0000 (09:59 -0500)]
rbd: set mapping info earlier

Set the mapping size and features earlier in rbd_dev_probe_finish().

Define rbd_dev_mapping_clear() as an inverse for setting those
fields, and use it both in error handling in rbd_dev_image_probe()
and in the final cleanup in rbd_dev_release().  Change the name
of rbd_dev_set_mapping() to of rbd_dev_mapping_set().

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: encapsulate removing parent devices
Alex Elder [Fri, 26 Apr 2013 20:44:36 +0000 (15:44 -0500)]
rbd: encapsulate removing parent devices

Encapsulate the code that removes an rbd device's parent images into
a new function, rbd_dev_remove_parent().

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: encapsulate probing for parent devices
Alex Elder [Fri, 26 Apr 2013 20:44:36 +0000 (15:44 -0500)]
rbd: encapsulate probing for parent devices

Encapsulate the code that probes for an rbd device's parent images
into a new function, rbd_dev_probe_parent().

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: defer setting disk capacity
Alex Elder [Fri, 26 Apr 2013 20:44:36 +0000 (15:44 -0500)]
rbd: defer setting disk capacity

Don't set the disk capacity until right before we announce the
device as available for use.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: only set device exists flag when ready
Alex Elder [Fri, 26 Apr 2013 20:44:36 +0000 (15:44 -0500)]
rbd: only set device exists flag when ready

Hold off setting the EXISTS rbd device flag until just before we
announce the disk as available for use.  There's no point in doing
so any earlier than that, and at that point the device truly is
fully set up and ready to use.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: fix up some sysfs stuff
Alex Elder [Fri, 26 Apr 2013 20:44:36 +0000 (15:44 -0500)]
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>
11 years agorbd: fix a bug in resizing a mapping
Alex Elder [Fri, 26 Apr 2013 20:44:35 +0000 (15:44 -0500)]
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>
11 years agorbd: refactor rbd_dev_probe_update_spec()
Alex Elder [Fri, 26 Apr 2013 14:43:48 +0000 (09:43 -0500)]
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>
11 years agorbd: rename rbd_dev_probe()
Alex Elder [Fri, 26 Apr 2013 14:43:48 +0000 (09:43 -0500)]
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>
11 years agorbd: make rbd_dev_destroy() match rbd_dev_create()
Alex Elder [Fri, 26 Apr 2013 14:43:47 +0000 (09:43 -0500)]
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>
11 years agorbd: define rbd snap context routines
Alex Elder [Fri, 26 Apr 2013 14:43:47 +0000 (09:43 -0500)]
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>
11 years agorbd: use rbd_warn(), not WARN_ON()
Alex Elder [Fri, 26 Apr 2013 14:43:47 +0000 (09:43 -0500)]
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>
11 years agorbd: move stripe_unit and stripe_count into header
Alex Elder [Fri, 26 Apr 2013 14:43:47 +0000 (09:43 -0500)]
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>
11 years agorbd: make rbd spec names pointer to const
Alex Elder [Fri, 26 Apr 2013 14:43:47 +0000 (09:43 -0500)]
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>
11 years agorbd: set snapshot id in rbd_dev_probe_update_spec()
Alex Elder [Fri, 26 Apr 2013 04:15:08 +0000 (23:15 -0500)]
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>
11 years agorbd: have snap_by_name() return a snapshot
Alex Elder [Fri, 26 Apr 2013 04:15:08 +0000 (23:15 -0500)]
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>
11 years agorbd: fix image id leak in initial probe
Alex Elder [Fri, 26 Apr 2013 04:15:08 +0000 (23:15 -0500)]
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>
11 years agorbd: have rbd_dev_image_id() set format 1 image id
Alex Elder [Fri, 26 Apr 2013 04:15:08 +0000 (23:15 -0500)]
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>
11 years agorbd: avoid dropping extra reference in rbd_free_disk()
Alex Elder [Fri, 26 Apr 2013 04:15:08 +0000 (23:15 -0500)]
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>
11 years agorbd: use rbd_obj_method_sync() return value
Alex Elder [Thu, 25 Apr 2013 20:09:42 +0000 (15:09 -0500)]
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>
11 years agorbd: fix leak of format 2 snapshot names
Alex Elder [Thu, 25 Apr 2013 20:09:42 +0000 (15:09 -0500)]
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>
11 years agorbd: rename __rbd_add_snap_dev()
Alex Elder [Thu, 25 Apr 2013 20:09:41 +0000 (15:09 -0500)]
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>
11 years agorbd: only update values on snap_info success
Alex Elder [Thu, 25 Apr 2013 20:09:41 +0000 (15:09 -0500)]
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>
11 years agorbd: make snap_size order parameter optional
Alex Elder [Thu, 25 Apr 2013 20:09:41 +0000 (15:09 -0500)]
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>
11 years agorbd: fix leak of snapshots during initial probe
Alex Elder [Thu, 25 Apr 2013 20:09:41 +0000 (15:09 -0500)]
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>
11 years agorbd: don't create sysfs entries for non-mapped snapshots
Alex Elder [Tue, 23 Apr 2013 18:52:53 +0000 (13:52 -0500)]
rbd: don't create sysfs entries for non-mapped snapshots

When an rbd image gets mapped a device entry gets created for it
under /sys/bus/rbd/devices/<id>/.  Inside that directory there are
sysfs files that contain information about the image: its size,
feature bits, major device number, and so on.

Additionally, if that image has any snapshots, a device entry gets
created for each of those as a "child" of the mapped device.  Each
of these is a subdirectory of the mapped device, and each directory
contains a few files with information about the snapshot (its
snapshot id, size, and feature mask).

There is no clear benefit to having those device entries for the
snapshots.  The information provided via sysfs of of little real
value--and all of it is available via rbd CLI commands.  If we
still wanted to see the kernel's view of this information it could
be done much more simply by including it in a single sysfs file for
the mapped image.

But there *is* a clear cost to supporting them.  Every time a snapshot
context changes, these entries need to be updated (deleted snapshots
removed, new snapshots created).  The rbd driver is notified of
changes to the snapshot context via callbacks from an osd, and care
must be taken to coordinate removal of snapshot data structures
with the possibility of one these notifications occurring.

Things would be considerably simpler if we just didn't have to
maintain device entries for the snapshots.

So get rid of them.

The ability to map a snapshot of an rbd image will remain; the only
thing lost will be the ability to query these sysfs directories for
information about snapshots of mapped images.

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: fix byte order mismatch
Alex Elder [Sun, 21 Apr 2013 21:51:50 +0000 (16:51 -0500)]
libceph: fix byte order mismatch

A WATCH op includes an object version.  The version that's supplied
is incorrectly byte-swapped osd_req_op_watch_init() where it's first
assigned (it's been this way since that code was first added).

The result is that the version sent to the osd is wrong, because
that value gets byte-swapped again in osd_req_encode_op().  This
is the source of a sparse warning related to improper byte order in
the assignment.

The approach of using the version to avoid a race is deprecated
(see http://tracker.ceph.com/issues/3871), and the watch parameter
is no longer even examined by the osd.  So fix the assignment in
osd_req_op_watch_init() so it no longer does the byte swap.

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: activate support for layered images
Alex Elder [Fri, 26 Oct 2012 04:34:40 +0000 (23:34 -0500)]
rbd: activate support for layered images

Now that we have most everything in place to support layered rbd
images, enable support for them in the kernel client.  Issue a
warning to the log that the support is considered experimental
whenever a format 2 layered image is mapped.

Note that we also have to claim to support the STRIPINGV2 feature,
due to a mistake in the way the rbd CLI set up those flags.  This
feature can work if it has the right parameters, and safeguards
have been put in place to reject those images that do not have
compatible parameters.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: get and check striping parameters
Alex Elder [Sun, 21 Apr 2013 17:14:45 +0000 (12:14 -0500)]
rbd: get and check striping parameters

If an rbd format 2 image indicates it supports the STRIPINGV2
feature we need to find out its stripe unit and stripe count in
order to know whether we can use it.  We don't yet support fancy
striping fully, but if the default parameters are used the behavior
is indistinguishible from non-fancy striping.

This is necessary because some images require the STRIPINGV2 feature
even if they use the default parameters.  (Which is to say the feature
bit was erroneously set even if the feature was not used.)

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: have rbd_obj_method_sync() return transfer count
Alex Elder [Sun, 21 Apr 2013 17:14:45 +0000 (12:14 -0500)]
rbd: have rbd_obj_method_sync() return transfer count

Callers of rbd_obj_method_sync() don't know how many bytes of data
got returned by the class method call.  As a result, they have been
assuming enough got returned to decode whatever was expected.

This isn't safe.  We know how many bytes got transferred, so have
rbd_obj_method_sync() return that amount (rather than just 0) if
the call is successful.

Change all callers to use this return value to ensure decoding of
the results is done safely.

On the other hand, most callers of rbd_obj_method_sync() only
indicate success or failure, so all of *their* callers can simply
test for non-zero result.

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: void data pointers for rbd_obj_method_sync()
Alex Elder [Sun, 21 Apr 2013 17:14:45 +0000 (12:14 -0500)]
rbd: void data pointers for rbd_obj_method_sync()

Make the inbound and outbound data parameters have void rather than
character type for rbd_obj_method_sync().  This makes it more clear
they don't expect typed data, and eliminates the need for some silly
type casts.

One more unrelated change: define the features buffer used in
_rbd_dev_v2_snap_features() to be a packed data structure.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: give rbd_obj_read_sync() buffer void type
Alex Elder [Sun, 21 Apr 2013 17:14:45 +0000 (12:14 -0500)]
rbd: give rbd_obj_read_sync() buffer void type

Make the buf parameter into which the data is to be read have type
void pointer.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: validate timespec conversions
Alex Elder [Fri, 19 Apr 2013 20:34:50 +0000 (15:34 -0500)]
libceph: validate timespec conversions

A ceph timespec contains 32-bit unsigned values for its seconds and
nanoseconds components.  For a standard timespec, both fields are
signed, and the seconds field is almost surely 64 bits.

Add some explicit casts so the fact that this conversion is taking
place is obvious.  Also trip a bug if we ever try to put out of
range (negative or too big) values into a ceph timespec.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: add signed type limits
Alex Elder [Fri, 19 Apr 2013 20:34:50 +0000 (15:34 -0500)]
libceph: add signed type limits

Flesh out the limits defined in <linux/ceph/decode.h> to include the
maximum and minimum values for signed type S8, S16, S32, and S64.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: enforce parent overlap
Alex Elder [Sun, 21 Apr 2013 05:32:07 +0000 (00:32 -0500)]
rbd: enforce parent overlap

A clone image has a defined overlap point with its parent image.
That is the byte offset beyond which the parent image has no
defined data to back the clone, and anything thereafter can be
viewed as being zero-filled by the clone image.

This is needed because a clone image can be resized.  If it gets
resized larger than the snapshot it is based on, the overlap defines
the original size.  If the clone gets resized downward below the
original size the new clone size defines the overlap.  If the clone
is subsequently resized to be larger, the overlap won't be increased
because the previous resize invalidated any parent data beyond that
point.

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: issue a copyup for layered writes
Alex Elder [Fri, 19 Apr 2013 20:34:50 +0000 (15:34 -0500)]
rbd: issue a copyup for layered writes

This implements the main copyup functionality for layered writes.

Here we add a copyup_pages field to the object request, which is
used only for copyup requests to keep track of the page array
containing data read from the parent image.

A copyup request is currently the only request rbd has that requires
two osd operations.  Because of this we handle copyup specially.
All image object requests get an osd request allocated when they are
created.  For a write request, if a copyup is required, the osd
request originally allocated is released, and a new one (with room
for two osd ops) is allocated to replace it.  A new function
rbd_osd_req_create_copyup() allocates an osd request suitable for
a copyup request.

The first op is then filled with a copyup object class method call,
supplying the array of pages containing data read from the parent.
The second op is filled in with the original write request.

The original request otherwise remains intact, and it describes the
original write request (found in the second osd op).  The presence
of the copyup op is sort of implicit; a non-null copyup_pages field
could be used to distinguish between a "normal" write request and a
request containing both a copyup call and a write.

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: implement full object parent reads
Alex Elder [Fri, 19 Apr 2013 20:34:50 +0000 (15:34 -0500)]
rbd: implement full object parent reads

As a step toward implementing layered writes, implement reading the
data for a target object from the parent image for a write request
whose target object is known to not exist.  Add a copyup_pages field
to an image request to track the page array used (only) for such a
request.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: revalidate_disk upon rbd resize
Laurent Barbe [Wed, 10 Apr 2013 22:47:46 +0000 (17:47 -0500)]
rbd: revalidate_disk upon rbd resize

If rbd disk is open and rbd resize is done, new size is not
visible by filesystem.  Like is done in virtio-blk and dm driver,
revalidate_disk() permits to update the bd_inode size.

Signed-off-by: Laurent Barbe <laurent@ksperis.com>
Reviewed-by: Alex Elder <elder@inktank.com>
11 years agorbd: support page array image requests
Alex Elder [Fri, 19 Apr 2013 20:34:50 +0000 (15:34 -0500)]
rbd: support page array image requests

This patch adds the ability to build an image request whose data
will be written from or read into memory described by a page array.
(Previously only bio lists were supported.)

Originally this was going to define a new function for this purpose
but it was largely identical to the rbd_img_request_fill_bio().  So
instead, rbd_img_request_fill_bio() has been generalized to handle
both types of image request.

For the moment we still only fill image requests with bio data.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: define zero_pages()
Alex Elder [Fri, 19 Apr 2013 20:34:50 +0000 (15:34 -0500)]
rbd: define zero_pages()

Define a new function zero_pages() that zeroes a range of memory
defined by a page array, along the lines of zero_bio_chain().  It
saves and the irq flags like bvec_kmap_irq() does, though I'm not
sure at this point that it's necessary.

Update rbd_img_obj_request_read_callback() to use the new function
if the object request contains page rather than bio data.

For the moment, only bio data is used for osd READ ops.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: encapsulate submission of image object requests
Alex Elder [Fri, 19 Apr 2013 20:34:50 +0000 (15:34 -0500)]
rbd: encapsulate submission of image object requests

Object requests that are part of an image request are subject to
some additional handling.  Define rbd_img_obj_request_submit() to
encapsulate that, and use it when initially submitting an image
object request, and when re-submitting it during callback of
an object existence check.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: define separate read and write format funcs
Alex Elder [Fri, 19 Apr 2013 20:34:50 +0000 (15:34 -0500)]
rbd: define separate read and write format funcs

Separate rbd_osd_req_format() into two functions, one for read
requests and the other for write requests.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: support pages for class request data
Alex Elder [Fri, 19 Apr 2013 20:34:49 +0000 (15:34 -0500)]
libceph: support pages for class request data

Add the ability to provide an array of pages as outbound request
data for object class method calls.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: fix two messenger bugs
Alex Elder [Fri, 19 Apr 2013 20:34:49 +0000 (15:34 -0500)]
libceph: fix two messenger bugs

This patch makes four small changes in the ceph messenger.

While getting copyup functionality working I found two bugs in the
messenger.  Existing paths through the code did not trigger these
problems, but they're fixed here:
    - In ceph_msg_data_pagelist_cursor_init(), the cursor's
      last_piece field was being checked against the length
      supplied.  This was OK until this commit: ccba6d98 libceph:
      implement multiple data items in a message That commit changed
      the cursor init routines to allow lengths to be supplied that
      exceeded the size of the current data item. Because of this,
      we have to use the assigned cursor resid field rather than the
      provided length in determining whether the cursor points to
      the last piece of a data item.
    - In ceph_msg_data_add_pages(), a BUG_ON() was erroneously
      catching attempts to add page data to a message if the message
      already had data assigned to it. That was OK until that same
      commit, at which point it was fine for messages to have
      multiple data items. It slipped through because that BUG_ON()
      call was present twice in that function. (You can never be too
      careful.)

In addition two other minor things are changed:
    - In ceph_msg_data_cursor_init(), the local variable "data" was
      getting assigned twice.
    - In ceph_msg_data_advance(), it was assumed that the
      type-specific advance routine would set new_piece to true
      after it advanced past the last piece. That may have been
      fine, but since we check for that case we might as well set it
      explicitly in ceph_msg_data_advance().

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: issue stat request before layered write
Alex Elder [Mon, 11 Feb 2013 18:33:24 +0000 (12:33 -0600)]
rbd: issue stat request before layered write

This is a step toward fully implementing layered writes.

Add checks before request submission for the object(s) associated
with an image request.  For write requests, if we don't know that
the target object exists, issue a STAT request to find out.  When
that request completes, mark the known and exists flags for the
original object request accordingly and re-submit the object
request.  (Note that this still does the existence check only; the
copyup operation is not yet done.)

A new object request is created to perform the existence check.  A
pointer to the original request is added to that object request to
allow the stat request to re-issue the original request after
updating its flags.  If there is a failure with the stat request
the error code is stored with the original request, which is then
completed.

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: add target object existence flags
Alex Elder [Mon, 11 Feb 2013 18:33:24 +0000 (12:33 -0600)]
rbd: add target object existence flags

This creates two new flags for object requests to indicate what is
known about the existence of the object to which a request is to be
sent.  The KNOWN flag will be true if the the EXISTS flag is
meaningful.  That is:

    KNOWN   EXISTS
    -----   ------
      0       0     don't know whether the object exists
      0       1     (not used/invalid)
      1       0     object is known to not exist
      1       0     object is known to exist

This will be used in determining how to handle write requests for
data objects for layered rbd images.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: always check IMG_DATA flag
Alex Elder [Mon, 11 Feb 2013 18:33:24 +0000 (12:33 -0600)]
rbd: always check IMG_DATA flag

In a few spots, whether the an object request's img_request pointer
is null is used to determine whether an object request is being done
as part of an image data request.

Stop doing that, and instead always use the object request IMG_DATA
flag for that purpose.  Swap the order of the definition of the
IMG_DATA and DONE flag helpers, because obj_request_done_set() now
refers to obj_request_img_data_set() to get its rbd_dev value.

This will become important because the img_request pointer is
about to become part of a union.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: adjust image object request ref counting
Alex Elder [Mon, 15 Apr 2013 19:50:37 +0000 (14:50 -0500)]
rbd: adjust image object request ref counting

An extra reference is taken when an object request is added as one
of the requests making up an image object.  A reference is dropped
again when the image's object requests get submitted.

The original reference for the object request will remain throughout
this period, so we don't need to add and then take away an extra
one.

This can be interpreted as the image request inheriting the original
object request's reference.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: support raw data requests
Alex Elder [Mon, 11 Feb 2013 18:33:24 +0000 (12:33 -0600)]
libceph: support raw data requests

Allow osd request ops that aren't otherwise structured (not class,
extent, or watch ops) to specify "raw" data to be used to hold
incoming data for the op.  Make use of this capability for the osd
STAT op.

Prefix the name of the private function osd_req_op_init() with "_",
and expose a new function by that (earlier) name whose purpose is to
initialize osd ops with (only) implied data.

For now we'll just support the use of a page array for an osd op
with incoming raw data.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: clean up osd data field access functions
Alex Elder [Mon, 15 Apr 2013 19:50:36 +0000 (14:50 -0500)]
libceph: clean up osd data field access functions

There are a bunch of functions defined to encapsulate getting the
address of a data field for a particular op in an osd request.
They're all defined the same way, so create a macro to take the
place of all of them.

Two of these are used outside the osd client code, so preserve them
(but convert them to use the new macro internally).  Stop exporting
the ones that aren't used elsewhere.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: kill off osd data write_request parameters
Alex Elder [Mon, 15 Apr 2013 19:50:36 +0000 (14:50 -0500)]
libceph: kill off osd data write_request parameters

In the incremental move toward supporting distinct data items in an
osd request some of the functions had "write_request" parameters to
indicate, basically, whether the data belonged to in_data or the
out_data.  Now that we maintain the data fields in the op structure
there is no need to indicate the direction, so get rid of the
"write_request" parameters.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agoceph: fix printk format warnings in file.c
Randy Dunlap [Fri, 19 Apr 2013 21:20:07 +0000 (14:20 -0700)]
ceph: fix printk format warnings in file.c

Fix printk format warnings by using %zd for 'ssize_t' variables:

fs/ceph/file.c:751:2: warning: format '%ld' expects argument of type 'long int', but argument 11 has type 'ssize_t' [-Wformat]
fs/ceph/file.c:762:2: warning: format '%ld' expects argument of type 'long int', but argument 11 has type 'ssize_t' [-Wformat]

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: ceph-devel@vger.kernel.org
Signed-off-by: Sage Weil <sage@inktank.com>
11 years agoceph: fix race between writepages and truncate
Yan, Zheng [Fri, 12 Apr 2013 13:45:42 +0000 (21:45 +0800)]
ceph: fix race between writepages and truncate

ceph_writepages_start() reads inode->i_size in two places. It can get
different values between successive read, because truncate can change
inode->i_size at any time. The race can lead to mismatch between data
length of osd request and pages marked as writeback. When osd request
finishes, it clear writeback page according to its data length. So
some pages can be left in writeback state forever. The fix is only
read inode->i_size once, save its value to a local variable and use
the local variable when i_size is needed.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Reviewed-by: Alex Elder <elder@inktank.com>
11 years agoceph: apply write checks in ceph_aio_write
Yan, Zheng [Fri, 12 Apr 2013 08:11:13 +0000 (16:11 +0800)]
ceph: apply write checks in ceph_aio_write

copy write checks in __generic_file_aio_write to ceph_aio_write.
To make these checks cover sync write path.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Reviewed-by: Alex Elder <elder@inktank.com>
11 years agoceph: take i_mutex before getting Fw cap
Yan, Zheng [Fri, 12 Apr 2013 08:11:10 +0000 (16:11 +0800)]
ceph: take i_mutex before getting Fw cap

There is deadlock as illustrated bellow. The fix is taking i_mutex
before getting Fw cap reference.

      write                    truncate                 MDS
---------------------     --------------------      --------------
get Fw cap
                          lock i_mutex
lock i_mutex (blocked)
                          request setattr.size  ->
                                                <-   revoke Fw cap

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Reviewed-by: Alex Elder <elder@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agolibceph: change how "safe" callback is used
Alex Elder [Mon, 15 Apr 2013 16:20:42 +0000 (11:20 -0500)]
libceph: change how "safe" callback is used

An osd request currently has two callbacks.  They inform the
initiator of the request when we've received confirmation for the
target osd that a request was received, and when the osd indicates
all changes described by the request are durable.

The only time the second callback is used is in the ceph file system
for a synchronous write.  There's a race that makes some handling of
this case unsafe.  This patch addresses this problem.  The error
handling for this callback is also kind of gross, and this patch
changes that as well.

In ceph_sync_write(), if a safe callback is requested we want to add
the request on the ceph inode's unsafe items list.  Because items on
this list must have their tid set (by ceph_osd_start_request()), the
request added *after* the call to that function returns.  The
problem with this is that there's a race between starting the
request and adding it to the unsafe items list; the request may
already be complete before ceph_sync_write() even begins to put it
on the list.

To address this, we change the way the "safe" callback is used.
Rather than just calling it when the request is "safe", we use it to
notify the initiator the bounds (start and end) of the period during
which the request is *unsafe*.  So the initiator gets notified just
before the request gets sent to the osd (when it is "unsafe"), and
again when it's known the results are durable (it's no longer
unsafe).  The first call will get made in __send_request(), just
before the request message gets sent to the messenger for the first
time.  That function is only called by __send_queued(), which is
always called with the osd client's request mutex held.

We then have this callback function insert the request on the ceph
inode's unsafe list when we're told the request is unsafe.  This
will avoid the race because this call will be made under protection
of the osd client's request mutex.  It also nicely groups the setup
and cleanup of the state associated with managing unsafe requests.

The name of the "safe" callback field is changed to "unsafe" to
better reflect its new purpose.  It has a Boolean "unsafe" parameter
to indicate whether the request is becoming unsafe or is now safe.
Because the "msg" parameter wasn't used, we drop that.

This resolves the original problem reportedin:
    http://tracker.ceph.com/issues/4706

Reported-by: Yan, Zheng <zheng.z.yan@intel.com>
Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Yan, Zheng <zheng.z.yan@intel.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agoceph: let osd client clean up for interrupted request
Alex Elder [Mon, 15 Apr 2013 16:18:01 +0000 (11:18 -0500)]
ceph: let osd client clean up for interrupted request

In ceph_sync_write(), if a safe callback is supplied with a request,
and an error is returned by ceph_osdc_wait_request(), a block of
code is executed to remove the request from the unsafe writes list
and drop references to capabilities acquired just prior to a call to
ceph_osdc_wait_request().

The only function used for this callback is sync_write_commit(),
and it does *exactly* what that block of error handling code does.

Now in ceph_osdc_wait_request(), if an error occurs (due to an
interupt during a wait_for_completion_interruptible() call),
complete_request() gets called, and that calls the request's
safe_callback method if it's defined.

So this means that this cleanup activity gets called twice in this
case, which is erroneous (and in fact leads to a crash).

Fix this by just letting the osd client handle the cleanup in
the event of an interrupt.

This resolves one problem mentioned in:
    http://tracker.ceph.com/issues/4706

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Yan, Zheng <zheng.z.yan@intel.com>
11 years agoceph: fix symlink inode operations
Yan, Zheng [Sun, 7 Apr 2013 08:28:49 +0000 (16:28 +0800)]
ceph: fix symlink inode operations

add getattr/setattr and xattrs related methods.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Reviewed-by: Greg Farnum <greg@inktank.com>
11 years agoceph: Use pseudo-random numbers to choose mds
Sam Lang [Tue, 9 Apr 2013 21:49:11 +0000 (16:49 -0500)]
ceph: Use pseudo-random numbers to choose mds

We don't need to use up entropy to choose an mds,
so use prandom_u32() to get a pseudo-random number.

Also, we don't need to choose a random mds if only
one mds is available, so add special casing for the
common case.

Fixes http://tracker.ceph.com/issues/3579

Signed-off-by: Sam Lang <sam.lang@inktank.com>
Reviewed-by: Greg Farnum <greg@inktank.com>
Reviewed-by: Alex Elder <elder@inktank.com>
11 years agorbd: implement layered reads
Alex Elder [Thu, 24 Jan 2013 22:13:36 +0000 (16:13 -0600)]
rbd: implement layered reads

Implement layered read requests for format 2 rbd images.

If an rbd image is a clone of a snapshot, the snapshot will be the
clone's "parent" image.  When an object read request on a clone
comes back with ENOENT it indicates that the clone is not yet
populated with that portion of the image's data, and the parent
image should be consulted to satisfy the read.

When this occurs, a new image request is created, directed to the
parent image.  The offset and length of the image are the same as
the image-relative offset and length of the object request that
produced ENOENT.  Data from the parent image therefore satisfies the
object read request for the original image request.

While this code works, it will not be active until we enable the
layering feature (by adding RBD_FEATURE_LAYERING to the value of
RBD_FEATURES_SUPPORTED).

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: probe the parent of an image if present
Alex Elder [Wed, 31 Oct 2012 00:40:33 +0000 (19:40 -0500)]
rbd: probe the parent of an image if present

Call the probe function for the parent device if one is present.
Since we don't formally support the layering feature we won't
be using this functionality just yet.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: add an object request flag for image data objects
Alex Elder [Mon, 11 Feb 2013 18:33:24 +0000 (12:33 -0600)]
rbd: add an object request flag for image data objects

Add a flag to distinguish between object requests being done on
standalone objects and requests being sent for objects representing
rbd image data (i.e., object requests that are the result of image
request).

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: define an rbd object request flags field
Alex Elder [Mon, 11 Feb 2013 18:33:24 +0000 (12:33 -0600)]
rbd: define an rbd object request flags field

We're going to need some more Boolean values for object requests,
so create a flags bit field and use it to record whether the request
is done.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: encapsulate image object end request handling
Alex Elder [Fri, 8 Feb 2013 15:55:49 +0000 (09:55 -0600)]
rbd: encapsulate image object end request handling

Encapsulate the code that completes processing of an object request
that's part of an image request.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: define image request layered flag
Alex Elder [Thu, 24 Jan 2013 22:13:36 +0000 (16:13 -0600)]
rbd: define image request layered flag

Define a flag indicating whether an image request is for a layered
image (one with a parent image to which requests will be redirected
if the target object of a request does not exist).  The code that
checks this flag will be added shortly.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: define image request originator flag
Alex Elder [Thu, 24 Jan 2013 22:13:36 +0000 (16:13 -0600)]
rbd: define image request originator flag

Define a flag indicating whether an image request originated from
the Linux block layer (from blk_fetch_request()) or whether it was
initiated in order to satisfy an object request for a child image
of a layered rbd device.  For image requests initiated by objects of
child images we'll save a pointer to the object request rather than
the Linux block request.

For now, only block requests are used.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: define image request flags
Alex Elder [Fri, 8 Feb 2013 15:55:49 +0000 (09:55 -0600)]
rbd: define image request flags

There are several Boolean values we'll be maintaining for image
requests.  Switch from the single write_request field to a
general-purpose flags field, and use one if its bits to represent
the direction of I/O for the image request.  Define helper functions
for setting and testing that flag.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: record image-relative offset in object requests
Alex Elder [Thu, 24 Jan 2013 22:13:36 +0000 (16:13 -0600)]
rbd: record image-relative offset in object requests

For an image object request we will need to know what offset within
the rbd image the request covers.  Record that when the object
request gets created.

Update the I/O error warnings so they use this so what's reported
is more informative.

Rename a local variable to fit the convention used everywhere else.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: record aggregate image transfer count
Alex Elder [Wed, 10 Apr 2013 17:34:25 +0000 (12:34 -0500)]
rbd: record aggregate image transfer count

Compute the total number of bytes transferred for an image
request--the sum across each of the request's object requests.
To avoid contention do it only when all object requests are
complete, in rbd_img_request_complete().

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: record overall image request result
Alex Elder [Thu, 24 Jan 2013 22:13:36 +0000 (16:13 -0600)]
rbd: record overall image request result

If any image object request produces a non-zero result, preserve
that as the result of the overall image request.  If multiple
objects have non-zero results, save only the first one.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: update feature bits
Alex Elder [Thu, 11 Apr 2013 14:29:48 +0000 (09:29 -0500)]
rbd: update feature bits

There is a new rbd feature bit defined for "fancy striping." Add
it to the ones defined in the kernel client.

Change RBD_FEATURES_ALL so it represents the set of all feature
bits (rather than just the ones we support).  Define a new symbol
RBD_FEATURES_SUPPORTED to indicate the supported ones.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: make method call data be a separate data item
Alex Elder [Fri, 5 Apr 2013 19:46:02 +0000 (14:46 -0500)]
libceph: make method call data be a separate data item

Right now the data for a method call is specified via a pointer and
length, and it's copied--along with the class and method name--into
a pagelist data item to be sent to the osd.  Instead, encode the
data in a data item separate from the class and method names.

This will allow large amounts of data to be supplied to methods
without copying.  Only rbd uses the class functionality right now,
and when it really needs this it will probably need to use a page
array rather than a page list.  But this simple implementation
demonstrates the functionality on the osd client, and that's enough
for now.

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: add, don't set data for a message
Alex Elder [Fri, 5 Apr 2013 19:46:01 +0000 (14:46 -0500)]
libceph: add, don't set data for a message

Change the names of the functions that put data on a pagelist to
reflect that we're adding to whatever's already there rather than
just setting it to the one thing.  Currently only one data item is
ever added to a message, but that's about to change.

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: implement multiple data items in a message
Alex Elder [Fri, 5 Apr 2013 19:46:01 +0000 (14:46 -0500)]
libceph: implement multiple data items in a message

This patch adds support to the messenger for more than one data item
in its data list.

A message data cursor has two more fields to support this:
    - a count of the number of bytes left to be consumed across
      all data items in the list, "total_resid"
    - a pointer to the head of the list (for validation only)

The cursor initialization routine has been split into two parts: the
outer one, which initializes the cursor for traversing the entire
list of data items; and the inner one, which initializes the cursor
to start processing a single data item.

When a message cursor is first initialized, the outer initialization
routine sets total_resid to the length provided.  The data pointer
is initialized to the first data item on the list.  From there, the
inner initialization routine finishes by setting up to process the
data item the cursor points to.

Advancing the cursor consumes bytes in total_resid.  If the resid
field reaches zero, it means the current data item is fully
consumed.  If total_resid indicates there is more data, the cursor
is advanced to point to the next data item, and then the inner
initialization routine prepares for using that.  (A check is made at
this point to make sure we don't wrap around the front of the list.)

The type-specific init routines are modified so they can be given a
length that's larger than what the data item can support.  The resid
field is initialized to the smaller of the provided length and the
length of the entire data item.

When total_resid reaches zero, we're done.

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

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: replace message data pointer with list
Alex Elder [Thu, 14 Mar 2013 19:09:06 +0000 (14:09 -0500)]
libceph: replace message data pointer with list

In place of the message data pointer, use a list head which links
through message data items.  For now we only support a single entry
on that list.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: have cursor point to data
Alex Elder [Thu, 14 Mar 2013 19:09:06 +0000 (14:09 -0500)]
libceph: have cursor point to data

Rather than having a ceph message data item point to the cursor it's
associated with, have the cursor point to a data item.  This will
allow a message cursor to be used for more than one data item.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: move cursor into message
Alex Elder [Thu, 14 Mar 2013 19:09:06 +0000 (14:09 -0500)]
libceph: move cursor into message

A message will only be processing a single data item at a time, so
there's no need for each data item to have its own cursor.

Move the cursor embedded in the message data structure into the
message itself.  To minimize the impact, keep the data->cursor
field, but make it be a pointer to the cursor in the message.

Move the definition of ceph_msg_data above ceph_msg_data_cursor so
the cursor can point to the data without a forward definition rather
than vice-versa.

This and the upcoming patches are part of:
    http://tracker.ceph.com/issues/3761

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: record bio length
Alex Elder [Fri, 5 Apr 2013 19:46:01 +0000 (14:46 -0500)]
libceph: record bio length

The bio is the only data item type that doesn't record its full
length.  Fix that.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: skip message if too big to receive
Alex Elder [Fri, 5 Apr 2013 19:46:01 +0000 (14:46 -0500)]
libceph: skip message if too big to receive

We know the length of our message buffers.  If we get a message
that's too long, just dump it and ignore it.  If skip was set
then con->in_msg won't be valid, so be careful not to dereference
a null pointer in the process.

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

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