[RFC] [PATCH] SCSI passthrough for virtio-blk

Hi all,

I got bored and implemented SCSI passthrough for the virtio-blk driver.
Principle is quite simple, just put the missing fields (cdb, sense and
status header) on the virtio queue and then call the SG_IO ioctl on the
host.

So when using '-drive file=/dev/sgXX,if=virtio,format=host_device' you
can happily call any sg_XX command on the resulting vdX device. Quite
neat, methinks. And it's even backwards compatible, so each of these
patches should work without the other one applied.

As one would have guessed there are two patches, one for the linux
kernel to modify the virtio-blk driver in the guest and one for the
qemu/kvm userland program to modify the virtio-blk driver on the host.
This patch is relative to avi's kvm-userland tree from kernel.org.

As usual, comments etc to me.

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)

Am Freitag, 29. August 2008 schrieb Hannes Reinecke:

So when using '-drive file=/dev/sgXX,if=virtio,format=host_device' you
can happily call any sg_XX command on the resulting vdX device. Quite
neat, methinks. And it's even backwards compatible, so each of these
patches should work without the other one applied.

Does not work here. If the host does not support the pass-through, the device 
drivers waits for an response. I tried  sdparm /dev/vda with a patched kernel 
and an unpatched userspace.

I think you should use a feature bit to avoid problems.

Christian

Hi Christian,

Christian Borntraeger wrote:

Am Freitag, 29. August 2008 schrieb Hannes Reinecke:

quoted

So when using '-drive file=/dev/sgXX,if=virtio,format=host_device' you
can happily call any sg_XX command on the resulting vdX device. Quite
neat, methinks. And it's even backwards compatible, so each of these
patches should work without the other one applied.

Does not work here. If the host does not support the pass-through, the device drivers waits for an response. I tried  sdparm /dev/vda with a patched kernel and an unpatched userspace.

Hmm. Works here, using an unpatched kvm-73.
Which version did you use?

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)

Am Freitag, 29. August 2008 schrieb Hannes Reinecke:

Hmm. Works here, using an unpatched kvm-73.
Which version did you use?

I use the s390 userspace prototype kuli which uses an virtio transport similar 
to lguest.

I retried and it seems to race. Most of the time it works fine, but sometimes 
sdparm hangs. I will have a 2nd look. 

sysrq-t gives me the following trace:

Call Trace:
([<040000000755bc78>] 0x40000000755bc78)
sdparm        D 000000000043659e     0  2493      1
000000000012004a 000000000744f740 000000000744f778 001896469fd23785
       000000000744f778 00000000009e5500 000000000043f230 0000000000120130
       000000000744f778 0000000006d39400 0000000006d39f80 0000000000000001
       00000000009e6f00 00000000076bf8e8 000000000744f7c8 0000000007530670
       000000000043f610 0000000000435e66 000000000744f7c8 000000000744f868
Call Trace:
([<0000000000435e66>] schedule+0x32e/0x7ec)
 [<000000000043659e>] schedule_timeout+0xba/0x10c
 [<00000000004358da>] wait_for_common+0xbe/0x1a8
 [<000000000027ec3e>] blk_execute_rq+0x86/0xc4
 [<0000000000282768>] sg_io+0x1a4/0x360
 [<0000000000282f8c>] scsi_cmd_ioctl+0x2bc/0x3f0
 [<00000000002c3108>] virtblk_ioctl+0x44/0x58
 [<000000000027ff18>] blkdev_driver_ioctl+0x98/0xa4
 [<000000000027ffd8>] blkdev_ioctl+0xb4/0x7f8
 [<00000000001e1572>] block_ioctl+0x3a/0x48
 [<00000000001bca0a>] vfs_ioctl+0x52/0xdc
 [<00000000001bcb0a>] do_vfs_ioctl+0x76/0x350
 [<00000000001bce6e>] sys_ioctl+0x8a/0xa0
 [<000000000011282c>] sysc_tracego+0xe/0x14
 [<0000020000114286>] 0x20000114286

Hi Christian,

Christian Borntraeger wrote:

Am Freitag, 29. August 2008 schrieb Hannes Reinecke:

quoted

Hmm. Works here, using an unpatched kvm-73.
Which version did you use?

I use the s390 userspace prototype kuli which uses an virtio transport similar to lguest.

I retried and it seems to race. Most of the time it works fine, but sometimes sdparm hangs. I will have a 2nd look.

sysrq-t gives me the following trace:

Call Trace:
([<040000000755bc78>] 0x40000000755bc78)
sdparm        D 000000000043659e     0  2493      1
000000000012004a 000000000744f740 000000000744f778 001896469fd23785
       000000000744f778 00000000009e5500 000000000043f230 0000000000120130
       000000000744f778 0000000006d39400 0000000006d39f80 0000000000000001
       00000000009e6f00 00000000076bf8e8 000000000744f7c8 0000000007530670
       000000000043f610 0000000000435e66 000000000744f7c8 000000000744f868
