blkback making assumptions about the id of the requests

[Roger Pau Monné]

On 02/09/13 22:03, Justin T. Gibbs wrote:
> On Sep 2, 2013, at 11:58 AM, Roger Pau Monné <roger.pau at citrix.com> wrote:
> 
>> Hello,
>>
>> While playing with driver domains using FreeBSD I've found out that
>> blkback in FreeBSD makes assumptions about the id of a request instead
>> of actually using the id of the request on the shared ring. This seems
>> wrong to me, since a frontend might choose whatever ids it like for the
>> requests (like using 100-131 instead of 0-31). The patch attached fixes
>> it by copying the id from the request on the ring to blkback internal
>> request structure.
>>
>> Roger.
> 
> It looks to me like the id is set in xbb_dispatch_io().  Why it is done there
> and not earlier, I don't recall.

Sorry, I've missed to spot that the id is set there, the problem is that
requests of type BLKIF_OP_FLUSH_DISKCACHE never reach that point, so
they end up having incorrect requests ids. I've reworded the commit
message and removed the late setting of the request id, now it is set
earlier so requests of type flush also have a valid id when writing the
response on the ring.


-------------- next part --------------
From 80dc3cbf5ed88be890d28ae198ab69968caf9d63 Mon Sep 17 00:00:00 2001
From: Roger Pau Monne <roger.pau at citrix.com>
Date: Mon, 2 Sep 2013 15:51:47 +0200
Subject: [PATCH] xen-blkback: set request id correctly for flush OP

Originally the internal request id is set on xbb_dispatch_io, but
requests of type BLKIF_OP_FLUSH_DISKCACHE never reach this point, so
they end up having wrong request ids when writting the response on the
ring, breaking the frontend.

This patch fixes it by setting the request id for all request types
earlier, so that BLKIF_OP_FLUSH_DISKCACHE operations have the right
id.
---
 sys/dev/xen/blkback/blkback.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sys/dev/xen/blkback/blkback.c b/sys/dev/xen/blkback/blkback.c
index 2a220c4..fa9a744 100644
--- a/sys/dev/xen/blkback/blkback.c
+++ b/sys/dev/xen/blkback/blkback.c
@@ -1239,6 +1239,7 @@ xbb_get_resources(struct xbb_softc *xbb, struct xbb_xen_reqlist **reqlist,
 
 	nreq->reqlist = *reqlist;
 	nreq->req_ring_idx = ring_idx;
+	nreq->id = ring_req->id;
 
 	if (xbb->abi != BLKIF_PROTOCOL_NATIVE) {
 		bcopy(ring_req, &nreq->ring_req_storage, sizeof(*ring_req));
@@ -1608,7 +1609,6 @@ xbb_dispatch_io(struct xbb_softc *xbb, struct xbb_xen_reqlist *reqlist)
 		req_ring_idx	      = nreq->req_ring_idx;
 		nr_sects              = 0;
 		nseg                  = ring_req->nr_segments;
-		nreq->id              = ring_req->id;
 		nreq->nr_pages        = nseg;
 		nreq->nr_512b_sectors = 0;
 		req_seg_idx	      = 0;
-- 
1.7.7.5 (Apple Git-26)