]> Pileus Git - ~andy/linux/blobdiff - drivers/block/rbd.c
rbd: fix parent request size assumption
[~andy/linux] / drivers / block / rbd.c
index a62e59a0aea7e3480a858502a3335a916b74d2db..597b9bbe2fc7100d11c0de18b9e37d81c5f7e2f1 100644 (file)
  * block device image metadata (in-memory version)
  */
 struct rbd_image_header {
-       /* These four fields never change for a given rbd image */
+       /* These six fields never change for a given rbd image */
        char *object_prefix;
-       u64 features;
        __u8 obj_order;
        __u8 crypt_type;
        __u8 comp_type;
+       u64 stripe_unit;
+       u64 stripe_count;
+       u64 features;           /* Might be changeable someday? */
 
        /* The remaining fields need to be updated occasionally */
        u64 image_size;
        struct ceph_snap_context *snapc;
-       char *snap_names;
-       u64 *snap_sizes;
-
-       u64 stripe_unit;
-       u64 stripe_count;
+       char *snap_names;       /* format 1 only */
+       u64 *snap_sizes;        /* format 1 only */
 };
 
 /*
@@ -225,6 +224,7 @@ struct rbd_obj_request {
                };
        };
        struct page             **copyup_pages;
+       u32                     copyup_page_count;
 
        struct ceph_osd_request *osd_req;
 
@@ -257,6 +257,7 @@ struct rbd_img_request {
                struct rbd_obj_request  *obj_request;   /* obj req initiator */
        };
        struct page             **copyup_pages;
+       u32                     copyup_page_count;
        spinlock_t              completion_lock;/* protects next_completion */
        u32                     next_completion;
        rbd_img_callback_t      callback;
@@ -359,7 +360,7 @@ static ssize_t rbd_add(struct bus_type *bus, const char *buf,
                       size_t count);
 static ssize_t rbd_remove(struct bus_type *bus, const char *buf,
                          size_t count);
