From nobody Thu Apr 25 05:46:00 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) client-ip=170.10.133.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1619711569; cv=none; d=zohomail.com; s=zohoarc; b=iFTVjP8pg4pKgKTRvSzhn9cZQYRLzxBWBL8AtMhDdWYlA0T8OFN1/cnyXZY2CguL1MxTo7ooD+EWOX1Mhja8p4nxKXzdz/w9eiitSkWLuTycGvuJMI6FXs/UbG5K2HvMwKBiwdGQz7ouatrtwsAFDpybxVPc4SqyqxRiTDKiWOU= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1619711569; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=TjKo52d26+m5cn+9j0kGgR49T528RBjdsQkse/Q953U=; b=ef1N+NP5/Z99C2i/7q3FOKUu47BYvXQaxxHRs/OfAuNPVgQ5Gm/ZcLayy+Rq38+R21h0pKMIT2G0TCijG6bOGCJyz9ANhJpncfWi6ZJTsuDb5bOImwtTOlcWIm0Uecl0E25azDrBVwejNQGcLq1ZN0bkb6oDcufA6Y0r9yrP5p8= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.zohomail.com with SMTPS id 161971156953492.73264237867318; Thu, 29 Apr 2021 08:52:49 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-148-3617U21TMDC_6W_BokF3vw-1; Thu, 29 Apr 2021 11:52:46 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E86FB801FCE; Thu, 29 Apr 2021 15:52:40 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id D2919772E1; Thu, 29 Apr 2021 15:52:39 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 220C918095C3; Thu, 29 Apr 2021 15:52:34 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 13TFqX8V010397 for ; Thu, 29 Apr 2021 11:52:33 -0400 Received: by smtp.corp.redhat.com (Postfix) id AC07B6EF4F; Thu, 29 Apr 2021 15:52:33 +0000 (UTC) Received: from localhost (ovpn-115-28.ams2.redhat.com [10.36.115.28]) by smtp.corp.redhat.com (Postfix) with ESMTP id DB83460C5B; Thu, 29 Apr 2021 15:52:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1619711568; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=TjKo52d26+m5cn+9j0kGgR49T528RBjdsQkse/Q953U=; b=V3uKOfNvOL/EKR2tdSNPxplO3JYwJrDa+h0zHHGUBCe4xxbuhCae5tKC+fD6hGC6YQsvDv z2Q7Zw5IaVMIc4Xproi7UBJtLLyh2PtTOMOAOjJI6QNniPTFXMLtIEuMSo4IGPAeGPBWCx LfvySsvSau+NEmP94PjrwEdA3BPLBEQ= X-MC-Unique: 3617U21TMDC_6W_BokF3vw-1 From: Stefan Hajnoczi To: qemu-devel@nongnu.org Subject: [PATCH] virtio-blk: drop deprecated scsi=on|off property Date: Thu, 29 Apr 2021 16:52:21 +0100 Message-Id: <20210429155221.1226561-1-stefanha@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-loop: libvir-list@redhat.com Cc: Kevin Wolf , Peter Krempa , Eduardo Habkost , qemu-block@nongnu.org, "Michael S. Tsirkin" , libvir-list@redhat.com, Max Reitz , "Dr . David Alan Gilbert" , Stefan Hajnoczi , Christoph Hellwig , Marcel Apfelbaum X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" The scsi=3Don|off property was deprecated in QEMU 5.0 and can be removed completely at this point. Drop the scsi=3Don|off option. It was only available on Legacy virtio-blk devices. Linux v5.6 already dropped support for it. Remove the hw_compat_2_4[] property assignment since scsi=3Don|off no longer exists. Old guests with Legacy virtio-blk devices no longer see the SCSI host features bit. Live migrating old guests from an old QEMU with the SCSI feature bit enabled will fail with "Features 0x... unsupported. Allowed features: 0x...". We've followed the QEMU deprecation policy so users have been warned... I have tested that libvirt still works when the property is absent. It no longer adds scsi=3Don|off to the command-line. Cc: Markus Armbruster Cc: Christoph Hellwig Cc: Peter Krempa Cc: Dr. David Alan Gilbert Signed-off-by: Stefan Hajnoczi Reviewed-by: Michal Privoznik --- docs/specs/tpm.rst | 2 +- docs/system/deprecated.rst | 13 --- docs/pci_expander_bridge.txt | 2 +- hw/block/virtio-blk.c | 192 +---------------------------------- hw/core/machine.c | 2 - 5 files changed, 3 insertions(+), 208 deletions(-) diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst index 3be190343a..0ec017ab95 100644 --- a/docs/specs/tpm.rst +++ b/docs/specs/tpm.rst @@ -328,7 +328,7 @@ In case a pSeries machine is emulated, use the followin= g command line: -tpmdev emulator,id=3Dtpm0,chardev=3Dchrtpm \ -device tpm-spapr,tpmdev=3Dtpm0 \ -device spapr-vscsi,id=3Dscsi0,reg=3D0x00002000 \ - -device virtio-blk-pci,scsi=3Doff,bus=3Dpci.0,addr=3D0x3,drive=3Ddrive= -virtio-disk0,id=3Dvirtio-disk0 \ + -device virtio-blk-pci,bus=3Dpci.0,addr=3D0x3,drive=3Ddrive-virtio-dis= k0,id=3Dvirtio-disk0 \ -drive file=3Dtest.img,format=3Draw,if=3Dnone,id=3Ddrive-virtio-disk0 =20 In case an Arm virt machine is emulated, use the following command line: diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 80cae86252..1abb64b669 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -248,19 +248,6 @@ machines have been renamed ``raspi2b`` and ``raspi3b``. Device options -------------- =20 -Emulated device options -''''''''''''''''''''''' - -``-device virtio-blk,scsi=3Don|off`` (since 5.0.0) -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -The virtio-blk SCSI passthrough feature is a legacy VIRTIO feature. VIRTI= O 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 requi= red. - -Note this also applies to ``-device virtio-blk-pci,scsi=3Don|off``, which = is an -alias. - Block device options '''''''''''''''''''' =20 diff --git a/docs/pci_expander_bridge.txt b/docs/pci_expander_bridge.txt index 36750273bb..540191f5e0 100644 --- a/docs/pci_expander_bridge.txt +++ b/docs/pci_expander_bridge.txt @@ -25,7 +25,7 @@ A detailed command line would be: -object memory-backend-ram,size=3D1024M,policy=3Dbind,host-nodes=3D1,id=3D= ram-node1 -numa node,nodeid=3D1,cpus=3D1,memdev=3Dram-node1 -device pxb,id=3Dbridge1,bus=3Dpci.0,numa_node=3D1,bus_nr=3D4 -netdev user= ,id=3Dnd -device e1000,bus=3Dbridge1,addr=3D0x4,netdev=3Dnd -device pxb,id=3Dbridge2,bus=3Dpci.0,numa_node=3D0,bus_nr=3D8 -device e100= 0,bus=3Dbridge2,addr=3D0x3 --device pxb,id=3Dbridge3,bus=3Dpci.0,bus_nr=3D40 -drive if=3Dnone,id=3Ddri= ve0,file=3D[img] -device virtio-blk-pci,drive=3Ddrive0,scsi=3Doff,bus=3Dbri= dge3,addr=3D1 +-device pxb,id=3Dbridge3,bus=3Dpci.0,bus_nr=3D40 -drive if=3Dnone,id=3Ddri= ve0,file=3D[img] -device virtio-blk-pci,drive=3Ddrive0,bus=3Dbridge3,addr= =3D1 =20 Here you have: - 2 NUMA nodes for the guest, 0 and 1. (both mapped to the same NUMA node= in host, but you can and should put it in different host NUMA nodes) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index d28979efb8..5023e046fc 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -25,10 +25,6 @@ #include "sysemu/runstate.h" #include "hw/virtio/virtio-blk.h" #include "dataplane/virtio-blk.h" -#include "scsi/constants.h" -#ifdef __linux__ -# include -#endif #include "hw/virtio/virtio-bus.h" #include "migration/qemu-file-types.h" #include "hw/virtio/virtio-access.h" @@ -200,59 +196,6 @@ out: aio_context_release(blk_get_aio_context(s->conf.conf.blk)); } =20 -#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 =3D opaque; - VirtIOBlockReq *req =3D ioctl_req->req; - VirtIOBlock *s =3D req->dev; - VirtIODevice *vdev =3D VIRTIO_DEVICE(s); - struct virtio_scsi_inhdr *scsi; - struct sg_io_hdr *hdr; - - scsi =3D (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base; - - if (status) { - status =3D VIRTIO_BLK_S_UNSUPP; - virtio_stl_p(vdev, &scsi->errors, 255); - goto out; - } - - hdr =3D &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_TERMINAT= ED - * status has occurred. However they do set DRIVER_SENSE in driver_st= atus - * field. Also a (sb_len_wr > 0) indicates there is a sense buffer. - */ - if (hdr->status =3D=3D 0 && hdr->sb_len_wr > 0) { - hdr->status =3D 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: - aio_context_acquire(blk_get_aio_context(s->conf.conf.blk)); - virtio_blk_req_complete(req, status); - virtio_blk_free_request(req); - aio_context_release(blk_get_aio_context(s->conf.conf.blk)); - g_free(ioctl_req); -} - -#endif - static VirtIOBlockReq *virtio_blk_get_request(VirtIOBlock *s, VirtQueue *v= q) { VirtIOBlockReq *req =3D virtqueue_pop(vq, sizeof(VirtIOBlockReq)); @@ -263,126 +206,6 @@ static VirtIOBlockReq *virtio_blk_get_request(VirtIOB= lock *s, VirtQueue *vq) return req; } =20 -static int virtio_blk_handle_scsi_req(VirtIOBlockReq *req) -{ - int status =3D VIRTIO_BLK_S_OK; - struct virtio_scsi_inhdr *scsi =3D NULL; - VirtIOBlock *blk =3D req->dev; - VirtIODevice *vdev =3D VIRTIO_DEVICE(blk); - VirtQueueElement *elem =3D &req->elem; - -#ifdef __linux__ - int i; - VirtIOBlockIoctlReq *ioctl_req; - BlockAIOCB *acb; -#endif - - /* - * We require at least one output segment each for the virtio_blk_outh= dr - * and the SCSI command block. - * - * We also at least require the virtio_blk_inhdr, the virtio_scsi_inhdr - * and the sense buffer pointer in the input segments. - */ - if (elem->out_num < 2 || elem->in_num < 3) { - status =3D VIRTIO_BLK_S_IOERR; - goto fail; - } - - /* - * The scsi inhdr is placed in the second-to-last input segment, just - * before the regular inhdr. - */ - scsi =3D (void *)elem->in_sg[elem->in_num - 2].iov_base; - - if (!virtio_has_feature(blk->host_features, VIRTIO_BLK_F_SCSI)) { - status =3D VIRTIO_BLK_S_UNSUPP; - goto fail; - } - - /* - * No support for bidirection commands yet. - */ - if (elem->out_num > 2 && elem->in_num > 3) { - status =3D VIRTIO_BLK_S_UNSUPP; - goto fail; - } - -#ifdef __linux__ - ioctl_req =3D g_new0(VirtIOBlockIoctlReq, 1); - ioctl_req->req =3D req; - ioctl_req->hdr.interface_id =3D 'S'; - ioctl_req->hdr.cmd_len =3D elem->out_sg[1].iov_len; - ioctl_req->hdr.cmdp =3D elem->out_sg[1].iov_base; - ioctl_req->hdr.dxfer_len =3D 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 =3D SG_DXFER_TO_DEV; - ioctl_req->hdr.iovec_count =3D elem->out_num - 2; - - for (i =3D 0; i < ioctl_req->hdr.iovec_count; i++) { - ioctl_req->hdr.dxfer_len +=3D elem->out_sg[i + 2].iov_len; - } - - ioctl_req->hdr.dxferp =3D elem->out_sg + 2; - - } else if (elem->in_num > 3) { - /* - * If we have more than 3 input segments the guest wants to actual= ly - * read data. - */ - ioctl_req->hdr.dxfer_direction =3D SG_DXFER_FROM_DEV; - ioctl_req->hdr.iovec_count =3D elem->in_num - 3; - for (i =3D 0; i < ioctl_req->hdr.iovec_count; i++) { - ioctl_req->hdr.dxfer_len +=3D elem->in_sg[i].iov_len; - } - - ioctl_req->hdr.dxferp =3D elem->in_sg; - } else { - /* - * Some SCSI commands don't actually transfer any data. - */ - ioctl_req->hdr.dxfer_direction =3D SG_DXFER_NONE; - } - - ioctl_req->hdr.sbp =3D elem->in_sg[elem->in_num - 3].iov_base; - ioctl_req->hdr.mx_sb_len =3D elem->in_sg[elem->in_num - 3].iov_len; - - acb =3D blk_aio_ioctl(blk->blk, SG_IO, &ioctl_req->hdr, - virtio_blk_ioctl_complete, ioctl_req); - if (!acb) { - g_free(ioctl_req); - status =3D VIRTIO_BLK_S_UNSUPP; - goto fail; - } - return -EINPROGRESS; -#else - abort(); -#endif - -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 =3D virtio_blk_handle_scsi_req(req); - if (status !=3D -EINPROGRESS) { - virtio_blk_req_complete(req, status); - virtio_blk_free_request(req); - } -} - static inline void submit_requests(BlockBackend *blk, MultiReqBuffer *mrb, int start, int num_reqs, int niov) { @@ -699,9 +522,6 @@ static int virtio_blk_handle_request(VirtIOBlockReq *re= q, MultiReqBuffer *mrb) case VIRTIO_BLK_T_FLUSH: virtio_blk_handle_flush(req, mrb); break; - case VIRTIO_BLK_T_SCSI_CMD: - virtio_blk_handle_scsi(req); - break; case VIRTIO_BLK_T_GET_ID: { /* @@ -1010,14 +830,8 @@ 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=3Doff 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); - virtio_add_feature(&features, VIRTIO_BLK_F_SCSI); } =20 if (blk_enable_write_cache(s->blk) || @@ -1289,10 +1103,6 @@ static Property virtio_blk_properties[] =3D { 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 40def78183..286f18ec6d 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -194,8 +194,6 @@ GlobalProperty hw_compat_2_5[] =3D { const size_t hw_compat_2_5_len =3D G_N_ELEMENTS(hw_compat_2_5); =20 GlobalProperty hw_compat_2_4[] =3D { - /* Optional because the 'scsi' property is Linux-only */ - { "virtio-blk-device", "scsi", "true", .optional =3D true }, { "e1000", "extra_mac_registers", "off" }, { "virtio-pci", "x-disable-pcie", "on" }, { "virtio-pci", "migrate-extra", "off" }, --=20 2.30.2