Re: [libvirt] [PATCH] qemu: Do not override config XML in case of snapshot revert
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