-static int rbd_dev_image_probe(struct rbd_device *rbd_dev);
+static int rbd_dev_image_probe(struct rbd_device *rbd_dev, bool mapping);
 
 static struct bus_attribute rbd_bus_attrs[] = {
        __ATTR(add, S_IWUSR, NULL, rbd_add),
@@ -426,7 +427,8 @@ static void rbd_img_parent_read(struct rbd_obj_request *obj_request);
 static void rbd_dev_remove_parent(struct rbd_device *rbd_dev);
 
 static int rbd_dev_refresh(struct rbd_device *rbd_dev);
-static int rbd_dev_v2_refresh(struct rbd_device *rbd_dev);
+static int rbd_dev_v2_header_onetime(struct rbd_device *rbd_dev);
+static int rbd_dev_v2_header_info(struct rbd_device *rbd_dev);
 static const char *rbd_dev_v2_snap_name(struct rbd_device *rbd_dev,
                                        u64 snap_id);
 static int _rbd_dev_v2_snap_size(struct rbd_device *rbd_dev, u64 snap_id,
@@ -728,88 +730,123 @@ static bool rbd_dev_ondisk_valid(struct rbd_image_header_ondisk *ondisk)
 }
 
 /*
- * Create a new header structure, translate header format from the on-disk
- * header.
+ * Fill an rbd image header with information from the given format 1
+ * on-disk header.
  */
-static int rbd_header_from_disk(struct rbd_image_header *header,
+static int rbd_header_from_disk(struct rbd_device *rbd_dev,
                                 struct rbd_image_header_ondisk *ondisk)
 {
+       struct rbd_image_header *header = &rbd_dev->header;
+       bool first_time = header->object_prefix == NULL;
+       struct ceph_snap_context *snapc;
+       char *object_prefix = NULL;
+       char *snap_names = NULL;
+       u64 *snap_sizes = NULL;
        u32 snap_count;
-       size_t len;
        size_t size;
+       int ret = -ENOMEM;
        u32 i;
 
-       memset(header, 0, sizeof (*header));
+       /* Allocate this now to avoid having to handle failure below */
 
-       snap_count = le32_to_cpu(ondisk->snap_count);
+       if (first_time) {
+               size_t len;
 
-       len = strnlen(ondisk->object_prefix, sizeof (ondisk->object_prefix));
-       header->object_prefix = kmalloc(len + 1, GFP_KERNEL);
-       if (!header->object_prefix)
-               return -ENOMEM;
-       memcpy(header->object_prefix, ondisk->object_prefix, len);
-       header->object_prefix[len] = '\0';
+               len = strnlen(ondisk->object_prefix,
+                               sizeof (ondisk->object_prefix));
+               object_prefix = kmalloc(len + 1, GFP_KERNEL);
+               if (!object_prefix)
+                       return -ENOMEM;
+               memcpy(object_prefix, ondisk->object_prefix, len);
+               object_prefix[len] = '\0';
+       }
 
+       /* Allocate the snapshot context and fill it in */
+
+       snap_count = le32_to_cpu(ondisk->snap_count);
+       snapc = ceph_create_snap_context(snap_count, GFP_KERNEL);
+       if (!snapc)
+               goto out_err;
+       snapc->seq = le64_to_cpu(ondisk->snap_seq);
        if (snap_count) {
+               struct rbd_image_snap_ondisk *snaps;
                u64 snap_names_len = le64_to_cpu(ondisk->snap_names_len);
 
-               /* Save a copy of the snapshot names */
+               /* We'll keep a copy of the snapshot names... */
+
+               if (snap_names_len > (u64)SIZE_MAX)
+                       goto out_2big;
+               snap_names = kmalloc(snap_names_len, GFP_KERNEL);
+               if (!snap_names)
+                       goto out_err;
+
+               /* ...as well as the array of their sizes. */
 
-               if (snap_names_len > (u64) SIZE_MAX)
-                       return -EIO;
-               header->snap_names = kmalloc(snap_names_len, GFP_KERNEL);
-               if (!header->snap_names)
+               size = snap_count * sizeof (*header->snap_sizes);
+               snap_sizes = kmalloc(size, GFP_KERNEL);
+               if (!snap_sizes)
                        goto out_err;
+
                /*
-                * Note that rbd_dev_v1_header_read() guarantees
-                * the ondisk buffer we're working with has
+                * Copy the names, and fill in each snapshot's id
+                * and size.
+                *
+                * Note that rbd_dev_v1_header_info() guarantees the
+                * ondisk buffer we're working with has
                 * snap_names_len bytes beyond the end of the
                 * snapshot id array, this memcpy() is safe.
                 */
-               memcpy(header->snap_names, &ondisk->snaps[snap_count],
-                       snap_names_len);
+               memcpy(snap_names, &ondisk->snaps[snap_count], snap_names_len);
+               snaps = ondisk->snaps;
+               for (i = 0; i < snap_count; i++) {
+                       snapc->snaps[i] = le64_to_cpu(snaps[i].id);
+                       snap_sizes[i] = le64_to_cpu(snaps[i].image_size);
+               }
+       }
 
-               /* Record each snapshot's size */
+       /* We won't fail any more, fill in the header */
 
-               size = snap_count * sizeof (*header->snap_sizes);
-               header->snap_sizes = kmalloc(size, GFP_KERNEL);
-               if (!header->snap_sizes)
-                       goto out_err;
-               for (i = 0; i < snap_count; i++)
-                       header->snap_sizes[i] =
-                               le64_to_cpu(ondisk->snaps[i].image_size);
+       down_write(&rbd_dev->header_rwsem);
+       if (first_time) {
+               header->object_prefix = object_prefix;
+               header->obj_order = ondisk->options.order;
+               header->crypt_type = ondisk->options.crypt_type;
+               header->comp_type = ondisk->options.comp_type;
+               /* The rest aren't used for format 1 images */
+               header->stripe_unit = 0;
+               header->stripe_count = 0;
+               header->features = 0;
        } else {
-               header->snap_names = NULL;
-               header->snap_sizes = NULL;
+               ceph_put_snap_context(header->snapc);
+               kfree(header->snap_names);
+               kfree(header->snap_sizes);
        }
 
-       header->features = 0;   /* No features support in v1 images */
-       header->obj_order = ondisk->options.order;
-       header->crypt_type = ondisk->options.crypt_type;
-       header->comp_type = ondisk->options.comp_type;
-
-       /* Allocate and fill in the snapshot context */
+       /* The remaining fields always get updated (when we refresh) */
 
        header->image_size = le64_to_cpu(ondisk->image_size);
+       header->snapc = snapc;
+       header->snap_names = snap_names;
+       header->snap_sizes = snap_sizes;
 
-       header->snapc = ceph_create_snap_context(snap_count, GFP_KERNEL);
-       if (!header->snapc)
-               goto out_err;
-       header->snapc->seq = le64_to_cpu(ondisk->snap_seq);
-       for (i = 0; i < snap_count; i++)
-               header->snapc->snaps[i] = le64_to_cpu(ondisk->snaps[i].id);
+       /* Make sure mapping size is consistent with header info */
 
-       return 0;
+       if (rbd_dev->spec->snap_id == CEPH_NOSNAP || first_time)
+               if (rbd_dev->mapping.size != header->image_size)
+                       rbd_dev->mapping.size = header->image_size;
 
+       up_write(&rbd_dev->header_rwsem);
+
+       return 0;
+out_2big:
+       ret = -EIO;
 out_err:
-       kfree(header->snap_sizes);
-       header->snap_sizes = NULL;
-       kfree(header->snap_names);
-       header->snap_names = NULL;
-       kfree(header->object_prefix);
-       header->object_prefix = NULL;
+       kfree(snap_sizes);
+       kfree(snap_names);
+       ceph_put_snap_context(snapc);
+       kfree(object_prefix);
 
-       return -ENOMEM;
+       return ret;
 }
 
 static const char *_rbd_dev_v1_snap_name(struct rbd_device *rbd_dev, u32 which)
@@ -951,11 +988,6 @@ static int rbd_dev_mapping_set(struct rbd_device *rbd_dev)
        rbd_dev->mapping.size = size;
        rbd_dev->mapping.features = features;
 
-       /* If we are mapping a snapshot it must be marked read-only */
-
-       if (snap_id != CEPH_NOSNAP)
-               rbd_dev->mapping.read_only = true;
-
        return 0;
 }
 
@@ -963,7 +995,6 @@ static void rbd_dev_mapping_clear(struct rbd_device *rbd_dev)
 {
        rbd_dev->mapping.size = 0;
        rbd_dev->mapping.features = 0;
-       rbd_dev->mapping.read_only = true;
 }
 
 static const char *rbd_segment_name(struct rbd_device *rbd_dev, u64 offset)
@@ -1328,13 +1359,6 @@ static void rbd_obj_request_put(struct rbd_obj_request *obj_request)
        kref_put(&obj_request->kref, rbd_obj_request_destroy);
 }
 
