]> Pileus Git - ~andy/linux/blobdiff - drivers/block/rbd.c
rbd: stop tracking header object version
[~andy/linux] / drivers / block / rbd.c
index 1e01f0d8312a0041949c4ecd7afbeb015b4fe182..8875bebbacfca13e46536c31c8c961da74ec19c7 100644 (file)
@@ -108,7 +108,8 @@ struct rbd_image_header {
        char *snap_names;
        u64 *snap_sizes;
 
-       u64 obj_version;
+       u64 stripe_unit;
+       u64 stripe_count;
 };
 
 /*
@@ -138,13 +139,13 @@ struct rbd_image_header {
  */
 struct rbd_spec {
        u64             pool_id;
-       char            *pool_name;
+       const char      *pool_name;
 
-       char            *image_id;
-       char            *image_name;
+       const char      *image_id;
+       const char      *image_name;
 
        u64             snap_id;
-       char            *snap_name;
+       const char      *snap_name;
 
        struct kref     kref;
 };
@@ -316,9 +317,6 @@ struct rbd_device {
        u64                     parent_overlap;
        struct rbd_device       *parent;
 
-       u64                     stripe_unit;
-       u64                     stripe_count;
-
        /* protects updating the header */
        struct rw_semaphore     header_rwsem;
 
@@ -358,14 +356,14 @@ static int rbd_img_request_submit(struct rbd_img_request *img_request);
 
 static int rbd_dev_snaps_update(struct rbd_device *rbd_dev);
 
-static void rbd_dev_release(struct device *dev);
-static void rbd_remove_snap_dev(struct rbd_snap *snap);
+static void rbd_dev_device_release(struct device *dev);
+static void rbd_snap_destroy(struct rbd_snap *snap);
 
 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_probe(struct rbd_device *rbd_dev);
+static int rbd_dev_image_probe(struct rbd_device *rbd_dev);
 
 static struct bus_attribute rbd_bus_attrs[] = {
        __ATTR(add, S_IWUSR, NULL, rbd_add),
@@ -427,8 +425,9 @@ void rbd_warn(struct rbd_device *rbd_dev, const char *fmt, ...)
 #  define rbd_assert(expr)     ((void) 0)
 #endif /* !RBD_DEBUG */
 
-static void rbd_img_parent_read(struct rbd_obj_request *obj_request);
 static int rbd_img_obj_request_submit(struct rbd_obj_request *obj_request);
+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, u64 *hver);
 static int rbd_dev_v2_refresh(struct rbd_device *rbd_dev, u64 *hver);
@@ -777,7 +776,6 @@ static int rbd_header_from_disk(struct rbd_image_header *header,
                        header->snap_sizes[i] =
                                le64_to_cpu(ondisk->snaps[i].image_size);
        } else {
-               WARN_ON(ondisk->snap_names_len);
                header->snap_names = NULL;
                header->snap_sizes = NULL;
        }
@@ -790,18 +788,13 @@ static int rbd_header_from_disk(struct rbd_image_header *header,
        /* Allocate and fill in the snapshot context */
 
        header->image_size = le64_to_cpu(ondisk->image_size);
-       size = sizeof (struct ceph_snap_context);
-       size += snap_count * sizeof (header->snapc->snaps[0]);
-       header->snapc = kzalloc(size, GFP_KERNEL);
+
+       header->snapc = ceph_create_snap_context(snap_count, GFP_KERNEL);
        if (!header->snapc)
                goto out_err;
-
-       atomic_set(&header->snapc->nref, 1);
        header->snapc->seq = le64_to_cpu(ondisk->snap_seq);
-       header->snapc->num_snaps = snap_count;
        for (i = 0; i < snap_count; i++)
-               header->snapc->snaps[i] =
-                       le64_to_cpu(ondisk->snaps[i].id);
+               header->snapc->snaps[i] = le64_to_cpu(ondisk->snaps[i].id);
 
        return 0;
 
@@ -830,56 +823,50 @@ static const char *rbd_snap_name(struct rbd_device *rbd_dev, u64 snap_id)
        return NULL;
 }
 
-static int snap_by_name(struct rbd_device *rbd_dev, const char *snap_name)
+static struct rbd_snap *snap_by_name(struct rbd_device *rbd_dev,
+                                       const char *snap_name)
 {
-
        struct rbd_snap *snap;
 
-       list_for_each_entry(snap, &rbd_dev->snaps, node) {
-               if (!strcmp(snap_name, snap->name)) {
-                       rbd_dev->spec->snap_id = snap->id;
-                       rbd_dev->mapping.size = snap->size;
-                       rbd_dev->mapping.features = snap->features;
-
-                       return 0;
-               }
-       }
+       list_for_each_entry(snap, &rbd_dev->snaps, node)
+               if (!strcmp(snap_name, snap->name))
+                       return snap;
 
-       return -ENOENT;
+       return NULL;
 }
 
-static int rbd_dev_set_mapping(struct rbd_device *rbd_dev)
+static int rbd_dev_mapping_set(struct rbd_device *rbd_dev)
 {
-       int ret;
-
        if (!memcmp(rbd_dev->spec->snap_name, RBD_SNAP_HEAD_NAME,
                    sizeof (RBD_SNAP_HEAD_NAME))) {
-               rbd_dev->spec->snap_id = CEPH_NOSNAP;
                rbd_dev->mapping.size = rbd_dev->header.image_size;
                rbd_dev->mapping.features = rbd_dev->header.features;
-               ret = 0;
        } else {
-               ret = snap_by_name(rbd_dev, rbd_dev->spec->snap_name);
-               if (ret < 0)
-                       goto done;
+               struct rbd_snap *snap;
+
+               snap = snap_by_name(rbd_dev, rbd_dev->spec->snap_name);
+               if (!snap)
+                       return -ENOENT;
+               rbd_dev->mapping.size = snap->size;
+               rbd_dev->mapping.features = snap->features;
                rbd_dev->mapping.read_only = true;
        }
-       set_bit(RBD_DEV_FLAG_EXISTS, &rbd_dev->flags);
 
-done:
-       return ret;
+       return 0;
 }
 
-static void rbd_header_free(struct rbd_image_header *header)
+static void rbd_dev_mapping_clear(struct rbd_device *rbd_dev)
 {
-       kfree(header->object_prefix);
-       header->object_prefix = NULL;
-       kfree(header->snap_sizes);
-       header->snap_sizes = NULL;
-       kfree(header->snap_names);
-       header->snap_names = NULL;
-       ceph_put_snap_context(header->snapc);
-       header->snapc = NULL;
+       rbd_dev->mapping.size = 0;
+       rbd_dev->mapping.features = 0;
+       rbd_dev->mapping.read_only = true;
+}
+
+static void rbd_dev_clear_mapping(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)
@@ -1728,7 +1715,6 @@ static struct rbd_img_request *rbd_img_request_create(
                                        bool child_request)
 {
        struct rbd_img_request *img_request;
-       struct ceph_snap_context *snapc = NULL;
 
        img_request = kmalloc(sizeof (*img_request), GFP_ATOMIC);
        if (!img_request)
@@ -1736,13 +1722,8 @@ static struct rbd_img_request *rbd_img_request_create(
 
        if (write_request) {
                down_read(&rbd_dev->header_rwsem);
-               snapc = ceph_get_snap_context(rbd_dev->header.snapc);
+               ceph_get_snap_context(rbd_dev->header.snapc);
                up_read(&rbd_dev->header_rwsem);
-               if (WARN_ON(!snapc)) {
-                       kfree(img_request);
-                       return NULL;    /* Shouldn't happen */
-               }
-
        }
 
        img_request->rq = NULL;
@@ -1752,7 +1733,7 @@ static struct rbd_img_request *rbd_img_request_create(
        img_request->flags = 0;
        if (write_request) {
                img_request_write_set(img_request);
-               img_request->snapc = snapc;
+               img_request->snapc = rbd_dev->header.snapc;
        } else {
                img_request->snap_id = rbd_dev->spec->snap_id;
        }
@@ -2571,8 +2552,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,
-                               rbd_dev->header.obj_version, start);
+                               rbd_dev->watch_event->cookie, 0, start);
        rbd_osd_req_format_write(obj_request);
 
        ret = rbd_obj_request_submit(osdc, obj_request);
@@ -2614,7 +2594,8 @@ out_cancel:
 }
 
 /*
- * Synchronous osd object method call
+ * Synchronous osd object method call.  Returns the number of bytes
+ * returned in the outbound buffer, or a negative error code.
  */
 static int rbd_obj_method_sync(struct rbd_device *rbd_dev,
                             const char *object_name,
@@ -2761,8 +2742,11 @@ static void rbd_request_fn(struct request_queue *q)
                }
 
                result = -EINVAL;
-               if (WARN_ON(offset && length > U64_MAX - offset + 1))
+               if (offset && length > U64_MAX - offset + 1) {
+                       rbd_warn(rbd_dev, "bad request range (%llu~%llu)\n",
+                               offset, length);
                        goto end_request;       /* Shouldn't happen */
+               }
 
                result = -ENOMEM;
                img_request = rbd_img_request_create(rbd_dev, offset, length,
@@ -2843,10 +2827,12 @@ static void rbd_free_disk(struct rbd_device *rbd_dev)
        if (!disk)
                return;
 
-       if (disk->flags & GENHD_FL_UP)
+       rbd_dev->disk = NULL;
+       if (disk->flags & GENHD_FL_UP) {
                del_gendisk(disk);
-       if (disk->queue)
-               blk_cleanup_queue(disk->queue);
+               if (disk->queue)
+                       blk_cleanup_queue(disk->queue);
+       }
        put_disk(disk);
 }
 
@@ -2959,7 +2945,7 @@ rbd_dev_v1_header_read(struct rbd_device *rbd_dev, u64 *version)
                                       0, size, ondisk, version);
                if (ret < 0)
                        goto out_err;
