rbd: drop extra header_rwsem init

In commit c666601a there was inadvertently added an extra
initialization of rbd_dev->header_rwsem.  This gets rid of the
duplicate.

Reported-by: Guangliang Zhao <gzhao@suse.com>
Signed-off-by: Alex Elder <elder@inktank.com>
rbd: simplify __rbd_remove_all_snaps()

This just replaces a while loop with list_for_each_entry_safe()
in __rbd_remove_all_snaps().

Signed-off-by: Alex Elder <elder@inktank.com>
rbd: clean up a few dout() calls

There was a dout() call in rbd_do_request() that was reporting
the reporting the offset as the length and vice versa.  While
fixing that I did a quick scan of other dout() calls and fixed
a couple of other minor things.

Signed-off-by: Alex Elder <elder@inktank.com>
ceph: define snap counts as u32 everywhere

There are two structures in which a count of snapshots are
maintained:

    struct ceph_snap_context {
	...
        u32 num_snaps;
	...
    }
and
    struct ceph_snap_realm {
	...
        u32 num_prior_parent_snaps;   /*  had prior to parent_since */
	...
        u32 num_snaps;
	...
    }

These fields never take on negative values (e.g., to hold special
meaning), and so are really inherently unsigned.  Furthermore they
take their value from over-the-wire or on-disk formatted 32-bit
values.

So change their definition to have type u32, and change some spots
elsewhere in the code to account for this change.

Signed-off-by: Alex Elder <elder@inktank.com>
rbd: snapc is unused in rbd_req_sync_read()

The "snapc" parameter to in rbd_req_sync_read() is not used, so
get rid of it.

Reported-by: Josh Durgin <josh.durgin@inktank.com>
Signed-off-by: Alex Elder <elder@inktank.com>
rbd: drop rbd_header_from_disk() gfp_flags parameter

The function rbd_header_from_disk() is only called in one spot, and
it passes GFP_KERNEL as its value for the gfp_flags parameter.