-static void rbd_img_request_get(struct rbd_img_request *img_request)
-{
-       dout("%s: img %p (was %d)\n", __func__, img_request,
-               atomic_read(&img_request->kref.refcount));
-       kref_get(&img_request->kref);
-}
-
 static void rbd_img_request_destroy(struct kref *kref);
 static void rbd_img_request_put(struct rbd_img_request *img_request)
 {
@@ -1859,9 +1883,6 @@ static struct rbd_img_request *rbd_img_request_create(
        INIT_LIST_HEAD(&img_request->obj_requests);
        kref_init(&img_request->kref);
 
-       rbd_img_request_get(img_request);       /* Avoid a warning */
-       rbd_img_request_put(img_request);       /* TEMPORARY */
-
        dout("%s: rbd_dev %p %s %llu/%llu -> img %p\n", __func__, rbd_dev,
                write_request ? "write" : "read", offset, length,
                img_request);
@@ -2100,7 +2121,7 @@ rbd_img_obj_copyup_callback(struct rbd_obj_request *obj_request)
 {
        struct rbd_img_request *img_request;
        struct rbd_device *rbd_dev;
-       u64 length;
+       struct page **pages;
        u32 page_count;
 
        rbd_assert(obj_request->type == OBJ_REQUEST_BIO);
@@ -2110,12 +2131,14 @@ rbd_img_obj_copyup_callback(struct rbd_obj_request *obj_request)
 
        rbd_dev = img_request->rbd_dev;
        rbd_assert(rbd_dev);
-       length = (u64)1 << rbd_dev->header.obj_order;
-       page_count = (u32)calc_pages_for(0, length);
 
-       rbd_assert(obj_request->copyup_pages);
-       ceph_release_page_vector(obj_request->copyup_pages, page_count);
+       pages = obj_request->copyup_pages;
+       rbd_assert(pages != NULL);
        obj_request->copyup_pages = NULL;
+       page_count = obj_request->copyup_page_count;
+       rbd_assert(page_count);
+       obj_request->copyup_page_count = 0;
+       ceph_release_page_vector(pages, page_count);
 
        /*
         * We want the transfer count to reflect the size of the
@@ -2139,9 +2162,9 @@ rbd_img_obj_parent_read_full_callback(struct rbd_img_request *img_request)
        struct ceph_osd_client *osdc;
        struct rbd_device *rbd_dev;
        struct page **pages;
+       u32 page_count;
        int result;
-       u64 obj_size;
-       u64 xferred;
+       u64 parent_length;
 
        rbd_assert(img_request_child_test(img_request));
 
@@ -2150,19 +2173,21 @@ rbd_img_obj_parent_read_full_callback(struct rbd_img_request *img_request)
        pages = img_request->copyup_pages;
        rbd_assert(pages != NULL);
        img_request->copyup_pages = NULL;
+       page_count = img_request->copyup_page_count;
+       rbd_assert(page_count);
+       img_request->copyup_page_count = 0;
 
        orig_request = img_request->obj_request;
        rbd_assert(orig_request != NULL);
        rbd_assert(orig_request->type == OBJ_REQUEST_BIO);
        result = img_request->result;
-       obj_size = img_request->length;
-       xferred = img_request->xferred;
+       parent_length = img_request->length;
+       rbd_assert(parent_length == img_request->xferred);
+       rbd_img_request_put(img_request);
 
-       rbd_dev = img_request->rbd_dev;
+       rbd_assert(orig_request->img_request);
+       rbd_dev = orig_request->img_request->rbd_dev;
        rbd_assert(rbd_dev);
-       rbd_assert(obj_size == (u64)1 << rbd_dev->header.obj_order);
-
-       rbd_img_request_put(img_request);
 
        if (result)
                goto out_err;
@@ -2176,11 +2201,12 @@ rbd_img_obj_parent_read_full_callback(struct rbd_img_request *img_request)
                goto out_err;
        orig_request->osd_req = osd_req;
        orig_request->copyup_pages = pages;
+       orig_request->copyup_page_count = page_count;
 
        /* Initialize the copyup op */
 
        osd_req_op_cls_init(osd_req, 0, CEPH_OSD_OP_CALL, "rbd", "copyup");
-       osd_req_op_cls_request_data_pages(osd_req, 0, pages, obj_size, 0,
+       osd_req_op_cls_request_data_pages(osd_req, 0, pages, parent_length, 0,
                                                false, false);
 
        /* Then the original write request op */
@@ -2293,6 +2319,7 @@ static int rbd_img_obj_parent_read_full(struct rbd_obj_request *obj_request)
        if (result)
                goto out_err;
        parent_request->copyup_pages = pages;
+       parent_request->copyup_page_count = page_count;
 
        parent_request->callback = rbd_img_obj_parent_read_full_callback;
        result = rbd_img_request_submit(parent_request);
@@ -2300,6 +2327,7 @@ static int rbd_img_obj_parent_read_full(struct rbd_obj_request *obj_request)
                return 0;
 
        parent_request->copyup_pages = NULL;
+       parent_request->copyup_page_count = 0;
        parent_request->obj_request = NULL;
        rbd_obj_request_put(obj_request);
 out_err:
@@ -2545,7 +2573,7 @@ static void rbd_img_parent_read(struct rbd_obj_request *obj_request)
        rbd_assert(obj_request_img_data_test(obj_request));
        rbd_assert(obj_request->img_request != NULL);
        rbd_assert(obj_request->result == (s32) -ENOENT);
-       rbd_assert(obj_request->type == OBJ_REQUEST_BIO);
+       rbd_assert(obj_request_type_valid(obj_request->type));
 
        rbd_dev = obj_request->img_request->rbd_dev;
        rbd_assert(rbd_dev->parent != NULL);
@@ -2561,8 +2589,12 @@ static void rbd_img_parent_read(struct rbd_obj_request *obj_request)
        rbd_obj_request_get(obj_request);
        img_request->obj_request = obj_request;
 
-       result = rbd_img_request_fill(img_request, OBJ_REQUEST_BIO,
-                                       obj_request->bio_list);
+       if (obj_request->type == OBJ_REQUEST_BIO)
+               result = rbd_img_request_fill(img_request, OBJ_REQUEST_BIO,
+                                               obj_request->bio_list);
+       else
+               result = rbd_img_request_fill(img_request, OBJ_REQUEST_PAGES,
+                                               obj_request->pages);
        if (result)
                goto out_err;
 
@@ -2631,7 +2663,7 @@ static void rbd_watch_cb(u64 ver, u64 notify_id, u8 opcode, void *data)
  * Request sync osd watch/unwatch.  The value of "start" determines
  * whether a watch request is being initiated or torn down.
  */
-static int rbd_dev_header_watch_sync(struct rbd_device *rbd_dev, int start)
+static int rbd_dev_header_watch_sync(struct rbd_device *rbd_dev, bool start)
 {
        struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
        struct rbd_obj_request *obj_request;
@@ -2665,7 +2697,7 @@ static int rbd_dev_header_watch_sync(struct rbd_device *rbd_dev, int start)
                                        rbd_dev->watch_request->osd_req);
 
        osd_req_op_watch_init(obj_request->osd_req, 0, CEPH_OSD_OP_WATCH,
-                               rbd_dev->watch_event->cookie, 0, start);
+                               rbd_dev->watch_event->cookie, 0, start ? 1 : 0);
        rbd_osd_req_format_write(obj_request);
 
        ret = rbd_obj_request_submit(osdc, obj_request);
@@ -3018,17 +3050,11 @@ out:
 }
 
 /*
- * Read the complete header for the given rbd device.
- *
- * Returns a pointer to a dynamically-allocated buffer containing
- * the complete and validated header.  Caller can pass the address
- * of a variable that will be filled in with the version of the
- * header object at the time it was read.
- *
- * Returns a pointer-coded errno if a failure occurs.
+ * Read the complete header for the given rbd device.  On successful
+ * return, the rbd_dev->header field will contain up-to-date
+ * information about the image.
  */
-static struct rbd_image_header_ondisk *
-rbd_dev_v1_header_read(struct rbd_device *rbd_dev)
+static int rbd_dev_v1_header_info(struct rbd_device *rbd_dev)
 {
        struct rbd_image_header_ondisk *ondisk = NULL;
        u32 snap_count = 0;
@@ -3053,22 +3079,22 @@ rbd_dev_v1_header_read(struct rbd_device *rbd_dev)
                size += names_size;
                ondisk = kmalloc(size, GFP_KERNEL);
                if (!ondisk)
-                       return ERR_PTR(-ENOMEM);
+                       return -ENOMEM;
 
                ret = rbd_obj_read_sync(rbd_dev, rbd_dev->header_name,
                                       0, size, ondisk);
                if (ret < 0)
-                       goto out_err;
+                       goto out;
                if ((size_t)ret < size) {
                        ret = -ENXIO;
                        rbd_warn(rbd_dev, "short header read (want %zd got %d)",
                                size, ret);
-                       goto out_err;
+                       goto out;
                }
                if (!rbd_dev_ondisk_valid(ondisk)) {
                        ret = -ENXIO;
                        rbd_warn(rbd_dev, "invalid header");
-                       goto out_err;
+                       goto out;
                }
 
                names_size = le64_to_cpu(ondisk->snap_names_len);
@@ -3076,72 +3102,13 @@ rbd_dev_v1_header_read(struct rbd_device *rbd_dev)
                snap_count = le32_to_cpu(ondisk->snap_count);
        } while (snap_count != want_count);
 
-       return ondisk;
-
-out_err:
-       kfree(ondisk);
-
-       return ERR_PTR(ret);
-}
-
-/*
- * reload the ondisk the header
- */
-static int rbd_read_header(struct rbd_device *rbd_dev,
-                          struct rbd_image_header *header)
-{
-       struct rbd_image_header_ondisk *ondisk;
-       int ret;
-
-       ondisk = rbd_dev_v1_header_read(rbd_dev);
-       if (IS_ERR(ondisk))
-               return PTR_ERR(ondisk);
-       ret = rbd_header_from_disk(header, ondisk);
+       ret = rbd_header_from_disk(rbd_dev, ondisk);
+out:
        kfree(ondisk);
 
        return ret;
 }
 
-/*
- * only read the first part of the ondisk header, without the snaps info
- */
-static int rbd_dev_v1_refresh(struct rbd_device *rbd_dev)
-{
-       int ret;
-       struct rbd_image_header h;
-
-       ret = rbd_read_header(rbd_dev, &h);
-       if (ret < 0)
-               return ret;
-
-       down_write(&rbd_dev->header_rwsem);
-
-       /* Update image size, and check for resize of mapped image */
-       rbd_dev->header.image_size = h.image_size;
-       if (rbd_dev->spec->snap_id == CEPH_NOSNAP)
-               if (rbd_dev->mapping.size != rbd_dev->header.image_size)
-                       rbd_dev->mapping.size = rbd_dev->header.image_size;
-
-       /* rbd_dev->header.object_prefix shouldn't change */
-       kfree(rbd_dev->header.snap_sizes);
-       kfree(rbd_dev->header.snap_names);
-       /* osd requests may still refer to snapc */
-       ceph_put_snap_context(rbd_dev->header.snapc);
-
-       rbd_dev->header.image_size = h.image_size;
-       rbd_dev->header.snapc = h.snapc;
-       rbd_dev->header.snap_names = h.snap_names;
-       rbd_dev->header.snap_sizes = h.snap_sizes;
-       /* Free the extra copy of the object prefix */
-       if (strcmp(rbd_dev->header.object_prefix, h.object_prefix))
-               rbd_warn(rbd_dev, "object prefix changed (ignoring)");
-       kfree(h.object_prefix);
-
-       up_write(&rbd_dev->header_rwsem);
-
-       return ret;
-}
-
 /*
  * Clear the rbd device's EXISTS flag if the snapshot it's mapped to
  * has disappeared from the (just updated) snapshot context.
@@ -3170,9 +3137,9 @@ static int rbd_dev_refresh(struct rbd_device *rbd_dev)
        mapping_size = rbd_dev->mapping.size;
        mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING);
        if (rbd_dev->image_format == 1)
-               ret = rbd_dev_v1_refresh(rbd_dev);
+               ret = rbd_dev_v1_header_info(rbd_dev);
        else
-               ret = rbd_dev_v2_refresh(rbd_dev);
+               ret = rbd_dev_v2_header_info(rbd_dev);
 
        /* If it's a mapped snapshot, validate its EXISTS flag */
 
@@ -4042,12 +4009,19 @@ out:
        return snap_name;
 }
 
