From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <010701a1455a5bcb-8972df05-a51e-418b-bc92-2daf73d2bbf8-000000@eu-central-1.amazonses.com> Received: from GVXPR05CU001.outbound.protection.outlook.com (mail-swedencentralazon11023097.outbound.protection.outlook.com [52.101.83.97]) by lore.virtuozzo.com (Postfix) with ESMTPS id CD17A8017D for ; Fri, 16 Oct 2026 15:35:18 +0000 (UTC) ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=hQb7NByUt3SPinOW2tiIDA+4RmAUqMBp4o06Zz+1+L3FIigpBhnoY6uj9MG7PVwE1nbGK5N2yoX9sWy38c1bpEYoWodV1nO7LAkWCXh1NJzznPIE4HSX3Lb4ydtVlNXrwl/H0h5VaDSyn9hINUixe6DV4k91m3JsWwtgtgj98NlEwPr0aMeO0teV+tFzpFXNcqP8tZOTwUx5hnpGiAWQFKXbyQQemlgdgHkvSw07U5fLtOZriUe+QKSLO1k8K5rw67MOd19iL141dBbbxwWZHbtTgCy6w+O6Okg6tpv5DQB9SytFQZKJEg4X0dDM1gZ8q/XbHEnrMYsxEqRf1rY9iA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=mtqg4GvuhFEiIqoGrZlyQBIUJawAYE5VmmWobTFtt4Q=; b=jpbUtH9O0DUnW1rEQV3BnmlH0AEgXrih/RXW2IFLc5IyevAhCbk4I87EEZmF+DCkVvJ0LckVNl0pNsWMVa4PSrvSHY+BLzJrLjjL8NWR6RL+1+Ct4hhmsstg35JMnQ+ieyLabvr89awAZcUlnRCTauipydVHL4bqAjNPfqcIBQzf+dEe+jPAMgGNDOdRu6gkknj2E+LMCVXz1NQxERLApBGs5t3xINL2v9K3wdMsSWZ5OOKAc5sjNxp0gACcNL6Rul/mfeXi8tVOD1+vfLSvBdfXljQmCHf/mjqLff9YlHpOHIGq+vgrCNf5+WMoIPx9PY8SG86dywps1E9RMMCUVg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 69.169.224.5) smtp.rcpttodomain=virtuozzo.com smtp.mailfrom=eu-central-1.amazonses.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=sashiko.virtuozzo.com; dkim=pass (signature was verified) header.d=sashiko.virtuozzo.com; dkim=pass (signature was verified) header.d=amazonses.com; arc=none (0) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=virtuozzo.onmicrosoft.com; s=selector2-virtuozzo-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=mtqg4GvuhFEiIqoGrZlyQBIUJawAYE5VmmWobTFtt4Q=; b=SFwET30gu1QAm32W+MvsozbtG7ipLBpKyR16lQv8XlO5BVSsI8qryV/z+xgXDFpKpn2fOHqtJucJ3oT6jMgw/WVgXJ1DFtwf85QD3vCiNZa2tVIQuR+a1mUM81goZ/Kut3Z6vBFo356MKGM3cOI6lNgCEHiISAfZR2kgDMIaOwg= Received: from AS4P190CA0019.EURP190.PROD.OUTLOOK.COM (2603:10a6:20b:5d0::18) by VI0PR08MB11597.eurprd08.prod.outlook.com (2603:10a6:800:2e7::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.21.546.9; Fri, 16 Oct 2026 15:35:12 +0000 Received: from AMS0EPF0000056B.eurprd02.prod.outlook.com (2603:10a6:20b:5d0:cafe::8c) by AS4P190CA0019.outlook.office365.com (2603:10a6:20b:5d0::18) with Microsoft SMTP Server (version=TLS1_3, cipher=TLS_AES_256_GCM_SHA384) id 15.21.522.4 via Frontend Transport; Fri, 16 Oct 2026 15:35:12 +0000 Authentication-Results: mx.microsoft.com 1; spf=pass (sender IP is 69.169.224.5) smtp.mailfrom=eu-central-1.amazonses.com; dkim=pass (signature was verified) header.d=sashiko.virtuozzo.com;dkim=pass (signature was verified) header.d=amazonses.com;dmarc=pass action=none header.from=sashiko.virtuozzo.com; Received-SPF: Pass (protection.outlook.com: domain of eu-central-1.amazonses.com designates 69.169.224.5 as permitted sender) receiver=protection.outlook.com; client-ip=69.169.224.5; helo=b224-5.smtp-out.eu-central-1.amazonses.com; pr=C Received: from b224-5.smtp-out.eu-central-1.amazonses.com (69.169.224.5) by AMS0EPF0000056B.mail.protection.outlook.com (10.167.242.121) with Microsoft SMTP Server (version=TLS1_3, cipher=TLS_AES_256_GCM_SHA384) id 15.21.522.6 via Frontend Transport; Fri, 16 Oct 2026 15:35:12 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=23yjbh4k34hwzfubhuiim7m4inbd3ick; d=sashiko.virtuozzo.com; t=1792164912; h=Message-ID:From:Subject:To:Cc:In-Reply-To:References:Content-Type:Content-Transfer-Encoding:Date; bh=cLR7oTQb3/QPnh+8gLaRDXWXOScq6Arj/GebUEwT0ho=; b=v47CL1bL8Cv3WShjYOECxS4xslmspRPF4z7MykLuI7vAUzPUx2tB+i32WPkNLv1s bzB44V7m9YGRFelIw/A+qvMG7DjYxUasSGiuVwBSAYJjyZAQl24bZcPA/F7s+N1P57F X9YHmj0WEauBy3Fr02G/Besme+xDneMCIhcadPm7NMB482OY9juyOt5zmB79VMRTf4j 2XB0N9YgrFSxLKSa/3E+jwljTXTkzsJC9pFvaDRd7TAE9OPEm2XK/fnkWtsHw/wkWi3 G5QOKcvmBb1N+dtKAUmr88A2BiPAvA7B4d5wCFZrcYpbfNf7f2q4yZBjAlejltCtDeI 7xjr7Xwx3g== DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=qftdzk2dqsatjnlrq4r5brjbihpfcrsh; d=amazonses.com; t=1792164912; h=Message-ID:From:Subject:To:Cc:In-Reply-To:References:Content-Type:Content-Transfer-Encoding:Date:Feedback-ID; bh=cLR7oTQb3/QPnh+8gLaRDXWXOScq6Arj/GebUEwT0ho=; b=bEn5wZesWzn/28Rhj5PiK3ScZnVUICL/9W3GiMxOUs3kad04Xp/aMjzLvKeQugJ7 fB0neT4NJGRdTCaNCaA8iMswKUSiJrJpLO1CSehMSI2dptnSehbJ4HyvXdtjn8Pgtxi ob1RmBg7amSenEHx2jhmnh3vizZCfYLXaVq44iRY= Message-ID: <010701a1455a5bcb-8972df05-a51e-418b-bc92-2daf73d2bbf8-000000@eu-central-1.amazonses.com> From: no-reply@sashiko.virtuozzo.com Subject: Re: [PATCH QEMU HCI-8.1 v3 12/28] migration: Fix regression of passing error_fatal into vmstate_load_state() To: Andrey Drobyshev , svt-core@virtuozzo.com Cc: andrey.drobyshev@virtuozzo.com In-Reply-To: <20261016-vfio_v1-v3-12-47487cb5a8e5@virtuozzo.com> References: <20261016-vfio_v1-v3-0-47487cb5a8e5@virtuozzo.com> <20261016-vfio_v1-v3-12-47487cb5a8e5@virtuozzo.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Fri, 16 Oct 2026 15:35:12 +0000 Feedback-ID: ::1.eu-central-1.2wRQJBpg9g1fGRYexx3dUuYGS25Bux+2TEOx72/m6U4=:AmazonSES X-SES-Outgoing: 2026.10.16-69.169.224.5 X-EOPAttributedMessage: 0 X-EOPTenantAttributedMessage: 0bc7f26d-0264-416e-a6fc-8352af79c58f:0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: AMS0EPF0000056B:EE_|VI0PR08MB11597:EE_ MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: 3962debe-9743-416e-4916-08df2b9b11a4 X-LD-Processed: 0bc7f26d-0264-416e-a6fc-8352af79c58f,ExtAddr X-MS-Exchange-AtpMessageProperties: SA List-Id: svt-core@virtuozzo.com X-MS-Exchange-Transport-Rules-Loop: 1 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230040|376014|48200799018|9140799003|61400799027|32142699015|55112099003|4113699003|5113699003|10067099003|22082099003|16102099003|18002099003|13003099007|4143699003|261009223027099003|56012099006|5063699009; X-Microsoft-Antispam-Message-Info: noI/Lpd0rxHrGhnm25ziApeH7Xp7U3GN6vTw+JVOlvMaPC2KQtjUYTH2YEPYLFqZkINkuZGQo+Ran93VSSV9vBM84MiJGO3Qw1B+x4bHFvH+9LfmvSHD93VecTrGDY8nV/zJuDUyMO0Y1LNOoVgyxT98eaadz6Ii6UCH6bBz94n6g3DdkAb9tVVYzMZmjjyz/e9o+58XamEeL4jYn/npTODjVyS4CrKknnppUhHByPhMR/AljIrZvUoMCVLSTNGEykLeoMU7+OTK2VqcDzRrMIXs2R0/L/zU0ktBaIcb4ymwd9JyFKE0fgblHBcJbqxDO9qRqmC3sSZPFCe/DileObXclXc+UQigeXLgyO3b0g+67IORN40epLzGKSXJUXaU/zswOWbFAaa+hWRdki/wuK8UiXxDeHK3GynqGLCMIwiDpgKJdJz+GVKWdgD8hnT1AKCu/wUvLp4EzpHgxoAiPnZX72GWvSBRfq5jLveKyUPed5XvbnW6rt5RusEIxxjCTfDNAZGtO2JYfCSR5R99aLO6hBDihL2TJ1gtyo7K/j8i9WXbjsdWuB5SBtM+/chwlxMu5FzhwssHAuoHPeYZN6k++AnzrGNejN5U6PARFNiHMithGL8ddnm5NUFn0BaMGnJdelejLZLeLwz+5eGM228ZBm+gYyeMjV3C/YI2aEU764bvY34V2/gSKi3YuqodmFuE6j0WsgExmY6rDVPGxoYl+NBbacXKXrUQnNq9VPkx8RAJ3LeOjZciV0Bn/PFoM3RsZdfavUNvTyvrCwBguIYjRJjIHTfbZs7r4aomReYerqMLPC3prvse8wdYWf5Dxc78CnmN1ddgRYXQXEP5tA== X-Forefront-Antispam-Report: CIP:69.169.224.5;CTRY:US;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:b224-5.smtp-out.eu-central-1.amazonses.com;PTR:b224-5.smtp-out.eu-central-1.amazonses.com;CAT:NONE;SFS:(13230040)(376014)(48200799018)(9140799003)(61400799027)(32142699015)(55112099003)(4113699003)(5113699003)(10067099003)(22082099003)(16102099003)(18002099003)(13003099007)(4143699003)(261009223027099003)(56012099006)(5063699009);DIR:OUT;SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: FGONISnkNx61hxzYmUxTzcPt/ISi+ZAw/8SzKfQh6YoVbws36pEZ9ybaVL3AgWi8hlmzVNiknJk2ZHQzXIqNTZxmj4c/VWEKtj+uJ2uqtF2JueftFqnrZdIgto/gDeKKPEX+yhVg/LvH4hSKNjx+t9j7Qsp1+YpAArwzNn6JbnCFR24qKpRozhuY4fzoSz23Iz/YvKM6c/pQ3OSvYqijXXOn1k95xN7or28fd5rF2tMXG+SSolGDjikMinYTq/ktDfCJ/lVsUgkDb2BOBafod43Mfsrq6sly6844vXvCkFcY7tEbsUos49GsJpj0RKe4zTxBqeByAyYO2j2mRQ1HOTtvc8LNPLDRNQBlt+zwyBXjIwwHJwed5OWu4s2KbDJ9LkPpltiaNaV0Xo3NnQ+Sshw1WP9+9f29zrBMdPIO5+WZ/1k6FNRg4Pt+pWDg/fQY X-ExternalRecipientOutboundConnectors: 0bc7f26d-0264-416e-a6fc-8352af79c58f X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Oct 2026 15:35:12.5067 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 3962debe-9743-416e-4916-08df2b9b11a4 X-MS-Exchange-CrossTenant-Id: 0bc7f26d-0264-416e-a6fc-8352af79c58f X-MS-Exchange-CrossTenant-AuthSource: AMS0EPF0000056B.eurprd02.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: Internet X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI0PR08MB11597 Thank you for your contribution! Sashiko AI review found 2 potential issue(= s) to consider: - [Low] migration: Error leaked when vmstate hooks return positive ret (ret= < 0 check) - [Low] migration: void vmstate save wrappers only report errors and leave = QEMUFile unmarked -- commit e28996c9e3a0b9aff33ccbdf09f181f304ec549c Author: Arun Menon migration: Fix regression of passing error_fatal into vmstate_load_state() This commit replaces error_fatal with a local Error object in the callers of vmstate_load_state() and vmstate_save_state(). Failures are now reported wi= th error_report_err() and, where the caller can return a value, propagated. Link: https://lore.kernel.org/r/20251028-solve_error_fatal_regression-v2-1-= dab24c808a28@redhat.com > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c [ ... ] > void pci_device_save(PCIDevice *s, QEMUFile *f) > { > + Error *local_err =3D NULL; > + int ret; > + > /* Clear interrupt status bit: it is implicit > * in irq_state which we are saving. > * This makes us compatible with old devices > * which never set or clear this bit. */ > s->config[PCI_STATUS] &=3D ~PCI_STATUS_INTERRUPT; > - vmstate_save_state(f, &vmstate_pci_device, s, NULL, &error_fatal); > + ret =3D vmstate_save_state(f, &vmstate_pci_device, s, NULL, &local_e= rr); > + if (ret < 0) { > + error_report_err(local_err); > + } > /* Restore the interrupt status bit. */ > pci_update_irq_status(s); > } [Severity: Low] pci_device_save() is a void function. It only prints the error and carries on to pci_update_irq_status(). vmstate_save_state_v() does not call qemu_file_set_error() on failure, so the QEMUFile is not marked as failed here. Can a failed nested save (a nonzero pre_save, put or post_save return) in pci_device_save() leave a truncated device section in the stream while the save still looks successful? The same pattern is present in virtio_ccw_save_config(), virtio_mmio_save_extra_state() and virtio_pci_save_extra_state(). [ ... ] > diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c > --- a/hw/scsi/spapr_vscsi.c > +++ b/hw/scsi/spapr_vscsi.c [ ... ] > + rc =3D vmstate_save_state(f, &vmstate_spapr_vscsi_req, req, NULL, &l= ocal_err); > + if (rc < 0) { > + error_report_err(local_err); > + return; > + } [Severity: Low] In vscsi_save_request(), the error is printed and then dropped. The QEMUFile is not marked failed, and the caller cannot tell that the request was not saved. Should the failure be recorded on the stream, for example with qemu_file_set_error(), so that this and the other void save helpers do not hide it? The commit message says the change is meant to keep "either propagating the error to the caller or reporting it". That holds for the load paths, virtio_gpu_save() and virtio_save(), but not for these void helpers. [ ... ] > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c [ ... ] > /* Subsections */ > - return vmstate_save_state(f, &vmstate_virtio, vdev, NULL, &error_fat= al); > + ret =3D vmstate_save_state(f, &vmstate_virtio, vdev, NULL, &local_er= r); > + if (ret < 0) { > + error_report_err(local_err); > + } > + return ret; > } [Severity: Low] The earlier vdc->vmsd call in virtio_save() tests "if (ret)". This new call tests "ret < 0". vmstate_save_state_v() calls error_setg(errp, ...) for any nonzero return from pre_save, and vmstate_load_state() does the same for any nonzero pre_load return. The load side also returns early on a nonzero vmstate_subsection_load() result. If one of those hooks returns a positive value, local_err is set but never passed to error_report_err(). Can that leak the Error and leave the failure unreported? The same "ret < 0" check is used in pci_device_save(), pci_device_load(), virtio_ccw_save_config(), virtio_ccw_load_config(), virtio_mmio_save_extra_state(), virtio_mmio_load_extra_state(), virtio_pci_save_extra_state(), virtio_pci_load_extra_state(), virtio_gpu_save(), virtio_gpu_load() and vscsi_save_request(). I did not find an in-tree hook that returns a positive value. This looks like a conditional problem, because hooks conventionally return negative errno. Would testing "if (ret)" here, as the vdc->vmsd call does, be more consistent? The follow-up commit e68e95a3b426 ("migration: vmstate_save_state_v: fix double error_setg") does not change this. The nonzero-return error_setg() calls in vmstate.c are unchanged, and the call sites still use "ret < 0". --=20 Sashiko AI review =C2=B7 https://sashiko.vzint.dev/#/patchset/20261016-vfio= _v1-v3-0-47487cb5a8e5@virtuozzo.com?part=3D12