On Fri, Sep 19, 2025 at 5:44 PM Peter Krempa <pkrempa@redhat.com> wrote:
On Fri, Sep 19, 2025 at 17:09:08 +0800, yong.huang@smartx.com wrote:
From: Hyman Huang <yong.huang@smartx.com>
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 them after restart the service. --- 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]); }
- if (xmlopt->privateData.format && - xmlopt->privateData.format(&buf, obj) < 0) - return NULL; + if (xmlopt->privateData.format) { + if (xmlopt->privateData.format(&buf, obj) < 0) + return NULL; + } else { + /* + * 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 them after restart + * the service. + */ + if (virDomainObjIsActive(obj) && + (obj->def->virtType == VIR_DOMAIN_VIRT_KVM)) { + VIR_WARN("Do not omit private data when formatting the" + " status XML for a %s VM", + virDomainStateTypeToString(state)); + return NULL; + } + }
So IIUC (based on the description in the cover letter) you're saying that there's a possibility that on the shutdown of the qemu driver something unregisters the private data formatting functions and thus we'd overwrite the status XML with a XML missing private data, right?
In such case this is not the right fix. It just covers up the that fact and also prints unactionable warnings caused by internal bug.
The proper fix is that the private data formatters must not be unregistered if we have internal state.
Can you elaborate when that happens? This seems like a race condition in the shutdown code.
The xmlopt driver contains privateData that implements the parse/format interface:
virQEMUDriverCreateXMLConf virDomainXMLOptionNew xmlopt->privateData = &virQEMUDriverPrivateDataCallbacks; The shutdown code frees the xmlopt field in qemu_driver, thereby releasing the parse/format interface: qemuStateCleanup virObjectUnref(qemu_driver->xmlopt); Meanwhile, the migration thread may call the virDomainObjSave function during the execution of qemuMigrationSrcConfirmPhase or qemuMigrationDstFinish: virDomainObjSave virDomainObjFormat if (xmlopt->privateData.format && xmlopt->privateData.format(&buf, obj) < 0) return NULL; As shown above, the shutdown code and the migration thread race to access the privateData format interface. -- Best regards