From nobody Sun Feb 8 10:40:41 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.libvirt.org designates 8.43.85.245 as permitted sender) client-ip=8.43.85.245; envelope-from=devel-bounces@lists.libvirt.org; helo=lists.libvirt.org; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of lists.libvirt.org designates 8.43.85.245 as permitted sender) smtp.mailfrom=devel-bounces@lists.libvirt.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.libvirt.org (lists.libvirt.org [8.43.85.245]) by mx.zohomail.com with SMTPS id 1726673294934602.7557486937754; Wed, 18 Sep 2024 08:28:14 -0700 (PDT) Received: by lists.libvirt.org (Postfix, from userid 996) id DD18F159A; Wed, 18 Sep 2024 11:28:13 -0400 (EDT) Received: from lists.libvirt.org (localhost [IPv6:::1]) by lists.libvirt.org (Postfix) with ESMTP id 118EE152D; Wed, 18 Sep 2024 11:26:45 -0400 (EDT) Received: by lists.libvirt.org (Postfix, from userid 996) id 316511462; Wed, 18 Sep 2024 11:26:39 -0400 (EDT) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.libvirt.org (Postfix) with ESMTPS id 8BFDB1459 for ; Wed, 18 Sep 2024 11:26:38 -0400 (EDT) Received: from mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-442-MLgO1LPzOxSqDrX_53b58Q-1; Wed, 18 Sep 2024 11:26:33 -0400 Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 12B2019560A7 for ; Wed, 18 Sep 2024 15:26:33 +0000 (UTC) Received: from vhost3.router.laine.org (unknown [10.22.17.190]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 77DC630001A1 for ; Wed, 18 Sep 2024 15:26:32 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on lists.libvirt.org X-Spam-Level: X-Spam-Status: No, score=-0.5 required=5.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE autolearn=unavailable autolearn_force=no version=3.4.4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1726673198; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=xgoedzSnpQYsUlRet5JLLV5mw1ATcqaGdB6lnZaFn5A=; b=H4YjMiTwLISUBDh/j0bvTmxVREjNvYMAQ/DONo7CBHPqLGZo1hqkElwUx1+adlzoCCGd3F GI3g7PYEgi2eDWE6xURggQd9RzuTbzy8AVt10rgkD014MU+CsHqQFktQ6jhxD95pfn7V1T MFpJVkhEGcH3KWaHSofhh/Ezm6r5rC4= X-MC-Unique: MLgO1LPzOxSqDrX_53b58Q-1 From: Laine Stump To: devel@lists.libvirt.org Subject: [PATCH 1/4] qemu: prevent unnecessarily failing live interface update Date: Wed, 18 Sep 2024 11:26:27 -0400 Message-ID: <20240918152630.875195-2-laine@redhat.com> In-Reply-To: <20240918152630.875195-1-laine@redhat.com> References: <20240918152630.875195-1-laine@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Message-ID-Hash: OXWC7IZ5HY6VFO45W6SABYAAE37TFTLF X-Message-ID-Hash: OXWC7IZ5HY6VFO45W6SABYAAE37TFTLF X-MailFrom: laine@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; header-match-config-2; header-match-config-3; header-match-devel.lists.libvirt.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header X-Mailman-Version: 3.2.2 Precedence: list List-Id: Development discussions about the libvirt library & tools Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZM-MESSAGEID: 1726673296700116600 Content-Type: text/plain; charset="utf-8"; x-default="true" Attempts to use update-device to modify just the link state of a guest interface were failing due to a supposed attempt to modify something in the interface that can't be modified live (even though the only thing that was changing was the link state, which *can* be modified live). It turned out that this failure happened because the guest interface in question was type=3D'network', and the network in question was a 'direct' network that provides each guest interface with one device from a pool of network devices. As a part of qemuDomainChangeNet() we would always allocate a new port from the network driver for the updated interface definition (by way of calling virDomainNetAllocateActualDevice(newdev)), and this new port (ie the ActualNetDef in newdev) would of course be allocated a new host device from the pool (which would of course be different from the one currently in use by the guest interface (in olddev)). Because direct interfaces don't support changing the host device in a live update, this would cause the update to fail. The solution to this is to realize that as long as the interface doesn't get switched to a different network as a part of the update, the network port information (ie the ActualNetDef) will not change as a part of updating the guest interface itself. So for sake of comparison we can just point the newdev at the ActualNetDef of olddev, and then clear out one or the other when we're done (to avoid a double free or, more likely, attempt to reference freed memory). (If, on the other hand, the name of the network has changed, or if the interface type has changed to type=3D'network' from something else, then we *do* need to allocate a new port (actual device) from the network driver (as we used to do in all cases when the new type was 'network'), and also indicate that we'll need to replace olddev in the domain with newdev (because either of these changes is major enough that we shouldn't just try to fix up olddev) Resolves: https://issues.redhat.com/browse/RHEL-7036 Signed-off-by: Laine Stump --- src/qemu/qemu_hotplug.c | 92 +++++++++++++++++++++++++++++++++++------ 1 file changed, 80 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 75b97cf736..a187466c5b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3675,6 +3675,7 @@ qemuDomainChangeNet(virQEMUDriver *driver, virDomainNetDef **devslot =3D NULL; virDomainNetDef *olddev; virDomainNetType oldType, newType; + bool actualSame =3D false; bool needReconnect =3D false; bool needBridgeChange =3D false; bool needFilterChange =3D false; @@ -3895,15 +3896,49 @@ qemuDomainChangeNet(virQEMUDriver *driver, * free it if we fail for any reason */ if (newdev->type =3D=3D VIR_DOMAIN_NET_TYPE_NETWORK) { - if (!(conn =3D virGetConnectNetwork())) - goto cleanup; - if (virDomainNetAllocateActualDevice(conn, vm->def, newdev) < 0) - goto cleanup; - } + if (olddev->type =3D=3D VIR_DOMAIN_NET_TYPE_NETWORK + && STREQ(olddev->data.network.name, newdev->data.network.name)= ) { + /* old and new are type=3D'network', and the network name + * hasn't changed. In this case we *don't* want to get a + * new port ("actual device") from the network because we + * can use the old one (since it hasn't changed). + * + * So instead we just duplicate *the pointer to* the + * actualNetDef from olddev to newdev so that comparisons + * of actualNetDef will show no change. If the update is + * successful, we will clear the actualNetDef pointer from + * olddev before destroying it (or if the update fails, + * then we need to clear the pointer from newdev before + * destroying it) + */ + newdev->data.network.actual =3D olddev->data.network.actual; + memcpy(newdev->data.network.portid, olddev->data.network.porti= d, + sizeof(newdev->data.network.portid)); + actualSame =3D true; /* old and new actual are pointing to sam= e object */ + } else { + /* either the olddev wasn't type=3D'network', or else the + * name of the network changed. In this case we *do* want + * to get a new port from the new network (because we know + * that it *will* change), and then if the update is + * successful, we will release the port ("actual device") + * in olddev. Or if the update is a failure, we will + * release this new port + */ + if (!(conn =3D virGetConnectNetwork()) + || virDomainNetAllocateActualDevice(conn, vm->def, newdev)= < 0) { + goto cleanup; + } =20 - /* final validation now that we have full info on the type */ - if (qemuDomainValidateActualNetDef(newdev, priv->qemuCaps) < 0) - goto cleanup; + /* final validation now that we have full info on the type */ + if (qemuDomainValidateActualNetDef(newdev, priv->qemuCaps) < 0) + goto cleanup; + + /* since there is a new actual, we definitely will want to + * replace olddev with newdev in the domain + */ + needReplaceDevDef =3D true; + } + } =20 newType =3D virDomainNetGetActualType(newdev); =20 @@ -4169,7 +4204,21 @@ qemuDomainChangeNet(virQEMUDriver *driver, =20 /* this function doesn't work with HOSTDEV networks yet, thus * no need to change the pointer in the hostdev structure */ - if (olddev->type =3D=3D VIR_DOMAIN_NET_TYPE_NETWORK) { + if (actualSame) { + /* olddev and newdev have both been pointing at the + * same actual device object. Now that we know we're + * going to use newdev and dispose of olddev, we clear + * olddev->...actual so it doesn't get freed by upcoming + * virDomainNetDefFree(olddev) (which would be + * catastrophic because it is still being used by + * newdev) + */ + olddev->data.network.actual =3D NULL; + + } else if (olddev->type =3D=3D VIR_DOMAIN_NET_TYPE_NETWORK) { + /* olddev had a port (actual device) and we aren't + * reusing it in newdev, so we need to release it + */ if (conn || (conn =3D virGetConnectNetwork())) virDomainNetReleaseActualDevice(conn, olddev); else @@ -4211,10 +4260,29 @@ qemuDomainChangeNet(virQEMUDriver *driver, * that the changes were minor enough that we didn't need to * replace the entire device object. */ - if (newdev && newdev->type =3D=3D VIR_DOMAIN_NET_TYPE_NETWORK && conn) - virDomainNetReleaseActualDevice(conn, newdev); - virErrorRestore(&save_err); + if (newdev) { + if (actualSame) { + /* newdev->...actual was previously pointing to the + * olddev->...actual, but we've decided to free newdev and + * continue using olddev. So we need to clear + * newdev->...actual to avoid freeing the actualNetDef while + * olddev is still using it. + */ + newdev->data.network.actual =3D NULL; =20 + } else if (newdev->type =3D=3D VIR_DOMAIN_NET_TYPE_NETWORK) { + /* we had allocated a new port (actual device) for newdev, + * but now we're not going to use it, so release it back to + * the network + */ + if (conn || (conn =3D virGetConnectNetwork())) + virDomainNetReleaseActualDevice(conn, newdev); + else + VIR_WARN("Unable to release network device '%s'", NULLSTR(= newdev->ifname)); + } + } + + virErrorRestore(&save_err); return ret; } =20 --=20 2.46.0