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
