From mboxrd@z Thu Jan 1 00:00:00 1970 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 0B6A6CF58D1 for ; Thu, 20 Nov 2025 02:05:12 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1vLu1z-0002GD-4A; Wed, 19 Nov 2025 21:04:19 -0500 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 1vLu1x-0002Fs-P6 for qemu-devel@nongnu.org; Wed, 19 Nov 2025 21:04:17 -0500 Received: from sender4-pp-f112.zoho.com ([136.143.188.112]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1vLu1v-0007I2-Ud for qemu-devel@nongnu.org; Wed, 19 Nov 2025 21:04:17 -0500 ARC-Seal: i=1; a=rsa-sha256; t=1763604240; cv=none; d=zohomail.com; s=zohoarc; b=BvUqI2T9MpLmCVmtPGILOOOJowqvJYaeCCWniKE3Z1rtbBHUXkSCm5HCvpkvqrlgLQHIaLglYkOTu5yrQcD88JpaLBlnfEXPNIcLsdFxhswpBHxR95i42k3N2s7PYnpaTGS6F0Q13ZuKT3+hFd+1P9sm6T7C61w6GHiYFOHNQA8= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1763604240; h=Content-Type:Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To; bh=ATnlm3Ww+kykN6/YxEM1CzSC3aeS2C741HcK3gIGw3A=; b=hTghAITA14O7D59mH6AtXsbIUU2KuI8L0QI0MmwCjeGW8NQ7w5DkucxZDYgrMxNfkbu3Sf8EA61Kr9Ow55+lUDA6Re7xveYffYEck7WIxm31Ht3Vg5vLs7KEjRK+YKf8l14dECxjS3WpcxinsTEjrEjbORvT+pdAgkSxKyOv+cc= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass header.i=collabora.com; spf=pass smtp.mailfrom=dmitry.osipenko@collabora.com; dmarc=pass header.from= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1763604240; s=zohomail; d=collabora.com; i=dmitry.osipenko@collabora.com; h=Message-ID:Date:Date:MIME-Version:Subject:Subject:From:From:To:To:Cc:Cc:References:In-Reply-To:Content-Type:Content-Transfer-Encoding:Message-Id:Reply-To; bh=ATnlm3Ww+kykN6/YxEM1CzSC3aeS2C741HcK3gIGw3A=; b=decWkJipxxTJq95Y0xeOuMgRLapqpsC+fyAN5PvjNuMy/ANuaLoShGLMPOWKjfCh LNBdZLU7db+VDbOFZJjdDxtawWXgTXXbkatNzLot3KvQNqFTsVHDb4YbCO7FKlLr5Sz ApWTx/ilp9iy9SYSFD65vHmHeXVlFtxVvmCvVPTM= Received: by mx.zohomail.com with SMTPS id 176360423760948.452024500188486; Wed, 19 Nov 2025 18:03:57 -0800 (PST) Message-ID: <8ce3a9dc-e4a8-4dd7-99dc-30b02b07f4c9@collabora.com> Date: Thu, 20 Nov 2025 05:03:49 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [v4] virtio-gpu: use consistent error checking style for virtio_gpu_create_mapping_iov From: Dmitry Osipenko To: Honglei Huang , Markus Armbruster Cc: alex.bennee@linaro.org, odaki@rsg.ci.i.u-tokyo.ac.jp, mst@redhat.com, cohuck@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, Ray.Huang@amd.com References: <20251117105104.258513-1-honghuan@amd.com> <9f547fbe-ea57-4784-9e37-f79b52b551a9@collabora.com> <87bjl0pzeq.fsf@pond.sub.org> Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-ZohoMailClient: External Received-SPF: pass client-ip=136.143.188.112; envelope-from=dmitry.osipenko@collabora.com; helo=sender4-pp-f112.zoho.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, 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, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=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 On 11/19/25 22:16, Dmitry Osipenko wrote: > On 11/18/25 15:32, Honglei Huang wrote: >> >> >> On 2025/11/18 09:48, Dmitry Osipenko wrote: >>> On 11/17/25 16:22, Markus Armbruster wrote: >>>> Dmitry Osipenko writes: >>>> >>>>> On 11/17/25 13:51, Honglei Huang wrote: >>>>>> diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio- >>>>>> gpu-rutabaga.c >>>>>> index ed5ae52acb..ea2928b706 100644 >>>>>> --- a/hw/display/virtio-gpu-rutabaga.c >>>>>> +++ b/hw/display/virtio-gpu-rutabaga.c >>>>>> @@ -466,7 +466,7 @@ rutabaga_cmd_attach_backing(VirtIOGPU *g, >>>>>> struct virtio_gpu_ctrl_command *cmd) >>>>>>         ret = virtio_gpu_create_mapping_iov(g, att_rb.nr_entries, >>>>>> sizeof(att_rb), >>>>>>                                           cmd, NULL, &res->iov, >>>>>> &res->iov_cnt); >>>>>> -    CHECK(!ret, cmd); >>>>>> +    CHECK(ret >= 0, cmd); >>>>> >>>>> virtio_gpu_create_mapping_iov() doesn't return positive values, don't >>>>> see how this change improves anything. You now saying that ret > 0 is >>>>> okay, while it shall never happen. >>>> >>>> Please see >>>> >>>>      Subject: Re: [PATCH] virtio-gpu-virgl: fix error handling in >>>> virgl_cmd_resource_create_blob >>>>      Date: Mon, 17 Nov 2025 08:49:42 +0100 >>>>      Message-ID: <87ms4lrtd5.fsf@pond.sub.org> >>>>      https://lore.kernel.org/qemu-devel/87ms4lrtd5.fsf@pond.sub.org/ >>> >>> It's a rather common bug when errno isn't negated by mistake and a >>> positive error code is returned. Ignoring positive values when they >>> aren't expected opens door to unnecessary problems, IMO. >>> >> >> How about apply the v2 or v3 firstly to fix the >> virtio_gpu_create_mapping_iov() block issue in virtio-gpu? >> >> I will create another thread for the `CHECK(!ret, cmd);` thing in rutabaga. > > There was a precedent of virtio-gpu not handling positive error codes > properly [1]. To me there is no problem that needs to be fixed when > virtio_gpu_create_mapping_iov() is never expected to return positive > values and doesn't return them. > > [1] > https://lore.kernel.org/qemu-devel/20240129073921.446869-1-dmitry.osipenko@collabora.com/ > > It's a common expectation that errors are negative. But in practice it's > not always true, especially when interacting with external code. > > Functionally this patch doesn't change anything. Will leave to Alex and > Akihiko to decide on it. Alright, I changed my mind after just typing a code where a fact of error occurrence needs to be propagated. For a code that is internal to QEMU, it should be fine to check for err < 0 because static analysis tools should catch invalid error handling and using same style makes code look more consistent and pleasant to read. For errors originated externally, I'll continue to advocate for checking of both pos and neg values. -- Best regards, Dmitry