From 2e9f7f1c0de23156e225046f10fad939a4017e97 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Fri, 26 Apr 2013 09:43:48 -0500 Subject: [PATCH] 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 Reviewed-by: Josh Durgin --- drivers/block/rbd.c | 83 ++++++++++++++++++++++++--------------------- 1 file changed, 44 insertions(+), 39 deletions(-) diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 09062c48705b..3bd12ead5091 100644 --- 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; -- 2.30.2