Reviewed-by: Josh Durgin <josh.durgin@xxxxxxxxxxx>
On 04/26/2013 07:52 AM, Alex Elder wrote:
Currently, rbd_dev_probe() assumes that any error returned by
rbd_dev_image_id() is most likely -ENOENT, and responds by
calling the format 1 probe routine, rbd_dev_v1_probe(). Then,
at the top of rbd_dev_v1_probe(), an empty string is allocated
for the image id.
This is sort of unbalanced. Fix this by having rbd_dev_image_id()
look for -ENOENT from its "get_id" method call. If that is seen,
have it allocate the empty string there rather than depending on
rbd_dev_v1_probe() to do it.
Also drop a redundant hunk of code in rbd_dev_image_id().
Signed-off-by: Alex Elder <elder@xxxxxxxxxxx>
---
drivers/block/rbd.c | 47 ++++++++++++++++++++++-------------------------
1 file changed, 22 insertions(+), 25 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 9e38967..0774ae1 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -4476,12 +4476,7 @@ 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
@@ -4511,24 +4506,28 @@ 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;
+ if (ret == -ENOENT) {
+ image_id = kstrdup("", GFP_KERNEL);
+ ret = image_id ? 0 : -ENOMEM;
+ } else if (ret > sizeof (__le32)) {
+ void *p = response;
- p = response;
- rbd_dev->spec->image_id = ceph_extract_encoded_string(&p,
- p + ret,
+ 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;
} 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);
@@ -4542,12 +4541,6 @@ static int rbd_dev_v1_probe(struct rbd_device
*rbd_dev)
int ret;
size_t size;
- /* Version 1 images have no id; empty string is used */
-
- rbd_dev->spec->image_id = kstrdup("", GFP_KERNEL);
- if (!rbd_dev->spec->image_id)
- return -ENOMEM;
-
/* Record the header object name for this rbd image. */
size = strlen(rbd_dev->spec->image_name) + sizeof (RBD_SUFFIX);
@@ -4794,9 +4787,13 @@ static int rbd_dev_probe(struct rbd_device *rbd_dev)
*/
ret = rbd_dev_image_id(rbd_dev);
if (ret)
- ret = rbd_dev_v1_probe(rbd_dev);
- else
+ return ret;
+
+ rbd_assert(rbd_dev->spec->image_id);
+ if (*rbd_dev->spec->image_id)
ret = rbd_dev_v2_probe(rbd_dev);
+ else
+ ret = rbd_dev_v1_probe(rbd_dev);
if (ret) {
dout("probe failed, returning %d\n", ret);
--
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
