From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from AM0PR02CU008.outbound.protection.outlook.com (mail-westeuropeazon11023099.outbound.protection.outlook.com [52.101.72.99]) by lore.virtuozzo.com (Postfix) with ESMTPS id AF8A5802A5 for ; Fri, 9 Oct 2026 15:17:39 +0000 (UTC) ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=cnq1tfcP5H2I65Bpq8NjBwSUdE5e28SWHYdp2aD/Rzt7MRr+3Ub//YB91zpOQYwWYGkoYGkS7FrR2XpxTCfPS+YKD1ek4L1+w+MyJ3DiHGMY13rpIZx3LTRqmIAZZmCucgivukY3fERaDrjzb59P8qeX/moHwCCJK/i5fhuAEuhSHGrxIftg+tdlPE2xb+rOL024CZel0sm83jMVxNlwOgXIEeQgToApZ8AjT1/cYfcgDunWtyNdZ1HqZgz2J+L37c6UIcXZwNztqBabZKdgNrlC5ysnYQ7vrtdwtxiN6q7bez6WFWuYqiDdfXZ+zfljbzgv6BJtQzH/+Ra8T6F0+w== 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=I5K7rMeJDy4V7rgxhkc+U33ont77XuS8nd8U4szoT6w=; b=gkf5lB2Px7lFNbl0BVJVI9FnMKvhoS41F1f67d9x1FXcAZFYX9Y6jpydXmpQLDGO7rWH9y2ylZQsgLxk0kg6Mdft5LABU9BfUE8L+DYrqjGDJdHwvh4wSDG4z0gCHW6QaFMjVgfJmWHOhUx6zWbem/nzqMZn7MeiAG7DQFEbXAx2ELXKOywpkiS0i6hwrpo0I31HzQo/78JYgcpOBnnkGg0MR4F9Uv1WoRwrO2QhAdlolN+hZwgxyKL9nOT5LsSLiVHpZjXyXVi3Cyu1XcGpkkZ8CEFIQKXlEmM6EH3cwTnE5ZruCvy71c/CijRW3SBq86ZbxrjHrPlSnNydyIqeQw== 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=I5K7rMeJDy4V7rgxhkc+U33ont77XuS8nd8U4szoT6w=; b=KmOaD5QjbHGhnTk2ExTG9hldgvkg8o/aSkYd/sm37cKb8jQnbs6mq3LgGm3nT9P07T1fpmrhqHU04Kswmclknm1oR76uvX9uT66ZhZldofcyFWm2hJzhJaCRVp04+wSiVVfW5scaifPpQTUPZOqASGHGGfOqUKBUfimiP+bMBpQ= Received: from OS6P279CA0045.NORP279.PROD.OUTLOOK.COM (2603:10a6:e10:32::18) by DB9PR08MB11462.eurprd08.prod.outlook.com (2603:10a6:10:605::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.21.496.15; Fri, 9 Oct 2026 15:17:36 +0000 Received: from CPH1EPF00000520.eurprd04.prod.outlook.com (2603:10a6:e10:32:cafe::85) by OS6P279CA0045.outlook.office365.com (2603:10a6:e10:32::18) with Microsoft SMTP Server (version=TLS1_3, cipher=TLS_AES_256_GCM_SHA384) id 15.21.472.21 via Frontend Transport; Fri, 9 Oct 2026 15:17:36 +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 CPH1EPF00000520.mail.protection.outlook.com (10.167.241.235) with Microsoft SMTP Server (version=TLS1_3, cipher=TLS_AES_256_GCM_SHA384) id 15.21.522.6 via Frontend Transport; Fri, 9 Oct 2026 15:17:36 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=23yjbh4k34hwzfubhuiim7m4inbd3ick; d=sashiko.virtuozzo.com; t=1791559055; h=Message-ID:From:Subject:To:In-Reply-To:References:Content-Type:Content-Transfer-Encoding:Date; bh=tBuD672XuZ+3ukSmndOU2gpcL9LC6XXHpPHhko8yYqs=; b=vXenILXLUr4E75ksdFK35xVpMvSP8BMGfcs6LeP6tPlhLJWfAWRo7LAmR2WpJEY4 XbtY/zC7HsPyNloPU9xkj6GjAqAEEIPv6mgQtWdyophxLLt4D1SlyFFkj9lSdY4BbHJ w5Oc2ws7WswprlfZ4OpPWBj1BxWxlaEg8bXgjKjQVw0VgNkE68YED4ZCCO7lKE+LZUC 0+FPquV2OfcqiJ89wr1C3VA7UWulKvdKbWvnbT329H/Kx/sVN2VCVIs/r7/lA15deGj YEo+/qv3tZ6bR51MGKXhgNe/RqwWK+AGhZdDML+LdpxVJ1NP4Zu+746DQOHmXEQjaOq 2MIeLtnC9Q== DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=qftdzk2dqsatjnlrq4r5brjbihpfcrsh; d=amazonses.com; t=1791559055; h=Message-ID:From:Subject:To:In-Reply-To:References:Content-Type:Content-Transfer-Encoding:Date:Feedback-ID; bh=tBuD672XuZ+3ukSmndOU2gpcL9LC6XXHpPHhko8yYqs=; b=UY2UWT7AYvf4rSCDWPsnAfCukJLldeNqk9NENB50pPaQAOB+N1sBzfZXyITvbfLm bEIotrJG29fLPI8SsJcr0e+DNwPqV8EYA6j4tEw8JG/YSxhVKXWuhHky9Q3ixLedLzS Hp8WdlF17Xi8tWVBZVOPMgt8HdU2qsq5JhC7vWjQ= Message-ID: <010701a1213db9d8-9abf6782-5d31-427a-b86e-09226ae177ba-000000@eu-central-1.amazonses.com> From: no-reply@sashiko.virtuozzo.com Subject: Re: [PATCH QEMU HCI-8.1 09/21] migration: Fix regression of passing error_fatal into vmstate_load_state() To: svt-core@virtuozzo.com, Andrey Drobyshev In-Reply-To: <20261009-vfio_v1-v1-9-2ae91d5b8048@virtuozzo.com> References: <20261009-vfio_v1-v1-0-2ae91d5b8048@virtuozzo.com> <20261009-vfio_v1-v1-9-2ae91d5b8048@virtuozzo.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Fri, 9 Oct 2026 15:17:35 +0000 Feedback-ID: ::1.eu-central-1.2wRQJBpg9g1fGRYexx3dUuYGS25Bux+2TEOx72/m6U4=:AmazonSES X-SES-Outgoing: 2026.10.09-69.169.224.5 X-EOPAttributedMessage: 0 X-EOPTenantAttributedMessage: 0bc7f26d-0264-416e-a6fc-8352af79c58f:0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CPH1EPF00000520:EE_|DB9PR08MB11462:EE_ MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: 4f3d94c9-4ecc-4720-7006-08df26187347 X-LD-Processed: 0bc7f26d-0264-416e-a6fc-8352af79c58f,ExtAddr X-MS-Exchange-AtpMessageProperties: SA List-Id: svt-core@virtuozzo.com X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230040|704161411799003|32142699015|61400799027|9140799003|48200799018|376014|10067099003|16102099003|18002099003|22082099003|4143699003|5063699009|56012099006|5023799004|13003099007|4113699003|5113699003|55112099003; X-Microsoft-Antispam-Message-Info: S+EGvOadXaHi2fNH0cMIMOZDTbY0yIIdGrfce+IWN8kJ6x/xKZsh/tPR/CjgvEyey8oKII4O5ndjTJxAU3E9a4BcG7gcqFhZLqH+CNkFxuKSmXU3p7eJxkNFSgMmss+ch5abB1tc9yW09NR+XJHmh7ilv7ML+mkMBER37CgKavO+WdYOMItbm1mpbIeJMc18DFiX1YC6JOYgrZlciJ2xfZEE+IAM7GaHHpzH7Q9Ziv94XcjXpZ4nGNxOic47kHZv+xleOPXTvDOGsOkf2MTUoYkHPn/i4pQ0NjsB2plBM+6/RrJ66FjCi5lWBvy6YH8nJeIscgJBLCccfG2WPGcC/LzLKZoE9PV30KqlO9OMZyGAJyqffCwwfJEGKU14CrdAHm/WGDy+N5lTkxlppcpnQwrlIPgjeLjz/0VRaTpzfUoKVcSJwbqVrp4BXxwWi7U77AaklPOCVXQiakvGtlLkc8vIstjXTqloiV5dxDWcPNzfLxakWkq/xQCtBQ1QDd/rRjxhrwT/r1MwVXBJj7H0Gq5e3hYXJR/8aBnJqk/FVifXycxKMPiP5FFeJbbRgh2hTs+nczWKlLQh+XJzgXrtGmNc7PZXCDjXnAWCX77cApHZg8bHlgULefELiL8ywfES46LBADkcTHlMCc3Vj1c7xd2AQ4uvd4fuKeL19xoNuzdOAJzjBXk1vrOqvBsrm0g93VNmHMo9qlNgZk80HuCOTxXBeOxpZl2CBoOP+1IMZIXAo7gmJYjgKY6eWQeTGvxBfUVaydUL54EmtABSAdiVjA== 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)(704161411799003)(32142699015)(61400799027)(9140799003)(48200799018)(376014)(10067099003)(16102099003)(18002099003)(22082099003)(4143699003)(5063699009)(56012099006)(5023799004)(13003099007)(4113699003)(5113699003)(55112099003);DIR:OUT;SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: D1gpzj5oE/tSGRBUYJlqfZaqdw5WtwKfrWWIqfIJFJsdR0jP5zPj6KMBsdT847QAb1hglBGdxlXxrMXr8YOJi1aW8vxDX5aSZr7sfw6GSla9QSuNhdqb0rWJYmbgvhRgN1ocH8JvswICra6ebEQ2DOAuvYXcKa9CK3XfTBklZB+PMNL4EocJoFHQMlQZtNyhogJogHv/2SY+rOarKjkgyU2VCvdCWbRFZiw9bfmm1MpP5Ka02r/3/VG1hKoOnjSdruF3iqDvXYiEbZ8o0DwkjE+ir1Rp+L7tTjHKMvexrkmVoGJH3HUi1kcOtCQIkC20P37c/qh0jJOfkGS3ZsxfyWipnRyV1BvrL8R8zf4qlLOH2WWwwQgwEcr1h7t8k3QFM8o1+n4goK4hMUTG22/+go0WZsK9k42Gn0xipfT35Uhu0bmB91KvlR5C/rnFezo4 X-ExternalRecipientOutboundConnectors: 0bc7f26d-0264-416e-a6fc-8352af79c58f X-Auto-Response-Suppress: DR, OOF, AutoReply X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Oct 2026 15:17:36.3213 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 4f3d94c9-4ecc-4720-7006-08df26187347 X-MS-Exchange-CrossTenant-Id: 0bc7f26d-0264-416e-a6fc-8352af79c58f X-MS-Exchange-CrossTenant-AuthSource: CPH1EPF00000520.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: Internet X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR08MB11462 Thank you for your contribution! Sashiko AI review found 2 potential issue(= s) to consider: - [Medium] migration: vmstate_save_state_v() calls error_setg() on an errp = already set by the nested call - [Low] migration: void vmstate save helpers log failure but never flag the= QEMUFile -- Commit e12869dadb039e65c940e64b56a6161151d7d971 Author: Arun Menon migration: Fix regression of passing error_fatal into vmstate_load_state() This replaces &error_fatal in the vmstate_save_state() and vmstate_load_state() callers with a local Error object. Errors are now reported with error_report_err() and the return value is propagated where the caller has one. 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: Medium] Can this abort QEMU through the assertion in error_setv()? In vmstate_save_state_v(), a nested VMS_STRUCT, VMS_VSTRUCT or subsection save is called with the same errp: if (inner_field->flags & VMS_STRUCT) { ret =3D vmstate_save_state(f, inner_field->vmsd, curr_elem, vmdesc_loop, errp); } ... if (ret) { error_setg(errp, "Save of field %s/%s failed", vmsd->name, field->name); If the nested call fails, it has already set *errp. The outer loop then calls error_setg() on the same errp, which reaches: assert(*errp =3D=3D NULL); With &error_fatal, QEMU exited before this point. With a real Error * as passed here, the second error_setg() looks like it would hit the assertion instead of propagating the error. The same applies to the other converted save callers in this patch, such as virtio_pci_save_extra_state() and virtio_save(). vmstate_save_state_v() does not appear to use ERRP_GUARD or check for an already set error. Could the outer error_setg() be skipped when *errp is already set, or could error_prepend() be used there instead? [Severity: Low] Is logging the error here enough? pci_device_save() returns void and does not call qemu_file_set_error(). vmstate_save_state_v() only does error_setg() and returns ret on failure, unlike the load path, which calls qemu_file_set_error(). So the QEMUFile error state stays clean after a failed save. The source then continues writing the stream, and the failure would show up later as a load error on the destination. Before this patch, QEMU exited at the point of failure. > 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] Does vscsi_save_request() leave its caller unaware of the failed save? This void function reports the error and returns. Nothing calls qemu_file_set_error(f, rc), so the caller continues as if the save succeeded. > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c [ ... ] > static void virtio_pci_save_extra_state(DeviceState *d, QEMUFile *f) > { > VirtIOPCIProxy *proxy =3D to_virtio_pci_proxy(d); > + Error *local_err =3D NULL; > + int ret; > =20 > - vmstate_save_state(f, &vmstate_virtio_pci, proxy, NULL, &error_fatal= ); > + ret =3D vmstate_save_state(f, &vmstate_virtio_pci, proxy, NULL, &loc= al_err); > + if (ret < 0) { > + error_report_err(local_err); > + } > } [Severity: Low] Does this void save callback have the same gap as pci_device_save()? The failure is only logged and the QEMUFile is not flagged with qemu_file_set_error(). The same pattern appears in virtio_mmio_save_extra_state() and virtio_ccw_save_config(). Would it make sense for vmstate_save_state_v() or these helpers to set the QEMUFile error on failure? --=20 Sashiko AI review =C2=B7 https://sashiko.vzint.dev/#/patchset/20261009-vfio= _v1-v1-0-2ae91d5b8048@virtuozzo.com?part=3D9