-static int rbd_dev_v2_refresh(struct rbd_device *rbd_dev)
+static int rbd_dev_v2_header_info(struct rbd_device *rbd_dev)
 {
+       bool first_time = rbd_dev->header.object_prefix == NULL;
        int ret;
 
        down_write(&rbd_dev->header_rwsem);
 
+       if (first_time) {
+               ret = rbd_dev_v2_header_onetime(rbd_dev);
+               if (ret)
+                       goto out;
+       }
+
        ret = rbd_dev_v2_image_size(rbd_dev);
        if (ret)
                goto out;
@@ -4496,51 +4470,18 @@ static void rbd_dev_unprobe(struct rbd_device *rbd_dev)
        memset(header, 0, sizeof (*header));
 }
 
-static int rbd_dev_v1_probe(struct rbd_device *rbd_dev)
+static int rbd_dev_v2_header_onetime(struct rbd_device *rbd_dev)
 {
        int ret;
 
-       /* Populate rbd image metadata */
-
-       ret = rbd_read_header(rbd_dev, &rbd_dev->header);
-       if (ret < 0)
-               goto out_err;
-
-       /* Version 1 images have no parent (no layering) */
-
-       rbd_dev->parent_spec = NULL;
-       rbd_dev->parent_overlap = 0;
-
-       dout("discovered version 1 image, header name is %s\n",
-               rbd_dev->header_name);
-
-       return 0;
-
-out_err:
-       kfree(rbd_dev->header_name);
-       rbd_dev->header_name = NULL;
-       kfree(rbd_dev->spec->image_id);
-       rbd_dev->spec->image_id = NULL;
-
-       return ret;
-}
-
-static int rbd_dev_v2_probe(struct rbd_device *rbd_dev)
-{
-       int ret;
-
-       ret = rbd_dev_v2_image_size(rbd_dev);
-       if (ret)
-               goto out_err;
-
-       /* Get the object prefix (a.k.a. block_name) for the image */
-
        ret = rbd_dev_v2_object_prefix(rbd_dev);
        if (ret)
                goto out_err;
 
-       /* Get the and check features for the image */
-
+       /*
+        * Get the and check features for the image.  Currently the
+        * features are assumed to never change.
+        */
        ret = rbd_dev_v2_features(rbd_dev);
        if (ret)
                goto out_err;
@@ -4570,20 +4511,7 @@ static int rbd_dev_v2_probe(struct rbd_device *rbd_dev)
                if (ret < 0)
                        goto out_err;
        }