-               if (WARN_ON((size_t) ret < size)) {
+               if ((size_t)ret < size) {
                        ret = -ENXIO;
                        rbd_warn(rbd_dev, "short header read (want %zd got %d)",
                                size, ret);
@@ -2998,8 +2984,6 @@ static int rbd_read_header(struct rbd_device *rbd_dev,
        if (IS_ERR(ondisk))
                return PTR_ERR(ondisk);
        ret = rbd_header_from_disk(header, ondisk);
-       if (ret >= 0)
-               header->obj_version = ver;
        kfree(ondisk);
 
        return ret;
@@ -3010,21 +2994,25 @@ static void rbd_remove_all_snaps(struct rbd_device *rbd_dev)
        struct rbd_snap *snap;
        struct rbd_snap *next;
 
-       list_for_each_entry_safe(snap, next, &rbd_dev->snaps, node)
-               rbd_remove_snap_dev(snap);
+       list_for_each_entry_safe(snap, next, &rbd_dev->snaps, node) {
+               list_del(&snap->node);
+               rbd_snap_destroy(snap);
+       }
 }
 
 static void rbd_update_mapping_size(struct rbd_device *rbd_dev)
 {
-       sector_t size;
-
        if (rbd_dev->spec->snap_id != CEPH_NOSNAP)
                return;
 
-       size = (sector_t) rbd_dev->header.image_size / SECTOR_SIZE;
-       dout("setting size to %llu sectors", (unsigned long long) size);
-       rbd_dev->mapping.size = (u64) size;
-       set_capacity(rbd_dev->disk, size);
+       if (rbd_dev->mapping.size != rbd_dev->header.image_size) {
+               sector_t size;
+
+               rbd_dev->mapping.size = rbd_dev->header.image_size;
+               size = (sector_t)rbd_dev->mapping.size / SECTOR_SIZE;
+               dout("setting size to %llu sectors", (unsigned long long)size);
+               set_capacity(rbd_dev->disk, size);
+       }
 }
 
 /*
@@ -3051,15 +3039,13 @@ static int rbd_dev_v1_refresh(struct rbd_device *rbd_dev, u64 *hver)
        /* osd requests may still refer to snapc */
        ceph_put_snap_context(rbd_dev->header.snapc);
 
-       if (hver)
-               *hver = h.obj_version;
-       rbd_dev->header.obj_version = h.obj_version;
        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 */
-       WARN_ON(strcmp(rbd_dev->header.object_prefix, h.object_prefix));
+       if (strcmp(rbd_dev->header.object_prefix, h.object_prefix))
+               rbd_warn(rbd_dev, "object prefix changed (ignoring)");
        kfree(h.object_prefix);
 
        ret = rbd_dev_snaps_update(rbd_dev);
@@ -3071,19 +3057,22 @@ static int rbd_dev_v1_refresh(struct rbd_device *rbd_dev, u64 *hver)
 
 static int rbd_dev_refresh(struct rbd_device *rbd_dev, u64 *hver)
 {
+       u64 image_size;
        int ret;
 
        rbd_assert(rbd_image_format_valid(rbd_dev->image_format));
+       image_size = rbd_dev->header.image_size;
        mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING);
        if (rbd_dev->image_format == 1)
                ret = rbd_dev_v1_refresh(rbd_dev, hver);
        else
                ret = rbd_dev_v2_refresh(rbd_dev, hver);
        mutex_unlock(&ctl_mutex);
-       revalidate_disk(rbd_dev->disk);
        if (ret)
                rbd_warn(rbd_dev, "got notification but failed to "
                           " update snaps: %d\n", ret);
+       if (image_size != rbd_dev->header.image_size)
+               revalidate_disk(rbd_dev->disk);
 
        return ret;
 }
@@ -3127,8 +3116,6 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)
 
        rbd_dev->disk = disk;
 
