]> rtime.felk.cvut.cz Git - linux-imx.git/log
linux-imx.git
11 years agolibceph: fix undefined behavior when using snprintf()
Cong Ding [Fri, 25 Jan 2013 23:48:59 +0000 (17:48 -0600)]
libceph: fix undefined behavior when using snprintf()

The variable "str" is used as both the source and destination in
function snprintf(), which is undefined behavior based on C11. The
original description in C11 is:
"If copying takes place between objects that
overlap, the behavior is undefined."

And, the function of ceph_osdmap_state_str() is to return the osdmap
state, so it should return "doesn't exist" when all the conditions
are not satisfied. I fix it in this patch.

[elder@inktank.com: shortened the commit message]

Signed-off-by: Cong Ding <dinggnu@gmail.com>
Reviewed-by: Alex Elder <elder@inktank.com>
11 years agorbd: don't retry setting up header watch
Alex Elder [Fri, 18 Jan 2013 18:31:09 +0000 (12:31 -0600)]
rbd: don't retry setting up header watch

When an rbd image is initially mapped a watch event is registered so
we can do something if the header object changes.

The code that does this currently loops if initiating the watch
request results in an ERANGE error.  The osds will never return
ERANGE, so there's no reason to do this loop, so get rid of it.

This resolves:
    http://tracker.newdream.net/issues/3860

Note that the problem this loop was intended to solve is a race
between collecting image header information and setting up the watch
on the header object.  The real fix for that problem is described
here:
    http://tracker.newdream.net/issues/3871

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: check for overflow in rbd_get_num_segments()
Alex Elder [Thu, 10 Jan 2013 18:56:58 +0000 (12:56 -0600)]
rbd: check for overflow in rbd_get_num_segments()

The return type of rbd_get_num_segments() is int, but the values it
operates on are u64.  Although it's not likely, there's no guarantee
the result won't exceed what can be respresented in an int.  The
function is already designed to return -ERANGE on error, so just add
this possible overflow as another reason to return that.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Dan Mick <dan.mick@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: small changes
Alex Elder [Sun, 20 Jan 2013 20:44:42 +0000 (14:44 -0600)]
rbd: small changes

A few very minor changes to the rbd code:
    - RBD_MAX_OPT_LEN is unused, so get rid of it
    - Consolidate rbd options definitions
    - Make rbd_segment_name() return pointer to const char

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Dan Mick <dan.mick@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: fix type of snap_id in rbd_dev_v2_snap_info()
Alex Elder [Wed, 9 Jan 2013 20:44:18 +0000 (14:44 -0600)]
rbd: fix type of snap_id in rbd_dev_v2_snap_info()

The type of the snap_id local variable is defined with the
wrong byte order.  Fix that.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: assign watch request more directly
Alex Elder [Tue, 20 Nov 2012 20:17:17 +0000 (14:17 -0600)]
rbd: assign watch request more directly

Both rbd_req_sync_op() and rbd_do_request() have a "linger"
parameter, which is the address of a pointer that should refer to
the osd request structure used to issue a request to an osd.

Only one case ever supplies a non-null "linger" argument: an
CEPH_OSD_OP_WATCH start.  And in that one case it is assigned
&rbd_dev->watch_request.

Within rbd_do_request() (where the assignment ultimately gets made)
we know the rbd_dev and therefore its watch_request field.  We
also know whether the op being sent is CEPH_OSD_OP_WATCH start.

Stop opaquely passing down the "linger" pointer, and instead just
assign the value directly inside rbd_do_request() when it's needed.

This makes it unnecessary for rbd_req_sync_watch() to make
arrangements to hold a value that's not available until a
bit later.  This more clearly separates setting up a watch
request from submitting it.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: move remaining osd op setup into rbd_osd_req_op_create()
Alex Elder [Tue, 20 Nov 2012 04:55:21 +0000 (22:55 -0600)]
rbd: move remaining osd op setup into rbd_osd_req_op_create()

The two remaining osd ops used by rbd are CEPH_OSD_OP_WATCH and
CEPH_OSD_OP_NOTIFY_ACK.  Move the setup of those operations into
rbd_osd_req_op_create(), and get rid of rbd_create_rw_op() and
rbd_destroy_op().

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: move call osd op setup into rbd_osd_req_op_create()
Alex Elder [Tue, 20 Nov 2012 04:55:21 +0000 (22:55 -0600)]
rbd: move call osd op setup into rbd_osd_req_op_create()

Move the initialization of the CEPH_OSD_OP_CALL operation into
rbd_osd_req_op_create().

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: don't assign extent info in rbd_req_sync_op()
Alex Elder [Tue, 20 Nov 2012 04:55:21 +0000 (22:55 -0600)]
rbd: don't assign extent info in rbd_req_sync_op()

Move the assignment of the extent offset and length and payload
length out of rbd_req_sync_op() and into its caller in the one spot
where a read (and note--no write) operation might be initiated.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: don't assign extent info in rbd_do_request()
Alex Elder [Tue, 20 Nov 2012 04:55:21 +0000 (22:55 -0600)]
rbd: don't assign extent info in rbd_do_request()

In rbd_do_request() there's a sort of last-minute assignment of the
extent offset and length and payload length for read and write
operations.  Move those assignments into the caller (in those spots
that might initiate read or write operations)

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: don't leak rbd_req for rbd_req_sync_notify_ack()
Alex Elder [Fri, 30 Nov 2012 15:59:47 +0000 (09:59 -0600)]
rbd: don't leak rbd_req for rbd_req_sync_notify_ack()

When rbd_req_sync_notify_ack() calls rbd_do_request() it supplies
rbd_simple_req_cb() as its callback function.  Because the callback
is supplied, an rbd_req structure gets allocated and populated so it
can be used by the callback.  However rbd_simple_req_cb() is not
freeing (or even using) the rbd_req structure, so it's getting
leaked.

Since rbd_simple_req_cb() has no need for the rbd_req structure,
just avoid allocating one for this case.  Of the three calls to
rbd_do_request(), only the one from rbd_do_op() needs the rbd_req
structure, and that call can be distinguished from the other two
because it supplies a non-null rbd_collection pointer.

So fix this leak by only allocating the rbd_req structure if a
non-null "coll" value is provided to rbd_do_request().

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: don't leak rbd_req on synchronous requests
Alex Elder [Fri, 30 Nov 2012 15:59:47 +0000 (09:59 -0600)]
rbd: don't leak rbd_req on synchronous requests

When rbd_do_request() is called it allocates and populates an
rbd_req structure to hold information about the osd request to be
sent.  This is done for the benefit of the callback function (in
particular, rbd_req_cb()), which uses this in processing when
the request completes.