Call Trace:
([<0000000000435e66>] schedule+0x32e/0x7ec)
 [<000000000043659e>] schedule_timeout+0xba/0x10c
 [<00000000004358da>] wait_for_common+0xbe/0x1a8
 [<000000000027ec3e>] blk_execute_rq+0x86/0xc4
 [<0000000000282768>] sg_io+0x1a4/0x360
 [<0000000000282f8c>] scsi_cmd_ioctl+0x2bc/0x3f0
 [<00000000002c3108>] virtblk_ioctl+0x44/0x58
 [<000000000027ff18>] blkdev_driver_ioctl+0x98/0xa4
 [<000000000027ffd8>] blkdev_ioctl+0xb4/0x7f8
 [<00000000001e1572>] block_ioctl+0x3a/0x48
 [<00000000001bca0a>] vfs_ioctl+0x52/0xdc
 [<00000000001bcb0a>] do_vfs_ioctl+0x76/0x350
 [<00000000001bce6e>] sys_ioctl+0x8a/0xa0
 [<000000000011282c>] sysc_tracego+0xe/0x14
 [<0000020000114286>] 0x20000114286

I'm tempted to say 'not my fault'; the submitted SCSI request on
the _host_ hangs and doesn't come back.
Looks more like a SCSI problem on the host ...

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)

Am Freitag, 29. August 2008 schrieb Hannes Reinecke:

quoted

([<0000000000435e66>] schedule+0x32e/0x7ec)
 [<000000000043659e>] schedule_timeout+0xba/0x10c
 [<00000000004358da>] wait_for_common+0xbe/0x1a8
 [<000000000027ec3e>] blk_execute_rq+0x86/0xc4
 [<0000000000282768>] sg_io+0x1a4/0x360
 [<0000000000282f8c>] scsi_cmd_ioctl+0x2bc/0x3f0
 [<00000000002c3108>] virtblk_ioctl+0x44/0x58
 [<000000000027ff18>] blkdev_driver_ioctl+0x98/0xa4
 [<000000000027ffd8>] blkdev_ioctl+0xb4/0x7f8
 [<00000000001e1572>] block_ioctl+0x3a/0x48
 [<00000000001bca0a>] vfs_ioctl+0x52/0xdc
 [<00000000001bcb0a>] do_vfs_ioctl+0x76/0x350
 [<00000000001bce6e>] sys_ioctl+0x8a/0xa0
 [<000000000011282c>] sysc_tracego+0xe/0x14
 [<0000020000114286>] 0x20000114286

I'm tempted to say 'not my fault'; the submitted SCSI request on
the _host_ hangs and doesn't come back.
Looks more like a SCSI problem on the host ...

It is a guest process trace.

Anyway, after you suggested to look at the len field it appeared to me, that 
the new code sets the data_len, sense_len and changes the number of reported 
bytes to random values even if the host returns VIRTIO_BLK_S_UNSUPP.

Moving these assignments to the VIRTIO_BLK_S_OK case seems to fix it.
Can you test if SG_IO still works for you after applying this patch:

Signed-off-by: Christian Borntraeger <redacted>
---
 drivers/block/virtio_blk.c |   17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

Index: kvm/drivers/block/virtio_blk.c
===================================================================

--- kvm.orig/drivers/block/virtio_blk.c
+++ kvm/drivers/block/virtio_blk.c

@@ -50,9 +50,17 @@ static void blk_done(struct virtqueue *v
 	while ((vbr = vblk->vq->vq_ops->get_buf(vblk->vq, &len)) != NULL) {
 		int error;
 		unsigned int bytes;
+
+		bytes = blk_rq_bytes(vbr->req);
 		switch (vbr->status) {
 		case VIRTIO_BLK_S_OK:
 			error = 0;
+			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;
+			}
 			break;
 		case VIRTIO_BLK_S_UNSUPP:
 			error = -ENOTTY;

@@ -61,15 +69,6 @@ static void blk_done(struct virtqueue *v
 			error = -EIO;
 			break;
 		}
-
-		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);

Thanks for your feedback.

Here is a second try to allows to propagate scsi error code from host->guest.
Makes sense?

Signed-off-by: Christian Borntraeger <redacted>

---
 drivers/block/virtio_blk.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: kvm/drivers/block/virtio_blk.c
===================================================================

--- kvm.orig/drivers/block/virtio_blk.c
+++ kvm/drivers/block/virtio_blk.c

@@ -62,7 +62,7 @@ static void blk_done(struct virtqueue *v
 			break;
 		}
 
-		if (blk_pc_request(vbr->req)) {
+		if (blk_pc_request(vbr->req) && len >= sizeof(vbr->in_hdr)) {
 			vbr->req->data_len = vbr->in_hdr.residual;
 			bytes = vbr->in_hdr.data_len;
 			vbr->req->sense_len = vbr->in_hdr.sense_len;