From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from AM0PR02CU008.outbound.protection.outlook.com (mail-westeuropeazon11023131.outbound.protection.outlook.com [52.101.72.131]) by lore.virtuozzo.com (Postfix) with ESMTPS id 612318017D for ; Tue, 6 Oct 2026 15:45:41 +0000 (UTC) ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=Vh27i+OKAdipyRugPwlU1ZEZE8UP6ZZj6XPzokkrdXJv/ke4y+qMb/WBAYH8jA9W5hlk9DuF0Dlz9OHhp7ThIBW2MhxpP9siItCNJ0oKVXtzpNF1D7moj440R0ST7yDjKP0WKwW8UBTbAmG2Eqr7rVag2gACeK2IaPqYXZ5opvA87Yw+PSGmBJBsqmsvutKNVtIreF/C1r9OILnLVHTOsGvVOvw/boZonFDlmnFSsjtDm5FzhM549F29X/zCaPGdeOEDpEDPxiWq1Yz7tDrHFZy0dx6OTE5e2qanR6rLWzMlz0dTk/D46xUjbrN2b+aWp1MleX/LjnU1lN1LKzSFag== 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=herfOG2gtBabmFq3O39eSbHosgIZ9ApQKfJKcdc8KCI=; b=WwTmCAdOLNASNBbfO6HbukeAleQlQ50mJ1mA22nIKfXIp55tV6KqnFtmDAs9Di/aT4rZUwzNdbuwIU+l8J4TnvnNOg2ydbV1VJZZMwj2vreHkcett7RwFt858cmi+L2FqfuNY5tfDj3hTIJseB0HGTx5yPKyThiLJHsGn/GeMjhESbbYq5j5x8CLMan7zzvrIQS5KIn2FmUsMU7Llxlw+kZUFb8fI5Ojs6Z5WpACAfQhDgkO9cXTaRLbX8iKhdgl7R2tai9jrPrMMoGHegH6TgWIhtmIQiO+Teq6USxdhm2SmNoV5I6lv5JH3pmvPRAzpf25bxt9T2GTIktHcxyYJQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 69.169.224.6) 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=herfOG2gtBabmFq3O39eSbHosgIZ9ApQKfJKcdc8KCI=; b=NcrA1QEgGPE6TBa4ReEHHGduQK2UORQaJFXSb6325Qx0R7n42tvbnjToHnEtZ+DEurkqiC/6hRzMm8OGN+73Cf9ngaFGyeOnE3hDPAk+41e0UCq23rkiZhFIxIOiNJyFZ37LE6oPFmZ5uJMjCUiNH8AHBkU70MbKCwCYSIDR1OM= Received: from DB9PR05CA0006.eurprd05.prod.outlook.com (2603:10a6:10:1da::11) by GV1PR08MB10955.eurprd08.prod.outlook.com (2603:10a6:150:1f0::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.21.496.14; Tue, 6 Oct 2026 15:45:33 +0000 Received: from DU2PEPF00028CFE.eurprd03.prod.outlook.com (2603:10a6:10:1da:cafe::3b) by DB9PR05CA0006.outlook.office365.com (2603:10a6:10:1da::11) with Microsoft SMTP Server (version=TLS1_3, cipher=TLS_AES_256_GCM_SHA384) id 15.21.472.21 via Frontend Transport; Tue, 6 Oct 2026 15:45:33 +0000 Authentication-Results: mx.microsoft.com 1; spf=pass (sender IP is 69.169.224.6) 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.6 as permitted sender) receiver=protection.outlook.com; client-ip=69.169.224.6; helo=b224-6.smtp-out.eu-central-1.amazonses.com; pr=C Received: from b224-6.smtp-out.eu-central-1.amazonses.com (69.169.224.6) by DU2PEPF00028CFE.mail.protection.outlook.com (10.167.242.182) with Microsoft SMTP Server (version=TLS1_3, cipher=TLS_AES_256_GCM_SHA384) id 15.21.472.14 via Frontend Transport; Tue, 6 Oct 2026 15:45:33 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=23yjbh4k34hwzfubhuiim7m4inbd3ick; d=sashiko.virtuozzo.com; t=1791301532; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Content-Transfer-Encoding:Date:Message-ID; bh=07Mu+Pocg1O+gOQ8pgkHzrTbKr3+0WQ0QvrbFF81zpw=; b=zfLn1PLnDQQZNsEc7ZORHy9/PWZfOL2AnbikGWtUFaWPIsP328lyK8qnyOodCPs0 Z97au/e01Ze3GpucnPautlRgySAgBxmEkx2XXOai8d6Oi6KCB+h7gwMGtAMnhnhp3Y5 70ybzf/KpWprrdUHWfloyUqHGJHxddDxbWmXrsRwHgf2gxyJgdG/6crH16qlzaP6Hjf br79MYtvZi0mty6VUaka0VGAU7iXCZL9GwvtxEVdzt/7uBMu5OpVma95PlD4GMSphNE OM3uZiNPVjHmeH0SmaKZd5dB6Fmj4WHmlZdSDSQMdWulEias3Z7o/UOeb6v4j3zAH5+ ogzKYAehIA== DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=sokbgaaqhfgd6qjht2wmdajpuuanpimv; d=amazonses.com; t=1791301532; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Content-Transfer-Encoding:Date:Message-ID:Feedback-ID; bh=07Mu+Pocg1O+gOQ8pgkHzrTbKr3+0WQ0QvrbFF81zpw=; b=GmC/mcWwJBsMK/XESAW4EsCDd+GeeA2imCQFXOeZAnOSUo+/VGqer1J4x2hMU4lI 0MSq/KhoaL3wYUN3TEdD5QBEBsKJCYYYMAHKhkCzIbUkghy3tm3ruAAo8flFAT+AADf nfRpvyG6kuGHgmA0RHHZBmQFH4iOoPiA/YpzoAMM= From: no-reply@sashiko.virtuozzo.com Subject: Re: [PATCH QEMU HCI-8.0 v7 06/14] usb-host: drain in-flight URBs across CPR #VSTOR-137800 To: svt-core@virtuozzo.com, Andrey Drobyshev Cc: den@openvz.org, andrey.drobyshev@virtuozzo.com In-Reply-To: <20261006-usb-host_v3-v7-6-044f668789a1@virtuozzo.com> References: <20261006-usb-host_v3-v7-0-044f668789a1@virtuozzo.com> <20261006-usb-host_v3-v7-6-044f668789a1@virtuozzo.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Tue, 6 Oct 2026 15:45:32 +0000 Message-ID: <010701a111e43bdd-0d96d6f4-b25d-4d22-9f27-a9c0df194465-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.6 X-EOPAttributedMessage: 0 X-EOPTenantAttributedMessage: 0bc7f26d-0264-416e-a6fc-8352af79c58f:0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DU2PEPF00028CFE:EE_|GV1PR08MB10955:EE_ MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: b691192b-ea2a-48b7-5563-08df23c0db8b 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|9140799003|61400799027|376014|48200799018|32142699015|5023799004|4113699003|5113699003|56012099006|5063699009|4143699003|6133799003|10067099003|22082099003|13003099007|16102099003|18002099003|55112099003; X-Microsoft-Antispam-Message-Info: GigfDVL524gUy1xWygd/D0eJjqO5kzE9OrptknvhuIztuJ3tZHGInJhtsakYOGBfXeY6KNfOGwq82/S9NRTVAtqSf6b3y0sJ4Ss1KGBXDZ4bLGUTJ6W2P0+z0wSWALu1/2kwnewOdnW9wZNbQiEmF27qQ/ylOCLOw0WIPUjOBVdGdfG4xupuw6WMMSTzPHnf+ODzW/x82FKAo6uM/mzhPww3K6MYMPydPyxHATKkWdW5hVmLg9RwwY7VJHSYqJCUa3BgVGwzkeZ0S1dwhDmm3IETSijNghmvxfUYBMGwmQX6Lo/XJs3By/SQilsApGDKP1K9KQWSioSF8u0xaFfks+EQr7fJnURsed+vwyBJ2Z3kWaskuPqA5CwmXQJ0LZSYKy9jeWqUOeKXeyXQd7nLJ5+Ul6Sxg7eyg4JlSn+Xuz9F+/W3xYMsIIXrfDc4NNXQ9NQJZEBNzmnefiHskrw2Glqh21LB4DtMdwIw1AoJCQ85ES6kcf/9qg3yQGDsHmSLviyCW555mZ06SIptLylvon9q2YbUexhO73KTGcPp4RD0YjOlkZ8SsQJQXfV/sIwiqd3CLzOtrJjLRLAhadlZBeAfJCMecHqulzqxqdMEe1wZukU+7t7KYvKsgtWXbTe7gKdESiL3eMtOQcbAQEN2MGWF28jQ76azAA66xeHCs5WTiFXMsSXE6ySrcZQ/1+ny75TJAaDsVb40194BwGj5LIPfAoylaH1t/U7JEi0aEvaobdEcw6MAjEntTKO1qfcGvfFqqJFpZ+/C/S8XSAafscBJLH41UfDbMEfbFbK07b6c0dm/agdVt7rkKhAf6ZtFsrppVj52OacUrnWKShoLB4oE65UqNx52Ea2xOZTtApA= X-Forefront-Antispam-Report: CIP:69.169.224.6;CTRY:US;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:b224-6.smtp-out.eu-central-1.amazonses.com;PTR:b224-6.smtp-out.eu-central-1.amazonses.com;CAT:NONE;SFS:(13230040)(704161411799003)(9140799003)(61400799027)(376014)(48200799018)(32142699015)(5023799004)(4113699003)(5113699003)(56012099006)(5063699009)(4143699003)(6133799003)(10067099003)(22082099003)(13003099007)(16102099003)(18002099003)(55112099003);DIR:OUT;SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: s03i+8UNyKJMMfx1VMhCoGR/tUamyMpbogFTfYxkPvgoliUh1oFa9u289Z426j0KzVaMgIyTj9JnIKLNAr4iBHh801We3CHMly9R6pgGKszB0dZ9g5II70SM+k6rmjrhMUeC/BC3AHLwuDYaAnko3JI3Sd5x02QlzvOfku0dd92fbGfi8cqXufO7No9OsJLxyWSA8NGxJ6xWPUwD9gJzbV58Jx+Qjrh52lWCGMv+6/G36DaWEKV1mM2UPnSGz//WjJo85PTNOJK+DSaoqJujoUtDNPKBtlXo1P6vSquWWVOa7WjJQw9AzwAbyAU8DXe0osT7vR+ljIBa5scnamiPp/Nld35SVRYK5yGEi8LBOw9qt+EzexEiQSlfI6sFszepqh+8ZBrfw6RiEYDBQt2ebt6K8FJkRDnlfVz/XFWZTq9dHYR9cRRNtVk+kWrWc+4l X-ExternalRecipientOutboundConnectors: 0bc7f26d-0264-416e-a6fc-8352af79c58f X-Auto-Response-Suppress: DR, OOF, AutoReply X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Oct 2026 15:45:33.3060 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: b691192b-ea2a-48b7-5563-08df23c0db8b X-MS-Exchange-CrossTenant-Id: 0bc7f26d-0264-416e-a6fc-8352af79c58f X-MS-Exchange-CrossTenant-AuthSource: DU2PEPF00028CFE.eurprd03.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: Internet X-MS-Exchange-Transport-CrossTenantHeadersStamped: GV1PR08MB10955 Thank you for your contribution! Sashiko AI review found 4 potential issue(= s) to consider: - [Medium] usb-host: CPR drain suppresses completed URBs and reports short = OUT as full - [Medium] usb-host: iso drain cancels in-flight URBs once, so resubmitted = URBs survive - [Medium] usb-host: timeout path completes packets mid-iteration then drop= s live URBs - [Medium] usb-host: usb_host_req_cpr_replay() not gated on an active CPR d= rain -- commit e57108ef005a6c96c6eda4382d78b25d2524c377 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 CPR. It also adds a .pre_save() fallback for a VM that is already paused, and cancels and reaps isochronous URBs on their own rings. Transfers that moved no data are suppressed so the target can replay them. > 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 [ ... ] > +/* > + * Classify a reap during the CPR drain (r->p is still set). The paused= -VM > + * fallback (cpr_replay_all) can't deliver completions, so it suppresses= every > + * reap for the target to replay; otherwise one that moved no data is su= ppressed > + * and one that moved some is kept as a short transfer. Returns true if > + * suppressed, so the caller frees the request. > + */ > +static bool usb_host_req_cpr_replay(USBHostRequest *r, > + struct libusb_transfer *xfer) > +{ > + 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; > + return false; > + } > + } > + return true; > +} [Severity: Medium] Does this function lose data or duplicate side effects across CPR? In the paused-VM fallback, usb_host_pre_save() sets cpr_replay_all =3D true. usb_host_req_cpr_replay() then skips its status check and returns true for every reap, including LIBUSB_TRANSFER_COMPLETED ones. The packet is suppressed and replayed by the target. An IN URB that had already received data loses it, since the device will not resend. A bulk OUT or control URB that already executed is sent again. This also seems to contradict the commit message statement "A transfer that already completed in the kernel is delivered normally". The final series state still has this behaviour. The later patch "usb-host: re-issue drained URBs if CPR is aborted" only documents it in a comment ("an OUT fragment the device consumed may be sent twice") and does not change it. On the normal vm_change_state path, a CANCELLED URB with actual_length !=3D= 0 is rewritten to COMPLETED: if (xfer->actual_length !=3D 0) { /* deliver as a short transfer */ xfer->status =3D LIBUSB_TRANSFER_COMPLETED; return false; } usb_host_req_complete_data() only trims the packet length for IN: r->p->status =3D status_map[xfer->status]; if (r->in && xfer->actual_length) { usb_packet_copy(r->p, r->buffer, xfer->actual_length); } For an OUT packet, p->actual_length was already set to the full length when the guest data was copied at submit time. Can the guest be told that the whole OUT TD succeeded when the device received only xfer->actual_length bytes, with nothing replaying the remainder? [Severity: Medium] Separately, is usb_host_req_cpr_replay() meant to apply outside an active CPR drain? usb_host_req_complete_ctrl() and usb_host_req_complete_data() now call it unconditionally for every completion with r->p !=3D NULL. With cpr_replay_all false, nothing checks cpr_drained or any other drain-active state. A transfer that arrives with LIBUSB_TRANSFER_CANCELLED outside a CPR drain would behave differently: - actual_length =3D=3D 0: the request is freed and the packet is never completed, which could leave it in USB_PACKET_ASYNC. - actual_length !=3D 0: the status is rewritten to COMPLETED. Previously such a transfer was mapped by status_map[] to USB_RET_IOERROR. Guest-initiated cancels clear r->p in usb_host_cancel_packet() and usb_host_req_abort(), so they are not affected. I could not confirm that libusb reports CANCELLED for a transfer with a live packet outside CPR, so this one is speculative. Should the logic be gated on an active drain? [ ... ] > +static int usb_host_cpr_drain_xfers(USBHostDevice *s) > +{ [ ... ] > + 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: Medium] Can this timeout path leave live, uncancelled URBs behind? The call chain is: usb_host_cpr_drain_xfers() usb_host_req_complete_err() usb_packet_complete() usb_process_one() usb_host_handle_data() libusb_submit_transfer() For a non-ep0 packet, usb_packet_complete() pops the next queued packet on the endpoint and processes it. That can allocate a new request, append it to s->requests, and submit a new URB while QTAILQ_FOREACH_SAFE is running. usb_host_drop_requests() then unlinks every request, including the newly submitted ones, without cancelling them. Would such a URB stay on the preserved fd with no tracking request, and later complete into a callback with stale r->p or r->host? This needs the 100-iteration timeout plus a queued packet behind the failed one on the same endpoint, which xHCI can have. [ ... ] > + QTAILQ_FOREACH(ring, &s->isorings, next) { > + QTAILQ_FOREACH(xfer, &ring->inflight, next) { > + libusb_cancel_transfer(xfer->xfer); > + } > + } > + > + /* > + * Cap the reap at 2x a full ring: > + * iso_urb_count URBs x iso_urb_frames packets each > + */ > + 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 }; > + libusb_handle_events_timeout(ctx, &tv); > + if (limit-- =3D=3D 0) { > + return -1; > + } > + } > + usb_host_iso_free_all(s); > + return 0; > +} [Severity: Medium] Can iso URBs be resubmitted while this reap loop runs, and then escape cancellation? The iso URBs on ring->inflight are cancelled in a single pass, and the reap loop does not cancel again. For an IN endpoint, usb_host_req_complete_iso() moves the xfer to ring->copy and calls usb_wakeup(): QTAILQ_INSERT_TAIL(&xfer->ring->copy, xfer, next); usb_wakeup(xfer->ring->ep, 0); usb_wakeup() reaches xhci_wakeup_endpoint(), which calls xhci_kick_ep() and xhci_kick_epctx() without checking the VM run state. The kick can reprocess an iso IN packet through usb_host_handle_data() and usb_host_iso_data_in(), which moves ring->unused entries to inflight with libusb_submit_transfer(). If that happens, the reap loop either runs out its limit and returns -1, or it exits with new iso URBs from the old process still submitted on the preserved fd. The control/bulk loop above re-cancels on every iteration, but this one does not. I did not prove that a guest iso TD is always available at that point. Should the iso phase re-cancel inflight URBs on each iteration as well? --=20 Sashiko AI review =C2=B7 https://sashiko.vzint.dev/#/patchset/20261006-usb-= host_v3-v7-0-044f668789a1@virtuozzo.com?part=3D6