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

[Eric Blake]

On 5/15/19 10:22 AM, Maxiwell S. Garcia wrote:
> On Tue, May 14, 2019 at 09:38:12AM -0500, Eric Blake wrote:
>> On 5/14/19 8:37 AM, Michal Privoznik wrote:
>>> On 5/13/19 8:48 PM, Daniel Henrique Barboza wrote:
>>>> Tried to reproduce the error using my x86 laptop but got hit by
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1689216 when trying
>>>> to create the snapshot using upstream code:
>>>>
>>
>>>
>>> Actually, I think that this is not the proper fix. The proper fix would
>>> be to store both active AND inactive XMLs when creating a snapshot and
>>> then restore them both on snapshot revert. While this fix may fix one
>>> use case, it's not dealing with the issue properly IMO. But I'm not
>>> snapshot expert really.
>>
>> Sadly, if we were to modify snapshot XML to store both the active and
>> inactive XML with a snapshot, all existing snapshots of a running guest
>> would be incomplete (right now, the snapshot XML only allows the storage
>> of a single <domain> sub-element).  I think the best we can do is store
>> an active XML with a snapshot of an active domain, and an inactive XML
>> with a snapshot of an offline domain. Reverting to an offline snapshot
>> only has to worry about inactive XML; reverting to an online snapshot
>> needs the active XML to properly restore the domain, but we then have to
>> decide whether to also update the inactive XML to match the fact that
>> the active XML has changed. So changing both inactive and active XML to
>> describe the same state during a revert to a snapshot of an active
>> domain seems like the best approach to me, short of rewriting snapshot
>> XML to store two different domain definitions at once.
>>
> 
> Hi Eric,
> 
> The actual behavior of ‘snapshot revert’ changes both inactive and active
> XML to describe the same state saved in the ‘snapshot create’.
> The steps below were commented in a bugzilla:
> 
> 1. Create a guest say VM1
> 2. Attach a virtual NIC to the guest :
>     virsh attach-device VM1 network.xml --live
>     interface will be seen in the guest.
> 3. Take a snapshot of the guest :
>     virsh snapshot-create VM1
>     Domain snapshot 1505299135 created
> 4. Destroy the guest
> 5. Now, revert back the snapshot:
>     virsh snapshot-revert VM1 1505299135
>     Interface is seen in the guest.
> 6. Now destroy the guest and start it again
> Interface is still seen in the guest.
> It is making the attach persistent.
> 
> So, after a revert, the guest always loses the inactive configuration.
> Why do you think this behavior is the best approach?

In your sequence, at point 2, the live XML differs from the inactive XML
(the live reflects the hot-plug, the inactive will revert to the state
at the time of the creation). You are correct that at step 5, we are
updating the inactive XML in addition to the live XML.

But consider this counter-sequence:

1. Create guest
2. Take snapshot
3. Hotplug a device
4. Revert to the snapshot

There, we MUST make sure that the revert does NOT have the hotplugged
device (since it was added only after the snapshot was created).

Then couple it with the fact that you can have:

1. Create guest
2. Take snapshot
3. Define new inactive XML that includes new device

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).

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?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190515/063685ab/attachment-0001.sig>