Just drop that parameter and substitute GFP_KERNEL everywhere within
that function it had been used.  (If we find we need the parameter
again in the future it's easy enough to add back again.)

Signed-off-by: Alex Elder <elder@inktank.com>
rbd: drop rbd_dev parameter in snap functions

Both rbd_register_snap_dev() and __rbd_remove_snap_dev() have
rbd_dev parameters that are unused.  Remove them.

Signed-off-by: Alex Elder <elder@inktank.com>
rbd: drop rbd_req_sync_exec() "ver" parameter

The only place that passes a version pointer to rbd_req_sync_exec()
is in rbd_header_add_snap(), and that spot ignores the result.

The only thing rbd_req_sync_exec() does with its "ver" parameter is
pass it directly to rbd_req_sync_op().  So we can just use a null
pointer there, and drop the "ver" parameter to rbd_req_sync_exec().

Signed-off-by: Alex Elder <elder@inktank.com>
rbd: have __rbd_add_snap_dev() return a pointer

It's not obvious whether the snapshot pointer whose address is
provided to __rbd_add_snap_dev() will be assigned by that function.
Change it to return the snapshot, or a pointer-coded errno in the
event of a failure.

Signed-off-by: Alex Elder <elder@inktank.com>
rbd: make rbd_create_rw_ops() return a pointer

Either rbd_create_rw_ops() will succeed, or it will fail because a
memory allocation failed.  Have it just return a valid pointer or
null rather than stuffing a pointer into a provided address and
returning an errno.

Signed-off-by: Alex Elder <elder@inktank.com>
rbd: always pass ops array to rbd_req_sync_op()

All of the callers of rbd_req_sync_op() except one pass a non-null
"ops" pointer.  The only one that does not is rbd_req_sync_read(),
which passes CEPH_OSD_OP_READ as its "opcode" and, CEPH_OSD_FLAG_READ
for "flags".

By allocating the ops array in rbd_req_sync_read() and moving the
special case code for the null ops pointer into it, it becomes
clear that much of that code is not even necessary.

In addition, the "opcode" argument to rbd_req_sync_op() is never
actually used, so get rid of that.

Signed-off-by: Alex Elder <elder@inktank.com>
rbd: fixes in rbd_header_from_disk()

This fixes a few issues in rbd_header_from_disk():
    - The memcmp() call at the beginning of the function is really
      looking at the "text" field of struct rbd_image_header_ondisk.
      While it does lie at the beginning of the structure, the
      comparison should be done against the field, not the structure
      as a whole.
    - There is a check intended to catch overflow, but it's wrong in
      two ways.
	- First, the type we don't want to overflow is size_t, not
	  unsigned int, and there is now a SIZE_MAX we can use for
	  use with that type.
	- Second, we're allocating the snapshot ids and snapshot
	  image sizes separately (each has type u64; on disk they
          grouped together as a rbd_image_header_ondisk structure).
	  So we can use the size of u64 in this overflow check.
    - If there are no snapshots, then there should be no snapshot
      names.  Enforce this, and issue a warning if we encounter a
      header with no snapshots but a non-zero snap_names_len.
    - When saving the snapshot names into the header, be more direct
      in defining the offset in the on-disk structure from which
      they're being copied by using "snap_count" rather than "i"
      in the array index.
    - If an error occurs, the "snapc" and "snap_names" fields are
      freed at the end of the function.  Make those fields be null
      pointers after they're freed, to be explicit that they are
      no longer valid.
    - Finally, move the definition of the local variable "i" to the
      innermost scope in which it's needed.

Signed-off-by: Alex Elder <elder@inktank.com>
rbd: __rbd_init_snaps_header() bug

There is a bug in the way __rbd_init_snaps_header() handles newly-
discovered snapshots, in the unusual case where the new snapshot id
is less than an existing rbd snapshot id.


When a new snapshot for an rbd image is created it is assigned a new
snapshot id.  The client requests the id from a monitor, and later
updates the osds with information about the snapshot.  The sequence
of snapshot ids is monotonically increasing, which guarantees each
is unique.  And typically all new snapshots will have higher ids
than any others a client has encountered.

Because of the delay between getting assigned a snapshot id and
recording the snapshot, there is a possible race between two clients
requesting new snapshots on the same rbd image.  It's possible in
this case for the client granted a higher id to be the first to
update the set of snapshots for the rbd image.  This leaves a sort
of gap in the set of snapshots until the other client's update gets
recorded.

This is an unlikely scenario, but it appears to be possible.  I'm
not sure whether we even support multiple concurrent clients for the
same rbd image.  However there is code in place to handle this case
and it's wrong, so it needs to be fixed.


The job of __rbd_init_snaps_header() is to compare an rbd's existing
list of snapshots to a new set, and remove any existing snapshots
that are not present in the new set and create any new ones not
present in the existing set.

If any new snapshots exist after the entire list of existing ones
has been examined, all the new ones that remain need to be created.
The logic for handling these last snapshots correct.

However, if a new snapshot is found whose id is less than an
existing one, the logic for handling this is wrong.  The basic
problem is that the name and current id used here are taken from
a place one position beyond where they should be.  In the event
this block of code is executed the first pass through the outer
loop, for example, these values will refer to invalid memory.

The fix is to make this block of code more closely match the
block that handles adding new snapshots at the end.  There are
three differences:
    - we use a do..while loop because we know we initially have an
      entry to process.
    - we insert the new entry at a position within the list instead
      of at the beginning
    - because we use it in the loop condition, we need to update
      our notion of "current id" each pass through.

Signed-off-by: Alex Elder <elder@inktank.com>
rbd: assign header name later

Move the assignment of the header name for an rbd image a bit later,
outside rbd_add_parse_args() and into its caller.

Signed-off-by: Alex Elder <elder@inktank.com>
rbd: do some header initialization earlier

Move some of the code that initializes an rbd header out of
rbd_init_disk and into its caller.  In order to do this we need
to supply the disk size (either the size of the base image or
the size of the snapshot) to rbd_init_disk().

Signed-off-by: Alex Elder <elder@inktank.com>
rbd: register device after header init

Now move registration of the device with the system down, after
doing all of the rbd header initialization.

This was done in steps in hopes it would make reviewing the change a
little easier.

Signed-off-by: Alex Elder <elder@inktank.com>
rbd: rearrange rbd_header_from_disk()

This just moves code around for the most part.  It was pulled out as
a separate patch to avoid cluttering up some upcoming patches which
are more substantive.  The point is basically to group everything
related to initializing the snapshot context together.

The only functional change is that rbd_read_header() now ensures
the header it passes is zero-filled.  This allows a simpler
error handling path.

Signed-off-by: Alex Elder <elder@inktank.com>
rbd: define struct snap_context

Currently a ceph snapshot context stores an array of ceph snapshot
ids.  This is consistent with how they're stored in the rbd image
header.  But there's no reason the in-core representation of a
snapshot has to match what's on disk.

Define a new struct type to hold the snapshot id, and eventually
hold other information we know about the snapshot.  This is
motivated by upcoming changes to support rbd image format 2.

Signed-off-by: Alex Elder <elder@inktank.com>
rbd: incorporate snapshot sizes in snap context

Rather than allocating a separate array of sizes to record each
snapshot's size, just include each snapshot's size in its
rbd_snap structure that's incorporated in the snapshot context.

Signed-off-by: Alex Elder <elder@inktank.com>
rbd: incorporate snapshot names in snap context

Rather than allocating a copy of the on-disk chunk of snapshot
names, allocate individual name strings for each snapshot and
include each snapshot's size in its rbd_snap structure that's
incorporated in the snapshot context.

This requires a bit more work than was needed for the snapshot id
and snapshot size.  rbd_header_from_disk() now makes the copy of
each snapshot name, and to protect from possible corruption it
also now takes and end pointer to ensure we don't overrun the
(source) names buffer in the process of copying.

In addition, snap_by_name() now just indexes the snaphot array to
get at the name rather than having to traverse the snap_names block.

Finally,

Signed-off-by: Alex Elder <elder@inktank.com>