[PATCH 0/4] rbd: cleanup and leaks #2

[PATCH 0/4] rbd: cleanup and leaks #2

[Alex Elder]

Cleanup related to spec ids.  This one fixes a small leak
of the version 2 image id.

					-Alex

[PATCH 1/4] rbd: have rbd_dev_image_id() set format 1 image id
[PATCH 2/4] rbd: fix image id leak in initial probe
[PATCH 3/4] rbd: have snap_by_name() return a snapshot
[PATCH 4/4] rbd: set snapshot id in rbd_dev_probe_update_spec()

[PATCH 1/4] rbd: have rbd_dev_image_id() set format 1 image id

[Alex Elder]

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@inktank.com>
---
 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);

-- 
1.7.9.5

Re: [PATCH 1/4] rbd: have rbd_dev_image_id() set format 1 image id

[Josh Durgin]

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

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@inktank.com>
> ---
>   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);
>

[PATCH 2/4] rbd: fix image id leak in initial probe

[Alex Elder]

If a format 2 image id is found for an image being mapped, but the
subsequent probe of the image fails, rbd_dev_probe() quits without
freeing the image id.  Fix that.

Also drop a redundant hunk of code in rbd_dev_image_id().

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 0774ae1..e79dfe2 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -4794,17 +4794,21 @@ static int rbd_dev_probe(struct rbd_device *rbd_dev)
 		ret = rbd_dev_v2_probe(rbd_dev);
 	else
 		ret = rbd_dev_v1_probe(rbd_dev);
-	if (ret) {
-		dout("probe failed, returning %d\n", ret);
-
-		return ret;
-	}
+	if (ret)
+		goto out_err;

 	ret = rbd_dev_probe_finish(rbd_dev);
 	if (ret)
 		rbd_header_free(&rbd_dev->header);

 	return ret;
+out_err:
+	kfree(rbd_dev->spec->image_id);
+	rbd_dev->spec->image_id = NULL;
+
+	dout("probe failed, returning %d\n", ret);
+
+	return ret;
 }

 static ssize_t rbd_add(struct bus_type *bus,
-- 
1.7.9.5

Re: [PATCH 2/4] rbd: fix image id leak in initial probe

[Josh Durgin]

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

On 04/26/2013 07:52 AM, Alex Elder wrote:
> If a format 2 image id is found for an image being mapped, but the
> subsequent probe of the image fails, rbd_dev_probe() quits without
> freeing the image id.  Fix that.
>
> Also drop a redundant hunk of code in rbd_dev_image_id().
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 0774ae1..e79dfe2 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -4794,17 +4794,21 @@ static int rbd_dev_probe(struct rbd_device *rbd_dev)
>   		ret = rbd_dev_v2_probe(rbd_dev);
>   	else
>   		ret = rbd_dev_v1_probe(rbd_dev);
> -	if (ret) {
> -		dout("probe failed, returning %d\n", ret);
> -
> -		return ret;
> -	}
> +	if (ret)
> +		goto out_err;
>
>   	ret = rbd_dev_probe_finish(rbd_dev);
>   	if (ret)
>   		rbd_header_free(&rbd_dev->header);
>
>   	return ret;
> +out_err:
> +	kfree(rbd_dev->spec->image_id);
> +	rbd_dev->spec->image_id = NULL;
> +
> +	dout("probe failed, returning %d\n", ret);
> +
> +	return ret;
>   }
>
>   static ssize_t rbd_add(struct bus_type *bus,
>

[PATCH 3/4] rbd: have snap_by_name() return a snapshot

[Alex Elder]

A function called snap_by_name() ought to just look up a snapshot by
name.  It does that, but then it assigns some stuff to the rbd
device structure as well.

Change the function to do just the lookup, and have the caller do
the assignments that follow.

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   35 +++++++++++++++--------------------
 1 file changed, 15 insertions(+), 20 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index e79dfe2..5f97137 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -830,44 +830,39 @@ 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)
 {
-	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->spec->snap_id = snap->id;
+		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)
-- 
1.7.9.5

Re: [PATCH 3/4] rbd: have snap_by_name() return a snapshot

[Josh Durgin]

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

On 04/26/2013 07:52 AM, Alex Elder wrote:
> A function called snap_by_name() ought to just look up a snapshot by
> name.  It does that, but then it assigns some stuff to the rbd
> device structure as well.
>
> Change the function to do just the lookup, and have the caller do
> the assignments that follow.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   35 +++++++++++++++--------------------
>   1 file changed, 15 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index e79dfe2..5f97137 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -830,44 +830,39 @@ 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)
>   {
> -	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->spec->snap_id = snap->id;
> +		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)
>

