On Fri, Sep 19, 2025 at 17:09:07 +0800, yong.huang@smartx.com wrote:
> From: Hyman Huang <yong.huang@smartx.com>
>
> When saving the domain status in the path of the virDomainObjSave
> function, Libvirtd will omit the private data if the format
> interface is not provided.
>
> When the Libvirtd service is starting up and the driver has not
> yet been initialized, this behavior is justified.
>
> However, when the service is shutting down, the driver is being
> finalized and the interface is being released, a migration job
> or another job may call the qemuDomainSaveStatus and save the
> domain status at the same time. For the latter, this behavior

I had another look and `virQEMUDriverPrivateDataCallbacks` is a global
variable in the qemu driver which is just populated and never changed.

This struct is then copied (by assignment)  into the virDomainXMLOption
instance that is used inside the qemu driver. Thus while this does have
a private copy of the function pointers. The virDomainXMLOption
instance lives the whole time the virt drivers live, so it's only ever
cleared in virStateCleanup, which in the daemons happens right before
exit.

Now looking at the formatter code (let's use your patch as an example):

---
 src/conf/domain_conf.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 281846dfbe..74af08e584 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -29542,9 +29542,24 @@ virDomainObjFormat(virDomainObj *obj,
                               obj->deprecations[i]);
     }
=20
-    if (xmlopt->privateData.format &&
-        xmlopt->privateData.format(&buf, obj) < 0)
-        return NULL;
+    if (xmlopt->privateData.format) {
+        if (xmlopt->privateData.format(&buf, obj) < 0)

Here you see that the code assumes that 'xmlopt' is *not NULL*. Inside
of XMLopt you have only pointers that don't change.

As of such the 'format' callback being NULL is only if something
overwrote the data inside 'xmlopt', which we don't do anywhere because
it doesn't make sense.

The other possibility is that 'xmlopt' is used after free, thus was
randomly overwritten, but that would be a much different kind of bug and
your fix wouldn't fix that at all.


> causes the XML to be saved without private information (such as
> monitor path and qemuCaps), which is required for the Libvirtd
> service to manage a VM during startup. As a result, after restarting
> the Libvirtd service, a running VM that is being migrated previously
> might escape management.
>
> Thus, when formatting the status XML for a running virtual machine,
> we need to presume that the "privateData.format" interface is present.
>
> Add a thorough check in the virDomainObjSave path to make sure that
> private data in the status XML file always exists for the running VM
> so that we won't lose it after restart the Libvirtd service.

Now this outlines the implications of losing the private data but
doesn't really give any information how this happened.

Can you please clarify:
 1) What did you do to trigger this bug? What was the state of the host?
 2) How did you figure out that what you claim happened?
 3) Can you reproduce the bug?

If you can reproduce the bug please make sure to collect debug logs of
libvirtd, and potentially consider running the daemon inside e.g.
valgrind to see if it reproduces with such heavy instrumentation.