Re: [PATCH] rbd: send snapshot context with writes

[Alex Elder <alex.elder@linaro.org>]

On 06/26/2013 03:06 PM, Josh Durgin wrote:
> Sending the right snapshot context with each write is required for
> snapshots to work. Due to the ordering of calls, the snapshot context
> is never set for any requests. This causes writes to the current
> version of the image to be reflected in all snapshots, which are
> supposed to be read-only.
> 
> This happens because rbd_osd_req_format_write() sets the snapshot
> context based on obj_request->img_request. At this point, however,
> obj_request->img_request has not been set yet, to the snapshot context
> is set to NULL. Fix this by moving rbd_img_obj_request_add(), which
> sets obj_request->img_request, before the osd request formatting
> calls.
> 
> This resolves:
>     http://tracker.ceph.com/issues/5465

That appears to be the wrong bug number.

One fix needed for commenting style (don't use "//"), but otherwise
this looks good.

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

> Reported-by: Karol Jurak <karol.jurak@gmail.com>
> Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
> ---
>  drivers/block/rbd.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index d79831a..4b03d02 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -2266,13 +2266,15 @@ static int rbd_img_request_fill(struct rbd_img_request *img_request,
>  					obj_request->pages, length,
>  					offset & ~PAGE_MASK, false, false);
>  
> +		// writes get the snapc from the img_request, so
> +		// set it before formatting the osd_req

Don't use C++ comments, use /* */.

> +		rbd_img_obj_request_add(img_request, obj_request);
>  		if (write_request)
>  			rbd_osd_req_format_write(obj_request);
>  		else
>  			rbd_osd_req_format_read(obj_request);
>  
>  		obj_request->img_offset = img_offset;
> -		rbd_img_obj_request_add(img_request, obj_request);
>  
>  		img_offset += length;
>  		resid -= length;
>