rbd: refactor rbd_dev_probe_update_spec()

Fairly straightforward refactoring of rbd_dev_probe_update_spec().
The name is changed to rbd_dev_spec_update().

Rearrange it so nothing gets assigned to the spec until all of the
names have been successfully acquired.

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

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 09062c48705b9258174e8e6d1ade5b4489fec9f1..3bd12ead509199799aaa8e0124d53eb84828af8c 100644 (file)
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -3774,83 +3774,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;
 
        /*
         * An image being mapped will have the pool name (etc.), but
         * we need to look up the snapshot id.
         */
-       if (rbd_dev->spec->pool_name) {
-               if (strcmp(rbd_dev->spec->snap_name, RBD_SNAP_HEAD_NAME)) {
+       if (spec->pool_name) {
+               if (strcmp(spec->snap_name, RBD_SNAP_HEAD_NAME)) {
                        struct rbd_snap *snap;
 
-                       snap = snap_by_name(rbd_dev, rbd_dev->spec->snap_name);
+                       snap = snap_by_name(rbd_dev, spec->snap_name);
                        if (!snap)
                                return -ENOENT;
-                       rbd_dev->spec->snap_id = snap->id;
+                       spec->snap_id = snap->id;
                } else {
-                       rbd_dev->spec->snap_id = CEPH_NOSNAP;
+                       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;
 }
@@ -4710,7 +4715,7 @@ static int rbd_dev_probe_finish(struct rbd_device *rbd_dev)
        if (ret)
                return ret;
 
-       ret = rbd_dev_probe_update_spec(rbd_dev);
+       ret = rbd_dev_spec_update(rbd_dev);
        if (ret)
                goto err_out_snaps;