[PATCH 4/4] rbd: set snapshot id in rbd_dev_probe_update_spec()

[Alex Elder]

Set the rbd spec's snapshot id for an image getting mapped in
rbd_dev_probe_update_spec() rather than rbd_dev_set_mapping().
This is the more logical place for that to happen (even though
it means we might look up the snapshot by name twice).

Signed-off-by: Alex Elder <elder@inktank.com>
---
 drivers/block/rbd.c |   26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 5f97137..5099f44 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -846,7 +846,6 @@ static int rbd_dev_set_mapping(struct rbd_device
*rbd_dev)
 {
 	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;
 	} else {
@@ -855,7 +854,6 @@ static int rbd_dev_set_mapping(struct rbd_device
*rbd_dev)
 		snap = snap_by_name(rbd_dev, rbd_dev->spec->snap_name);
 		if (!snap)
 			return -ENOENT;
-		rbd_dev->spec->snap_id = snap->id;
 		rbd_dev->mapping.size = snap->size;
 		rbd_dev->mapping.features = snap->features;
 		rbd_dev->mapping.read_only = true;
@@ -3759,6 +3757,10 @@ out:
  * rbd_dev_snaps_update() has completed because some of the
  * information (in particular, snapshot name) is not available
  * until then.
+ *
+ * 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.
  */
 static int rbd_dev_probe_update_spec(struct rbd_device *rbd_dev)
 {
@@ -3767,8 +3769,24 @@ static int rbd_dev_probe_update_spec(struct
rbd_device *rbd_dev)
 	void *reply_buf = NULL;
 	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 (rbd_dev->spec->pool_name) {
+		if (strcmp(rbd_dev->spec->snap_name, RBD_SNAP_HEAD_NAME)) {
+			struct rbd_snap *snap;
+
+			snap = snap_by_name(rbd_dev, rbd_dev->spec->snap_name);
+			if (!snap)
+				return -ENOENT;
+			rbd_dev->spec->snap_id = snap->id;
+		} else {
+			rbd_dev->spec->snap_id = CEPH_NOSNAP;
+		}
+
+		return 0;
+	}

 	/* Look up the pool name */

-- 
1.7.9.5

Re: [PATCH 4/4] rbd: set snapshot id in rbd_dev_probe_update_spec()

[Josh Durgin]

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

On 04/26/2013 07:53 AM, Alex Elder wrote:
> Set the rbd spec's snapshot id for an image getting mapped in
> rbd_dev_probe_update_spec() rather than rbd_dev_set_mapping().
> This is the more logical place for that to happen (even though
> it means we might look up the snapshot by name twice).
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
>   drivers/block/rbd.c |   26 ++++++++++++++++++++++----
>   1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 5f97137..5099f44 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -846,7 +846,6 @@ static int rbd_dev_set_mapping(struct rbd_device
> *rbd_dev)
>   {
>   	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;
>   	} else {
> @@ -855,7 +854,6 @@ static int rbd_dev_set_mapping(struct rbd_device
> *rbd_dev)
>   		snap = snap_by_name(rbd_dev, rbd_dev->spec->snap_name);
>   		if (!snap)
>   			return -ENOENT;
> -		rbd_dev->spec->snap_id = snap->id;
>   		rbd_dev->mapping.size = snap->size;
>   		rbd_dev->mapping.features = snap->features;
>   		rbd_dev->mapping.read_only = true;
> @@ -3759,6 +3757,10 @@ out:
>    * rbd_dev_snaps_update() has completed because some of the
>    * information (in particular, snapshot name) is not available
>    * until then.
> + *
> + * 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.
>    */
>   static int rbd_dev_probe_update_spec(struct rbd_device *rbd_dev)
>   {
> @@ -3767,8 +3769,24 @@ static int rbd_dev_probe_update_spec(struct
> rbd_device *rbd_dev)
>   	void *reply_buf = NULL;
>   	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 (rbd_dev->spec->pool_name) {
> +		if (strcmp(rbd_dev->spec->snap_name, RBD_SNAP_HEAD_NAME)) {
> +			struct rbd_snap *snap;
> +
> +			snap = snap_by_name(rbd_dev, rbd_dev->spec->snap_name);
> +			if (!snap)
> +				return -ENOENT;
> +			rbd_dev->spec->snap_id = snap->id;
> +		} else {
> +			rbd_dev->spec->snap_id = CEPH_NOSNAP;
> +		}
> +
> +		return 0;
> +	}
>
>   	/* Look up the pool name */
>