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>
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>
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>
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>
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.
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.
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.
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.
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.
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.
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.
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.
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
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.
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.
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.
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.
Alex Elder [Wed, 14 Nov 2012 03:11:15 +0000 (21:11 -0600)]
rbd: 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 ot get_num_ops(), and as a result we
can also get rid of op_needs_trail() which is no longer used.
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.
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.
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.
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.
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.
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.
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.
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 and release the header semaphore every time we
process a request for an rbd image. We do this because for write
requests we need to supply the snapshot context, and we can't
safely get a reference to it without holding that semaphore.
There's no need to get the snap context if we're doing a read,
so avoid doing so in that case.
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. Although I don't think synchronizing
access to this carefully is that critical, it is converted here to
be an atomic variable so a request is aware the flag is clear as
soon as that is known.
Alex Elder [Fri, 9 Nov 2012 14:43:15 +0000 (08:43 -0600)]
rbd: fix reference leak in rbd_do_op()
This commit introduced a bug: 4634246d rbd: consolidate rbd_do_op() calls
When a read request is being issued, the snapshot context provided
isn't needed. But that snap context pointer was acquired in
rbd_rq_fn() and it carries with it a reference to that structure.
So before discarding it, the reference needs to be dropped.
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.
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.
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.
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.
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.
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.
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().
Alex Elder [Thu, 1 Nov 2012 15:17:15 +0000 (10:17 -0500)]
rbd: fix ceph_pg_poolid_by_name()
Currently ceph_pg_poolid_by_name() returns an int, which is used to
encode a ceph pool id. This could be a problem because a pool id
(at least in some cases) is a 64-bit value. We have a defined pool
id value that represents "no pool," and that's a very sensible
return value here.
This patch changes ceph_pg_poolid_by_name() to return a 64-bit
pool id value, or CEPH_NOPOOL if the named pool is not found.
The patch also gratuitously renames the function, separating "pool"
from "id" in the name by an underscore.
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.
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.
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.
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.
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>
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>
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>
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>
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>
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>
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>
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>
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>
Alex Elder [Fri, 26 Oct 2012 04:34:42 +0000 (23:34 -0500)]
rbd: fill rbd_spec in rbd_add_parse_args()
Pass the address of an rbd_spec structure to rbd_add_parse_args().
Use it to hold the information defining the rbd image to be mapped
in an rbd_add() call.
Use the result in the caller to initialize the rbd_dev->id field.
This means rbd_dev is no longer needed in rbd_add_parse_args(),
so get rid of it.
Now that this transformation of rbd_add_parse_args() is complete,
correct and expand on the its header documentation to reflect the
new reality.
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Alex Elder [Fri, 26 Oct 2012 04:34:41 +0000 (23:34 -0500)]
rbd: define image specification structure
Group the fields that uniquely specify an rbd image into a new
reference-counted rbd_spec structure. This structure will be used
to describe the desired image when mapping an image, and when
probing parent images in layered rbd devices. Replace the set of
fields in the rbd device structure with a pointer to a dynamically
allocated rbd_spec.
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Alex Elder [Fri, 26 Oct 2012 04:34:41 +0000 (23:34 -0500)]
rbd: have rbd_add_parse_args() return error
Change the interface to rbd_add_parse_args() so it returns an
error code rather than a pointer. Return the ceph_options result
via a pointer whose address is passed as an argument.
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Alex Elder [Fri, 26 Oct 2012 04:34:41 +0000 (23:34 -0500)]
rbd: pass and populate rbd_options structure
Have the caller pass the address of an rbd_options structure to
rbd_add_parse_args(), to be initialized with the information
gleaned as a result of the parse.
I know, this is another near-reversal of a recent change...
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Alex Elder [Fri, 26 Oct 2012 04:34:41 +0000 (23:34 -0500)]
rbd: remove snap_name arg from rbd_add_parse_args()
The snapshot name returned by rbd_add_parse_args() just gets saved
in the rbd_dev eventually. So just do that inside that function and
do away with the snap_name argument, both in rbd_add_parse_args()
and rbd_dev_set_mapping().
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Alex Elder [Fri, 26 Oct 2012 04:34:41 +0000 (23:34 -0500)]
rbd: remove options args from rbd_add_parse_args()
They "options" argument to rbd_add_parse_args() (and it's partner
options_size) is now only needed within the function, so there's no
need to have the caller allocate and pass the options buffer. Just
allocate the options buffer within the function using dup_token().
Also distinguish between failures due to failed memory allocation
and failing because a required argument was missing.
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Alex Elder [Fri, 26 Oct 2012 04:34:41 +0000 (23:34 -0500)]
rbd: do all argument parsing in one place
This patch makes rbd_add_parse_args() be the single place all
argument parsing occurs for an image map request:
- Move the ceph_parse_options() call into that function
- Use local variables rather than parameters to hold the list
of monitor addresses supplied
- Rather than returning it, pass the snapshot name (and its
length) back via parameters
- Have the function return a ceph_options structure pointer
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Alex Elder [Fri, 26 Oct 2012 22:25:23 +0000 (17:25 -0500)]
rbd: rename snap_exists field
A Boolean field "snap_exists" in an rbd mapping is used to indicate
whether a mapped snapshot has been removed from an image's snapshot
context, to stop sending requests for that snapshot as soon as we
know it's gone.
Generalize the interpretation of this field so it applies to
non-snapshot (i.e. "head") mappings. That is, define its value
to be false until the mapping has been set, and then define it to be
true for both snapshot mappings or head mappings.
Rename the field "exists" to reflect the broader interpretation.
The rbd_mapping structure is on its way out, so move the field
back into the rbd_device structure.
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Alex Elder [Fri, 26 Oct 2012 04:34:41 +0000 (23:34 -0500)]
rbd: move snap info out of rbd_mapping struct
Moving the snap_id and snap_name fields into the separate
rbd_mapping structure was misguided. (And in time, perhaps
we'll do away with that structure altogether...)
Move these fields back into struct rbd_device.
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Alex Elder [Fri, 26 Oct 2012 04:34:40 +0000 (23:34 -0500)]
rbd: remove snapshots on error in rbd_add()
If rbd_dev_snaps_update() has ever been called for an rbd device
structure there could be snapshot structures on its snaps list.
In rbd_add(), this function is called but a subsequent error
path neglected to clean up any of these snapshots.
Add a call to rbd_remove_all_snaps() in the appropriate spot to
remedy this. Change a couple of error labels to be a little
clearer while there.
Drop the leading underscores from the function name; there's nothing
special about that function that they might signify. As suggested
in review, the leading underscores in __rbd_remove_snap_dev() have
been removed as well.
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Alex Elder [Sun, 21 Oct 2012 03:17:27 +0000 (22:17 -0500)]
rbd: simplify rbd_rq_fn()
When processing a request, rbd_rq_fn() makes clones of the bio's in
the request's bio chain and submits the results to osd's to be
satisfied. If a request bio straddles the boundary between objects
backing the rbd image, it must be represented by two cloned bio's,
one for the first part (at the end of one object) and one for the
second (at the beginning of the next object).
This has been handled by a function bio_chain_clone(), which
includes an interface only a mother could love, and which has
been found to have other problems.
This patch defines two new fairly generic bio functions (one which
replaces bio_chain_clone()) to help out the situation, and then
revises rbd_rq_fn() to make use of them.
First, bio_clone_range() clones a portion of a single bio, starting
at a given offset within the bio and including only as many bytes
as requested. As a convenience, a request to clone the entire bio
is passed directly to bio_clone().
Second, bio_chain_clone_range() performs a similar function,
producing a chain of cloned bio's covering a sub-range of the
source chain. No bio_pair structures are used, and if successful
the result will represent exactly the specified range.
Using bio_chain_clone_range() makes bio_rq_fn() a little easier
to understand, because it avoids the need to pass very much
state information between consecutive calls. By avoiding the need
to track a bio_pair structure, it also eliminates the problem
described here: http://tracker.newdream.net/issues/2933
Note that a block request (and therefore the complete length of
a bio chain processed in rbd_rq_fn()) is an unsigned int, while
the result of rbd_segment_length() is u64. This change makes
this range trunctation explicit, and trips a bug if the the
segment boundary is too far off.
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Sage Weil [Mon, 29 Oct 2012 18:01:42 +0000 (11:01 -0700)]
libceph: fix osdmap decode error paths
Ensure that we set the err value correctly so that we do not pass a 0
value to ERR_PTR and confuse the calling code. (In particular,
osd_client.c handle_map() will BUG(!newmap)).
Signed-off-by: Sage Weil <sage@inktank.com> Reviewed-by: Alex Elder <elder@inktank.com>
Alex Elder [Mon, 22 Oct 2012 16:31:27 +0000 (11:31 -0500)]
rbd: kill rbd_device->rbd_opts
The rbd_device structure has an embedded rbd_options structure.
Such a structure is needed to work with the generic ceph argument
parsing code, but there's no need to keep it around once argument
parsing is done.
Use a local variable to hold the rbd options used in parsing in
rbd_get_client(), and just transfer its content (it's just a
read_only flag) into the field in the rbd_mapping sub-structure
that requires that information.
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com> Reviewed-by: Dan Mick <dan.mick@inktank.com>
Alex Elder [Sun, 21 Oct 2012 03:17:27 +0000 (22:17 -0500)]
rbd: simplify rbd_merge_bvec()
The aim of this patch is to make what's going on rbd_merge_bvec() a
bit more obvious than it was before. This was an issue when a
recent btrfs bug led us to question whether the merge function was
working correctly.
Use "obj" rather than "chunk" to indicate the units whose boundaries
we care about we call (rados) "objects".
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Dan Mick <dan.mick@inktank.com>
Alex Elder [Tue, 3 Jul 2012 21:01:19 +0000 (16:01 -0500)]
rbd: increase maximum snapshot name length
Change RBD_MAX_SNAP_NAME_LEN to be based on NAME_MAX. That is a
practical limit for the length of a snapshot name (based on the
presence of a directory using the name under /sys/bus/rbd to
represent the snapshot).
The /sys entry is created by prefixing it with "snap_"; define that
prefix symbolically, and take its length into account in defining
the snapshot name length limit.
Enforce the limit in rbd_add_parse_args(). Also delete a dout()
call in that function that was not meant to be committed.
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Dan Mick <dan.mick@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Alex Elder [Sun, 21 Oct 2012 03:17:27 +0000 (22:17 -0500)]
rbd: verify rbd image order value
This adds a verification that an rbd image's object order is
within the upper and lower bounds supported by this implementation.
It must be at least 9 (SECTOR_SHIFT), because the Linux bio system
assumes that minimum granularity.
It also must be less than 32 (at the moment anyway) because there
exist spots in the code that store the size of a "segment" (object
backing an rbd image) in a signed int variable, which can be 32 bits
including the sign. We should be able to relax this limit once
we've verified the code uses 64-bit types where needed.
Note that the CLI tool already limits the order to the range 12-25.
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Alex Elder [Thu, 11 Oct 2012 01:59:29 +0000 (18:59 -0700)]
rbd: consolidate rbd_do_op() calls
The two calls to rbd_do_op() from rbd_rq_fn() differ only in the
value passed for the snapshot id and the snapshot context.
For reads the snapshot always comes from the mapping, and for writes
the snapshot id is always CEPH_NOSNAP.
The snapshot context is always null for reads. For writes, the
snapshot context always comes from the rbd header, but it is
acquired under protection of header semaphore and could change
thereafter, so we can't simply use what's available inside
rbd_do_op().
Eliminate the snapid parameter from rbd_do_op(), and set it
based on the I/O direction inside that function instead. Always
pass the snapshot context acquired in the caller, but reset it
to a null pointer inside rbd_do_op() if the operation is a read.
As a result, there is no difference in the read and write calls
to rbd_do_op() made in rbd_rq_fn(), so just call it unconditionally.
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Alex Elder [Thu, 11 Oct 2012 01:59:29 +0000 (18:59 -0700)]
rbd: drop rbd_do_op() opcode and flags
The only callers of rbd_do_op() are in rbd_rq_fn(), where call one
is used for writes and the other used for reads. The request passed
to rbd_do_op() already encodes the I/O direction, and that
information can be used inside the function to set the opcode and
flags value (rather than passing them in as arguments).
So get rid of the opcode and flags arguments to rbd_do_op().
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Alex Elder [Thu, 11 Oct 2012 01:59:29 +0000 (18:59 -0700)]
rbd: kill rbd_req_{read,write}()
Both rbd_req_read() and rbd_req_write() are simple wrapper routines
for rbd_do_op(), and each is only called once. Replace each wrapper
call with a direct call to rbd_do_op(), and get rid of the wrapper
functions.
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Alex Elder [Thu, 11 Oct 2012 04:19:13 +0000 (21:19 -0700)]
rbd: zero return code in rbd_dev_image_id()
When rbd_dev_probe() calls rbd_dev_image_id() it expects to get
a 0 return code if successful, but it is getting a positive value.
The reason is that rbd_dev_image_id() returns the value it gets from
rbd_req_sync_exec(), which returns the number of bytes read in as a
result of the request. (This ultimately comes from
ceph_copy_from_page_vector() in rbd_req_sync_op()).
Force the return value to 0 when successful in rbd_dev_image_id().
Do the same in rbd_dev_v2_object_prefix().
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com> Reviewed-by: Dan Mick <dan.mick@inktank.com>
Alex Elder [Thu, 11 Oct 2012 04:19:13 +0000 (21:19 -0700)]
rbd: fix bug in rbd_dev_id_put()
In rbd_dev_id_put(), there's a loop that's intended to determine
the maximum device id in use. But it isn't doing that at all,
the effect of how it's written is to simply use the just-put id
number, which ignores whole purpose of this function.
Fix the bug.
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Sage Weil [Thu, 25 Oct 2012 15:49:41 +0000 (08:49 -0700)]
libceph: avoid NULL kref_put from NULL alloc_msg return
The ceph_on_in_msg_alloc() method calls the ->alloc_msg() helper which
may return NULL. It also drops con->mutex while it allocates a message,
which means that the connection state may change (e.g., get closed). If
that happens, we clean up and bail out. Avoid calling ceph_msg_put() on
a NULL return value and triggering a crash.
This was observed when an ->alloc_msg() call races with a timeout that
resends a zillion messages and resets the connection, and ->alloc_msg()
returns NULL (because the request was resent to another target).
Fixes http://tracker.newdream.net/issues/3342
Signed-off-by: Sage Weil <sage@inktank.com> Reviewed-by: Alex Elder <elder@inktank.com>
Alex Elder [Tue, 9 Oct 2012 20:50:17 +0000 (13:50 -0700)]
rbd: implement feature checks
Version 2 images have two sets of feature bit fields. The first
indicates features possibly used by the image. The second indicates
features that the client *must* support in order to use the image.
When an image (or snapshot) is first examined, we need to make sure
that the local implementation supports the image's required
features. If not, fail the probe for the image.
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Alex Elder [Fri, 31 Aug 2012 22:29:55 +0000 (17:29 -0500)]
rbd: define rbd_dev_v2_refresh()
Define a new function rbd_dev_v2_refresh() to update/refresh the
snapshot context for a format version 2 rbd image. This function
will update anything that is not fixed for the life of an rbd
image--at the moment this is mainly the snapshot context and (for
a base mapping) the size.
Update rbd_refresh_header() so it selects which function to use
based on the image format.
Rename __rbd_refresh_header() to be rbd_dev_v1_refresh()
to be consistent with the naming of its version 2 counterpart.
Similarly rename rbd_refresh_header() to be rbd_dev_refresh().
Unrelated--we use rbd_image_format_valid() here. Delete the other
use of it, which was primarily put in place to ensure that function
was referenced at the time it was defined.
Signed-off-by: Alex Elder <elder@inktank.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
projects / ceph-client.git / log