-       set_capacity(rbd_dev->disk, rbd_dev->mapping.size / SECTOR_SIZE);
-
        return 0;
 out_disk:
        put_disk(disk);
@@ -3149,13 +3136,9 @@ static ssize_t rbd_size_show(struct device *dev,
                             struct device_attribute *attr, char *buf)
 {
        struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);
-       sector_t size;
-
-       down_read(&rbd_dev->header_rwsem);
-       size = get_capacity(rbd_dev->disk);
-       up_read(&rbd_dev->header_rwsem);
 
-       return sprintf(buf, "%llu\n", (unsigned long long) size * SECTOR_SIZE);
+       return sprintf(buf, "%llu\n",
+               (unsigned long long)rbd_dev->mapping.size);
 }
 
 /*
@@ -3168,7 +3151,7 @@ static ssize_t rbd_features_show(struct device *dev,
        struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);
 
        return sprintf(buf, "0x%016llx\n",
-                       (unsigned long long) rbd_dev->mapping.features);
+                       (unsigned long long)rbd_dev->mapping.features);
 }
 
 static ssize_t rbd_major_show(struct device *dev,
@@ -3176,7 +3159,11 @@ static ssize_t rbd_major_show(struct device *dev,
 {
        struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);
 
-       return sprintf(buf, "%d\n", rbd_dev->major);
+       if (rbd_dev->major)
+               return sprintf(buf, "%d\n", rbd_dev->major);
+
+       return sprintf(buf, "(none)\n");
+
 }
 
 static ssize_t rbd_client_id_show(struct device *dev,
@@ -3202,7 +3189,7 @@ static ssize_t rbd_pool_id_show(struct device *dev,
        struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);
 
        return sprintf(buf, "%llu\n",
-               (unsigned long long) rbd_dev->spec->pool_id);
+                       (unsigned long long) rbd_dev->spec->pool_id);
 }
 
 static ssize_t rbd_name_show(struct device *dev,
@@ -3406,66 +3393,61 @@ static struct rbd_device *rbd_dev_create(struct rbd_client *rbdc,
 
 static void rbd_dev_destroy(struct rbd_device *rbd_dev)
 {
-       rbd_spec_put(rbd_dev->parent_spec);
-       kfree(rbd_dev->header_name);
        rbd_put_client(rbd_dev->rbd_client);
        rbd_spec_put(rbd_dev->spec);
        kfree(rbd_dev);
 }
 
-static void rbd_remove_snap_dev(struct rbd_snap *snap)
+static void rbd_snap_destroy(struct rbd_snap *snap)
 {
-       list_del(&snap->node);
        kfree(snap->name);
        kfree(snap);
 }
 
-static struct rbd_snap *__rbd_add_snap_dev(struct rbd_device *rbd_dev,
+static struct rbd_snap *rbd_snap_create(struct rbd_device *rbd_dev,
                                                const char *snap_name,
                                                u64 snap_id, u64 snap_size,
                                                u64 snap_features)
 {
        struct rbd_snap *snap;
-       int ret;
 
        snap = kzalloc(sizeof (*snap), GFP_KERNEL);
        if (!snap)
                return ERR_PTR(-ENOMEM);
 
-       ret = -ENOMEM;
-       snap->name = kstrdup(snap_name, GFP_KERNEL);
-       if (!snap->name)
-               goto err;
-
+       snap->name = snap_name;
        snap->id = snap_id;
        snap->size = snap_size;
        snap->features = snap_features;
 
        return snap;
-
-err:
-       kfree(snap->name);
-       kfree(snap);
-
-       return ERR_PTR(ret);
 }
 
-static char *rbd_dev_v1_snap_info(struct rbd_device *rbd_dev, u32 which,
+/*
+ * Returns a dynamically-allocated snapshot name if successful, or a
+ * pointer-coded error otherwise.
+ */
+static const char *rbd_dev_v1_snap_info(struct rbd_device *rbd_dev, u32 which,
                u64 *snap_size, u64 *snap_features)
 {
-       char *snap_name;
+       const char *snap_name;
+       int i;
 
        rbd_assert(which < rbd_dev->header.snapc->num_snaps);
 
-       *snap_size = rbd_dev->header.snap_sizes[which];
-       *snap_features = 0;     /* No features for v1 */
-
        /* Skip over names until we find the one we are looking for */
 
        snap_name = rbd_dev->header.snap_names;
-       while (which--)
+       for (i = 0; i < which; i++)
                snap_name += strlen(snap_name) + 1;
 
+       snap_name = kstrdup(snap_name, GFP_KERNEL);
+       if (!snap_name)
+               return ERR_PTR(-ENOMEM);
+
+       *snap_size = rbd_dev->header.snap_sizes[which];
+       *snap_features = 0;     /* No features for v1 */
+
        return snap_name;
 }
 
@@ -3632,8 +3614,11 @@ static int rbd_dev_v2_parent_info(struct rbd_device *rbd_dev)
        /* The ceph file layout needs to fit pool id in 32 bits */
 
        ret = -EIO;
-       if (WARN_ON(parent_spec->pool_id > (u64)U32_MAX))
+       if (parent_spec->pool_id > (u64)U32_MAX) {
+               rbd_warn(NULL, "parent pool id too large (%llu > %u)\n",
+                       (unsigned long long)parent_spec->pool_id, U32_MAX);
                goto out_err;
+       }
 
        image_id = ceph_extract_encoded_string(&p, end, NULL, GFP_KERNEL);
        if (IS_ERR(image_id)) {
@@ -3700,8 +3685,8 @@ static int rbd_dev_v2_striping_info(struct rbd_device *rbd_dev)
                                "(got %llu want 1)", stripe_count);
                return -EINVAL;
        }
-       rbd_dev->stripe_unit = stripe_unit;
-       rbd_dev->stripe_count = stripe_count;
+       rbd_dev->header.stripe_unit = stripe_unit;
+       rbd_dev->header.stripe_count = stripe_count;
 
        return 0;
 }
@@ -3742,7 +3727,8 @@ static char *rbd_dev_image_name(struct rbd_device *rbd_dev)
        if (ret < 0)
                goto out;
        p = reply_buf;
