After v9.1.0-rc1~116 we track whether it's us who created a
macvtap or not. But when updating a vNIC its definition might be
replaced with a new one (though, ifname is not allowed to
change), e.g. to reflect new QoS, link state, etc.

Now, the fact whether we created macvtap for given vNIC is stored
in net->privateData->created. And replacing definition is done by
simply freeing the old definition and making the pointer point to
the new one. But this does not preserve the 'created' flag, which
in turn means when a domain is shutting off, the macvtap is not
removed (see loop inside of qemuProcessStop()).

Copy this flag into new definition and leave a note in
_qemuDomainNetworkPrivate struct.

Fixes: 61d1b9e6592660121aeda66bf7adbcd39de06aa8
Resolves: https://issues.redhat.com/browse/RHEL-22714
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---

I also contemplated idea of preserving whole privateData, e.g.:

  virObjectUnref(newdev->privateData);
  newdev->privateData = g_steal_pointer(&olddev->privateData);

but then decided against it. Looks like a heavy gun, though in my
debugging all members but 'created' were zero/NULL at point of calling
qemuDomainChangeNet(). Then there is qemuDomainNetworkPrivateFormat()
and qemuDomainNetworkPrivateParse() combo, but in order to use it I'd
need to create and then parse an XML doc (to get 'ctxt') and that looks
too much work for very little gain.

Does somebody have any other idea?

 src/qemu/qemu_domain.h  | 2 ++
 src/qemu/qemu_hotplug.c | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index b4512cc80e..6ba3f10e8d 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -420,6 +420,8 @@ typedef struct _qemuDomainNetworkPrivate qemuDomainNetworkPrivate;
 struct _qemuDomainNetworkPrivate {
     virObject parent;
 
+    /* Don't forget to possibly copy these members in qemuDomainChangeNet(). */
+
     /* True if the device was created by us. Otherwise we should
      * avoid removing it. Currently only used for
      * VIR_DOMAIN_NET_TYPE_DIRECT. */
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0e45bd53e1..31b00e05ca 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4166,6 +4166,11 @@ qemuDomainChangeNet(virQEMUDriver *driver,
             else
                 VIR_WARN("Unable to release network device '%s'", NULLSTR(olddev->ifname));
         }
+
+        /* Carry over fact whether we created the device or not. */
+        QEMU_DOMAIN_NETWORK_PRIVATE(newdev)->created =
+            QEMU_DOMAIN_NETWORK_PRIVATE(olddev)->created;
+
         virDomainNetDefFree(olddev);
         /* move newdev into the nets list, and NULL it out from the
          * virDomainDeviceDef that we were given so that the caller
-- 
2.43.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

On a Thursday in 2024, Michal Privoznik wrote:
>After v9.1.0-rc1~116 we track whether it's us who created a
>macvtap or not. But when updating a vNIC its definition might be
>replaced with a new one (though, ifname is not allowed to
>change), e.g. to reflect new QoS, link state, etc.
>
>Now, the fact whether we created macvtap for given vNIC is stored
>in net->privateData->created. And replacing definition is done by
>simply freeing the old definition and making the pointer point to
>the new one. But this does not preserve the 'created' flag, which
>in turn means when a domain is shutting off, the macvtap is not
>removed (see loop inside of qemuProcessStop()).
>
>Copy this flag into new definition and leave a note in
>_qemuDomainNetworkPrivate struct.
>
>Fixes: 61d1b9e6592660121aeda66bf7adbcd39de06aa8
>Resolves: https://issues.redhat.com/browse/RHEL-22714
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
>
>I also contemplated idea of preserving whole privateData, e.g.:
>
>  virObjectUnref(newdev->privateData);
>  newdev->privateData = g_steal_pointer(&olddev->privateData);
>
>but then decided against it. Looks like a heavy gun, though in my
>debugging all members but 'created' were zero/NULL at point of calling
>qemuDomainChangeNet(). Then there is qemuDomainNetworkPrivateFormat()
>and qemuDomainNetworkPrivateParse() combo, but in order to use it I'd
>need to create and then parse an XML doc (to get 'ctxt') and that looks
>too much work for very little gain.
>
>Does somebody have any other idea?
>
> src/qemu/qemu_domain.h  | 2 ++
> src/qemu/qemu_hotplug.c | 5 +++++
> 2 files changed, 7 insertions(+)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org