From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <010701a11214c87b-590e4ff4-13d8-4f35-bf55-7bbcb44beba9-000000@eu-central-1.amazonses.com> Received: from GVXPR05CU001.outbound.protection.outlook.com (mail-swedencentralazon11023076.outbound.protection.outlook.com [52.101.83.76]) by lore.virtuozzo.com (Postfix) with ESMTPS id 124AF802A7 for ; Tue, 6 Oct 2026 16:38:38 +0000 (UTC) ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=V9qmIDZWBvIN1eqCKopmgMKOimZwr3m2i0mOjmmxhEArfNdDMbGQRhb1R3w4I3jD5PPkPkvlDtwIxz+xEVxVJuUNHIvQBCdZRfemt5iCsqCwMBTz3hkdUMrPFl0CNbCeABuSc8udVnj8LAIqAwEUTLVvbGAMFiXfLUmR9fwXlD6F4hGw51opz8a/K9tbhwa+N2vUbPXw1n+orxTvXySk4DtLTdWas+iyBXyob1BAom7VWX6jwpDijs975yqhgq9JLeBBipZ3N2QTSWGZYCLSrNus+uXhEodbL6rnjUbf03RU9+v/i2ri6/jc6yFbdGSvGQDjlFxpAMT2GDUNQHmgzg== 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=sdv+R43DV5LAGg1efjN6wtup7ZGrZT8LfR02EvlT7KI=; b=TZjNQuH5BAW7Fixv28ghclgNY1hp77kCaimSmZ0R9DoA/mpd3lIwzQp5IKw4hLy4Pqdyk4BYEetbqWWA6EQ4FIYQU38akf+FLXWzBvDYk4x2NBFaM1l5Se+LbYDEcuU6jh0aHWh2hSI1I7/NApm2QkOly8IveWKiMbhROcTUh5Amo9cxle7N3Ihibl4zS5JioI8JRy++RiL5guD54/QYx6eHrmfh1iAUBhCEXCzer85E0G+ueL8CijkHJ66jx5o5edz0aBEE2nbodojlMgWdCJ6tJpGsIUTX4jFKFdAxZxtA/hMBKHStFkBz2MBzAi1XEMuF0FlOS4J0YcC7itIh/Q== 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=sdv+R43DV5LAGg1efjN6wtup7ZGrZT8LfR02EvlT7KI=; b=IXIvOL2dAqect89WMJqc+ZLYErzDh+s2sPtYU3oXnnf8+5OqO+pABw3ygryOzSflxNFgQDi9c42WyUV8qoz4bLn5vb2MBKzNQjMlG3sjjEjzMFcjdOdr5Dpob7LW/1RyAbV23osKRRP5jtFLLXyvoxwLJwJBjutFowufBJuLhRE= Received: from AM0PR10CA0006.EURPRD10.PROD.OUTLOOK.COM (2603:10a6:208:17c::16) by AS8PR08MB6520.eurprd08.prod.outlook.com (2603:10a6:20b:319::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.21.496.15; Tue, 6 Oct 2026 16:38:35 +0000 Received: from AM1PEPF000C1ADB.eurprd02.prod.outlook.com (2603:10a6:208:17c:cafe::44) by AM0PR10CA0006.outlook.office365.com (2603:10a6:208:17c::16) with Microsoft SMTP Server (version=TLS1_3, cipher=TLS_AES_256_GCM_SHA384) id 15.21.472.20 via Frontend Transport; Tue, 6 Oct 2026 16:38:34 +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 AM1PEPF000C1ADB.mail.protection.outlook.com (10.167.242.106) with Microsoft SMTP Server (version=TLS1_3, cipher=TLS_AES_256_GCM_SHA384) id 15.21.496.14 via Frontend Transport; Tue, 6 Oct 2026 16:38:34 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=23yjbh4k34hwzfubhuiim7m4inbd3ick; d=sashiko.virtuozzo.com; t=1791304714; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Content-Transfer-Encoding:Date:Message-ID; bh=DqRW5J45iKxFeVVAb9B1/wEVebIHhlODMS0pTho7BNE=; b=rDSHRA9QVLfFiiCXrpD3KnVHMRPToCcuY6+Hq+3RQGcEvL8BtTPwvx5kgEKhy84B 0yR3b3pTuh0x2KnIShdEnOqH+QNbHfZ+O3PLXIkKRDVv93jj+oVs0p+IIebjAouFBie Ws+OtR8UrXg3uwcbKdh/GRWSF3ba1gfau2j9ez7PA5FkSxWPhkjUe4WXHoyQ+WN8EOn UCn0DPYxSr7ZJ/po/B2dBwKhO1fPcufPzBf5MzEchVsWOSA5Pu70U9OkS9+ovBj/zGi zQiUfEby2uOcudJPHFzN4oemDu/XLBgeEZNW4ineYUrJEEXTnK8y9Dm7bOMPBL8XfWo jp2jbNv/QQ== DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=sokbgaaqhfgd6qjht2wmdajpuuanpimv; d=amazonses.com; t=1791304714; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Content-Transfer-Encoding:Date:Message-ID:Feedback-ID; bh=DqRW5J45iKxFeVVAb9B1/wEVebIHhlODMS0pTho7BNE=; b=gUKpaZ7y6xOc8ij/UtcMnw5aU2zWwWtkVqlqQO5Nog+c2iv+6b/F5H0OrFprDPmw Bq9iWTpuP/qPid1BzfDAKFu9XUSpR8RRmq4XGCbdRx+5zMUG+EI15K06HwFL1pL5W/g YEDC+yzaHKBP5RU7+6yBMOicgevR4dRO1Mx3Plmw= From: no-reply@sashiko.virtuozzo.com Subject: Re: [PATCH QEMU HCI-8.0 v8 06/14] usb-host: drain in-flight URBs across CPR #VSTOR-137800 To: Andrey Drobyshev , svt-core@virtuozzo.com Cc: den@openvz.org, andrey.drobyshev@virtuozzo.com In-Reply-To: <20261006-usb-host_v3-v8-6-72ae4fb8977b@virtuozzo.com> References: <20261006-usb-host_v3-v8-0-72ae4fb8977b@virtuozzo.com> <20261006-usb-host_v3-v8-6-72ae4fb8977b@virtuozzo.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Tue, 6 Oct 2026 16:38:34 +0000 Message-ID: <010701a11214c87b-590e4ff4-13d8-4f35-bf55-7bbcb44beba9-000000@eu-central-1.amazonses.com> Feedback-ID: ::1.eu-central-1.2wRQJBpg9g1fGRYexx3dUuYGS25Bux+2TEOx72/m6U4=:AmazonSES X-SES-Outgoing: 2026.10.06-69.169.224.5 X-EOPAttributedMessage: 0 X-EOPTenantAttributedMessage: 0bc7f26d-0264-416e-a6fc-8352af79c58f:0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: AM1PEPF000C1ADB:EE_|AS8PR08MB6520:EE_ MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: cc088ec4-fece-44ca-1372-08df23c843d1 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|704161411799003|48200799018|61400799027|32142699015|376014|9140799003|5113699003|4113699003|18002099003|55112099003|16102099003|22082099003|10067099003|13003099007|4143699003|5063699009|56012099006; X-Microsoft-Antispam-Message-Info: 6A5ah0QQVnDB7oje7BSaUx5WfrnbdnlYfjINxSe1MOUk6QVFiZq7xYcKqY4Wi/o5VlRVVNZjxwrg0nVm6J2ku22bGwWaQgjl3Hjwq+Idk8pj05AXZoCdijSeRLUhX42kxz677Y4m3Hypr/yRdvC/IQAqBYQK1H+afqRhhhGlIM0JaEguvy8BkoXlIVIiNjs+9U1wgygNoa69lue8d+6UQnTob6+lh19VNOHuMZg8mTiGatV9H/Zin6Lq9aF/v4zNONOrCM/ggqd+PjCllPz4VlRnUEKjjiSGnsBLnriG4P+tAhroXfLYhew8UQNCvs2WyvFXzn9D4tQ9dzfx/hUfr7yP7R25vpGLzyYEbjY57G4c6E7jf4746ygSXVFyCgqd0OmJSuc32mX0i9tCSfY9BnJinamuJ8hTLr+wh7dBPXYAntUlUa6H731I+H+kIjikvJ+JLW/bIVYpIM7MBkQgvI0NwSvnZtEPbydCehKAWwMpw99QgdIOEbGB9Z4KfvMUjSowA4ibaplnn64nDYZt/ipPwOrBWSXPkIHi+YiKN7u5CrO+GkljJFUou9wIp2ODo4cjGMgZ0Hdtq2uQe4IkU4IbycB3L10V1u0QaDrtlK9XE69v08F9ygeC+DZsDVS3tLTnON0xwyfe1/zrQshEfniIfK/z24LFUwdrZ+ScLxGw9RdbiEDJUvNyKsaWW0Ggiuzi2YjPdBsu5zuMaH+bt1gXzz8YS0HPRh0RiOZ5SLzBptFB9byCULVkLig9Ojpz9cxm5k0AxZT4TXpH+MpgnVfgERoBdqy3vXtPbPPGdNyovgrhXbs+7U5eROU2b5b2 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)(48200799018)(61400799027)(32142699015)(376014)(9140799003)(5113699003)(4113699003)(18002099003)(55112099003)(16102099003)(22082099003)(10067099003)(13003099007)(4143699003)(5063699009)(56012099006);DIR:OUT;SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: c3jgLRwzsxbUb9bqEK2qjQWNYuIN6bEZl2Gd58xJlkw6v3HB/ceX+2vKoNzgxWYpI2OuPs4CvDt5VZ74Ak+/rNsbTsUjIm0802lPpaaffaSVSvGNDnlR6gf7hPibCP88M1E4R/iWuVVb/bO0UgfsdZxPF5y+zF+2NiLvdqE1nXA+bISq6frEGLJREVJqJOedNHFaeq8T10mkFzesi7CRizFRzHoYzh9iivNd89Wpqk0wZ+8Yy6SOF6nGjLSl6Imbk9UDEa3ZAj+w/uJ4nO5CzOVVIYsFvXj+Ml0pFJA/U6avxO4xXmrQtO2aEDVl99JfnWPmaT5tAyE+8mIeRk/ovJmoiki4I6CpncXCYBvrJmrEgAdWn7ueN+97ux4zJF/HTNM4Ry/wTh+EmnaaQPW3KjnCo5HnyDYsDVDlMQTgiRg0vi9UMSn0GZ/YJnn41I0r X-ExternalRecipientOutboundConnectors: 0bc7f26d-0264-416e-a6fc-8352af79c58f X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Oct 2026 16:38:34.7438 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: cc088ec4-fece-44ca-1372-08df23c843d1 X-MS-Exchange-CrossTenant-Id: 0bc7f26d-0264-416e-a6fc-8352af79c58f X-MS-Exchange-CrossTenant-AuthSource: AM1PEPF000C1ADB.eurprd02.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: Internet X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR08MB6520 Thank you for your contribution! Sashiko AI review found 2 potential issue(= s) to consider: - [Medium] usb-host: cpr_replay_all drops already-completed URBs, losing IN= data - [Low] usb-host: drain timeout path drops newly queued URBs without cancel= ling them -- commit 5a6ed65d6723a2887c0ae5c9eb6d765f5c531174 Author: Andrey Drobyshev usb-host: drain in-flight URBs across CPR #VSTOR-137800 This patch registers a vm_change_state handler that cancels and reaps all pending usb-host URBs when the VM stops for a CPR switchover. It also adds a pre_save fallback for VMs that were already paused, and cancels isochronous URBs on their own rings. > diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c > --- a/hw/usb/host-libusb.c > +++ b/hw/usb/host-libusb.c [ ... ] > +static bool usb_host_req_cpr_replay(USBHostRequest *r, > + struct libusb_transfer *xfer) > +{ > + if (!r->host->cpr_drained) { > + /* no CPR drain in progress: normal completion */ > + return false; > + } > + if (!r->host->cpr_replay_all) { > + if (xfer->status !=3D LIBUSB_TRANSFER_CANCELLED) { > + return false; > + } > + if (xfer->actual_length !=3D 0) { > + /* deliver as a short transfer */ > + xfer->status =3D LIBUSB_TRANSFER_COMPLETED; > + if (!r->in) { > + r->p->actual_length =3D xfer->actual_length; > + } > + return false; > + } > + } > + return true; > +} [Severity: Medium] When cpr_replay_all is set, the status and actual_length checks are skipped and usb_host_req_cpr_replay() returns true for every reaped URB. Does this also discard URBs that the kernel already completed with LIBUSB_TRANSFER_COMPLETED and actual_length > 0? For an IN endpoint, the data the device already returned would be dropped. The target then replays the TD from its start, so that data appears to be lost. A completed OUT or control request could be sent a second time. The commit message says "A transfer that already completed in the kernel is delivered normally". That only holds when cpr_replay_all is false. The message also says the fallback suppresses every URB, so the two statements seem to disagree. Could the commit message be made consistent with the code, or could completed URBs be delivered in the replay_all case too? The path is: the VM is already paused when a CPR migration starts, so the FINISH_MIGRATE handler never runs. usb_host_pre_save() then sets cpr_replay_all and calls usb_host_cpr_quiesce(). A URB that completed in the kernel but has not been reaped yet then goes through usb_host_req_complete_data() or usb_host_req_complete_ctrl() and is suppressed. Later patches in the series do not change this. The final usb_host_req_cpr_replay() still returns true for every reap when cpr_replay_all is set. The later patch "usb-host: re-issue drained URBs if CPR is aborted" only adds replay for an aborted migration. Its own comment says the paused-VM fallback queues partially transferred OUT URBs and may send them twice. [ ... ] > +static int usb_host_cpr_drain_xfers(USBHostDevice *s) > +{ [ ... ] > + while (!QTAILQ_EMPTY(&s->requests)) { > + struct timeval tv =3D { .tv_usec =3D 2500 }; > + > + QTAILQ_FOREACH_SAFE(r, &s->requests, next, rtmp) { > + libusb_cancel_transfer(r->xfer); > + } > + libusb_handle_events_timeout(ctx, &tv); > + if (--limit =3D=3D 0) { > + /* > + * The device won't release these URBs. Error the packets o= ut so > + * the resumed guest retries them, leak the requests (libusb= still > + * owns their transfers) and abort the migration. > + */ > + QTAILQ_FOREACH_SAFE(r, &s->requests, next, rtmp) { > + usb_host_req_complete_err(r, USB_RET_IOERROR); > + } > + usb_host_drop_requests(s); > + return -1; > + } > + } [Severity: Low] On the 100-iteration timeout, can this path leave URBs live that were never cancelled? usb_host_req_complete_err() calls usb_packet_complete(), which can start the next queued packet: usb_host_cpr_drain_xfers() usb_host_req_complete_err() usb_packet_complete() usb_process_one() usb_host_handle_data() usb_host_req_alloc() /* new request appended to s->requests */ usb_host_drop_requests() then unlinks every request on s->requests, including the newly queued ones. Those were never passed to libusb_cancel_transfer(), and their packets stay in USB_PACKET_ASYNC. The URBs would stay live in the kernel on the preserved fd, which is what this patch is meant to prevent. Their late completion callbacks would run with r->host still set, so they could touch stale state if the device is closed or unrealized first. This is an error path on a migration that is already failing. It mirrors the existing leak in usb_host_reap_xfers(). Later patches, including "usb-host: re-issue drained URBs if CPR is aborted", do not change this timeout path. The final code still calls usb_host_req_complete_err() followed by usb_host_drop_requests(). [ ... ] > + limit =3D 2 * s->iso_urb_count * s->iso_urb_frames; > + while (usb_host_iso_inflight(s)) { > + struct timeval tv =3D { .tv_usec =3D 1000 }; > + > + QTAILQ_FOREACH(ring, &s->isorings, next) { > + QTAILQ_FOREACH(xfer, &ring->inflight, next) { > + libusb_cancel_transfer(xfer->xfer); > + } > + } > + libusb_handle_events_timeout(ctx, &tv); > + if (limit-- =3D=3D 0) { > + return -1; > + } > + } [Severity: Low] Does the iso timeout path also return with iso URBs still in flight and the rings still allocated? On limit-- =3D=3D 0, usb_host_cpr_drain_xfers() retu= rns -1 before usb_host_iso_free_all(). The cancelled iso URBs may remain on ring->inflight. [ ... ] > + if (usb_host_cpr_quiesce(s, mode) < 0) { > + migration_file_set_error(-EIO, NULL); > + } [Severity: Low] In usb_host_cpr_vm_state(), the drain failure is reported with migration_file_set_error(-EIO, NULL). Since the Error is NULL, no diagnostic reaches the user. Could an Error describing the usb-host drain failure be passed instead? [ ... ] > + if ((mode =3D=3D MIG_MODE_CPR_EXEC || mode =3D=3D MIG_MODE_CPR_TRANS= FER) && > + s->dh && !s->cpr_drained) { > + s->cpr_replay_all =3D true; > + if (usb_host_cpr_quiesce(s, mode) < 0) { > + return -1; > + } > + } [ ... ] --=20 Sashiko AI review =C2=B7 https://sashiko.vzint.dev/#/patchset/20261006-usb-= host_v3-v8-0-72ae4fb8977b@virtuozzo.com?part=3D6