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@xxxxxxxxxx> > Reported-by: Karol Jurak <karol.jurak@xxxxxxxxx> > Signed-off-by: Josh Durgin <josh.durgin@xxxxxxxxxxx> > --- > 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; > -- 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