Synchronous requests provide no callback function, in which case
rbd_do_request() waits for the request to complete before returning.
This case is not handling the needed free of the rbd_req structure
like it should, so it is getting leaked.

Note however that the synchronous case has no need for the rbd_req
structure at all.  So rather than simply freeing this structure for
synchronous requests, just don't allocate it to begin with.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: combine rbd sync watch/unwatch functions
Alex Elder [Wed, 14 Nov 2012 03:11:15 +0000 (21:11 -0600)]
rbd: combine rbd sync watch/unwatch functions

The rbd_req_sync_watch() and rbd_req_sync_unwatch() functions are
nearly identical.  Combine them into a single function with a flag
indicating whether a watch is to be initiated or torn down.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: use a common layout for each device
Alex Elder [Wed, 14 Nov 2012 18:25:19 +0000 (12:25 -0600)]
rbd: use a common layout for each device

Each osd message includes a layout structure, and for rbd it is
always the same (at least for osd's in a given pool).

Initialize a layout structure when an rbd_dev gets created and just
copy that into osd requests for the rbd image.

Replace an assertion that was done when initializing the layout
structures with code that catches and handles anything that would
trigger the assertion as soon as it is identified.  This precludes
that (bad) condition from ever occurring.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: don't bother calculating file mapping
Alex Elder [Wed, 14 Nov 2012 18:25:19 +0000 (12:25 -0600)]
rbd: don't bother calculating file mapping

When rbd_do_request() has a request to process it initializes a ceph
file layout structure and uses it to compute offsets and limits for
the range of the request using ceph_calc_file_object_mapping().

The layout used is fixed, and is based on RBD_MAX_OBJ_ORDER (30).
It sets the layout's object size and stripe unit to be 1 GB (2^30),
and sets the stripe count to be 1.

The job of ceph_calc_file_object_mapping() is to determine which
of a sequence of objects will contain data covered by range, and
within that object, at what offset the range starts.  It also
truncates the length of the range at the end of the selected object
if necessary.

This is needed for ceph fs, but for rbd it really serves no purpose.
It does its own blocking of images into objects, echo of which is
(1 << obj_order) in size, and as a result it ignores the "bno"
value returned by ceph_calc_file_object_mapping().  In addition,
by the point a request has reached this function, it is already
destined for a single rbd object, and its length will not exceed
that object's extent.  Because of this, and because the mapping will
result in blocking up the range using an integer multiple of the
image's object order, ceph_calc_file_object_mapping() will never
change the offset or length values defined by the request.

In other words, this call is a big no-op for rbd data requests.

There is one exception.  We read the header object using this
function, and in that case we will not have already limited the
request size.  However, the header is a single object (not a file or
rbd image), and should not be broken into pieces anyway.  So in fact
we should *not* be calling ceph_calc_file_object_mapping() when
operating on the header object.

So...

Don't call ceph_calc_file_object_mapping() in rbd_do_request(),
because useless for image data and incorrect to do sofor the image
header.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: open code rbd_calc_raw_layout()
Alex Elder [Wed, 14 Nov 2012 18:25:18 +0000 (12:25 -0600)]
rbd: open code rbd_calc_raw_layout()

This patch gets rid of rbd_calc_raw_layout() by simply open coding
it in its one caller.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: pull in ceph_calc_raw_layout()
Alex Elder [Wed, 14 Nov 2012 18:25:18 +0000 (12:25 -0600)]
rbd: pull in ceph_calc_raw_layout()

This is the first in a series of patches aimed at eliminating
the use of ceph_calc_raw_layout() by rbd.

It simply pulls in a copy of that function and renames it
rbd_calc_raw_layout().

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: kill ceph_osd_req_op->flags
Alex Elder [Wed, 14 Nov 2012 15:38:20 +0000 (09:38 -0600)]
rbd: kill ceph_osd_req_op->flags

The flags field of struct ceph_osd_req_op is never used, so just get
rid of it.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: assume single op in a request
Alex Elder [Wed, 14 Nov 2012 03:11:15 +0000 (21:11 -0600)]
rbd: assume single op in a request

We now know that every of rbd_req_sync_op() passes an array of
exactly one operation, as evidenced by all callers passing 1 as its
num_op argument.  So get rid of that argument, assuming a single op.

Similarly, we now know that all callers of rbd_do_request() pass 1
as the num_op value, so that parameter can be eliminated as well.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: there is really only one op
Alex Elder [Wed, 14 Nov 2012 03:11:15 +0000 (21:11 -0600)]
rbd: there is really only one op

Throughout the rbd code there are spots where it appears we can
handle an osd request containing more than one osd request op.

But that is only the way it appears.  In fact, currently only one
operation at a time can be supported, and supporting more than
one will require much more than fleshing out the support that's
there now.

This patch changes names to make it perfectly clear that anywhere
we're dealing with a block of ops, we're in fact dealing with
exactly one of them.  We'll be able to simplify some things as
a result.

When multiple op support is implemented, we can update things again
accordingly.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: pass num_op with ops
Alex Elder [Wed, 14 Nov 2012 03:11:15 +0000 (21:11 -0600)]
libceph: pass num_op with ops

Both ceph_osdc_alloc_request() and ceph_osdc_build_request() are
provided an array of ceph osd request operations.  Rather than just
passing the number of operations in the array, the caller is
required append an additional zeroed operation structure to signal
the end of the array.

All callers know the number of operations at the time these
functions are called, so drop the silly zero entry and supply that
number directly.  As a result, get_num_ops() is no longer needed.
This also means that ceph_osdc_alloc_request() never uses its ops
argument, so that can be dropped.

Also rbd_create_rw_ops() no longer needs to add one to reserve room
for the additional op.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: pass num_op with ops array
Alex Elder [Wed, 14 Nov 2012 03:11:15 +0000 (21:11 -0600)]
rbd: pass num_op with ops array

Add a num_op parameter to rbd_do_request() and rbd_req_sync_op() to
indicate the number of entries in the array.  The callers of these
functions always know how many entries are in the array, so just
pass that information down.

This is in anticipation of eliminating the extra zero-filled entry
in these ops arrays.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: don't set pages or bio in ceph_osdc_alloc_request()
Alex Elder [Wed, 14 Nov 2012 03:11:15 +0000 (21:11 -0600)]
libceph: don't set pages or bio in ceph_osdc_alloc_request()

Only one of the two callers of ceph_osdc_alloc_request() provides
page or bio data for its payload.  And essentially all that function
was doing with those arguments was assigning them to fields in the
osd request structure.

Simplify ceph_osdc_alloc_request() by having the caller take care of
making those assignments

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: don't set flags in ceph_osdc_alloc_request()
Alex Elder [Wed, 14 Nov 2012 03:11:15 +0000 (21:11 -0600)]
libceph: don't set flags in ceph_osdc_alloc_request()

The only thing ceph_osdc_alloc_request() really does with the
flags value it is passed is assign it to the newly-created
osd request structure.  Do that in the caller instead.

Both callers subsequently call ceph_osdc_build_request(), so have
that function (instead of ceph_osdc_alloc_request()) issue a warning
if a request comes through with neither the read nor write flags set.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: drop osdc from ceph_calc_raw_layout()
Alex Elder [Wed, 14 Nov 2012 03:11:14 +0000 (21:11 -0600)]
libceph: drop osdc from ceph_calc_raw_layout()

The osdc parameter to ceph_calc_raw_layout() is not used, so get rid
of it.  Consequently, the corresponding parameter in calc_layout()
becomes unused, so get rid of that as well.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: drop snapid in ceph_calc_raw_layout()
Alex Elder [Wed, 14 Nov 2012 03:11:15 +0000 (21:11 -0600)]
libceph: drop snapid in ceph_calc_raw_layout()

A snapshot id must be provided to ceph_calc_raw_layout() even though
it is not needed at all for calculating the layout.

Where the snapshot id *is* needed is when building the request
message for an osd operation.

Drop the snapid parameter from ceph_calc_raw_layout() and pass
that value instead in ceph_osdc_build_request().

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: pass length to ceph_calc_file_object_mapping()
Alex Elder [Wed, 14 Nov 2012 15:38:19 +0000 (09:38 -0600)]
libceph: pass length to ceph_calc_file_object_mapping()

ceph_calc_file_object_mapping() takes (among other things) a "file"
offset and length, and based on the layout, determines the object
number ("bno") backing the affected portion of the file's data and
the offset into that object where the desired range begins.  It also
computes the size that should be used for the request--either the
amount requested or something less if that would exceed the end of
the object.

This patch changes the input length parameter in this function so it
is used only for input.  That is, the argument will be passed by
value rather than by address, so the value provided won't get
updated by the function.

The value would only get updated if the length would surpass the
current object, and in that case the value it got updated to would
be exactly that returned in *oxlen.

Only one of the two callers is affected by this change.  Update
ceph_calc_raw_layout() so it records any updated value.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: pass length to ceph_osdc_build_request()
Alex Elder [Wed, 14 Nov 2012 15:38:19 +0000 (09:38 -0600)]
libceph: pass length to ceph_osdc_build_request()

The len argument to ceph_osdc_build_request() is set up to be
passed by address, but that function never updates its value
so there's no need to do this.  Tighten up the interface by
passing the length directly.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: kill op_needs_trail()
Alex Elder [Wed, 14 Nov 2012 03:11:15 +0000 (21:11 -0600)]
libceph: kill op_needs_trail()

Since every osd message is now prepared to include trailing data,
there's no need to check ahead of time whether any operations will
make use of the trail portion of the message.

We can drop the second argument to get_num_ops(), and as a result we
can also get rid of op_needs_trail() which is no longer used.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: always allow trail in osd request
Alex Elder [Wed, 14 Nov 2012 03:11:15 +0000 (21:11 -0600)]
libceph: always allow trail in osd request

An osd request structure contains an optional trail portion, which
if present will contain data to be passed in the payload portion of
the message containing the request.  The trail field is a
ceph_pagelist pointer, and if null it indicates there is no trail.

A ceph_pagelist structure contains a length field, and it can
legitimately hold value 0.  Make use of this to change the
interpretation of the "trail" of an osd request so that every osd
request has trailing data, it just might have length 0.

This means we change the r_trail field in a ceph_osd_request
structure from a pointer to a structure that is always initialized.

Note that in ceph_osdc_start_request(), the trail pointer (or now
address of that structure) is assigned to a ceph message's trail
field.  Here's why that's still OK (looking at net/ceph/messenger.c):
    - What would have resulted in a null pointer previously will now
      refer to a 0-length page list.  That message trail pointer
      is used in two functions, write_partial_msg_pages() and
      out_msg_pos_next().
    - In write_partial_msg_pages(), a null page list pointer is
      handled the same as a message with 0-length trail, and both
      result in a "in_trail" variable set to false.  The trail
      pointer is only used if in_trail is true.
    - The only other place the message trail pointer is used is
      out_msg_pos_next().  That function is only called by
      write_partial_msg_pages() and only touches the trail pointer
      if the in_trail value it is passed is true.
Therefore a null ceph_msg->trail pointer is equivalent to a non-null
pointer referring to a 0-length page list structure.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: don't bother setting snapid in rbd_do_request()
Alex Elder [Fri, 9 Nov 2012 18:50:10 +0000 (12:50 -0600)]
rbd: don't bother setting snapid in rbd_do_request()

For some reason, the snapid field of the osd request header is
explicitly set to CEPH_NOSNAP in rbd_do_request().  Just a few lines
later--with no code that would access this field in between--a call
is made to ceph_calc_raw_layout() passing the snapid provided to
rbd_do_request(), which encodes the snapid value it is provided into
that field instead.

In other words, there is no need to fill in CEPH_NOSNAP, and doing
so suggests it might be necessary.  Don't do that any more.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: kill rbd_req_sync_op() snapc and snapid parameters
Alex Elder [Fri, 9 Nov 2012 14:43:16 +0000 (08:43 -0600)]
rbd: kill rbd_req_sync_op() snapc and snapid parameters

The snapc and snapid parameters to rbd_req_sync_op() always take
the values NULL and CEPH_NOSNAP, respectively.  So just get rid
of them and use those values where needed.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: drop flags parameter from rbd_req_sync_exec()
Alex Elder [Fri, 9 Nov 2012 14:43:16 +0000 (08:43 -0600)]
rbd: drop flags parameter from rbd_req_sync_exec()

All callers of rbd_req_sync_exec() pass CEPH_OSD_FLAG_READ as their
flags argument.  Delete that parameter and use CEPH_OSD_FLAG_READ
within the function.  If we find a need to support write operations
we can add it back again.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: drop snapid parameter from rbd_req_sync_read()
Alex Elder [Fri, 9 Nov 2012 14:43:15 +0000 (08:43 -0600)]
rbd: drop snapid parameter from rbd_req_sync_read()

There is only one caller of rbd_req_sync_read(), and it passes
CEPH_NOSNAP as the snapshot id argument.  Delete that parameter
and just use CEPH_NOSNAP within the function.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: drop oid parameters from ceph_osdc_build_request()
Alex Elder [Fri, 9 Nov 2012 14:43:15 +0000 (08:43 -0600)]
rbd: drop oid parameters from ceph_osdc_build_request()

The last two parameters to ceph_osd_build_request() describe the
object id, but the values passed always come from the osd request
structure whose address is also provided.  Get rid of those last
two parameters.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: separate layout init
Alex Elder [Fri, 9 Nov 2012 14:43:15 +0000 (08:43 -0600)]
rbd: separate layout init

Pull a block of code that initializes the layout structure in an osd
request into its own function so it can be reused.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Dan Mick <dan.mick@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: only get snap context for write requests
Alex Elder [Fri, 9 Nov 2012 14:43:15 +0000 (08:43 -0600)]
rbd: only get snap context for write requests

Right now we get the snapshot context for an rbd image (under
protection of the header semaphore) for every request processed.

There's no need to get the snap context if we're doing a read,
so avoid doing so in that case.

Note that we no longer need to hold the header semaphore to
check the rbd_dev's existence flag.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: make exists flag atomic
Alex Elder [Fri, 9 Nov 2012 14:43:15 +0000 (08:43 -0600)]
rbd: make exists flag atomic

The rbd_device->exists field can be updated asynchronously, changing
from set to clear if a mapped snapshot disappears from the base
image's snapshot context.

Currently, value of the "exists" flag is only read and modified
under protection of the header semaphore, but that will change with
the next patch.  Making it atomic ensures this won't be a problem
because the a the non-existence of device will be immediately known.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: a little more cleanup of rbd_rq_fn()
Alex Elder [Thu, 8 Nov 2012 14:01:39 +0000 (08:01 -0600)]
rbd: a little more cleanup of rbd_rq_fn()

Now that a big hunk in the middle of rbd_rq_fn() has been moved
into its own routine we can simplify it a little more.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: end request on error in rbd_do_request() caller
Alex Elder [Thu, 8 Nov 2012 14:01:39 +0000 (08:01 -0600)]
rbd: end request on error in rbd_do_request() caller

Only one of the three callers of rbd_do_request() provide a
collection structure to aggregate status.

If an error occurs in rbd_do_request(), have the caller
take care of calling rbd_coll_end_req() if necessary in
that one spot.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: encapsulate handling for a single request
Alex Elder [Thu, 8 Nov 2012 14:01:39 +0000 (08:01 -0600)]
rbd: encapsulate handling for a single request

In rbd_rq_fn(), requests are fetched from the block layer and each
request is processed, looping through the request's list of bio's
until they've all been consumed.

Separate the handling for a single request into its own function to
make it a bit easier to see what's going on.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: be picky about osd request status type
Alex Elder [Thu, 8 Nov 2012 14:01:39 +0000 (08:01 -0600)]
rbd: be picky about osd request status type

The result field in a ceph osd reply header is a signed 32-bit type,
but rbd code often casually uses int to represent it.

The following changes the types of variables that handle this result
value to be "s32" instead of "int" to be completely explicit about
it.  Only at the point we pass that result to __blk_end_request()
does the type get converted to the plain old int defined for that
interface.

There is almost certainly no binary impact of this change, but I
prefer to show the exact size and signedness of the value since we
know it.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Dan Mick <dan.mick@inktank.com>
11 years agorbd: standardize ceph_osd_request variable names
Alex Elder [Thu, 8 Nov 2012 14:01:39 +0000 (08:01 -0600)]
rbd: standardize ceph_osd_request variable names

There are spots where a ceph_osds_request pointer variable is given
the name "req".  Since we're dealing with (at least) three types of
requests (block layer, rbd, and osd), I find this slightly
distracting.

Change such instances to use "osd_req" consistently to make the
abstraction represented a little more obvious.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Dan Mick <dan.mick@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: standardize rbd_request variable names
Alex Elder [Thu, 8 Nov 2012 14:01:39 +0000 (08:01 -0600)]
rbd: standardize rbd_request variable names

There are two names used for items of rbd_request structure type:
"req" and "req_data".  The former name is also used to represent
items of pointers to struct ceph_osd_request.

Change all variables that have these names so they are instead
called "rbd_req" consistently.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Dan Mick <dan.mick@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: add warnings to rbd_dev_probe_update_spec()
Alex Elder [Thu, 1 Nov 2012 15:17:15 +0000 (10:17 -0500)]
rbd: add warnings to rbd_dev_probe_update_spec()

Josh suggested adding warnings to this function to help users
diagnose problems.

Other than memory allocatino errors, there are two places where
errors can be returned.  Both represent problems that should
have been caught earlier, and as such might well have been
handled with BUG_ON() calls.  But if either ever did manage to
happen, it will be reported.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: add a warning in bio_chain_clone_range()
Alex Elder [Thu, 1 Nov 2012 15:17:15 +0000 (10:17 -0500)]
rbd: add a warning in bio_chain_clone_range()

Add a warning in bio_chain_clone_range() to help a user determine
what exactly might have led to a failure.  There is only one; please
say something if you disagree with the following reasoning.

There are three places this can return abnormally:
    - Initially, if there is nothing to clone.  It turns out that
      right now this cannot happen anyway.  The test is in place
      because the code below it doesn't work if those conditions
      don't hold.  As such they could be assertions but since I can
      return a null to indicate an error I just do that instead.
      I have not added a warning here because it won't happen.
    - While processing bio's, if none remain but there are supposed
      to be more bytes to clone.  Here I have added a warning.
    - If bio_clone_range() returns a null pointer.  That function
      will have already produced a warning (at least the first
      time, via WARN_ON_ONCE()) to distinguish the cause of the
      error.  The only exception is memory exhaustion, and I'd
      rather not pepper the code with warnings in all those spots.
      So no warning is added in that place.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: add warning messages for missing arguments
Alex Elder [Thu, 1 Nov 2012 15:17:15 +0000 (10:17 -0500)]
rbd: add warning messages for missing arguments

Tell the user (via dmesg) what was wrong with the arguments provided
via /sys/bus/rbd/add.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Dan Mick <dan.mick@inktank.com>
11 years agorbd: define and use rbd_warn()
Alex Elder [Thu, 1 Nov 2012 15:17:15 +0000 (10:17 -0500)]
rbd: define and use rbd_warn()

Define a new function rbd_warn() that produces a boilerplate warning
message, identifying in the resulting message the affected rbd
device in the best way available.  Use it in a few places that now
use pr_warning().

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Dan Mick <dan.mick@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agoceph: define ceph_encode_8_safe()
Alex Elder [Thu, 1 Nov 2012 13:39:27 +0000 (08:39 -0500)]
ceph: define ceph_encode_8_safe()

It's kind of a silly macro, but ceph_encode_8_safe() is the only one
missing from an otherwise pretty complete set.  It's not used, but
neither are a couple of the others in this set.

While in there, insert some whitespace to tidy up the alignment of
the line-terminating backslashes in some of the macro definitions.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Dan Mick <dan.mick@inktank.com>
11 years agorbd: use kmemdup()
Alex Elder [Thu, 1 Nov 2012 13:39:27 +0000 (08:39 -0500)]
rbd: use kmemdup()

This replaces two kmalloc()/memcpy() combinations with a single
call to kmemdup().

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: David Zafman <david.zafman@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: kill rbd_spec->image_id_len
Alex Elder [Thu, 1 Nov 2012 13:39:26 +0000 (08:39 -0500)]
rbd: kill rbd_spec->image_id_len

There is no real benefit to keeping the length of an image id, so
get rid of it.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: David Zafman <david.zafman@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: kill rbd_spec->image_name_len
Alex Elder [Thu, 1 Nov 2012 13:39:26 +0000 (08:39 -0500)]
rbd: kill rbd_spec->image_name_len

There may have been a benefit to hanging on to the length of an
image name before, but there is really none now.  The only time it's
used is when probing for rbd images, so we can just compute the
length then.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: David Zafman <david.zafman@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: document rbd_spec structure
Alex Elder [Thu, 1 Nov 2012 13:39:26 +0000 (08:39 -0500)]
rbd: document rbd_spec structure

I promised Josh I would document whether there were any restrictions
needed for accessing fields of an rbd_spec structure.  This adds a
big block of comments that documents the structure and how it is
used--including the fact that we don't attempt to synchronize access
to it.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: David Zafman <david.zafman@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: reformat __reset_osd()
Alex Elder [Fri, 7 Dec 2012 15:57:58 +0000 (09:57 -0600)]
libceph: reformat __reset_osd()

Reformat __reset_osd() into three distinct blocks of code
handling the three return cases.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agocrush: avoid recursion if we have already collided
Sage Weil [Wed, 16 Jan 2013 02:49:09 +0000 (18:49 -0800)]
crush: avoid recursion if we have already collided

This saves us some cycles, but does not affect the placement result at
all.

This corresponds to ceph.git commit 4abb53d4f.

Signed-off-by: Sage Weil <sage@inktank.com>
11 years agolibceph: for chooseleaf rules, retry CRUSH map descent from root if leaf is failed
Jim Schutt [Fri, 30 Nov 2012 16:15:25 +0000 (09:15 -0700)]
libceph: for chooseleaf rules, retry CRUSH map descent from root if leaf is failed

Add libceph support for a new CRUSH tunable recently added to Ceph servers.

Consider the CRUSH rule
  step chooseleaf firstn 0 type <node_type>

This rule means that <n> replicas will be chosen in a manner such that
each chosen leaf's branch will contain a unique instance of <node_type>.

When an object is re-replicated after a leaf failure, if the CRUSH map uses
a chooseleaf rule the remapped replica ends up under the <node_type> bucket
that held the failed leaf.  This causes uneven data distribution across the
storage cluster, to the point that when all the leaves but one fail under a
particular <node_type> bucket, that remaining leaf holds all the data from
its failed peers.

This behavior also limits the number of peers that can participate in the
re-replication of the data held by the failed leaf, which increases the
time required to re-replicate after a failure.

For a chooseleaf CRUSH rule, the tree descent has two steps: call them the
inner and outer descents.

If the tree descent down to <node_type> is the outer descent, and the descent
from <node_type> down to a leaf is the inner descent, the issue is that a
down leaf is detected on the inner descent, so only the inner descent is
retried.

In order to disperse re-replicated data as widely as possible across a
storage cluster after a failure, we want to retry the outer descent. So,
fix up crush_choose() to allow the inner descent to return immediately on
choosing a failed leaf.  Wire this up as a new CRUSH tunable.

Note that after this change, for a chooseleaf rule, if the primary OSD
in a placement group has failed, choosing a replacement may result in
one of the other OSDs in the PG colliding with the new primary.  This
requires that OSD's data for that PG to need moving as well.  This
seems unavoidable but should be relatively rare.

This corresponds to ceph.git commit 88f218181a9e6d2292e2697fc93797d0f6d6e5dc.

Signed-off-by: Jim Schutt <jaschut@sandia.gov>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agoceph: check mds_wanted for imported cap
Yan, Zheng [Fri, 4 Jan 2013 07:30:10 +0000 (15:30 +0800)]
ceph: check mds_wanted for imported cap

The MDS may have incorrect wanted caps after importing caps. So the
client should check the value mds has and send cap update if necessary.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agoceph: allocate cap_release message when receiving cap import
Yan, Zheng [Fri, 4 Jan 2013 06:45:18 +0000 (14:45 +0800)]
ceph: allocate cap_release message when receiving cap import

When client wants to release an imported cap, it's possible there
is no reserved cap_release message in corresponding mds session.
so __queue_cap_release causes kernel panic.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agoceph: allow revoking duplicated caps issued by non-auth MDS
Yan, Zheng [Fri, 4 Jan 2013 06:37:57 +0000 (14:37 +0800)]
ceph: allow revoking duplicated caps issued by non-auth MDS

Allow revoking duplicated caps issued by non-auth MDS if these caps
are also issued by auth MDS.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agoceph: move dirty inode to migrating list when clearing auth caps
Yan, Zheng [Fri, 4 Jan 2013 06:28:07 +0000 (14:28 +0800)]
ceph: move dirty inode to migrating list when clearing auth caps

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agoceph: re-calculate truncate_size for strip object
Yan, Zheng [Fri, 30 Nov 2012 05:49:51 +0000 (13:49 +0800)]
ceph: re-calculate truncate_size for strip object

Otherwise osd may truncate the object to larger size.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agoceph: Check for created flag in response from mds
Sam Lang [Fri, 28 Dec 2012 17:56:46 +0000 (09:56 -0800)]
ceph: Check for created flag in response from mds

The mds now sends back a created inode if the create request
performed the create.  If the file already existed, no inode is
returned in the reply.  This allows ceph to set the created flag
in atomic_open so that permissions are properly checked in the case
that the file wasn't created by the create call to the mds.

To ensure compability with previous kernels, a feature for sending
back the inode in the create reply was added, so that the mds will
only send back the inode if the client indicates it supports the
feature.

Signed-off-by: Sam Lang <sam.lang@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agoceph: Check for err on mds request in atomic_open
Sam Lang [Wed, 19 Dec 2012 19:44:23 +0000 (09:44 -1000)]
ceph: Check for err on mds request in atomic_open

The error returned by ceph_mdsc_do_request includes errors sending the
request, errors on timeout, or any errors coming from the mds.  If
ceph_mdsc_do_request returns an error, the reply struct will most likely
be bogus.  We need to bail out and propogate the error instead of
overwriting it.

Signed-off-by: Sam Lang <sam.lang@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agolibceph: fix protocol feature mismatch failure path
Sage Weil [Fri, 28 Dec 2012 02:27:04 +0000 (20:27 -0600)]
libceph: fix protocol feature mismatch failure path

We should not set con->state to CLOSED here; that happens in
ceph_fault() in the caller, where it first asserts that the state
is not yet CLOSED.  Avoids a BUG when the features don't match.

Since the fail_protocol() has become a trivial wrapper, replace
calls to it with direct calls to reset_connection().

Signed-off-by: Sage Weil <sage@inktank.com>
Reviewed-by: Alex Elder <elder@inktank.com>
11 years agolibceph: WARN, don't BUG on unexpected connection states
Alex Elder [Wed, 26 Dec 2012 16:43:57 +0000 (10:43 -0600)]
libceph: WARN, don't BUG on unexpected connection states

A number of assertions in the ceph messenger are implemented with
BUG_ON(), killing the system if connection's state doesn't match
what's expected.  At this point our state model is (evidently) not
well understood enough for these assertions to trigger a BUG().
Convert all BUG_ON(con->state...) calls to be WARN_ON(con->state...)
so we learn about these issues without killing the machine.

We now recognize that a connection fault can occur due to a socket
closure at any time, regardless of the state of the connection.  So
there is really nothing we can assert about the state of the
connection at that point so eliminate that assertion.

Reported-by: Ugis <ugis22@gmail.com>
Tested-by: Ugis <ugis22@gmail.com>
Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agolibceph: always reset osds when kicking
Alex Elder [Wed, 26 Dec 2012 20:31:40 +0000 (14:31 -0600)]
libceph: always reset osds when kicking

When ceph_osdc_handle_map() is called to process a new osd map,
kick_requests() is called to ensure all affected requests are
updated if necessary to reflect changes in the osd map.  This
happens in two cases:  whenever an incremental map update is
processed; and when a full map update (or the last one if there is
more than one) gets processed.

In the former case, the kick_requests() call is followed immediately
by a call to reset_changed_osds() to ensure any connections to osds
affected by the map change are reset.  But for full map updates
this isn't done.

Both cases should be doing this osd reset.

Rather than duplicating the reset_changed_osds() call, move it into
the end of kick_requests().

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agolibceph: move linger requests sooner in kick_requests()
Alex Elder [Wed, 19 Dec 2012 21:52:36 +0000 (15:52 -0600)]
libceph: move linger requests sooner in kick_requests()

The kick_requests() function is called by ceph_osdc_handle_map()
when an osd map change has been indicated.  Its purpose is to
re-queue any request whose target osd is different from what it
was when it was originally sent.

It is structured as two loops, one for incomplete but registered
requests, and a second for handling completed linger requests.
As a special case, in the first loop if a request marked to linger
has not yet completed, it is moved from the request list to the
linger list.  This is as a quick and dirty way to have the second
loop handle sending the request along with all the other linger
requests.

Because of the way it's done now, however, this quick and dirty
solution can result in these incomplete linger requests never
getting re-sent as desired.  The problem lies in the fact that
the second loop only arranges for a linger request to be sent
if it appears its target osd has changed.  This is the proper
handling for *completed* linger requests (it avoids issuing
the same linger request twice to the same osd).

But although the linger requests added to the list in the first loop
may have been sent, they have not yet completed, so they need to be
re-sent regardless of whether their target osd has changed.

The first required fix is we need to avoid calling __map_request()
on any incomplete linger request.  Otherwise the subsequent
__map_request() call in the second loop will find the target osd
has not changed and will therefore not re-send the request.

Second, we need to be sure that a sent but incomplete linger request
gets re-sent.  If the target osd is the same with the new osd map as
it was when the request was originally sent, this won't happen.
This can be fixed through careful handling when we move these
requests from the request list to the linger list, by unregistering
the request *before* it is registered as a linger request.  This
works because a side-effect of unregistering the request is to make
the request's r_osd pointer be NULL, and *that* will ensure the
second loop actually re-sends the linger request.

Processing of such a request is done at that point, so continue with
the next one once it's been moved.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agorbd: get rid of rbd_{get,put}_dev()
Alex Elder [Fri, 16 Nov 2012 15:29:16 +0000 (09:29 -0600)]
rbd: get rid of rbd_{get,put}_dev()

The functions rbd_get_dev() and rbd_put_dev() are trivial wrappers
that add no value, and their existence suggests they may do more
than what they do.

Get rid of them.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Dan Mick <dan.mick@inktank.com>
11 years agolibceph: register request before unregister linger
Alex Elder [Thu, 6 Dec 2012 13:22:04 +0000 (07:22 -0600)]
libceph: register request before unregister linger

In kick_requests(), we need to register the request before we
unregister the linger request.  Otherwise the unregister will
reset the request's osd pointer to NULL.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agolibceph: don't use rb_init_node() in ceph_osdc_alloc_request()
Alex Elder [Mon, 17 Dec 2012 18:23:48 +0000 (12:23 -0600)]
libceph: don't use rb_init_node() in ceph_osdc_alloc_request()

The red-black node in the ceph osd request structure is initialized
in ceph_osdc_alloc_request() using rbd_init_node().  We do need to
initialize this, because in __unregister_request() we call
RB_EMPTY_NODE(), which expects the node it's checking to have
been initialized.  But rb_init_node() is apparently overkill, and
may in fact be on its way out.  So use RB_CLEAR_NODE() instead.

For a little more background, see this commit:
    4c199a93 rbtree: empty nodes have no color"

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agolibceph: init event->node in ceph_osdc_create_event()
Alex Elder [Mon, 17 Dec 2012 18:23:48 +0000 (12:23 -0600)]
libceph: init event->node in ceph_osdc_create_event()

The red-black node node in the ceph osd event structure is not
initialized in create_osdc_create_event().  Because this node can
be the subject of a RB_EMPTY_NODE() call later on, we should ensure
the node is initialized properly for that.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agolibceph: init osd->o_node in create_osd()
Alex Elder [Thu, 6 Dec 2012 13:22:04 +0000 (07:22 -0600)]
libceph: init osd->o_node in create_osd()

The red-black node node in the ceph osd structure is not initialized
in create_osd().  Because this node can be the subject of a
RB_EMPTY_NODE() call later on, we should ensure the node is
initialized properly for that.  Add a call to RB_CLEAR_NODE()
initialize it.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agolibceph: report connection fault with warning
Alex Elder [Fri, 14 Dec 2012 22:47:41 +0000 (16:47 -0600)]
libceph: report connection fault with warning

When a connection's socket disconnects, or if there's a protocol
error of some kind on the connection, a fault is signaled and
the connection is reset (closed and reopened, basically).  We
currently get an error message on the log whenever this occurs.

A ceph connection will attempt to reestablish a socket connection
repeatedly if a fault occurs.  This means that these error messages
will get repeatedly added to the log, which is undesirable.

Change the error message to be a warning, so they don't get
logged by default.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agolibceph: socket can close in any connection state
Alex Elder [Sat, 8 Dec 2012 01:50:07 +0000 (19:50 -0600)]
libceph: socket can close in any connection state

A connection's socket can close for any reason, independent of the
state of the connection (and without irrespective of the connection
mutex).  As a result, the connectino can be in pretty much any state
at the time its socket is closed.

Handle those other cases at the top of con_work().  Pull this whole
block of code into a separate function to reduce the clutter.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agorbd: don't use ENOTSUPP
Alex Elder [Thu, 1 Nov 2012 13:39:26 +0000 (08:39 -0500)]
rbd: don't use ENOTSUPP

ENOTSUPP is not a standard errno (it shows up as "Unknown error 524"
in an error message).  This is what was getting produced when the
the local rbd code does not implement features required by a
discovered rbd image.

Change the error code returned in this case to ENXIO.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agorbd: remove linger unconditionally
Alex Elder [Thu, 6 Dec 2012 15:37:23 +0000 (09:37 -0600)]
rbd: remove linger unconditionally

In __unregister_linger_request(), the request is being removed
from the osd client's req_linger list only when the request
has a non-null osd pointer.  It should be done whether or not
the request currently has an osd.

This is most likely a non-issue because I believe the request
will always have an osd when this function is called.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agorbd: get rid of RBD_MAX_SEG_NAME_LEN
Alex Elder [Fri, 9 Nov 2012 21:05:54 +0000 (15:05 -0600)]
rbd: get rid of RBD_MAX_SEG_NAME_LEN

RBD_MAX_SEG_NAME_LEN represents the maximum length of an rbd object
name (i.e., one of the objects providing storage backing an rbd
image).

Another symbol, MAX_OBJ_NAME_SIZE, is used in the osd client code to
define the maximum length of any object name in an osd request.

Right now they disagree, with RBD_MAX_SEG_NAME_LEN being too big.

There's no real benefit at this point to defining the rbd object
name length limit separate from any other object name, so just
get rid of RBD_MAX_SEG_NAME_LEN and use MAX_OBJ_NAME_SIZE in its
place.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agolibceph: avoid using freed osd in __kick_osd_requests()
Alex Elder [Fri, 7 Dec 2012 15:57:58 +0000 (09:57 -0600)]
libceph: avoid using freed osd in __kick_osd_requests()

If an osd has no requests and no linger requests, __reset_osd()
will just remove it with a call to __remove_osd().  That drops
a reference to the osd, and therefore the osd may have been free
by the time __reset_osd() returns.  That function offers no
indication this may have occurred, and as a result the osd will
continue to be used even when it's no longer valid.

Change__reset_osd() so it returns an error (ENODEV) when it
deletes the osd being reset.  And change __kick_osd_requests() so it
returns immediately (before referencing osd again) if __reset_osd()
returns *any* error.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agoceph: don't reference req after put
Alex Elder [Thu, 29 Nov 2012 14:37:03 +0000 (08:37 -0600)]
ceph: don't reference req after put

In __unregister_request(), there is a call to list_del_init()
referencing a request that was the subject of a call to
ceph_osdc_put_request() on the previous line.  This is not
safe, because the request structure could have been freed
by the time we reach the list_del_init().

Fix this by reversing the order of these lines.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-off-by: Sage Weil <sage@inktank.com>
11 years agorbd: do not allow remove of mounted-on image
Alex Elder [Fri, 16 Nov 2012 15:29:16 +0000 (09:29 -0600)]
rbd: do not allow remove of mounted-on image

There is no check in rbd_remove() to see if anybody holds open the
image being removed.  That's not cool.

Add a simple open count that goes up and down with opens and closes
(releases) of the device, and don't allow an rbd image to be removed
if the count is non-zero.

Protect the updates of the open count value with ctl_mutex to ensure
the underlying rbd device doesn't get removed while concurrently
being opened.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
11 years agolibceph: Unlock unprocessed pages in start_read() error path
David Zafman [Tue, 4 Dec 2012 03:14:05 +0000 (19:14 -0800)]
libceph: Unlock unprocessed pages in start_read() error path

Function start_read() can get an error before processing all pages.
It must not only release the remaining pages, but unlock them too.

This fixes http://tracker.newdream.net/issues/3370

Signed-off-by: David Zafman <david.zafman@inktank.com>
Reviewed-by: Alex Elder <elder@inktank.com>
11 years agoceph: call handle_cap_grant() for cap import message
Yan, Zheng [Mon, 19 Nov 2012 02:49:09 +0000 (10:49 +0800)]
ceph: call handle_cap_grant() for cap import message

If client sends cap message that requests new max size during
exporting caps, the exporting MDS will drop the message quietly.
So the client may wait for the reply that updates the max size
forever. call handle_cap_grant() for cap import message can
avoid this issue.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Signed-off-by: Sage Weil <sage@inktank.com>
11 years agoceph: Fix __ceph_do_pending_vmtruncate
Yan, Zheng [Mon, 19 Nov 2012 02:49:08 +0000 (10:49 +0800)]
ceph: Fix __ceph_do_pending_vmtruncate

we should set i_truncate_pending to 0 after page cache is truncated
to i_truncate_size

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Signed-off-by: Sage Weil <sage@inktank.com>
11 years agoceph: Don't add dirty inode to dirty list if caps is in migration
Yan, Zheng [Mon, 19 Nov 2012 02:49:07 +0000 (10:49 +0800)]
ceph: Don't add dirty inode to dirty list if caps is in migration

Add dirty inode to cap_dirty_migrating list instead, this can avoid
ceph_flush_dirty_caps() entering infinite loop.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Signed-off-by: Sage Weil <sage@inktank.com>
11 years agoceph: Fix infinite loop in __wake_requests
Yan, Zheng [Mon, 19 Nov 2012 02:49:06 +0000 (10:49 +0800)]
ceph: Fix infinite loop in __wake_requests

__wake_requests() will enter infinite loop if we use it to wake
requests in the session->s_waiting list. __wake_requests() deletes
requests from the list and __do_request() adds requests back to
the list.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Signed-off-by: Sage Weil <sage@inktank.com>
11 years agoceph: Don't update i_max_size when handling non-auth cap
Yan, Zheng [Mon, 19 Nov 2012 02:49:04 +0000 (10:49 +0800)]
ceph: Don't update i_max_size when handling non-auth cap

The cap from non-auth mds doesn't have a meaningful max_size value.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Signed-off-by: Sage Weil <sage@inktank.com>
11 years agobdi_register: add __printf verification, fix arg mismatch
Joe Perches [Thu, 29 Nov 2012 14:37:03 +0000 (08:37 -0600)]
bdi_register: add __printf verification, fix arg mismatch

__printf is useful to verify format and arguments.

Signed-off-by: Joe Perches <joe@perches.com>
Reviewed-by: Alex Elder <elder@inktank.com>
11 years agolibceph: remove 'osdtimeout' option
Sage Weil [Wed, 28 Nov 2012 20:28:24 +0000 (12:28 -0800)]
libceph: remove 'osdtimeout' option

This would reset a connection with any OSD that had an outstanding
request that was taking more than N seconds.  The idea was that if the
OSD was buggy, the client could compensate by resending the request.

In reality, this only served to hide server bugs, and we haven't
actually seen such a bug in quite a while.  Moreover, the userspace
client code never did this.

More importantly, often the request is taking a long time because the
OSD is trying to recover, or overloaded, and killing the connection
and retrying would only make the situation worse by giving the OSD
more work to do.

Signed-off-by: Sage Weil <sage@inktank.com>
Reviewed-by: Alex Elder <elder@inktank.com>
11 years agoceph: fix dentry reference leak in ceph_encode_fh().
Cyril Roelandt [Tue, 20 Nov 2012 16:23:07 +0000 (10:23 -0600)]
ceph: fix dentry reference leak in ceph_encode_fh().

dput() was not called in the error path.

Signed-off-by: Cyril Roelandt <tipecaml@gmail.com>
Reviewed-by: Alex Elder <elder@inktank.com>
11 years agoceph: Fix i_size update race
Sage Weil [Mon, 5 Nov 2012 19:07:23 +0000 (11:07 -0800)]
ceph: Fix i_size update race

ceph_aio_write() has an optimization that marks cap EPH_CAP_FILE_WR
dirty before data is copied to page cache and inode size is updated.
If ceph_check_caps() flushes the dirty cap before the inode size is
updated, MDS can miss the new inode size. The fix is move
ceph_{get,put}_cap_refs() into ceph_write_{begin,end}() and call
__ceph_mark_dirty_caps() after inode size is updated.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Signed-off-by: Sage Weil <sage@inktank.com>
11 years agoceph: Hold caps_list_lock when adjusting caps_{use, total}_count
Yan, Zheng [Sat, 3 Nov 2012 02:32:37 +0000 (10:32 +0800)]
ceph: Hold caps_list_lock when adjusting caps_{use, total}_count

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Signed-off-by: Sage Weil <sage@inktank.com>
11 years agorbd: get additional info in parent spec
Alex Elder [Wed, 31 Oct 2012 00:40:33 +0000 (19:40 -0500)]
rbd: get additional info in parent spec

When a layered rbd image has a parent, that parent is identified
only by its pool id, image id, and snapshot id.  Images that have
been mapped also record *names* for those three id's.

Add code to look up these names for parent images so they match
mapped images more closely.  Skip doing this for an image if it
already has its pool name defined (this will be the case for images
mapped by the user).

It is possible that an the name of a parent image can't be
determined, even if the image id is valid.  If this occurs it
does not preclude correct operation, so don't treat this as
an error.

On the other hand, defined pools will always have both an id and a
name.   And any snapshot of an image identified as a parent for a
clone image will exist, and will have a name (if not it indicates
some other internal error).  So treat failure to get these bits
of information as errors.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agolibceph: define ceph_pg_pool_name_by_id()
Alex Elder [Wed, 31 Oct 2012 00:40:33 +0000 (19:40 -0500)]
libceph: define ceph_pg_pool_name_by_id()

Define and export function ceph_pg_pool_name_by_id() to supply
the name of a pg pool whose id is given.  This will be used by
the next patch.

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

Add support for getting the the information identifying the parent
image for rbd images that have them.  The child image holds a
reference to its parent image specification structure.  Create a new
entry "parent" in /sys/bus/rbd/image/N/ to report the identifying
information for the parent image, if any.

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

Format 2 parent images are partially identified by their image id,
but it may not be possible to determine their image name.  The name
is not strictly needed for correct operation, so we won't be
treating it as an error if we don't know it.  Handle this case
gracefully in rbd_name_show().

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

We will know the image id for format 2 parent images, but won't
initially know its image name.  Avoid making the query for an image
id in rbd_dev_image_id() if it's already known.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: encapsulate last part of probe
Alex Elder [Tue, 30 Oct 2012 20:47:17 +0000 (15:47 -0500)]
rbd: encapsulate last part of probe

Group the activities that now take place after an rbd_dev_probe()
call into a single function, and move the call to that function
into rbd_dev_probe() itself.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: define rbd_dev_{create,destroy}() helpers
Alex Elder [Fri, 26 Oct 2012 04:34:42 +0000 (23:34 -0500)]
rbd: define rbd_dev_{create,destroy}() helpers

Encapsulate the creation/initialization and destruction of rbd
device structures.  The rbd_client and the rbd_spec structures
provided on creation hold references whose ownership is transferred
to the new rbd_device structure.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: consolidate rbd_dev init in rbd_add()
Alex Elder [Fri, 26 Oct 2012 04:34:42 +0000 (23:34 -0500)]
rbd: consolidate rbd_dev init in rbd_add()

Group the allocation and initialization of fields of the rbd device
structure created in rbd_add().  Move the grouped code down later in
the function, just prior to the call to rbd_dev_probe().  This is
for the most part simple code movement.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
11 years agorbd: don't pass rbd_dev to rbd_get_client()
Alex Elder [Fri, 26 Oct 2012 04:34:42 +0000 (23:34 -0500)]
rbd: don't pass rbd_dev to rbd_get_client()

The only reason rbd_dev is passed to rbd_get_client() is so its
rbd_client field can get assigned.  Instead, just return the
rbd_client pointer as a result and have the caller do the
assignment.

Change rbd_put_client() so it takes an rbd_client structure,
so follows the more typical symmetry with rbd_get_client().

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