[PATCH] virtio-blk: remove SCSI passthrough functionality

[Paolo Bonzini]

The legacy SCSI passthrough functionality has never been enabled for
VIRTIO 1.0 and was deprecated more than four years ago.

Get rid of it---almost, because QEMU is advertising it unconditionally
for legacy virtio-blk devices.  Just parse the header and return a
nonzero status.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/about/deprecated.rst       |  10 --
 docs/about/removed-features.rst |   8 ++
 hw/block/virtio-blk.c           | 166 +++-----------------------------
 hw/core/machine.c               |   2 -
 4 files changed, 19 insertions(+), 167 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 03f8b1b655e..9bfaeda3adb 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -282,16 +282,6 @@ Device options
 Emulated device options
 '''''''''''''''''''''''
 
-``-device virtio-blk,scsi=on|off`` (since 5.0)
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-The virtio-blk SCSI passthrough feature is a legacy VIRTIO feature.  VIRTIO 1.0
-and later do not support it because the virtio-scsi device was introduced for
-full SCSI support.  Use virtio-scsi instead when SCSI passthrough is required.
-
-Note this also applies to ``-device virtio-blk-pci,scsi=on|off``, which is an
-alias.
-
 ``-device nvme-ns,eui64-default=on|off`` (since 7.1)
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 53ca08aba9c..1044d657c1a 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -505,6 +505,14 @@ configurations (e.g. -smp 8,sockets=0) is removed since 
9.0, users have
 to ensure that all the topology members described with -smp are greater
 than zero.
 
+``-device virtio-blk,scsi=on|off`` (since 9.1)
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The virtio-blk SCSI passthrough feature is a legacy VIRTIO feature.  VIRTIO 1.0
+and later do not support it because the virtio-scsi device was introduced for
+full SCSI support.  Use virtio-scsi instead when SCSI passthrough is required.
+
+
 User-mode emulator command line arguments
 -----------------------------------------
 
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index bb86e65f652..73bdfd6122a 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -172,57 +172,6 @@ static void virtio_blk_discard_write_zeroes_complete(void 
*opaque, int ret)
     virtio_blk_free_request(req);
 }
 
-#ifdef __linux__
-
-typedef struct {
-    VirtIOBlockReq *req;
-    struct sg_io_hdr hdr;
-} VirtIOBlockIoctlReq;
-
-static void virtio_blk_ioctl_complete(void *opaque, int status)
-{
-    VirtIOBlockIoctlReq *ioctl_req = opaque;
-    VirtIOBlockReq *req = ioctl_req->req;
-    VirtIOBlock *s = req->dev;
-    VirtIODevice *vdev = VIRTIO_DEVICE(s);
-    struct virtio_scsi_inhdr *scsi;
-    struct sg_io_hdr *hdr;
-
-    scsi = (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base;
-
-    if (status) {
-        status = VIRTIO_BLK_S_UNSUPP;
-        virtio_stl_p(vdev, &scsi->errors, 255);
-        goto out;
-    }
-
-    hdr = &ioctl_req->hdr;
-    /*
-     * From SCSI-Generic-HOWTO: "Some lower level drivers (e.g. ide-scsi)
-     * clear the masked_status field [hence status gets cleared too, see
-     * block/scsi_ioctl.c] even when a CHECK_CONDITION or COMMAND_TERMINATED
-     * status has occurred.  However they do set DRIVER_SENSE in driver_status
-     * field. Also a (sb_len_wr > 0) indicates there is a sense buffer.
-     */
-    if (hdr->status == 0 && hdr->sb_len_wr > 0) {
-        hdr->status = CHECK_CONDITION;
-    }
-
-    virtio_stl_p(vdev, &scsi->errors,
-                 hdr->status | (hdr->msg_status << 8) |
-                 (hdr->host_status << 16) | (hdr->driver_status << 24));
-    virtio_stl_p(vdev, &scsi->residual, hdr->resid);
-    virtio_stl_p(vdev, &scsi->sense_len, hdr->sb_len_wr);
-    virtio_stl_p(vdev, &scsi->data_len, hdr->dxfer_len);
-
-out:
-    virtio_blk_req_complete(req, status);
-    virtio_blk_free_request(req);
-    g_free(ioctl_req);
-}
-
-#endif
-
 static VirtIOBlockReq *virtio_blk_get_request(VirtIOBlock *s, VirtQueue *vq)
 {
     VirtIOBlockReq *req = virtqueue_pop(vq, sizeof(VirtIOBlockReq));
@@ -233,20 +182,14 @@ static VirtIOBlockReq *virtio_blk_get_request(VirtIOBlock 
*s, VirtQueue *vq)
     return req;
 }
 
-static int virtio_blk_handle_scsi_req(VirtIOBlockReq *req)
+static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
 {
-    int status = VIRTIO_BLK_S_OK;
-    struct virtio_scsi_inhdr *scsi = NULL;
+    int status;
+    struct virtio_scsi_inhdr *scsi;
     VirtIOBlock *blk = req->dev;
     VirtIODevice *vdev = VIRTIO_DEVICE(blk);
     VirtQueueElement *elem = &req->elem;
 
-#ifdef __linux__
-    int i;
-    VirtIOBlockIoctlReq *ioctl_req;
-    BlockAIOCB *acb;
-#endif
-
     /*
      * We require at least one output segment each for the virtio_blk_outhdr
      * and the SCSI command block.
@@ -262,95 +205,16 @@ static int virtio_blk_handle_scsi_req(VirtIOBlockReq *req)
     /*
      * The scsi inhdr is placed in the second-to-last input segment, just
      * before the regular inhdr.
+     *
+     * Just put anything nonzero so that the ioctl fails in the guest.
      */
     scsi = (void *)elem->in_sg[elem->in_num - 2].iov_base;
-
-    if (!virtio_has_feature(blk->host_features, VIRTIO_BLK_F_SCSI)) {
-        status = VIRTIO_BLK_S_UNSUPP;
-        goto fail;
-    }
-
-    /*
-     * No support for bidirection commands yet.
-     */
-    if (elem->out_num > 2 && elem->in_num > 3) {
-        status = VIRTIO_BLK_S_UNSUPP;
-        goto fail;
-    }
-
-#ifdef __linux__
-    ioctl_req = g_new0(VirtIOBlockIoctlReq, 1);
-    ioctl_req->req = req;
-    ioctl_req->hdr.interface_id = 'S';
-    ioctl_req->hdr.cmd_len = elem->out_sg[1].iov_len;
-    ioctl_req->hdr.cmdp = elem->out_sg[1].iov_base;
-    ioctl_req->hdr.dxfer_len = 0;
-
-    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.
-         */
-        ioctl_req->hdr.dxfer_direction = SG_DXFER_TO_DEV;
-        ioctl_req->hdr.iovec_count = elem->out_num - 2;
-
-        for (i = 0; i < ioctl_req->hdr.iovec_count; i++) {
-            ioctl_req->hdr.dxfer_len += elem->out_sg[i + 2].iov_len;
-        }
-
-        ioctl_req->hdr.dxferp = elem->out_sg + 2;
-
-    } else if (elem->in_num > 3) {
-        /*
-         * If we have more than 3 input segments the guest wants to actually
-         * read data.
-         */
-        ioctl_req->hdr.dxfer_direction = SG_DXFER_FROM_DEV;
-        ioctl_req->hdr.iovec_count = elem->in_num - 3;
-        for (i = 0; i < ioctl_req->hdr.iovec_count; i++) {
-            ioctl_req->hdr.dxfer_len += elem->in_sg[i].iov_len;
-        }
-
-        ioctl_req->hdr.dxferp = elem->in_sg;
-    } else {
-        /*
-         * Some SCSI commands don't actually transfer any data.
-         */
-        ioctl_req->hdr.dxfer_direction = SG_DXFER_NONE;
-    }
-
-    ioctl_req->hdr.sbp = elem->in_sg[elem->in_num - 3].iov_base;
-    ioctl_req->hdr.mx_sb_len = elem->in_sg[elem->in_num - 3].iov_len;
-
-    acb = blk_aio_ioctl(blk->blk, SG_IO, &ioctl_req->hdr,
-                        virtio_blk_ioctl_complete, ioctl_req);
-    if (!acb) {
-        g_free(ioctl_req);
-        status = VIRTIO_BLK_S_UNSUPP;
-        goto fail;
-    }
-    return -EINPROGRESS;
-#else
-    abort();
-#endif
+    virtio_stl_p(vdev, &scsi->errors, 255);
+    status = VIRTIO_BLK_S_UNSUPP;
 
 fail:
-    /* Just put anything nonzero so that the ioctl fails in the guest.  */
-    if (scsi) {
-        virtio_stl_p(vdev, &scsi->errors, 255);
-    }
-    return status;
-}
-
-static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
-{
-    int status;
-
-    status = virtio_blk_handle_scsi_req(req);
-    if (status != -EINPROGRESS) {
-        virtio_blk_req_complete(req, status);
-        virtio_blk_free_request(req);
-    }
+    virtio_blk_req_complete(req, status);
+    virtio_blk_free_request(req);
 }
 
 static inline void submit_requests(VirtIOBlock *s, MultiReqBuffer *mrb,
@@ -1379,13 +1243,9 @@ static uint64_t virtio_blk_get_features(VirtIODevice 
*vdev, uint64_t features,
     virtio_add_feature(&features, VIRTIO_BLK_F_GEOMETRY);
     virtio_add_feature(&features, VIRTIO_BLK_F_TOPOLOGY);
     virtio_add_feature(&features, VIRTIO_BLK_F_BLK_SIZE);
-    if (virtio_has_feature(features, VIRTIO_F_VERSION_1)) {
-        if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_SCSI)) {
-            error_setg(errp, "Please set scsi=off for virtio-blk devices in 
order to use virtio 1.0");
-            return 0;
-        }
-    } else {
+    if (!virtio_has_feature(features, VIRTIO_F_VERSION_1)) {
         virtio_clear_feature(&features, VIRTIO_F_ANY_LAYOUT);
+        /* Added for historical reasons, removing it could break migration.  */
         virtio_add_feature(&features, VIRTIO_BLK_F_SCSI);
     }
 
@@ -2132,10 +1992,6 @@ static Property virtio_blk_properties[] = {
     DEFINE_PROP_STRING("serial", VirtIOBlock, conf.serial),
     DEFINE_PROP_BIT64("config-wce", VirtIOBlock, host_features,
                       VIRTIO_BLK_F_CONFIG_WCE, true),
-#ifdef __linux__
-    DEFINE_PROP_BIT64("scsi", VirtIOBlock, host_features,
-                      VIRTIO_BLK_F_SCSI, false),
-#endif
     DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
                     true),
     DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues,
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 4ff60911e74..8cbbd529845 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -264,8 +264,6 @@ GlobalProperty hw_compat_2_5[] = {
 const size_t hw_compat_2_5_len = G_N_ELEMENTS(hw_compat_2_5);
 
 GlobalProperty hw_compat_2_4[] = {
-    /* Optional because the 'scsi' property is Linux-only */
-    { "virtio-blk-device", "scsi", "true", .optional = true },
     { "e1000", "extra_mac_registers", "off" },
     { "virtio-pci", "x-disable-pcie", "on" },
     { "virtio-pci", "migrate-extra", "off" },
-- 
2.45.0