-
-       /* crypto and compression type aren't (yet) supported for v2 images */
-
-       rbd_dev->header.crypt_type = 0;
-       rbd_dev->header.comp_type = 0;
-
-       /* Get the snapshot context, plus the header version */
-
-       ret = rbd_dev_v2_snap_context(rbd_dev);
-       if (ret)
-               goto out_err;
-
-       dout("discovered version 2 image, header name is %s\n",
-               rbd_dev->header_name);
+       /* No support for crypto and compression type format 2 images */
 
        return 0;
 out_err:
@@ -4620,7 +4548,7 @@ static int rbd_dev_probe_parent(struct rbd_device *rbd_dev)
        if (!parent)
                goto out_err;
 
-       ret = rbd_dev_image_probe(parent);
+       ret = rbd_dev_image_probe(parent, false);
        if (ret < 0)
                goto out_err;
        rbd_dev->parent = parent;
@@ -4643,10 +4571,6 @@ static int rbd_dev_device_setup(struct rbd_device *rbd_dev)
 {
        int ret;
 
-       ret = rbd_dev_mapping_set(rbd_dev);
-       if (ret)
-               return ret;
-
        /* generate unique id: find highest unique id, add one */
        rbd_dev_id_get(rbd_dev);
 
@@ -4668,13 +4592,17 @@ static int rbd_dev_device_setup(struct rbd_device *rbd_dev)
        if (ret)
                goto err_out_blkdev;
 
-       ret = rbd_bus_add_dev(rbd_dev);
+       ret = rbd_dev_mapping_set(rbd_dev);
        if (ret)
                goto err_out_disk;
+       set_capacity(rbd_dev->disk, rbd_dev->mapping.size / SECTOR_SIZE);
+
+       ret = rbd_bus_add_dev(rbd_dev);
+       if (ret)
+               goto err_out_mapping;
 
        /* Everything's ready.  Announce the disk to the world. */
 
-       set_capacity(rbd_dev->disk, rbd_dev->mapping.size / SECTOR_SIZE);
        set_bit(RBD_DEV_FLAG_EXISTS, &rbd_dev->flags);
        add_disk(rbd_dev->disk);
 
@@ -4683,6 +4611,8 @@ static int rbd_dev_device_setup(struct rbd_device *rbd_dev)
 
        return ret;
 
+err_out_mapping:
+       rbd_dev_mapping_clear(rbd_dev);
 err_out_disk:
        rbd_free_disk(rbd_dev);
 err_out_blkdev:
@@ -4723,12 +4653,7 @@ static int rbd_dev_header_name(struct rbd_device *rbd_dev)
 
 static void rbd_dev_image_release(struct rbd_device *rbd_dev)
 {
-       int ret;
-
        rbd_dev_unprobe(rbd_dev);
-       ret = rbd_dev_header_watch_sync(rbd_dev, 0);
-       if (ret)
-               rbd_warn(rbd_dev, "failed to cancel watch event (%d)\n", ret);
        kfree(rbd_dev->header_name);
        rbd_dev->header_name = NULL;
        rbd_dev->image_format = 0;
@@ -4740,10 +4665,11 @@ static void rbd_dev_image_release(struct rbd_device *rbd_dev)
 
 /*
  * Probe for the existence of the header object for the given rbd
- * device.  For format 2 images this includes determining the image
- * id.
+ * device.  If this image is the one being mapped (i.e., not a
+ * parent), initiate a watch on its header object before using that
+ * object to get detailed information about the rbd image.
  */
-static int rbd_dev_image_probe(struct rbd_device *rbd_dev)
+static int rbd_dev_image_probe(struct rbd_device *rbd_dev, bool mapping)
 {
        int ret;
        int tmp;
@@ -4763,14 +4689,16 @@ static int rbd_dev_image_probe(struct rbd_device *rbd_dev)
        if (ret)
                goto err_out_format;
 
-       ret = rbd_dev_header_watch_sync(rbd_dev, 1);
-       if (ret)
-               goto out_header_name;
+       if (mapping) {
+               ret = rbd_dev_header_watch_sync(rbd_dev, true);
+               if (ret)
+                       goto out_header_name;
+       }
 
        if (rbd_dev->image_format == 1)
-               ret = rbd_dev_v1_probe(rbd_dev);
+               ret = rbd_dev_v1_header_info(rbd_dev);
        else
-               ret = rbd_dev_v2_probe(rbd_dev);
+               ret = rbd_dev_v2_header_info(rbd_dev);
        if (ret)
                goto err_out_watch;
 
@@ -4779,15 +4707,22 @@ static int rbd_dev_image_probe(struct rbd_device *rbd_dev)
                goto err_out_probe;
 
        ret = rbd_dev_probe_parent(rbd_dev);
-       if (!ret)
-               return 0;
+       if (ret)
+               goto err_out_probe;
+
+       dout("discovered format %u image, header name is %s\n",
+               rbd_dev->image_format, rbd_dev->header_name);
 
+       return 0;
 err_out_probe:
        rbd_dev_unprobe(rbd_dev);
 err_out_watch:
-       tmp = rbd_dev_header_watch_sync(rbd_dev, 0);
-       if (tmp)
-               rbd_warn(rbd_dev, "unable to tear down watch request\n");
+       if (mapping) {
+               tmp = rbd_dev_header_watch_sync(rbd_dev, false);
+               if (tmp)
+                       rbd_warn(rbd_dev, "unable to tear down "
+                                       "watch request (%d)\n", tmp);
+       }
 out_header_name:
        kfree(rbd_dev->header_name);
        rbd_dev->header_name = NULL;
@@ -4811,6 +4746,7 @@ static ssize_t rbd_add(struct bus_type *bus,
        struct rbd_spec *spec = NULL;
        struct rbd_client *rbdc;
        struct ceph_osd_client *osdc;
+       bool read_only;
        int rc = -ENOMEM;
 
        if (!try_module_get(THIS_MODULE))
@@ -4820,6 +4756,9 @@ static ssize_t rbd_add(struct bus_type *bus,
        rc = rbd_add_parse_args(buf, &ceph_opts, &rbd_opts, &spec);
        if (rc < 0)
                goto err_out_module;
+       read_only = rbd_opts->read_only;
+       kfree(rbd_opts);
+       rbd_opts = NULL;        /* done with this */
 
        rbdc = rbd_get_client(ceph_opts);
        if (IS_ERR(rbdc)) {
@@ -4850,14 +4789,16 @@ static ssize_t rbd_add(struct bus_type *bus,
        rbdc = NULL;            /* rbd_dev now owns this */
        spec = NULL;            /* rbd_dev now owns this */
 
-       rbd_dev->mapping.read_only = rbd_opts->read_only;
-       kfree(rbd_opts);
-       rbd_opts = NULL;        /* done with this */
-
-       rc = rbd_dev_image_probe(rbd_dev);
+       rc = rbd_dev_image_probe(rbd_dev, true);
        if (rc < 0)
                goto err_out_rbd_dev;
 
+       /* If we are mapping a snapshot it must be marked read-only */
+
+       if (rbd_dev->spec->snap_id != CEPH_NOSNAP)
+               read_only = true;
+       rbd_dev->mapping.read_only = read_only;
+
        rc = rbd_dev_device_setup(rbd_dev);
        if (!rc)
                return count;
@@ -4970,10 +4911,13 @@ static ssize_t rbd_remove(struct bus_type *bus,
        spin_unlock_irq(&rbd_dev->lock);
        if (ret < 0)
                goto done;
-       ret = count;
        rbd_bus_del_dev(rbd_dev);
+       ret = rbd_dev_header_watch_sync(rbd_dev, false);
+       if (ret)
+               rbd_warn(rbd_dev, "failed to cancel watch event (%d)\n", ret);
        rbd_dev_image_release(rbd_dev);
        module_put(THIS_MODULE);
+       ret = count;
 done:
        mutex_unlock(&ctl_mutex);