-       end = reply_buf + size;
+       end = reply_buf + ret;
+
        image_name = ceph_extract_encoded_string(&p, end, &len, GFP_KERNEL);
        if (IS_ERR(image_name))
                image_name = NULL;
@@ -3756,63 +3742,88 @@ out:
 }
 
 /*
- * When a parent image gets probed, we only have the pool, image,
- * and snapshot ids but not the names of any of them.  This call
- * is made later to fill in those names.  It has to be done after
- * rbd_dev_snaps_update() has completed because some of the
- * information (in particular, snapshot name) is not available
- * until then.
+ * When an rbd image has a parent image, it is identified by the
+ * pool, image, and snapshot ids (not names).  This function fills
+ * in the names for those ids.  (It's OK if we can't figure out the
+ * name for an image id, but the pool and snapshot ids should always
+ * exist and have names.)  All names in an rbd spec are dynamically
+ * allocated.
+ *
+ * When an image being mapped (not a parent) is probed, we have the
+ * pool name and pool id, image name and image id, and the snapshot
+ * name.  The only thing we're missing is the snapshot id.
+ *
+ * The set of snapshots for an image is not known until they have
+ * been read by rbd_dev_snaps_update(), so we can't completely fill
+ * in this information until after that has been called.
  */
-static int rbd_dev_probe_update_spec(struct rbd_device *rbd_dev)
+static int rbd_dev_spec_update(struct rbd_device *rbd_dev)
 {
-       struct ceph_osd_client *osdc;
-       const char *name;
-       void *reply_buf = NULL;
+       struct ceph_osd_client *osdc = &rbd_dev->rbd_client->client->osdc;
+       struct rbd_spec *spec = rbd_dev->spec;
+       const char *pool_name;
+       const char *image_name;
+       const char *snap_name;
        int ret;
 
-       if (rbd_dev->spec->pool_name)
-               return 0;       /* Already have the names */
+       /*
+        * An image being mapped will have the pool name (etc.), but
+        * we need to look up the snapshot id.
+        */
+       if (spec->pool_name) {
+               if (strcmp(spec->snap_name, RBD_SNAP_HEAD_NAME)) {
+                       struct rbd_snap *snap;
+
+                       snap = snap_by_name(rbd_dev, spec->snap_name);
+                       if (!snap)
+                               return -ENOENT;
+                       spec->snap_id = snap->id;
+               } else {
+                       spec->snap_id = CEPH_NOSNAP;
+               }
+
+               return 0;
+       }
 
-       /* Look up the pool name */
+       /* Get the pool name; we have to make our own copy of this */
 
-       osdc = &rbd_dev->rbd_client->client->osdc;
-       name = ceph_pg_pool_name_by_id(osdc->osdmap, rbd_dev->spec->pool_id);
-       if (!name) {
-               rbd_warn(rbd_dev, "there is no pool with id %llu",
-                       rbd_dev->spec->pool_id);        /* Really a BUG() */
+       pool_name = ceph_pg_pool_name_by_id(osdc->osdmap, spec->pool_id);
+       if (!pool_name) {
+               rbd_warn(rbd_dev, "no pool with id %llu", spec->pool_id);
                return -EIO;
        }
-
-       rbd_dev->spec->pool_name = kstrdup(name, GFP_KERNEL);
-       if (!rbd_dev->spec->pool_name)
+       pool_name = kstrdup(pool_name, GFP_KERNEL);
+       if (!pool_name)
                return -ENOMEM;
 
        /* Fetch the image name; tolerate failure here */
 
-       name = rbd_dev_image_name(rbd_dev);
-       if (name)
-               rbd_dev->spec->image_name = (char *)name;
-       else
+       image_name = rbd_dev_image_name(rbd_dev);
+       if (!image_name)
                rbd_warn(rbd_dev, "unable to get image name");
 
-       /* Look up the snapshot name. */
+       /* Look up the snapshot name, and make a copy */
 
-       name = rbd_snap_name(rbd_dev, rbd_dev->spec->snap_id);
-       if (!name) {
-               rbd_warn(rbd_dev, "no snapshot with id %llu",
-                       rbd_dev->spec->snap_id);        /* Really a BUG() */
+       snap_name = rbd_snap_name(rbd_dev, spec->snap_id);
+       if (!snap_name) {
+               rbd_warn(rbd_dev, "no snapshot with id %llu", spec->snap_id);
                ret = -EIO;
                goto out_err;
        }
-       rbd_dev->spec->snap_name = kstrdup(name, GFP_KERNEL);
-       if(!rbd_dev->spec->snap_name)
+       snap_name = kstrdup(snap_name, GFP_KERNEL);
+       if (!snap_name) {
+               ret = -ENOMEM;
                goto out_err;
+       }
+
+       spec->pool_name = pool_name;
+       spec->image_name = image_name;
+       spec->snap_name = snap_name;
 
        return 0;
 out_err:
-       kfree(reply_buf);
-       kfree(rbd_dev->spec->pool_name);
-       rbd_dev->spec->pool_name = NULL;
+       kfree(image_name);
+       kfree(pool_name);
 
        return ret;
 }
@@ -3867,19 +3878,14 @@ static int rbd_dev_v2_snap_context(struct rbd_device *rbd_dev, u64 *ver)
        }
        if (!ceph_has_room(&p, end, snap_count * sizeof (__le64)))
                goto out;
+       ret = 0;
 
-       size = sizeof (struct ceph_snap_context) +
-                               snap_count * sizeof (snapc->snaps[0]);
-       snapc = kmalloc(size, GFP_KERNEL);
+       snapc = ceph_create_snap_context(snap_count, GFP_KERNEL);
        if (!snapc) {
                ret = -ENOMEM;
                goto out;
        }
-       ret = 0;
-
-       atomic_set(&snapc->nref, 1);
        snapc->seq = seq;
-       snapc->num_snaps = snap_count;
        for (i = 0; i < snap_count; i++)
                snapc->snaps[i] = ceph_decode_64(&p);
 
@@ -3893,7 +3899,7 @@ out:
        return ret;
 }
 
