]> git.ipfire.org Git - thirdparty/libvirt.git/commitdiff
qemu: re-use existing ActualNetDef for more interface types during update-device
authorLaine Stump <laine@redhat.com>
Tue, 26 Nov 2024 03:51:04 +0000 (22:51 -0500)
committerLaine Stump <laine@redhat.com>
Fri, 13 Dec 2024 16:44:05 +0000 (11:44 -0500)
For the full history behind this patch, look at the following:

   https://issues.redhat.com/browse/RHEL-7036
   commit v10.7.0-101-ga37bd2a15b
   commit v10.8.0-rc2-8-gbcd5ae4e73

Summary: original problem was unexpected failure of update-device when
the user hadn't changed anything other than online status of the guest
NIC (which should always be allowed).

The first commit "fixed" this by avoiding the allocation of a new
ActualNetDef (i.e. creating a new networkport) for *all* network
device updates (because that was inappropriately changing which
ethernet physdev should be used for a macvtap connection, which by
design can't be handled in an update-device).

But this commit caused a regression for update-device of bridge-based
network devices (because some the updates of certain attributes *do*
require the ActualNetDef be re-allocated), so...

The 2nd commit narrowed the list of network types that get the "don't
allocate new ActualNetDef" treatment (so that only interfaces
connected to a network that uses a pool of ethernet VFs *being used in
passthrough mode* qualify).

But then it was pointed out that this re-broke simple updates of
devices that used a direct/macvtap network in "bridge" mode (because
it's possible to list multiple physdevs to use for bridge mode, in
which case the network driver attempts to "load balance" (and so a new
allocation might have a different ethernet physdev which, again, can't
be supported in a device-update).

So this (single line of code) patch *widens* the list of network types
that don't allocate a new ActualNetDef to also include the other
direct (macvtap) modes, e.g. bridge, private, etc.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
src/qemu/qemu_hotplug.c

index 3c18af6b0c5f4099a8f85c542ed90533464040a3..de0777d330fd606f007c9be451ca0601a0c22159 100644 (file)
@@ -3935,25 +3935,29 @@ qemuDomainChangeNet(virQEMUDriver *driver,
     if (newdev->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
         if (olddev->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
             oldType == VIR_DOMAIN_NET_TYPE_DIRECT &&
-            virDomainNetGetActualDirectMode(olddev) == VIR_NETDEV_MACVLAN_MODE_PASSTHRU &&
             STREQ(olddev->data.network.name, newdev->data.network.name)) {
             /* old and new are type='network', and the network name
-             * hasn't changed *and* this is a network where each
-             * connection is allocated exclusive use of a VF
-             * device. In this case we *don't* want to get a new port
-             * ("actual device") from the network because attempting
-             * to allocate a new device would also allocate a
-             * new/different VF, causing the update to fail. And
-             * anyway we can use olddev's actualNetDef (since it
-             * hasn't changed).
+             * hasn't changed *and* this is a "direct" network (a pool
+             * of 1 or more host ethernet devices where each guest
+             * interface is allocated one of those physical devices
+             * that it then connects to via macvtap). In this case we
+             * *don't* want to get a new port ("actual device") from
+             * the network because attempting to allocate a new port
+             * would also allocate a new/different ethernet (physical
+             * device), causing the update to fail (because the
+             * physical device of a macvtap-based interface can't be
+             * changed without completely unplugging and re-plugging
+             * the guest NIC).
              *
-             * 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)
+             * We can work around this issue by just re-using olddev's
+             * actualNetDef (since it hasn't changed) rather than
+             * allocating a new one.  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 = olddev->data.network.actual;
             memcpy(newdev->data.network.portid, olddev->data.network.portid,