lkml.org 
[lkml]   [2009]   [Mar]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: VIRTIO_BLK_T_SCSI_CMD handling in virtio-blk
Christian Borntraeger wrote:
> Am Wednesday 18 March 2009 07:09:19 schrieb Christoph Hellwig:
>> Currently virtio-blk just sends down the payload for packet command
>> requests, setting the VIRTIO_BLK_T_SCSI_CMD flag in the type field and
>> zeroing out the sector field.
>>
>> But to make any sense of the payload of a packet command we need the
>> scsi command block (request->cmd) which specifies the operation,
>> location and length for this command.
>>
>> All backends that I checked just fail VIRTIO_BLK_T_SCSI_CMD commands, so
>> AFAICS no harm is done. But should we really keep this broken support
>> in the protocol around? If we do want to support packet commands in
>> the future we should probably just add the command as the first S/G list
>> entry.
>
> Hannes did some implementation for SCSI command passthrough:
> https://lists.linux-foundation.org/pipermail/virtualization/2008-August/011629.html
>
> After some addon fix this was working pretty well. I dont know why Hannes never
> pushed the final version upstream.
>
Sorry, being busy. Someone keeps sending me multipath bugs to worry about.
Patch is attached.

Please do keep me in the loop here, I'm not subscribed to lkml.

Cheers,

Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 4225109..46f03d2 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -35,6 +35,7 @@ struct virtblk_req
struct list_head list;
struct request *req;
struct virtio_blk_outhdr out_hdr;
+ struct virtio_blk_inhdr in_hdr;
u8 status;
};

@@ -47,20 +48,29 @@ static void blk_done(struct virtqueue *vq)

spin_lock_irqsave(&vblk->lock, flags);
while ((vbr = vblk->vq->vq_ops->get_buf(vblk->vq, &len)) != NULL) {
- int uptodate;
+ int error;
+ unsigned int bytes;
switch (vbr->status) {
case VIRTIO_BLK_S_OK:
- uptodate = 1;
+ error = 0;
break;
case VIRTIO_BLK_S_UNSUPP:
- uptodate = -ENOTTY;
+ error = -ENOTTY;
break;
default:
- uptodate = 0;
+ error = -EIO;
break;
}

- end_dequeued_request(vbr->req, uptodate);
+ if (blk_pc_request(vbr->req)) {
+ vbr->req->data_len = vbr->in_hdr.residual;
+ bytes = vbr->in_hdr.data_len;
+ vbr->req->sense_len = vbr->in_hdr.sense_len;
+ vbr->req->errors = vbr->in_hdr.status;
+ } else
+ bytes = blk_rq_bytes(vbr->req);
+
+ __blk_end_request(vbr->req, error, bytes);
list_del(&vbr->list);
mempool_free(vbr, vblk->pool);
}
@@ -72,7 +82,7 @@ static void blk_done(struct virtqueue *vq)
static bool do_req(struct request_queue *q, struct virtio_blk *vblk,
struct request *req)
{
- unsigned long num, out, in;
+ unsigned long num, out = 0, in = 0;
struct virtblk_req *vbr;

vbr = mempool_alloc(vblk->pool, GFP_ATOMIC);
@@ -99,20 +109,31 @@ static bool do_req(struct request_queue *q, struct virtio_blk *vblk,

/* This init could be done at vblk creation time */
sg_init_table(vblk->sg, VIRTIO_MAX_SG);
- sg_set_buf(&vblk->sg[0], &vbr->out_hdr, sizeof(vbr->out_hdr));
- num = blk_rq_map_sg(q, vbr->req, vblk->sg+1);
- sg_set_buf(&vblk->sg[num+1], &vbr->status, sizeof(vbr->status));
-
- if (rq_data_dir(vbr->req) == WRITE) {
- vbr->out_hdr.type |= VIRTIO_BLK_T_OUT;
- out = 1 + num;
- in = 1;
- } else {
- vbr->out_hdr.type |= VIRTIO_BLK_T_IN;
- out = 1;
- in = 1 + num;
+ sg_set_buf(&vblk->sg[out], &vbr->out_hdr, sizeof(vbr->out_hdr));
+ out++;
+ if (blk_pc_request(vbr->req)) {
+ sg_set_buf(&vblk->sg[out], vbr->req->cmd, vbr->req->cmd_len);
+ out++;
+ }
+ num = blk_rq_map_sg(q, vbr->req, vblk->sg+out);
+ if (blk_pc_request(vbr->req)) {
+ sg_set_buf(&vblk->sg[num+out+in], vbr->req->sense, 96);
+ in++;
+ sg_set_buf(&vblk->sg[num+out+in], &vbr->in_hdr,
+ sizeof(vbr->in_hdr));
+ in++;
+ }
+ sg_set_buf(&vblk->sg[num+out+in], &vbr->status, sizeof(vbr->status));
+ in++;
+ if (num) {
+ if (rq_data_dir(vbr->req) == WRITE) {
+ vbr->out_hdr.type |= VIRTIO_BLK_T_OUT;
+ out += num;
+ } else {
+ vbr->out_hdr.type |= VIRTIO_BLK_T_IN;
+ in += num;
+ }
}
-
if (vblk->vq->vq_ops->add_buf(vblk->vq, vblk->sg, out, in, vbr)) {
mempool_free(vbr, vblk->pool);
return false;
diff --git a/include/linux/virtio_blk.h b/include/linux/virtio_blk.h
index c1aef85..089e596 100644
--- a/include/linux/virtio_blk.h
+++ b/include/linux/virtio_blk.h
@@ -54,6 +54,14 @@ struct virtio_blk_outhdr
__u64 sector;
};

+struct virtio_blk_inhdr
+{
+ __u32 status;
+ __u32 data_len;
+ __u32 sense_len;
+ __u32 residual;
+};
+
/* And this is the final byte of the write scatter-gather list. */
#define VIRTIO_BLK_S_OK 0
#define VIRTIO_BLK_S_IOERR 1
\
 
 \ /
  Last update: 2009-03-18 10:23    [from the cache]
©2003-2011 Jasper Spaans