From nobody Sun Nov 24 15:02:19 2024 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1722892247; cv=none; d=zohomail.com; s=zohoarc; b=mchNTeltbwWz2lrYd6ERpVeHqEezYfJ+3yDZDCjvcSU6q2z7DHX6VOveOdgAZGp5NPEu0DwXqQbpI8Rhtn71SqVxFU5SHRtHA1pP1N+9L0ru8pEqtL7wzhPavZ0K5C7HH/bZKHLjqAH1YygWNUUOjb6Y9acoo1DMl1e4I8AxnuI= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1722892247; h=Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:Subject:To:To:Message-Id:Reply-To; bh=efcBaXWkHAOOawEycsLB2uZn397zNxBIPFnI8W1gVfM=; b=PgiE5Ml0AsIos5ahZ+T6UmDse6W+n5zA99NirOmMYOHIcfB4+XxYJEVrislm6PXDdBwqYS25gyhrNcXGiSs33qm10QbtP6HrDVF422u6w/PLr19ENDjnVfjnEotlBpuLB/SvjcVDIYkAZKl4kIrhP23lz27cbkWK1BLus2GXzik= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=pass header.from= (p=none dis=none) Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1722892247982321.21624059749183; Mon, 5 Aug 2024 14:10:47 -0700 (PDT) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sb4xH-0000Mb-6Q; Mon, 05 Aug 2024 17:09:23 -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 1sb4xD-0008VX-Ok for qemu-devel@nongnu.org; Mon, 05 Aug 2024 17:09:19 -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 1sb4xB-0008Ta-39 for qemu-devel@nongnu.org; Mon, 05 Aug 2024 17:09:19 -0400 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-261-3Nu1YoXkMtS0oV2URi73-A-1; Mon, 05 Aug 2024 17:09:13 -0400 Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (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 214B61955D44; Mon, 5 Aug 2024 21:09:13 +0000 (UTC) Received: from merkur.fritz.box (unknown [10.39.193.224]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 9001730001AA; Mon, 5 Aug 2024 21:09:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1722892156; 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=efcBaXWkHAOOawEycsLB2uZn397zNxBIPFnI8W1gVfM=; b=JomunbDWVhMxT3lZOhrhK+wEXQqdAagJkjd1Jy5cH377fph1+/yflMuJs4bEYgkFJkapzF JK0NdwkjgHW+UhhwkADkRvbbagIlOarrXBY0gtX2wCsyHov6m36fcM+92ctsAJMv0xJLLp zTsXhl1pwHvK3EDbfScG9nExwPU3XKo= X-MC-Unique: 3Nu1YoXkMtS0oV2URi73-A-1 From: Kevin Wolf To: qemu-block@nongnu.org Cc: kwolf@redhat.com, qemu-devel@nongnu.org Subject: [PULL 06/13] scsi-disk: Add warning comments that host_status errors take a shortcut Date: Mon, 5 Aug 2024 23:08:44 +0200 Message-ID: <20240805210851.314076-7-kwolf@redhat.com> In-Reply-To: <20240805210851.314076-1-kwolf@redhat.com> References: <20240805210851.314076-1-kwolf@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; 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.143, 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=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+importer=patchew.org@nongnu.org Sender: qemu-devel-bounces+importer=patchew.org@nongnu.org X-ZohoMail-DKIM: pass (identity @redhat.com) X-ZM-MESSAGEID: 1722892248952116600 Content-Type: text/plain; charset="utf-8" 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 Acked-by: Paolo Bonzini Message-ID: <20240731123207.27636-4-kwolf@redhat.com> 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 =3D=3D 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 direct= ly + * complete the request and never call the callback. */ DMAIOFunc *dma_readv; DMAIOFunc *dma_writev; @@ -381,6 +384,7 @@ done: scsi_req_unref(&r->req); } =20 +/* 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 =3D (SCSIDiskReq *)opaque; @@ -421,6 +425,7 @@ done: scsi_req_unref(&r->req); } =20 +/* 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 =3D (SCSIDiskReq *)opaque; @@ -560,6 +565,7 @@ done: scsi_req_unref(&r->req); } =20 +/* 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 =3D (SCSIDiskReq *)opaque; @@ -2821,6 +2827,7 @@ static void scsi_block_sgio_complete(void *opaque, in= t ret) sg_io_hdr_t *io_hdr =3D &req->io_header; =20 if (ret =3D=3D 0) { + /* FIXME This skips calling req->cb() and any cleanup in it */ if (io_hdr->host_status !=3D SCSI_HOST_OK) { scsi_req_complete_failed(&r->req, io_hdr->host_status); scsi_req_unref(&r->req); --=20 2.45.2