[PATCH 1/2] rbd: fetch object order before using it

[PATCH 1/2] rbd: fetch object order before using it

[Josh Durgin]

rbd_dev_v2_header_onetime() fetches striping information, and
checks whether the image can be read by compariing the stripe unit
to the object size. It determines the object size by shifting
the object order, which is 0 at this point since it has not been
read yet. Move the call to get the image size and object order
before rbd_dev_v2_header_onetime() so it is set before use.

Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
---
 drivers/block/rbd.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index cecf5c6..9f72125 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -4286,6 +4286,10 @@ static int rbd_dev_v2_header_info(struct rbd_device *rbd_dev)
 	bool first_time = rbd_dev->header.object_prefix == NULL;
 	int ret;
 
+	ret = rbd_dev_v2_image_size(rbd_dev);
+	if (ret)
+		return ret;
+
 	if (first_time) {
 		ret = rbd_dev_v2_header_onetime(rbd_dev);
 		if (ret)
@@ -4319,10 +4323,6 @@ static int rbd_dev_v2_header_info(struct rbd_device *rbd_dev)
 					"is EXPERIMENTAL!");
 	}
 
-	ret = rbd_dev_v2_image_size(rbd_dev);
-	if (ret)
-		return ret;
-
 	if (rbd_dev->spec->snap_id == CEPH_NOSNAP)
 		if (rbd_dev->mapping.size != rbd_dev->header.image_size)
 			rbd_dev->mapping.size = rbd_dev->header.image_size;
-- 
1.7.2.5

[PATCH 2/2] rbd: use the correct length for format 2 object names

[Josh Durgin]

Format 2 objects use 16 characters for the object name suffix to be
able to express the full 64-bit range of object numbers. Format 1
images only use 12 characters for this. Using 12-character names for
format 2 caused userspace and kernel rbd clients to read differently
named objects, which made an image written by one client look empty to
the other client.

Reported-by: Chris Dunlop <chris@onthe.net.au>
Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
---
 drivers/block/rbd.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 9f72125..39ee017 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1028,12 +1028,16 @@ static const char *rbd_segment_name(struct rbd_device *rbd_dev, u64 offset)
 	char *name;
 	u64 segment;
 	int ret;
+	char *name_format;
 
 	name = kmem_cache_alloc(rbd_segment_name_cache, GFP_NOIO);
 	if (!name)
 		return NULL;
 	segment = offset >> rbd_dev->header.obj_order;
