[libvirt] [PATCH] qemu: Do not override config XML in case of snapshot revert

[Daniel Henrique Barboza]

On 5/15/19 12:49 PM, Eric Blake wrote:
> If you reboot the guest, the inactive XML will become active and the
> guest will have a new device. But if the guest continues running without
> rebooting, the inactive XML will be different from the live XML. Whether
> or not the guest rebooted, a revert must NOT expose the new device to
> the live XML installed during the revert.  But there is still a question
> as to whether the revert should also undo the inactive XML change that
> was made after the snapshot was created, or leave the inactive XML alone
> (that is, after we revert, will a fresh boot once again pick up the new
> device, or will the fresh boot be stuck with the configuration as though
> step 3 had never happened).

Maybe we should consider that, in Libvirt, the snapshot state consists
of both inactive and live XMLs. Like Michal suggested in his first reply.
It appears to be the most consistent way of dealing with the revert of a
VM state - the change is that the VM state is now both inactive and
active XMLs.

This would cover all possible scenarios (at least the ones I can think
of) - we will not overwrite the inactive XML with stuff that weren't 
supposed
to be there ("but I used setvcpus --live, why the extra vcpus are showing in
the offline XML after revert") and the other way around ("I hotplugged
with --config, but after the revert my vcpus are gone").

This would mean that yeah, a user made change in the inactive XML will
be overwritten in a revert operation with the inactive XML of the snapshot.
The documentation already states that this would be case though:

---
snapshot-revert domain {snapshot | --current} [{--running | --paused}] 
[--force]
      Revert the given domain to the snapshot specified by snapshot, or 
to the current
      snapshot with --current.  Be aware that this is a destructive 
action; any
      changes in the domain since the last snapshot was taken will be 
lost.  Also note
      that the state of the domain after snapshot-revert is complete 
will be the state
      of the domain at the time the original snapshot was taken.
----

"any changes in the domain since the last snapshot was taken will be 
lost". This
includes the inactive XML, which is a domain change.

>
> I can buy the argument that when reverting to a live state, libvirt
> could be changed to only revert the live XML and leave the inactive XML
> unchanged.  But will that break any existing clients that have come to
> rely on several years of behavior where reverting sets both XML at once?
>   Or is it really that many years that we've had a bug and no one has
> noticed it until now?

I can't comment on how most users expect snapshot-revert to work, but I
would say that the initial bug that Maxiwell is addressing here (for 
reference:
https://bugzilla.redhat.com/show_bug.cgi?id=1662588 ) is something that
users will complain about if they face it. It is annoying, at the very 
least.

Hopefully, at least in my estimation, taking both XMLs into 
consideration when
snapshot reverting fixes this bug without creating other ones. Maxiwell, 
have you
had the chance to assess how hard would it be to make such change?


Thanks,


DHB