Il 22/05/2014 09:37, Fam Zheng ha scritto:
-static void virtio_blk_handle_scsi(VirtIOBlockReq *req) +int virtio_blk_handle_scsi_req(BlockDriverState *bs, + VirtIOBlkConf *conf, + VirtQueueElement *elem)
Two more comments... please pass the VirtIOBlock * instead of bs/conf here.
{ + int status = VIRTIO_BLK_S_OK; + struct virtio_scsi_inhdr *scsi = NULL; #ifdef __linux__ - int ret; int i; + struct sg_io_hdr hdr; #endif - int status = VIRTIO_BLK_S_OK; /* * We require at least one output segment each for the virtio_blk_outhdr @@ -140,63 +142,61 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req) * We also at least require the virtio_blk_inhdr, the virtio_scsi_inhdr * and the sense buffer pointer in the input segments. */ - if (req->elem.out_num < 2 || req->elem.in_num < 3) { - virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); - g_free(req); - return; + if (elem->out_num < 2 || elem->in_num < 3) { + status = VIRTIO_BLK_S_IOERR; + goto fail; } /* * The scsi inhdr is placed in the second-to-last input segment, just * before the regular inhdr. */ - req->scsi = (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base; + scsi = (void *)elem->in_sg[elem->in_num - 2].iov_base; - if (!req->dev->blk.scsi) { + /* + * No support for bidirection commands yet. + */ + if (elem->out_num > 2 && elem->in_num > 3) { status = VIRTIO_BLK_S_UNSUPP; goto fail; } - /* - * No support for bidirection commands yet. - */ - if (req->elem.out_num > 2 && req->elem.in_num > 3) { + if (!conf->scsi) { status = VIRTIO_BLK_S_UNSUPP; goto fail; }
Swapping the two conditionals unnecessarily makes the patch bigger. Paolo
#ifdef __linux__ - struct sg_io_hdr hdr; memset(&hdr, 0, sizeof(struct sg_io_hdr)); hdr.interface_id = 'S'; - hdr.cmd_len = req->elem.out_sg[1].iov_len; - hdr.cmdp = req->elem.out_sg[1].iov_base; + hdr.cmd_len = elem->out_sg[1].iov_len; + hdr.cmdp = elem->out_sg[1].iov_base; hdr.dxfer_len = 0; - if (req->elem.out_num > 2) { + if (elem->out_num > 2) { /* * If there are more than the minimally required 2 output segments * there is write payload starting from the third iovec. */ hdr.dxfer_direction = SG_DXFER_TO_DEV; - hdr.iovec_count = req->elem.out_num - 2; + hdr.iovec_count = elem->out_num - 2; for (i = 0; i < hdr.iovec_count; i++) - hdr.dxfer_len += req->elem.out_sg[i + 2].iov_len; + hdr.dxfer_len += elem->out_sg[i + 2].iov_len; - hdr.dxferp = req->elem.out_sg + 2; + hdr.dxferp = elem->out_sg + 2; - } else if (req->elem.in_num > 3) { + } else if (elem->in_num > 3) { /* * If we have more than 3 input segments the guest wants to actually * read data. */ hdr.dxfer_direction = SG_DXFER_FROM_DEV; - hdr.iovec_count = req->elem.in_num - 3; + hdr.iovec_count = elem->in_num - 3; for (i = 0; i < hdr.iovec_count; i++) - hdr.dxfer_len += req->elem.in_sg[i].iov_len; + hdr.dxfer_len += elem->in_sg[i].iov_len; - hdr.dxferp = req->elem.in_sg; + hdr.dxferp = elem->in_sg; } else { /* * Some SCSI commands don't actually transfer any data. @@ -204,11 +204,11 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req) hdr.dxfer_direction = SG_DXFER_NONE; } - hdr.sbp = req->elem.in_sg[req->elem.in_num - 3].iov_base; - hdr.mx_sb_len = req->elem.in_sg[req->elem.in_num - 3].iov_len; + hdr.sbp = elem->in_sg[elem->in_num - 3].iov_base; + hdr.mx_sb_len = elem->in_sg[elem->in_num - 3].iov_len; - ret = bdrv_ioctl(req->dev->bs, SG_IO, &hdr); - if (ret) { + status = bdrv_ioctl(bs, SG_IO, &hdr); + if (status) { status = VIRTIO_BLK_S_UNSUPP; goto fail; } @@ -224,23 +224,32 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req) hdr.status = CHECK_CONDITION; } - stl_p(&req->scsi->errors, + stl_p(&scsi->errors, hdr.status | (hdr.msg_status << 8) | (hdr.host_status << 16) | (hdr.driver_status << 24)); - stl_p(&req->scsi->residual, hdr.resid); - stl_p(&req->scsi->sense_len, hdr.sb_len_wr); - stl_p(&req->scsi->data_len, hdr.dxfer_len); + stl_p(&scsi->residual, hdr.resid); + stl_p(&scsi->sense_len, hdr.sb_len_wr); + stl_p(&scsi->data_len, hdr.dxfer_len); - virtio_blk_req_complete(req, status); - g_free(req); - return; + return status; #else abort(); #endif fail: /* Just put anything nonzero so that the ioctl fails in the guest. */ - stl_p(&req->scsi->errors, 255); + if (scsi) { + stl_p(&scsi->errors, 255); + } + return status; +} + +static void virtio_blk_handle_scsi(VirtIOBlockReq *req) +{ + int status; + + status = virtio_blk_handle_scsi_req(req->dev->bs, &req->dev->blk, + &req->elem); virtio_blk_req_complete(req, status); g_free(req); } diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index e4c41ff..1b00b48 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -155,4 +155,8 @@ typedef struct VirtIOBlock { void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk); +int virtio_blk_handle_scsi_req(BlockDriverState *bs, + VirtIOBlkConf *conf, + VirtQueueElement *elem); + #endif