Fix none native request decoding (to be squashed)

This commit moves the protocol-dependant decoding to where it belongs -
directly after request are taken from the ring and before the wrong message
format is asserted.

Prio to this fix, discard did not work properly for x86-guests, as discard
requests got corrupted during decoding.

Signed-off-by: Robert Breker <[email protected]>

Author: Robert Breker

drivers/td-ctx.c

@@ -125,6 +125,17 @@ xenio_pending_blkif(struct td_xenio_ctx * const ctx)
         dst->seg[i] = src->seg[i];              \
 }
 
+#define blkif_get_req_discard(dst, discard_src)                              \
+{                                                                            \
+    /* assert(sizeof(blkif_request_discard_t)<sizeof(blkif_request_t)) */    \
+    blkif_request_discard_t *discard_dst = (blkif_request_discard_t *) dst;  \
+    discard_dst->operation = src->operation;                                 \
+    discard_dst->flag = discard_src->flag;                                   \
+    discard_dst->id = discard_src->id;                                       \
+    discard_dst->sector_number = discard_src->sector_number;                 \
+    discard_dst->nr_sectors = discard_src->nr_sectors;                       \
+}
+
 /**
  * Utility function that retrieves a request using @idx as the ring index,
  * copying it to the @dst in a H/W independent way.
@@ -149,6 +160,7 @@ xenio_blkif_get_request(struct td_xenblkif * const blkif,
             {
                 blkif_request_t *src;
                 src = RING_GET_REQUEST(&rings->native, idx);
+                // sizeof(blkif_request_t)>sizeof(blkif_request_discard_t)
                 memcpy(dst, src, sizeof(blkif_request_t));
                 break;
             }
@@ -157,15 +169,27 @@ xenio_blkif_get_request(struct td_xenblkif * const blkif,
             {
                 blkif_x86_32_request_t *src;
                 src = RING_GET_REQUEST(&rings->x86_32, idx);
-                blkif_get_req(dst, src);
+                if (src->operation==BLKIF_OP_DISCARD) {
+                    blkif_x86_32_request_discard_t * discard_src;
+                    discard_src = (blkif_x86_32_request_discard_t *) src;
+                    blkif_get_req_discard(dst, discard_src);
+                } else {
+                    blkif_get_req(dst, src);
+                }
                 break;
             }
 
         case BLKIF_PROTOCOL_X86_64:
             {
                 blkif_x86_64_request_t *src;
                 src = RING_GET_REQUEST(&rings->x86_64, idx);
-                blkif_get_req(dst, src);
+                if (src->operation==BLKIF_OP_DISCARD) {
+                    blkif_x86_64_request_discard_t * discard_src;
+                    discard_src = (blkif_x86_64_request_discard_t *) src;
+                    blkif_get_req_discard(dst, discard_src);
+                } else {
+                    blkif_get_req(dst, src);
+                }
                 break;
             }
 

drivers/td-req.c

@@ -569,10 +569,6 @@ tapdisk_xenblkif_parse_request(struct td_xenblkif * const blkif,
     void *page, *next, *last;
     int err = 0;
     unsigned nr_sect = 0;
-    blkif_request_discard_t * request_discard_native_msg;
-    blkif_x86_32_request_discard_t * request_discard_x86_32_msg;
-    blkif_x86_64_request_discard_t * request_discard_x86_64_msg;
-
 
     ASSERT(blkif);
     ASSERT(req);
@@ -640,33 +636,6 @@ tapdisk_xenblkif_parse_request(struct td_xenblkif * const blkif,
     vreq->iovcnt = iov - req->iov + 1;
     vreq->sec = req->msg.sector_number;
 
-    if(req->msg.operation == BLKIF_OP_DISCARD) {
-        switch (blkif->proto) {
-            case BLKIF_PROTOCOL_NATIVE:
-                request_discard_native_msg = (blkif_request_discard_t*)&req->msg;
-                vreq->discard_nr_sectors = request_discard_native_msg->nr_sectors;
-                vreq->sec = request_discard_native_msg->sector_number;
-                break;
-            case BLKIF_PROTOCOL_X86_32:
-                request_discard_x86_32_msg = (blkif_x86_32_request_discard_t*)&req->msg;
-                vreq->discard_nr_sectors = request_discard_x86_32_msg->nr_sectors;
-                vreq->sec = request_discard_x86_32_msg->sector_number;
-                break;
-            case BLKIF_PROTOCOL_X86_64:
-                request_discard_x86_64_msg = (blkif_x86_64_request_discard_t*)&req->msg;
-                vreq->discard_nr_sectors = request_discard_x86_64_msg->nr_sectors;
-                vreq->sec = request_discard_x86_64_msg->sector_number;
-                break;
-            default:
-                ASSERT(0);
-                break;
-        }
-        nr_sect = 0;
-    }
-
-    vreq->iov = req->iov;
-    vreq->iovcnt = iov - req->iov + 1;
-
     if (blkif_rq_wr(&req->msg)) {
         err = guest_copy2(blkif, req);
         if (err) {
@@ -693,6 +662,37 @@ tapdisk_xenblkif_parse_request(struct td_xenblkif * const blkif,
     return err;
 }
 
+static inline int
+tapdisk_xenblkif_parse_request_discard(struct td_xenblkif * const blkif,
+                                       struct td_xenblkif_req * const req)
+{
+    int err = 0;
+    td_vbd_request_t *vreq;
+    blkif_request_discard_t * request_discard_msg;
+
+    vreq = &req->vreq;
+    ASSERT(vreq);
+
+    vreq->iov = 0;
+    vreq->iovcnt = 0;
+    vreq->sec = 0;
+
+    request_discard_msg = (blkif_request_discard_t*)&req->msg;
+    vreq->discard_nr_sectors = request_discard_msg->nr_sectors;
+    vreq->sec = request_discard_msg->sector_number;
+
+    /*
+     * TODO Isn't this kind of expensive to do for each requests? Why does
+     * the tapdisk need this in the first place?
+     */
+    snprintf(req->name, sizeof(req->name), "xenvbd-%d-%d.%"SCNx64"",
+             blkif->domid, blkif->devid, request_discard_msg->id);
+    vreq->name = req->name;
+    vreq->token = blkif;
+    vreq->cb = __tapdisk_xenblkif_request_cb;
+
+    return err;
+}
 
 /**
  * Initialises the standard tapdisk request (td_vbd_request_t) from the
@@ -757,8 +757,9 @@ tapdisk_xenblkif_make_vbd_request(struct td_xenblkif * const blkif,
         goto out;
     }
 
-    if (likely(tapreq->msg.nr_segments ||
-               tapreq->msg.operation == BLKIF_OP_DISCARD))
+    if (unlikely(tapreq->msg.operation == BLKIF_OP_DISCARD))
+        err = tapdisk_xenblkif_parse_request_discard(blkif, tapreq);
+    else if (likely(tapreq->msg.nr_segments))
         err = tapdisk_xenblkif_parse_request(blkif, tapreq);
     /*
      * If we only got one request from the ring and that was a barrier one,