-	ret = snprintf(name, MAX_OBJ_NAME_SIZE + 1, "%s.%012llx",
+	name_format = "%s.%012llx";
+	if (rbd_dev->image_format == 2)
+		name_format = "%s.%016llx";
+	ret = snprintf(name, MAX_OBJ_NAME_SIZE + 1, name_format,
 			rbd_dev->header.object_prefix, segment);
 	if (ret < 0 || ret > MAX_OBJ_NAME_SIZE) {
 		pr_err("error formatting segment name for #%llu (%d)\n",
-- 
1.7.2.5

Re: [PATCH 2/2] rbd: use the correct length for format 2 object names

[Alex Elder]

On 06/12/2013 10:10 PM, Josh Durgin wrote:
> Format 2 objects use 16 characters for the object name suffix to be
> able to express the full 64-bit range of object numbers. Format 1
> images only use 12 characters for this. Using 12-character names for
> format 2 caused userspace and kernel rbd clients to read differently
> named objects, which made an image written by one client look empty to
> the other client.
> 
> Reported-by: Chris Dunlop <chris@onthe.net.au>
> Signed-off-by: Josh Durgin <josh.durgin@inktank.com>

Looks good.

Reviewed-by: Alex Elder <elder@linaro.org>

>  drivers/block/rbd.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 9f72125..39ee017 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -1028,12 +1028,16 @@ static const char *rbd_segment_name(struct rbd_device *rbd_dev, u64 offset)
>  	char *name;
>  	u64 segment;
>  	int ret;
> +	char *name_format;
>  
>  	name = kmem_cache_alloc(rbd_segment_name_cache, GFP_NOIO);
>  	if (!name)
>  		return NULL;
>  	segment = offset >> rbd_dev->header.obj_order;
> -	ret = snprintf(name, MAX_OBJ_NAME_SIZE + 1, "%s.%012llx",
> +	name_format = "%s.%012llx";
> +	if (rbd_dev->image_format == 2)
> +		name_format = "%s.%016llx";
> +	ret = snprintf(name, MAX_OBJ_NAME_SIZE + 1, name_format,
>  			rbd_dev->header.object_prefix, segment);
>  	if (ret < 0 || ret > MAX_OBJ_NAME_SIZE) {
>  		pr_err("error formatting segment name for #%llu (%d)\n",
> 

Re: [PATCH 1/2] rbd: fetch object order before using it

[Sage Weil]

Both of these look good; pushing to testing branch shortly.  The name 
length one I also tagged for 3.9 stable (this one didn't apply cleanly).

Thanks!
sage


On Wed, 12 Jun 2013, Josh Durgin wrote:

> rbd_dev_v2_header_onetime() fetches striping information, and
> checks whether the image can be read by compariing the stripe unit
> to the object size. It determines the object size by shifting
> the object order, which is 0 at this point since it has not been
> read yet. Move the call to get the image size and object order
> before rbd_dev_v2_header_onetime() so it is set before use.
> 
> Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
> ---
>  drivers/block/rbd.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index cecf5c6..9f72125 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -4286,6 +4286,10 @@ static int rbd_dev_v2_header_info(struct rbd_device *rbd_dev)
>  	bool first_time = rbd_dev->header.object_prefix == NULL;
>  	int ret;
>  
> +	ret = rbd_dev_v2_image_size(rbd_dev);
> +	if (ret)
> +		return ret;
> +
>  	if (first_time) {
>  		ret = rbd_dev_v2_header_onetime(rbd_dev);
>  		if (ret)
> @@ -4319,10 +4323,6 @@ static int rbd_dev_v2_header_info(struct rbd_device *rbd_dev)
>  					"is EXPERIMENTAL!");
>  	}
>  
> -	ret = rbd_dev_v2_image_size(rbd_dev);
> -	if (ret)
> -		return ret;
> -
>  	if (rbd_dev->spec->snap_id == CEPH_NOSNAP)
>  		if (rbd_dev->mapping.size != rbd_dev->header.image_size)
>  			rbd_dev->mapping.size = rbd_dev->header.image_size;
> -- 
> 1.7.2.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH 1/2] rbd: fetch object order before using it

[Alex Elder]

On 06/12/2013 10:10 PM, Josh Durgin wrote:
> rbd_dev_v2_header_onetime() fetches striping information, and
> checks whether the image can be read by compariing the stripe unit
> to the object size. It determines the object size by shifting
> the object order, which is 0 at this point since it has not been
> read yet. Move the call to get the image size and object order
> before rbd_dev_v2_header_onetime() so it is set before use.
> 
> Signed-off-by: Josh Durgin <josh.durgin@inktank.com>

Looks good.

Reviewed-by: Alex Elder <elder@linaro.org>

> ---
>  drivers/block/rbd.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index cecf5c6..9f72125 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -4286,6 +4286,10 @@ static int rbd_dev_v2_header_info(struct rbd_device *rbd_dev)
>  	bool first_time = rbd_dev->header.object_prefix == NULL;
>  	int ret;
>  
> +	ret = rbd_dev_v2_image_size(rbd_dev);
> +	if (ret)
> +		return ret;
> +
>  	if (first_time) {
>  		ret = rbd_dev_v2_header_onetime(rbd_dev);
>  		if (ret)
> @@ -4319,10 +4323,6 @@ static int rbd_dev_v2_header_info(struct rbd_device *rbd_dev)
>  					"is EXPERIMENTAL!");
>  	}
>  
> -	ret = rbd_dev_v2_image_size(rbd_dev);
> -	if (ret)
> -		return ret;
> -
>  	if (rbd_dev->spec->snap_id == CEPH_NOSNAP)
>  		if (rbd_dev->mapping.size != rbd_dev->header.image_size)
>  			rbd_dev->mapping.size = rbd_dev->header.image_size;
>