[PATCH] qemu: save status xml after generating taint message

[Michal Prívozník]

On 7/21/21 3:02 PM, Daniel P. Berrangé wrote:
> On Wed, Jul 21, 2021 at 02:29:34PM +0200, Peter Krempa wrote:
>> On Wed, Jul 21, 2021 at 14:05:05 +0200, Kristina Hanicova wrote:
>>> We didn't always save status xml after generating new taint
>>> message, which resulted in it being deleted in case of a libvirtd
>>> restart.  Some taint messages were preserved thanks to saving
>>> status xml separately at the end of the calling functions (which
>>> makes sense, because qemuDomainObjTaint was usually called there
>>> multiple times).  But for special cases (e.g. When only few taint
>>> messages are generated) we need a separate function for
>>> generating them and saving status xml explicitly.
>>
>> Saving the status XML is a very common operation which we in some cases
>> repeat a few times when doing an multi-step operation, thus we can
>> reasonably assume that saving the status XML in all cases when we are
>> adding a taint on a VM object is okay without the need to special case
>> operations which don't save the status XML as part of their code.
> 
> The bug quoted shows a few examples where we fail to save status.
> 
> I'm very surprised we don't save status when hotplugging a NIC or a
> disk, as the BZ suggests.

I'm not convinced that the steps there are 100% correct. We do call
virDomainObjSave() after live attach:

https://gitlab.com/libvirt/libvirt/-/blob/master/src/qemu/qemu_driver.c#L7834

> 
> Missing status save in QMP monitor command passthrough is less
> surprising though since we're not actually changing the VM state
> when doing that, so would not have reason to save state except
> for the taint message.

Yep. For a few cases it is hidden in BeginJob() and EndJob() but not for
agent jobs.

Michal