-static char *rbd_dev_v2_snap_name(struct rbd_device *rbd_dev, u32 which)
+static const char *rbd_dev_v2_snap_name(struct rbd_device *rbd_dev, u32 which)
 {
        size_t size;
        void *reply_buf;
@@ -3908,52 +3914,63 @@ static char *rbd_dev_v2_snap_name(struct rbd_device *rbd_dev, u32 which)
        if (!reply_buf)
                return ERR_PTR(-ENOMEM);
 
+       rbd_assert(which < rbd_dev->header.snapc->num_snaps);
        snap_id = cpu_to_le64(rbd_dev->header.snapc->snaps[which]);
        ret = rbd_obj_method_sync(rbd_dev, rbd_dev->header_name,
                                "rbd", "get_snapshot_name",
                                &snap_id, sizeof (snap_id),
                                reply_buf, size, NULL);
        dout("%s: rbd_obj_method_sync returned %d\n", __func__, ret);
-       if (ret < 0)
+       if (ret < 0) {
+               snap_name = ERR_PTR(ret);
                goto out;
+       }
 
        p = reply_buf;
-       end = reply_buf + size;
+       end = reply_buf + ret;
        snap_name = ceph_extract_encoded_string(&p, end, NULL, GFP_KERNEL);
-       if (IS_ERR(snap_name)) {
-               ret = PTR_ERR(snap_name);
+       if (IS_ERR(snap_name))
                goto out;
-       } else {
-               dout("  snap_id 0x%016llx snap_name = %s\n",
-                       (unsigned long long)le64_to_cpu(snap_id), snap_name);
-       }
-       kfree(reply_buf);
 
-       return snap_name;
+       dout("  snap_id 0x%016llx snap_name = %s\n",
+               (unsigned long long)le64_to_cpu(snap_id), snap_name);
 out:
        kfree(reply_buf);
 
-       return ERR_PTR(ret);
+       return snap_name;
 }
 
