From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <010701a110d492d3-d9d7ad01-48bb-44b3-af1b-663730a00e2f-000000@eu-central-1.amazonses.com> Received: from GVXPR05CU001.outbound.protection.outlook.com (mail-swedencentralazon11023075.outbound.protection.outlook.com [52.101.83.75]) by lore.virtuozzo.com (Postfix) with ESMTPS id BA9A6802A7 for ; Tue, 6 Oct 2026 10:48:59 +0000 (UTC) ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=Np1qkIJIDvigZZhmZgGcmR6zHURY04u7Pg981PHeoE4rk64XBD13DCU4HrRfrDD+/iR7V018rJkXMQepvhzH2mXQMJ0nJPC37fVwrPIW/oO1+8El4c9K5aJmDjX2/Tf7CkQg4VayMNKZnOcnvxVHTwfy91zOMtjp3Q7b/uinqiOq5Ng07r1BLo6munh3pdGatgyg2gGD9+Se9EPGnirSAtVqu3V5qYIlot7u5rrMmkyJkJdDkIf0hLqvDfIxWuZr39TheYgAveD9FHaHVq+QhrVH6gbbkXB5mWyIpXz8RGS3bihVMCzWuVrGCEPm+N55CJfl+q0rbfC6+NXIiqssaw== 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=ujPqeq09EEZXOMBBkFtpg/vf3FUm1qwDzDNM1PvnxpI=; b=I52cuy1NHrMR6VkGKkkse3tAMaml2eik7Jgp1ad7CGS/7bZdXRHG5iD078pY9Oqp+lbrC1JiJcQVMYEn3h5K9qcVjTbqU4HLyWUKNdC1BP4g3118zVHgMOC6Wxvj3BeI83ZnJp2XJ6bPfdz92T49UxyUDb1ff8fuDVovFIvPRVMtv54BHXnHIPqw+2LjiPtx0/H0tT1wkTVlyLcfG25Ol0yGHuUtLd5j44WrAdl4adasZLczP/GTkhUfYMXefJEEUM4Xw1QqB19FPoq9ZvhU1387TPavbcZecbHRXF/5bqqBlL2Io8UeeowIeHDen/Hh2VSa4j8oD0cKt8GMVIT1Ag== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 69.169.224.2) 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=ujPqeq09EEZXOMBBkFtpg/vf3FUm1qwDzDNM1PvnxpI=; b=LdVqqrLaG88lRMbo1C+cXpOfHIj5kWLDd2FYigcY/9jQf7AZMqvulKUzwRsDG6Km5mXbMsiIjyHqnK2cfF7DZx0JL8uJY0uNDxkuQacuPNdnl1IDjVee14uopb8mNSuq7jHAH9rAinq7XgrUlqA1te1YUnn075QAdVAefe8/91E= Received: from PA7P264CA0036.FRAP264.PROD.OUTLOOK.COM (2603:10a6:102:34b::9) by GVXPR08MB10497.eurprd08.prod.outlook.com (2603:10a6:150:156::7) 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 10:48:50 +0000 Received: from MI3PEPF00008480.eurprd02.prod.outlook.com (2603:10a6:102:34b:cafe::1c) by PA7P264CA0036.outlook.office365.com (2603:10a6:102:34b::9) 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 10:48:49 +0000 Authentication-Results: mx.microsoft.com 1; spf=pass (sender IP is 69.169.224.2) 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.2 as permitted sender) receiver=protection.outlook.com; client-ip=69.169.224.2; helo=b224-2.smtp-out.eu-central-1.amazonses.com; pr=C Received: from b224-2.smtp-out.eu-central-1.amazonses.com (69.169.224.2) by MI3PEPF00008480.mail.protection.outlook.com (10.167.240.167) 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 10:48:49 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=23yjbh4k34hwzfubhuiim7m4inbd3ick; d=sashiko.virtuozzo.com; t=1791283729; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Content-Transfer-Encoding:Date:Message-ID; bh=3ucUiG8jGyVi5gerFDb28aclL87h5H/eMnDS5ZWPry8=; b=zuSPmjBnFEIZxg7fT9bmSi56tSvz2q6FSNc2ly+guzQe7NBU/RvOVsQww3ObrHBz 8RefJJ4c3EZi3eJCcPf71s28zueibR8LJHe1CAl2FXgg8LQ/2FE9YJvx2Ixhd2iWeUl ZA5SolkD3cx7y26V/2UnSptUCbHhaFROd9xzZ+kaftfIXHFymYRnyyTTBxppgxAsXqN X9/YhcIYQjBpO+h6T7jEWFJ815twsD9zTTx0hKQ5x6L5bh+mPwc4FwEiuIXmmCElLX/ LnYEisI1adPVjqsouuSvSORt/AW3jZkPODQkfuLg5Wy1lRXczrRJPik07Iy6X0RuQzA kfJox04FtA== DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=sokbgaaqhfgd6qjht2wmdajpuuanpimv; d=amazonses.com; t=1791283729; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Content-Transfer-Encoding:Date:Message-ID:Feedback-ID; bh=3ucUiG8jGyVi5gerFDb28aclL87h5H/eMnDS5ZWPry8=; b=dBMNltKJYbYlhDTa2Jm0rbVOnvtJT2bc6ljmBTzlKL2kqV3Akd6QaGQiLSPT5lEF 19koJfZ+ytesNOGcfADXBExeLETmS52TyohZLmn9EFVpoCil9ovG0Rs0pZurvtVilIZ D41KqJ9IVSb8Nh9m2I/EM20BayJ8tr5UkWKXGabk= From: no-reply@sashiko.virtuozzo.com Subject: Re: [PATCH QEMU HCI-8.0 v6 05/12] 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-v6-5-8688ec9576d1@virtuozzo.com> References: <20261006-usb-host_v3-v6-0-8688ec9576d1@virtuozzo.com> <20261006-usb-host_v3-v6-5-8688ec9576d1@virtuozzo.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Tue, 6 Oct 2026 10:48:49 +0000 Message-ID: <010701a110d492d3-d9d7ad01-48bb-44b3-af1b-663730a00e2f-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.2 X-EOPAttributedMessage: 0 X-EOPTenantAttributedMessage: 0bc7f26d-0264-416e-a6fc-8352af79c58f:0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: MI3PEPF00008480:EE_|GVXPR08MB10497:EE_ MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: bbbe9cf6-d6b7-4a31-fa02-08df239767af 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|32142699015|61400799027|9140799003|376014|48200799018|16102099003|22082099003|55112099003|18002099003|13003099007|4143699003|10067099003|5063699009|56012099006|5113699003|4113699003; X-Microsoft-Antispam-Message-Info: guGP//ElDNgIS85i0bv1Ite+yHHT/OTH+xe409mXfPMvWKLzcz8FbwB0CNdKcoDi3S2fN/BhgiKO66ngQiVs4L3spwKnzhYHE3xx1GusBmRxT7iyE+fRZZcQWRoUU78jfkdOfH0IccsydPuiVdarOdH33R529AoTV26RtTBkElTCFofgs9shvEVyMlJob31CNAk9jMLhqTG3cDIMa5HOVOTr6MVrlOgDQS4aIaMibOcBZ0vH78jjawEMkLiVgOYKs3EoJKXSM/pxh0byWJ6i33rPiplQVJahaoSrB1k9boQHLcCEq6U/XBzsnRP51wODdsK4FCrS/OgiIGLC3xpZPvA4/Ng8NgTeOhQ9FFVuDqhCFtEiLG8E3ebukaKpil1LFdTjHSD/oTM4vQQc9ClyiZE6TAQYr1+gTmWwcjUEM117nEplcioZ6LlZtUV6C8svocM6vw+WsB+f/4hkbBX/Ye4aFGd7YbF7e62gihiWzhYVol5Fg0dpF0z8TV0mYzyfJFuCGbdvylcNOeMWIGmz+7APKJR7AKdDka3X62/i9Rklt96GEgV5JsExlr2U7Xep2Pz1PwI/KeHhWZ7eodTq7CvnbKsgknZapSfH1sUz7wUhR2i70wGt0FMyBVuVibnfPMGfnfdR4vzQ7DTb97863iZ7wPhLW6Frg5MFOHDYicT5sPKu5kz6QPAfmp4H9q1LOhfCrAncQhlZ+SozM31W9mBdK4g6Wgy9XezcB618AVUxLkAPAqfo63s/gqcmh70Rlj8u4w3S3CCTwpUtSZkfPcHEsFkUf99RmORlGgmw4XxTmFUNpOxbWI5VLIafYx2RlYaLG0CjTLONGzHVvAyE8JgJXHiOPjWp7oULtoe5L0A= X-Forefront-Antispam-Report: CIP:69.169.224.2;CTRY:US;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:b224-2.smtp-out.eu-central-1.amazonses.com;PTR:b224-2.smtp-out.eu-central-1.amazonses.com;CAT:NONE;SFS:(13230040)(32142699015)(61400799027)(9140799003)(376014)(48200799018)(16102099003)(22082099003)(55112099003)(18002099003)(13003099007)(4143699003)(10067099003)(5063699009)(56012099006)(5113699003)(4113699003);DIR:OUT;SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: iVNy/9aZ/GcjllGN2/xNtcdbDnxxY+rcIac4vG7Fedy1IaNMiiY9ogqAk3M8PNYpl4sFGDSM8KiYjKQbddRCbip6V1RNwqyRwNv9LmDnjZ67IiZ/9JJi88PyuqtWlPG5wgkpSfQ2HQmAwJyj2DbOPLvf2x893YZi7teYvNMtA+obDu38o9WTrpqXDm1gMF4BY+o2OauyknkpUktTCu3dXPjJViM4Zg1CONge6Trb2h72SAS4IFQ/8isl4RmIHzt1hacZWMRqZm51JU3bXxqpvdjOxmZ//S8/85Pd/4CkxXsHsMbgX71rQyoOoppqccMH9CIhMpA1ac/FPPGxsugoiLSqQgLklmhfLaU9bn2YDrmMJFHntzzT9UMIUz3lSggfOm3O0WLlNhNja6wA92VcO5JTFFSDK6v0WGyT9lxsbse6tz4QjfM/e61O674SQlLC X-ExternalRecipientOutboundConnectors: 0bc7f26d-0264-416e-a6fc-8352af79c58f X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Oct 2026 10:48:49.5826 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: bbbe9cf6-d6b7-4a31-fa02-08df239767af X-MS-Exchange-CrossTenant-Id: 0bc7f26d-0264-416e-a6fc-8352af79c58f X-MS-Exchange-CrossTenant-AuthSource: MI3PEPF00008480.eurprd02.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: Internet X-MS-Exchange-Transport-CrossTenantHeadersStamped: GVXPR08MB10497 Thank you for your contribution! Sashiko AI review found 5 potential issue(= s) to consider: - [Medium] usb-host: xHCI replay of suppressed URBs relies on a dequeue poi= nter that is already advanced - [Medium] usb-host: pre_save fallback drain can deliver completions after = xHCI state is saved - [Medium] usb-host: iso drain cancels in-flight URBs once but completions = can resubmit new ones - [Medium] usb-host: a cancelled OUT or control URB that moved data is repo= rted as a short success - [Medium] usb-host: drain timeout path can leave newly submitted URBs live= and untracked -- commit e9cd7ce570abec8bcd0aa45a09ea274defa372c5 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 libusb transfers when the VM stops for CPR. It also adds a .pre_save() fallback for VMs that are already paused, and cancels and reaps isochronous URBs as well. Cancelled URBs 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). One that m= oved no > + * data is suppressed for the target to replay; one that moved some is k= ept 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 (xfer->status !=3D LIBUSB_TRANSFER_CANCELLED) { > + return false; > + } [Severity: Medium] The commit message says the .pre_save() fallback suppresses "every URB for the target to replay". Is that accurate? In usb_host_req_cpr_replay(), any status other than LIBUSB_TRANSFER_CANCELLED returns false, so a URB that completed in the kernel during the fallback drain is still delivered: usb_host_req_complete_data() -> usb_packet_complete() Can this run after the xHCI and interrupt controller state has already been saved? usb_host_pre_save() runs as part of device-state save, not before the snapshot as the vm-state handler does. If so, event ring, interrupter and endpoint state changed by that completion might be missing from the migrated image. > + if (xfer->actual_length !=3D 0 && !r->host->cpr_replay_all) { > + /* deliver as a short transfer */ > + xfer->status =3D LIBUSB_TRANSFER_COMPLETED; > + return false; > + } > + return true; > +} [Severity: Medium] Can this corrupt an OUT stream? The status is rewritten to LIBUSB_TRANSFER_COMPLETED for every direction and transfer type, with no check of r->in or the control stage. For a bulk or interrupt OUT, or a control write, that was cancelled after moving some bytes, the device has consumed a fragment. The guest is then told the transfer ended short, and the target can neither replay it nor see an error. A CBW or a data stage could be affected this way. Should this conversion be limited to IN transfers? [Severity: Medium] When usb_host_req_cpr_replay() returns true, the completion is suppressed and the request is freed. The replay then relies on the xHCI endpoint context in guest RAM still pointing at the start of this TD. Is that the case when several TDs are outstanding on one endpoint? xhci_kick_epctx() advances ring->dequeue via xhci_ring_fetch() for each TD it fires. xhci_complete() calls xhci_kick_epctx(), which calls xhci_set_ep_state(EP_RUNNING) at its start: xhci_complete() -> xhci_kick_epctx() -> xhci_set_ep_state() ctx[2] =3D ring->dequeue | ring->ccs; That writes the advanced dequeue to guest memory while later TDs are still queued or in flight. usb_xhci_post_load() rebuilds the ring from that value. Would a suppressed TD2 then be skipped on the target, so that its completion event is never generated? [ ... ] > +/* > + * Quiesce the device for a CPR switchover. Cancel and reap all URBs, > + * so that no URB submitted by this process can outlive it. > + */ > +static int usb_host_cpr_drain_xfers(USBHostDevice *s) > +{ [ ... ] > + 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] On this timeout path, can usb_host_drop_requests() orphan live URBs? usb_host_req_complete_err() calls usb_packet_complete(), which can process queued packets on the endpoint: usb_host_req_complete_err() -> usb_packet_complete() -> usb_process_one() -> usb_host_handle_data() / control handler -> libusb_submit_transfer() Those paths append new entries to s->requests. usb_host_drop_requests() then unlinks every entry without cancelling it, so the new URBs stay live on the preserved fd and are never reaped. > + /* > + * Iso URBs are on the rings, not s->requests. Cancel them: one in > + * flight when the guest stops may never complete on its own, and no= ne > + * may be left on the preserved fd. The target replays from the rin= g. > + */ > + 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; > + } > + } [Severity: Medium] Are the iso URBs cancelled only once here? Completing a cancelled IN iso URB runs usb_host_req_complete_iso(), which does: QTAILQ_INSERT_TAIL(&xfer->ring->copy, xfer, next); usb_wakeup(xfer->ring->ep, 0); That can reach xhci_wakeup_endpoint(), then a kick, then usb_host_iso_data_in(), which submits every xfer on ring->unused into ring->inflight. The loop above never re-cancels those, and the bulk loop earlier in usb_host_cpr_drain_xfers() has already finished, so it does not recheck s->requests. Can this either time out and return -1, which aborts the migration, or leave freshly submitted iso or bulk URBs live across exec? [ ... ] > +static int usb_host_pre_save(void *opaque) > +{ [ ... ] > + 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_drain_xfers(s) < 0) { > + return -1; > + } > + } [Severity: Medium] Is it safe to run the drain from usb_host_pre_save()? Packet completions raised here can modify xHCI state that may already have been serialized, depending on the order of the xHCI and usb-host vmstate entries in the save sequence. Could that ordering be confirmed, or the drain be moved earlier? --=20 Sashiko AI review =C2=B7 https://sashiko.vzint.dev/#/patchset/20261006-usb-= host_v3-v6-0-8688ec9576d1@virtuozzo.com?part=3D5