From patchwork Wed Jul 31 12:32:04 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 13748658 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 2219BC3DA7F for ; Wed, 31 Jul 2024 12:33:54 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sZ8VV-0002bI-Q4; Wed, 31 Jul 2024 08:32:41 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sZ8VS-0002N5-EM for qemu-devel@nongnu.org; Wed, 31 Jul 2024 08:32:38 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sZ8VQ-00047N-G3 for qemu-devel@nongnu.org; Wed, 31 Jul 2024 08:32:37 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1722429155; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=BdOLfUo1FinCPJEBrdvb3o1LkFOGtSrCp1ZUO0io0O8=; b=XdKz5FidNXrGwo0ijhY1ki3z9g9JPorQA3DMo43r3Pid2QzwuEDvJNSeQyoLfSpOzisaUT kWoIKWrMPvywuS9U0F5h/pkpwbdClrx8UKYLMBYjKN4abdemsJWEtRM4DSLmtsJD73PzZU Dokn7eJtPP/2LHl3h5CYnRd33PH870w= Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-322-F1tustNeMTu1fz2FrPlU9g-1; Wed, 31 Jul 2024 08:32:29 -0400 X-MC-Unique: F1tustNeMTu1fz2FrPlU9g-1 Received: from mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.12]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id C76D11955F06; Wed, 31 Jul 2024 12:32:27 +0000 (UTC) Received: from merkur.redhat.com (unknown [10.39.194.1]) by mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id A0B951955F2E; Wed, 31 Jul 2024 12:32:15 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Cc: kwolf@redhat.com, pbonzini@redhat.com, fam@euphon.net, stefanha@redhat.com, qemu-devel@nongnu.org Subject: [PATCH v2 1/4] scsi-disk: Use positive return value for status in dma_readv/writev Date: Wed, 31 Jul 2024 14:32:04 +0200 Message-ID: <20240731123207.27636-2-kwolf@redhat.com> In-Reply-To: <20240731123207.27636-1-kwolf@redhat.com> References: <20240731123207.27636-1-kwolf@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.12 Received-SPF: pass client-ip=170.10.133.124; envelope-from=kwolf@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -21 X-Spam_score: -2.2 X-Spam_bar: -- X-Spam_report: (-2.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.126, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org In some error cases, scsi_block_sgio_complete() never calls the passed callback, but directly completes the request. This leads to bugs because its error paths are not exact copies of what the callback would normally do. In preparation to fix this, allow passing positive return values to the callbacks that represent the status code that should be used to complete the request. scsi_handle_rw_error() already handles positive values for its ret parameter because scsi_block_sgio_complete() calls directly into it. Signed-off-by: Kevin Wolf --- hw/scsi/scsi-disk.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index a67092db6a..3ff6798bde 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -65,6 +65,10 @@ OBJECT_DECLARE_TYPE(SCSIDiskState, SCSIDiskClass, SCSI_DISK_BASE) struct SCSIDiskClass { SCSIDeviceClass parent_class; + /* + * Callbacks receive ret == 0 for success. Errors are represented either as + * negative errno values, or as positive SAM status codes. + */ DMAIOFunc *dma_readv; DMAIOFunc *dma_writev; bool (*need_fua_emulation)(SCSICommand *cmd); @@ -283,7 +287,7 @@ static bool scsi_disk_req_check_error(SCSIDiskReq *r, int ret, bool acct_failed) return true; } - if (ret < 0) { + if (ret != 0) { return scsi_handle_rw_error(r, ret, acct_failed); } @@ -360,7 +364,7 @@ static void scsi_write_do_fua(SCSIDiskReq *r) static void scsi_dma_complete_noio(SCSIDiskReq *r, int ret) { assert(r->req.aiocb == NULL); - if (scsi_disk_req_check_error(r, ret, false)) { + if (scsi_disk_req_check_error(r, ret, ret > 0)) { goto done; } @@ -385,9 +389,10 @@ static void scsi_dma_complete(void *opaque, int ret) assert(r->req.aiocb != NULL); r->req.aiocb = NULL; + /* ret > 0 is accounted for in scsi_disk_req_check_error() */ if (ret < 0) { block_acct_failed(blk_get_stats(s->qdev.conf.blk), &r->acct); - } else { + } else if (ret == 0) { block_acct_done(blk_get_stats(s->qdev.conf.blk), &r->acct); } scsi_dma_complete_noio(r, ret); @@ -403,7 +408,7 @@ static void scsi_read_complete_noio(SCSIDiskReq *r, int ret) qemu_get_current_aio_context()); assert(r->req.aiocb == NULL); - if (scsi_disk_req_check_error(r, ret, false)) { + if (scsi_disk_req_check_error(r, ret, ret > 0)) { goto done; } @@ -424,9 +429,10 @@ static void scsi_read_complete(void *opaque, int ret) assert(r->req.aiocb != NULL); r->req.aiocb = NULL; + /* ret > 0 is accounted for in scsi_disk_req_check_error() */ if (ret < 0) { block_acct_failed(blk_get_stats(s->qdev.conf.blk), &r->acct); - } else { + } else if (ret == 0) { block_acct_done(blk_get_stats(s->qdev.conf.blk), &r->acct); trace_scsi_disk_read_complete(r->req.tag, r->qiov.size); } @@ -534,7 +540,7 @@ static void scsi_write_complete_noio(SCSIDiskReq *r, int ret) qemu_get_current_aio_context()); assert (r->req.aiocb == NULL); - if (scsi_disk_req_check_error(r, ret, false)) { + if (scsi_disk_req_check_error(r, ret, ret > 0)) { goto done; } @@ -562,9 +568,10 @@ static void scsi_write_complete(void * opaque, int ret) assert (r->req.aiocb != NULL); r->req.aiocb = NULL; + /* ret > 0 is accounted for in scsi_disk_req_check_error() */ if (ret < 0) { block_acct_failed(blk_get_stats(s->qdev.conf.blk), &r->acct); - } else { + } else if (ret == 0) { block_acct_done(blk_get_stats(s->qdev.conf.blk), &r->acct); } scsi_write_complete_noio(r, ret); From patchwork Wed Jul 31 12:32:05 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 13748660 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 485BFC3DA7F for ; Wed, 31 Jul 2024 12:34:47 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sZ8VV-0002aj-LV; Wed, 31 Jul 2024 08:32:41 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sZ8VS-0002N1-EH for qemu-devel@nongnu.org; Wed, 31 Jul 2024 08:32:38 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sZ8VQ-00047b-JI for qemu-devel@nongnu.org; Wed, 31 Jul 2024 08:32:37 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1722429155; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=48tv8E771azAG/7TbiDrxaNgby/yNJ6CX75QSGZBrqU=; b=LkDiTdGREt18mt9c654fjjvWr4xXB4UO3zW2J7GVeko7eL2GCIk8FxrhYC9c5JRPIy4W6N DIUZxAFpSbc2o3KfToBcPRcVwSEAo2hPunBSBaGGPHlovppHCrgTXYsfyNQnkgDBhbSQGk UrvjsQEQIb3Yp4AAuFCSl5DIHFiOJ50= Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-379-R_9h4kqYPRmLUp_jehdavg-1; Wed, 31 Jul 2024 08:32:31 -0400 X-MC-Unique: R_9h4kqYPRmLUp_jehdavg-1 Received: from mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.12]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 723C119541AE; Wed, 31 Jul 2024 12:32:30 +0000 (UTC) Received: from merkur.redhat.com (unknown [10.39.194.1]) by mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 49D1D19560B2; Wed, 31 Jul 2024 12:32:22 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Cc: kwolf@redhat.com, pbonzini@redhat.com, fam@euphon.net, stefanha@redhat.com, qemu-devel@nongnu.org Subject: [PATCH v2 2/4] scsi-block: Don't skip callback for sgio error status/driver_status Date: Wed, 31 Jul 2024 14:32:05 +0200 Message-ID: <20240731123207.27636-3-kwolf@redhat.com> In-Reply-To: <20240731123207.27636-1-kwolf@redhat.com> References: <20240731123207.27636-1-kwolf@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.12 Received-SPF: pass client-ip=170.10.129.124; envelope-from=kwolf@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -21 X-Spam_score: -2.2 X-Spam_bar: -- X-Spam_report: (-2.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.126, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Instead of calling into scsi_handle_rw_error() directly from scsi_block_sgio_complete() and skipping the normal callback, go through the normal cleanup path by calling the callback with a positive error value. The important difference here is not only that the code path is cleaner, but that the callbacks set r->req.aiocb = NULL. If we skip setting this and the error action is BLOCK_ERROR_ACTION_STOP, resuming the VM runs into an assertion failure in scsi_read_data() or scsi_write_data() because the dangling aiocb pointer is unexpected. Fixes: a108557bbf ("scsi: inline sg_io_sense_from_errno() into the callers.") Buglink: https://issues.redhat.com/browse/RHEL-50000 Signed-off-by: Kevin Wolf --- hw/scsi/scsi-disk.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 3ff6798bde..6e1a5c98df 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2832,16 +2832,6 @@ static void scsi_block_sgio_complete(void *opaque, int ret) } else { ret = io_hdr->status; } - - if (ret > 0) { - if (scsi_handle_rw_error(r, ret, true)) { - scsi_req_unref(&r->req); - return; - } - - /* Ignore error. */ - ret = 0; - } } req->cb(req->cb_opaque, ret); From patchwork Wed Jul 31 12:32:06 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 13748657 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 826CCC49EA1 for ; Wed, 31 Jul 2024 12:32:59 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sZ8VU-0002Tw-1Q; Wed, 31 Jul 2024 08:32:40 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sZ8VQ-0002In-WB for qemu-devel@nongnu.org; Wed, 31 Jul 2024 08:32:37 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sZ8VO-000477-Hw for qemu-devel@nongnu.org; Wed, 31 Jul 2024 08:32:36 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1722429154; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=7fWS/MeBs6Kai+qgd4zCs2xGsxcdZvo3lCYuqY77YaM=; b=SSEMbtTANmO+zieKSnRiNfJ/CJS2D0C49GB2D0B1Cwo9aTH816tYaf7A5mZgAWMN2s/rnq NeKX2MhYo3mnS2+BJw+kQSj7ogw7G9OqamLFs6s1yhuh1iPPPvdABVbudpFufNZjJiFoUW 55O5jDI/Nx6d0j6F3vGt/MGsZ/CLjxo= Received: from mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-322-OS34m-dmMaWE102R_yoDtg-1; Wed, 31 Jul 2024 08:32:29 -0400 X-MC-Unique: OS34m-dmMaWE102R_yoDtg-1 Received: from mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.12]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 935D818B649A; Wed, 31 Jul 2024 12:32:28 +0000 (UTC) Received: from merkur.redhat.com (unknown [10.39.194.1]) by mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 107F51955F65; Wed, 31 Jul 2024 12:32:25 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Cc: kwolf@redhat.com, pbonzini@redhat.com, fam@euphon.net, stefanha@redhat.com, qemu-devel@nongnu.org Subject: [PATCH v2 3/4] scsi-disk: Add warning comments that host_status errors take a shortcut Date: Wed, 31 Jul 2024 14:32:06 +0200 Message-ID: <20240731123207.27636-4-kwolf@redhat.com> In-Reply-To: <20240731123207.27636-1-kwolf@redhat.com> References: <20240731123207.27636-1-kwolf@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.12 Received-SPF: pass client-ip=170.10.133.124; envelope-from=kwolf@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -21 X-Spam_score: -2.2 X-Spam_bar: -- X-Spam_report: (-2.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.126, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org scsi_block_sgio_complete() has surprising behaviour in that there are error cases in which it directly completes the request and never calls the passed callback. In the current state of the code, this doesn't seem to result in bugs, but with future code changes, we must be careful to never rely on the callback doing some cleanup until this code smell is fixed. For now, just add warnings to make people aware of the trap. Signed-off-by: Kevin Wolf --- hw/scsi/scsi-disk.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 6e1a5c98df..69a195177e 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -68,6 +68,9 @@ struct SCSIDiskClass { /* * Callbacks receive ret == 0 for success. Errors are represented either as * negative errno values, or as positive SAM status codes. + * + * Beware: For errors returned in host_status, the function may directly + * complete the request and never call the callback. */ DMAIOFunc *dma_readv; DMAIOFunc *dma_writev; @@ -381,6 +384,7 @@ done: scsi_req_unref(&r->req); } +/* May not be called in all error cases, don't rely on cleanup here */ static void scsi_dma_complete(void *opaque, int ret) { SCSIDiskReq *r = (SCSIDiskReq *)opaque; @@ -421,6 +425,7 @@ done: scsi_req_unref(&r->req); } +/* May not be called in all error cases, don't rely on cleanup here */ static void scsi_read_complete(void *opaque, int ret) { SCSIDiskReq *r = (SCSIDiskReq *)opaque; @@ -560,6 +565,7 @@ done: scsi_req_unref(&r->req); } +/* May not be called in all error cases, don't rely on cleanup here */ static void scsi_write_complete(void * opaque, int ret) { SCSIDiskReq *r = (SCSIDiskReq *)opaque; @@ -2821,6 +2827,7 @@ static void scsi_block_sgio_complete(void *opaque, int ret) sg_io_hdr_t *io_hdr = &req->io_header; if (ret == 0) { + /* FIXME This skips calling req->cb() and any cleanup in it */ if (io_hdr->host_status != SCSI_HOST_OK) { scsi_req_complete_failed(&r->req, io_hdr->host_status); scsi_req_unref(&r->req); From patchwork Wed Jul 31 12:32:07 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 13748661 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1EA8FC3DA7F for ; Wed, 31 Jul 2024 12:34:52 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sZ8VV-0002Yd-Hz; Wed, 31 Jul 2024 08:32:41 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sZ8VS-0002MG-0t for qemu-devel@nongnu.org; Wed, 31 Jul 2024 08:32:38 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sZ8VQ-00047U-5h for qemu-devel@nongnu.org; Wed, 31 Jul 2024 08:32:37 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1722429155; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=FOIXpdiXP48X03eoys7HTXHfkbmBuHCmyJnnxafRgkA=; b=AUNRzEkOdg6Xd1XnZbtqybWMD84Q6aVwjbVloAVHxNyCoBpWuG4ug1UjuvxY4yUFpMUuKv i3OdA+w+akXsUOif2tEuB3fvrFeh465Kz1XxqhUaGB/4pu5IEZynoOnpvLQugsPi9RXn6H s8sgM7tuMFlH2z8eHvE6GTJ1xAhLIyw= Received: from mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-518-Gk62xqpRNUynKS1g6Or1lw-1; Wed, 31 Jul 2024 08:32:32 -0400 X-MC-Unique: Gk62xqpRNUynKS1g6Or1lw-1 Received: from mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.12]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 7426C189A364; Wed, 31 Jul 2024 12:32:31 +0000 (UTC) Received: from merkur.redhat.com (unknown [10.39.194.1]) by mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 08DA61955E92; Wed, 31 Jul 2024 12:32:28 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Cc: kwolf@redhat.com, pbonzini@redhat.com, fam@euphon.net, stefanha@redhat.com, qemu-devel@nongnu.org Subject: [PATCH v2 4/4] scsi-disk: Always report RESERVATION_CONFLICT to guest Date: Wed, 31 Jul 2024 14:32:07 +0200 Message-ID: <20240731123207.27636-5-kwolf@redhat.com> In-Reply-To: <20240731123207.27636-1-kwolf@redhat.com> References: <20240731123207.27636-1-kwolf@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.12 Received-SPF: pass client-ip=170.10.133.124; envelope-from=kwolf@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -21 X-Spam_score: -2.2 X-Spam_bar: -- X-Spam_report: (-2.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.126, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org In the case of scsi-block, RESERVATION_CONFLICT is not a backend error, but indicates that the guest tried to make a request that it isn't allowed to execute. Pass the error to the guest so that it can decide what to do with it. Without this, if we stop the VM in response to a RESERVATION_CONFLICT (as is the default policy in management software such as oVirt or KubeVirt), it can happen that the VM cannot be resumed any more because every attempt to resume it immediately runs into the same error and stops the VM again. One case that expects RESERVATION_CONFLICT errors to be visible in the guest is running the validation tests in Windows 2019's Failover Cluster Manager, which intentionally tries to execute invalid requests to see if they are properly rejected. Buglink: https://issues.redhat.com/browse/RHEL-50000 Signed-off-by: Kevin Wolf --- hw/scsi/scsi-disk.c | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 69a195177e..4d94b2b816 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -224,7 +224,7 @@ static bool scsi_handle_rw_error(SCSIDiskReq *r, int ret, bool acct_failed) SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev); SCSIDiskClass *sdc = (SCSIDiskClass *) object_get_class(OBJECT(s)); SCSISense sense = SENSE_CODE(NO_SENSE); - int error = 0; + int error; bool req_has_sense = false; BlockErrorAction action; int status; @@ -235,11 +235,35 @@ static bool scsi_handle_rw_error(SCSIDiskReq *r, int ret, bool acct_failed) } else { /* A passthrough command has completed with nonzero status. */ status = ret; - if (status == CHECK_CONDITION) { + switch (status) { + case CHECK_CONDITION: req_has_sense = true; error = scsi_sense_buf_to_errno(r->req.sense, sizeof(r->req.sense)); - } else { + break; + case RESERVATION_CONFLICT: + /* + * Don't apply the error policy, always report to the guest. + * + * This is a passthrough code path, so it's not a backend error, but + * a response to an invalid guest request. + * + * Windows Failover Cluster validation intentionally sends invalid + * requests to verify that reservations work as intended. It is + * crucial that it sees the resulting errors. + * + * Treating a reservation conflict as a guest-side error is obvious + * when a pr-manager is in use. Without one, the situation is less + * clear, but there might be nothing that can be fixed on the host + * (like in the above example), and we don't want to be stuck in a + * loop where resuming the VM and retrying the request immediately + * stops it again. So always reporting is still the safer option in + * this case, too. + */ + error = 0; + break; + default: error = EINVAL; + break; } } @@ -249,8 +273,9 @@ static bool scsi_handle_rw_error(SCSIDiskReq *r, int ret, bool acct_failed) * are usually retried immediately, so do not post them to QMP and * do not account them as failed I/O. */ - if (req_has_sense && - scsi_sense_buf_is_guest_recoverable(r->req.sense, sizeof(r->req.sense))) { + if (!error || (req_has_sense && + scsi_sense_buf_is_guest_recoverable(r->req.sense, + sizeof(r->req.sense)))) { action = BLOCK_ERROR_ACTION_REPORT; acct_failed = false; } else {