-static char *rbd_dev_v2_snap_info(struct rbd_device *rbd_dev, u32 which,
+static const char *rbd_dev_v2_snap_info(struct rbd_device *rbd_dev, u32 which,
                u64 *snap_size, u64 *snap_features)
 {
        u64 snap_id;
+       u64 size;
+       u64 features;
+       const char *snap_name;
        int ret;
 
+       rbd_assert(which < rbd_dev->header.snapc->num_snaps);
        snap_id = rbd_dev->header.snapc->snaps[which];
-       ret = _rbd_dev_v2_snap_size(rbd_dev, snap_id, NULL, snap_size);
+       ret = _rbd_dev_v2_snap_size(rbd_dev, snap_id, NULL, &size);
        if (ret)
-               return ERR_PTR(ret);
-       ret = _rbd_dev_v2_snap_features(rbd_dev, snap_id, snap_features);
+               goto out_err;
+
+       ret = _rbd_dev_v2_snap_features(rbd_dev, snap_id, &features);
        if (ret)
-               return ERR_PTR(ret);
+               goto out_err;
+
+       snap_name = rbd_dev_v2_snap_name(rbd_dev, which);
+       if (!IS_ERR(snap_name)) {
+               *snap_size = size;
+               *snap_features = features;
+       }
 
-       return rbd_dev_v2_snap_name(rbd_dev, which);
+       return snap_name;
+out_err:
+       return ERR_PTR(ret);
 }
 
-static char *rbd_dev_snap_info(struct rbd_device *rbd_dev, u32 which,
+static const char *rbd_dev_snap_info(struct rbd_device *rbd_dev, u32 which,
                u64 *snap_size, u64 *snap_features)
 {
        if (rbd_dev->image_format == 1)
@@ -3968,20 +3985,12 @@ static char *rbd_dev_snap_info(struct rbd_device *rbd_dev, u32 which,
 static int rbd_dev_v2_refresh(struct rbd_device *rbd_dev, u64 *hver)
 {
        int ret;
-       __u8 obj_order;
 
        down_write(&rbd_dev->header_rwsem);
 
-       /* Grab old order first, to see if it changes */
-
-       obj_order = rbd_dev->header.obj_order,
        ret = rbd_dev_v2_image_size(rbd_dev);
        if (ret)
                goto out;
-       if (rbd_dev->header.obj_order != obj_order) {
-               ret = -EIO;
-               goto out;
-       }
        rbd_update_mapping_size(rbd_dev);
 
        ret = rbd_dev_v2_snap_context(rbd_dev, hver);
@@ -4028,7 +4037,7 @@ static int rbd_dev_snaps_update(struct rbd_device *rbd_dev)
        while (index < snap_count || links != head) {
                u64 snap_id;
                struct rbd_snap *snap;
-               char *snap_name;
+               const char *snap_name;
                u64 snap_size = 0;
                u64 snap_features = 0;
 
@@ -4056,7 +4065,9 @@ static int rbd_dev_snaps_update(struct rbd_device *rbd_dev)
                                rbd_dev->spec->snap_id == snap->id ?
                                                        "mapped " : "",
                                (unsigned long long)snap->id);
-                       rbd_remove_snap_dev(snap);
+
+                       list_del(&snap->node);
+                       rbd_snap_destroy(snap);
 
                        /* Done with this list entry; advance */
 
@@ -4079,7 +4090,7 @@ static int rbd_dev_snaps_update(struct rbd_device *rbd_dev)
 
                        /* We haven't seen this snapshot before */
 
-                       new_snap = __rbd_add_snap_dev(rbd_dev, snap_name,
+                       new_snap = rbd_snap_create(rbd_dev, snap_name,
                                        snap_id, snap_size, snap_features);
                        if (IS_ERR(new_snap)) {
                                ret = PTR_ERR(new_snap);
@@ -4132,7 +4143,7 @@ static int rbd_bus_add_dev(struct rbd_device *rbd_dev)
        dev->bus = &rbd_bus_type;
        dev->type = &rbd_device_type;
        dev->parent = &rbd_root_dev;
-       dev->release = rbd_dev_release;
+       dev->release = rbd_dev_device_release;
        dev_set_name(dev, "%d", rbd_dev->dev_id);
        ret = device_register(dev);
 
@@ -4346,6 +4357,7 @@ static int rbd_add_parse_args(const char *buf,
        size_t len;
        char *options;
        const char *mon_addrs;
+       char *snap_name;
        size_t mon_addrs_size;
        struct rbd_spec *spec = NULL;
        struct rbd_options *rbd_opts = NULL;
@@ -4404,10 +4416,11 @@ static int rbd_add_parse_args(const char *buf,
                ret = -ENAMETOOLONG;
                goto out_err;
        }
-       spec->snap_name = kmemdup(buf, len + 1, GFP_KERNEL);
-       if (!spec->snap_name)
+       snap_name = kmemdup(buf, len + 1, GFP_KERNEL);
+       if (!snap_name)
                goto out_mem;
-       *(spec->snap_name + len) = '\0';
+       *(snap_name + len) = '\0';
+       spec->snap_name = snap_name;
 
        /* Initialize all rbd options to the defaults */
 
@@ -4461,20 +4474,19 @@ static int rbd_dev_image_id(struct rbd_device *rbd_dev)
        size_t size;
        char *object_name;
        void *response;
-       void *p;
-
-       /* If we already have it we don't need to look it up */
-
-       if (rbd_dev->spec->image_id)
-               return 0;
+       char *image_id;
 
        /*
         * When probing a parent image, the image id is already
         * known (and the image name likely is not).  There's no
-        * need to fetch the image id again in this case.
+        * need to fetch the image id again in this case.  We
+        * do still need to set the image format though.
         */
-       if (rbd_dev->spec->image_id)
+       if (rbd_dev->spec->image_id) {
+               rbd_dev->image_format = *rbd_dev->spec->image_id ? 2 : 1;
+
                return 0;
+       }
 
        /*
         * First, see if the format 2 image id file exists, and if
@@ -4496,24 +4508,32 @@ static int rbd_dev_image_id(struct rbd_device *rbd_dev)
                goto out;
        }
 
+       /* If it doesn't exist we'll assume it's a format 1 image */
+
        ret = rbd_obj_method_sync(rbd_dev, object_name,
                                "rbd", "get_id", NULL, 0,
                                response, RBD_IMAGE_ID_LEN_MAX, NULL);
        dout("%s: rbd_obj_method_sync returned %d\n", __func__, ret);
-       if (ret < 0)
-               goto out;
-
-       p = response;
-       rbd_dev->spec->image_id = ceph_extract_encoded_string(&p,
-                                               p + ret,
+       if (ret == -ENOENT) {
+               image_id = kstrdup("", GFP_KERNEL);
+               ret = image_id ? 0 : -ENOMEM;
+               if (!ret)
+                       rbd_dev->image_format = 1;
+       } else if (ret > sizeof (__le32)) {
+               void *p = response;
+
+               image_id = ceph_extract_encoded_string(&p, p + ret,
                                                NULL, GFP_NOIO);
-       ret = 0;
-
-       if (IS_ERR(rbd_dev->spec->image_id)) {
-               ret = PTR_ERR(rbd_dev->spec->image_id);
-               rbd_dev->spec->image_id = NULL;
+               ret = IS_ERR(image_id) ? PTR_ERR(image_id) : 0;
+               if (!ret)
+                       rbd_dev->image_format = 2;
        } else {
-               dout("image_id is %s\n", rbd_dev->spec->image_id);
+               ret = -EINVAL;
+       }
+
+       if (!ret) {
+               rbd_dev->spec->image_id = image_id;
+               dout("image_id is %s\n", image_id);
        }
 out:
        kfree(response);
@@ -4522,27 +4542,30 @@ out:
        return ret;
 }
 
-static int rbd_dev_v1_probe(struct rbd_device *rbd_dev)
+/* Undo whatever state changes are made by v1 or v2 image probe */
+
+static void rbd_dev_unprobe(struct rbd_device *rbd_dev)
 {
-       int ret;
-       size_t size;
+       struct rbd_image_header *header;
 
-       /* Version 1 images have no id; empty string is used */
+       rbd_dev_remove_parent(rbd_dev);
+       rbd_spec_put(rbd_dev->parent_spec);
+       rbd_dev->parent_spec = NULL;
+       rbd_dev->parent_overlap = 0;
 
-       rbd_dev->spec->image_id = kstrdup("", GFP_KERNEL);
-       if (!rbd_dev->spec->image_id)
-               return -ENOMEM;
+       /* Free dynamic fields from the header, then zero it out */
 
-       /* Record the header object name for this rbd image. */
+       header = &rbd_dev->header;
+       ceph_put_snap_context(header->snapc);
+       kfree(header->snap_sizes);
+       kfree(header->snap_names);
+       kfree(header->object_prefix);
+       memset(header, 0, sizeof (*header));
+}
 
-       size = strlen(rbd_dev->spec->image_name) + sizeof (RBD_SUFFIX);
-       rbd_dev->header_name = kmalloc(size, GFP_KERNEL);
-       if (!rbd_dev->header_name) {
-               ret = -ENOMEM;
-               goto out_err;
-       }
-       sprintf(rbd_dev->header_name, "%s%s",
-               rbd_dev->spec->image_name, RBD_SUFFIX);
+static int rbd_dev_v1_probe(struct rbd_device *rbd_dev)
+{
+       int ret;
 
        /* Populate rbd image metadata */
 
@@ -4555,8 +4578,6 @@ static int rbd_dev_v1_probe(struct rbd_device *rbd_dev)
        rbd_dev->parent_spec = NULL;
        rbd_dev->parent_overlap = 0;
 
-       rbd_dev->image_format = 1;
-
        dout("discovered version 1 image, header name is %s\n",
                rbd_dev->header_name);
 
@@ -4573,22 +4594,9 @@ out_err:
 
 static int rbd_dev_v2_probe(struct rbd_device *rbd_dev)
 {
-       size_t size;
        int ret;
        u64 ver = 0;
 
-       /*
-        * Image id was filled in by the caller.  Record the header
-        * object name for this rbd image.
-        */
-       size = sizeof (RBD_HEADER_PREFIX) + strlen(rbd_dev->spec->image_id);
-       rbd_dev->header_name = kmalloc(size, GFP_KERNEL);
-       if (!rbd_dev->header_name)
-               return -ENOMEM;
-       sprintf(rbd_dev->header_name, "%s%s",
-                       RBD_HEADER_PREFIX, rbd_dev->spec->image_id);
-
-       /* Get the size and object order for the image */
        ret = rbd_dev_v2_image_size(rbd_dev);
        if (ret)
                goto out_err;
@@ -4611,8 +4619,15 @@ static int rbd_dev_v2_probe(struct rbd_device *rbd_dev)
                ret = rbd_dev_v2_parent_info(rbd_dev);
                if (ret)
                        goto out_err;
-               rbd_warn(rbd_dev, "WARNING: kernel support for "
-                                       "layered rbd images is EXPERIMENTAL!");
+
+               /*
+                * Don't print a warning for parent images.  We can
+                * tell this point because we won't know its pool
+                * name yet (just its pool id).
+                */
+               if (rbd_dev->spec->pool_name)
+                       rbd_warn(rbd_dev, "WARNING: kernel layering "
+                                       "is EXPERIMENTAL!");
        }
 
        /* If the image supports fancy striping, get its parameters */
@@ -4633,9 +4648,6 @@ static int rbd_dev_v2_probe(struct rbd_device *rbd_dev)
        ret = rbd_dev_v2_snap_context(rbd_dev, &ver);
        if (ret)
                goto out_err;
-       rbd_dev->header.obj_version = ver;
-
-       rbd_dev->image_format = 2;
 
        dout("discovered version 2 image, header name is %s\n",
                rbd_dev->header_name);
@@ -4653,25 +4665,54 @@ out_err:
        return ret;
 }
 
-static int rbd_dev_probe_finish(struct rbd_device *rbd_dev)
+static int rbd_dev_probe_parent(struct rbd_device *rbd_dev)
 {
        struct rbd_device *parent = NULL;
-       struct rbd_spec *parent_spec = NULL;
-       struct rbd_client *rbdc = NULL;
+       struct rbd_spec *parent_spec;
+       struct rbd_client *rbdc;
        int ret;
 
-       /* no need to lock here, as rbd_dev is not registered yet */
-       ret = rbd_dev_snaps_update(rbd_dev);
-       if (ret)
-               return ret;
+       if (!rbd_dev->parent_spec)
+               return 0;
+       /*
+        * We need to pass a reference to the client and the parent
+        * spec when creating the parent rbd_dev.  Images related by
+        * parent/child relationships always share both.
+        */
+       parent_spec = rbd_spec_get(rbd_dev->parent_spec);
+       rbdc = __rbd_get_client(rbd_dev->rbd_client);
 
-       ret = rbd_dev_probe_update_spec(rbd_dev);
-       if (ret)
-               goto err_out_snaps;
+       ret = -ENOMEM;
+       parent = rbd_dev_create(rbdc, parent_spec);
+       if (!parent)
+               goto out_err;
+
+       ret = rbd_dev_image_probe(parent);
+       if (ret < 0)
+               goto out_err;
+       rbd_dev->parent = parent;
 
-       ret = rbd_dev_set_mapping(rbd_dev);
+       return 0;
+out_err:
+       if (parent) {
+               rbd_spec_put(rbd_dev->parent_spec);
+               kfree(rbd_dev->header_name);
+               rbd_dev_destroy(parent);
+       } else {
+               rbd_put_client(rbdc);
+               rbd_spec_put(parent_spec);
+       }
+
+       return ret;
+}
+
+static int rbd_dev_device_setup(struct rbd_device *rbd_dev)
+{
+       int ret;
+
+       ret = rbd_dev_mapping_set(rbd_dev);
        if (ret)
-               goto err_out_snaps;
+               return ret;
 
        /* generate unique id: find highest unique id, add one */
        rbd_dev_id_get(rbd_dev);
@@ -4698,41 +4739,10 @@ static int rbd_dev_probe_finish(struct rbd_device *rbd_dev)
        if (ret)
                goto err_out_disk;
 
-       /*
-        * At this point cleanup in the event of an error is the job
-        * of the sysfs code (initiated by rbd_bus_del_dev()).
-        */
-       /* Probe the parent if there is one */
-
-       if (rbd_dev->parent_spec) {
-               /*
-                * We need to pass a reference to the client and the
-                * parent spec when creating the parent rbd_dev.
-                * Images related by parent/child relationships
-                * always share both.
-                */
-               parent_spec = rbd_spec_get(rbd_dev->parent_spec);
-               rbdc = __rbd_get_client(rbd_dev->rbd_client);
-
-               parent = rbd_dev_create(rbdc, parent_spec);
-               if (!parent) {
-                       ret = -ENOMEM;
-                       goto err_out_spec;
-               }
-               rbdc = NULL;            /* parent now owns reference */
-               parent_spec = NULL;     /* parent now owns reference */
-               ret = rbd_dev_probe(parent);
-               if (ret < 0)
-                       goto err_out_parent;
-               rbd_dev->parent = parent;
-       }
-
-       ret = rbd_dev_header_watch_sync(rbd_dev, 1);
-       if (ret)
-               goto err_out_bus;
-
        /* 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);
 
        pr_info("%s: added with size 0x%llx\n", rbd_dev->disk->disk_name,
@@ -4740,37 +4750,71 @@ static int rbd_dev_probe_finish(struct rbd_device *rbd_dev)
 
        return ret;
 
-err_out_parent:
-       rbd_dev_destroy(parent);
-err_out_spec:
-       rbd_spec_put(parent_spec);
-       rbd_put_client(rbdc);
-err_out_bus:
-       /* this will also clean up rest of rbd_dev stuff */
-
-       rbd_bus_del_dev(rbd_dev);
-
-       return ret;
 err_out_disk:
        rbd_free_disk(rbd_dev);
 err_out_blkdev:
        unregister_blkdev(rbd_dev->major, rbd_dev->name);
 err_out_id:
        rbd_dev_id_put(rbd_dev);
-err_out_snaps:
-       rbd_remove_all_snaps(rbd_dev);
+       rbd_dev_mapping_clear(rbd_dev);
 
        return ret;
 }
 
+static int rbd_dev_header_name(struct rbd_device *rbd_dev)
+{
+       struct rbd_spec *spec = rbd_dev->spec;
+       size_t size;
+
+       /* Record the header object name for this rbd image. */
+
+       rbd_assert(rbd_image_format_valid(rbd_dev->image_format));
+
+       if (rbd_dev->image_format == 1)
+               size = strlen(spec->image_name) + sizeof (RBD_SUFFIX);
+       else
+               size = sizeof (RBD_HEADER_PREFIX) + strlen(spec->image_id);
+
+       rbd_dev->header_name = kmalloc(size, GFP_KERNEL);
+       if (!rbd_dev->header_name)
+               return -ENOMEM;
+
+       if (rbd_dev->image_format == 1)
+               sprintf(rbd_dev->header_name, "%s%s",
+                       spec->image_name, RBD_SUFFIX);
+       else
+               sprintf(rbd_dev->header_name, "%s%s",
+                       RBD_HEADER_PREFIX, spec->image_id);
+       return 0;
+}
+
+static void rbd_dev_image_release(struct rbd_device *rbd_dev)
+{
+       int ret;
+
+       rbd_remove_all_snaps(rbd_dev);
+       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;
+       kfree(rbd_dev->spec->image_id);
+       rbd_dev->spec->image_id = NULL;
+
+       rbd_dev_destroy(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.
  */
-static int rbd_dev_probe(struct rbd_device *rbd_dev)
+static int rbd_dev_image_probe(struct rbd_device *rbd_dev)
 {
        int ret;
+       int tmp;
 
        /*
         * Get the id from the image id object.  If it's not a
@@ -4779,18 +4823,54 @@ static int rbd_dev_probe(struct rbd_device *rbd_dev)
         */
        ret = rbd_dev_image_id(rbd_dev);
        if (ret)
+               return ret;
+       rbd_assert(rbd_dev->spec->image_id);
+       rbd_assert(rbd_image_format_valid(rbd_dev->image_format));
+
+       ret = rbd_dev_header_name(rbd_dev);
+       if (ret)
+               goto err_out_format;
+
+       ret = rbd_dev_header_watch_sync(rbd_dev, 1);
+       if (ret)
+               goto out_header_name;
+
+       if (rbd_dev->image_format == 1)
                ret = rbd_dev_v1_probe(rbd_dev);
        else
                ret = rbd_dev_v2_probe(rbd_dev);
-       if (ret) {
-               dout("probe failed, returning %d\n", ret);
+       if (ret)
+               goto err_out_watch;
 
-               return ret;
-       }
+       ret = rbd_dev_snaps_update(rbd_dev);
+       if (ret)
+               goto err_out_probe;
 
-       ret = rbd_dev_probe_finish(rbd_dev);
+       ret = rbd_dev_spec_update(rbd_dev);
        if (ret)
-               rbd_header_free(&rbd_dev->header);
+               goto err_out_snaps;
+
+       ret = rbd_dev_probe_parent(rbd_dev);
+       if (!ret)
+               return 0;
+
+err_out_snaps:
+       rbd_remove_all_snaps(rbd_dev);
+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");
+out_header_name:
+       kfree(rbd_dev->header_name);
+       rbd_dev->header_name = NULL;
+err_out_format:
+       rbd_dev->image_format = 0;
+       kfree(rbd_dev->spec->image_id);
+       rbd_dev->spec->image_id = NULL;
+
+       dout("probe failed, returning %d\n", ret);
 
        return ret;
 }
@@ -4827,11 +4907,13 @@ static ssize_t rbd_add(struct bus_type *bus,
        rc = ceph_pg_poolid_by_name(osdc->osdmap, spec->pool_name);
        if (rc < 0)
                goto err_out_client;
-       spec->pool_id = (u64) rc;
+       spec->pool_id = (u64)rc;
 
        /* The ceph file layout needs to fit pool id in 32 bits */
 
-       if (WARN_ON(spec->pool_id > (u64) U32_MAX)) {
+       if (spec->pool_id > (u64)U32_MAX) {
+               rbd_warn(NULL, "pool id too large (%llu > %u)\n",
+                               (unsigned long long)spec->pool_id, U32_MAX);
                rc = -EIO;
                goto err_out_client;
        }
@@ -4846,11 +4928,15 @@ static ssize_t rbd_add(struct bus_type *bus,
        kfree(rbd_opts);
        rbd_opts = NULL;        /* done with this */
 
-       rc = rbd_dev_probe(rbd_dev);
+       rc = rbd_dev_image_probe(rbd_dev);
        if (rc < 0)
                goto err_out_rbd_dev;
 
-       return count;
+       rc = rbd_dev_device_setup(rbd_dev);
+       if (!rc)
+               return count;
+
+       rbd_dev_image_release(rbd_dev);
 err_out_rbd_dev:
        rbd_dev_destroy(rbd_dev);
 err_out_client:
@@ -4865,7 +4951,7 @@ err_out_module:
 
        dout("Error adding device %s\n", buf);
 
-       return (ssize_t) rc;
+       return (ssize_t)rc;
 }
 
 static struct rbd_device *__rbd_get_dev(unsigned long dev_id)
@@ -4885,33 +4971,43 @@ static struct rbd_device *__rbd_get_dev(unsigned long dev_id)
        return NULL;
 }
 
-static void rbd_dev_release(struct device *dev)
+static void rbd_dev_device_release(struct device *dev)
 {
        struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);
 
-       if (rbd_dev->watch_event)
-               rbd_dev_header_watch_sync(rbd_dev, 0);
-
-       /* clean up and free blkdev */
        rbd_free_disk(rbd_dev);
+       clear_bit(RBD_DEV_FLAG_EXISTS, &rbd_dev->flags);
+       rbd_dev_clear_mapping(rbd_dev);
        unregister_blkdev(rbd_dev->major, rbd_dev->name);
-
-       /* release allocated disk header fields */
-       rbd_header_free(&rbd_dev->header);
-
-       /* done with the id, and with the rbd_dev */
+       rbd_dev->major = 0;
        rbd_dev_id_put(rbd_dev);
-       rbd_assert(rbd_dev->rbd_client != NULL);
-       rbd_dev_destroy(rbd_dev);
-
-       /* release module ref */
-       module_put(THIS_MODULE);
+       rbd_dev_mapping_clear(rbd_dev);
 }
 
-static void __rbd_remove(struct rbd_device *rbd_dev)
+static void rbd_dev_remove_parent(struct rbd_device *rbd_dev)
 {
-       rbd_remove_all_snaps(rbd_dev);
-       rbd_bus_del_dev(rbd_dev);
+       while (rbd_dev->parent) {
+               struct rbd_device *first = rbd_dev;
+               struct rbd_device *second = first->parent;
+               struct rbd_device *third;
+
+               /*
+                * Follow to the parent with no grandparent and
+                * remove it.
+                */
+               while (second && (third = second->parent)) {
+                       first = second;
+                       second = third;
+               }
+               rbd_assert(second);
+               rbd_dev_image_release(second);
+               first->parent = NULL;
+               first->parent_overlap = 0;
+
+               rbd_assert(first->parent_spec);
+               rbd_spec_put(first->parent_spec);
+               first->parent_spec = NULL;
+       }
 }
 
 static ssize_t rbd_remove(struct bus_type *bus,
@@ -4919,13 +5015,13 @@ static ssize_t rbd_remove(struct bus_type *bus,
                          size_t count)
 {
        struct rbd_device *rbd_dev = NULL;
-       int target_id, rc;
+       int target_id;
        unsigned long ul;
-       int ret = count;
+       int ret;
 
-       rc = strict_strtoul(buf, 10, &ul);
-       if (rc)
-               return rc;
+       ret = strict_strtoul(buf, 10, &ul);
+       if (ret)
+               return ret;
 
        /* convert to int; abort if we lost anything in the conversion */
        target_id = (int) ul;
@@ -4948,28 +5044,10 @@ static ssize_t rbd_remove(struct bus_type *bus,
        spin_unlock_irq(&rbd_dev->lock);
        if (ret < 0)
                goto done;
-
-       while (rbd_dev->parent_spec) {
-               struct rbd_device *first = rbd_dev;
-               struct rbd_device *second = first->parent;
-               struct rbd_device *third;
-
-               /*
-                * Follow to the parent with no grandparent and
-                * remove it.
-                */
-               while (second && (third = second->parent)) {
-                       first = second;
-                       second = third;
-               }
-               __rbd_remove(second);
-               rbd_spec_put(first->parent_spec);
-               first->parent_spec = NULL;
-               first->parent_overlap = 0;
-               first->parent = NULL;
-       }
-       __rbd_remove(rbd_dev);
-
+       ret = count;
+       rbd_bus_del_dev(rbd_dev);
+       rbd_dev_image_release(rbd_dev);
+       module_put(THIS_MODULE);
 done:
        mutex_unlock(&ctl_mutex);