On 04/26/2013 11:01 AM, Alex Elder wrote:
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@xxxxxxxxxxx>
---
drivers/block/rbd.c | 81
++++++++++++++++++++++++++-------------------------
1 file changed, 42 insertions(+), 39 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index f65310c6..5918e0b 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -3774,83 +3774,86 @@ 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)
Probably want ret = -ENOMEM here. With that fixed:
Reviewed-by: Josh Durgin <josh.durgin@xxxxxxxxxxx>
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;
}
@@ -4706,7 +4709,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;
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
