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

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: > > $ sudo ./run tools/virsh snapshot-create-as ub1810-cpu-hotplug snap > error: operation failed: Failed to take snapshot: Error: Nested VMX > virtualization does not support live migration yet > > > Since this issue is arch independent I tried it out with a Power server. > Here's the behavior with current upstream: > > > - relevant piece of VM XML: > > <domain type='kvm'> >   <name>dhb</name> >   <uuid>d3fe0169-2be6-431c-85a5-cc178601bb69</uuid> >   <memory unit='KiB'>67108864</memory> >   <currentMemory unit='KiB'>67108864</currentMemory> >   <vcpu placement='static' current='4'>16</vcpu> > > $ sudo ./run tools/virsh setvcpus dhb 8 --live > > $ > $ sudo ./run tools/virsh snapshot-create-as dhb snap1 > Domain snapshot snap1 created > $ > $ sudo ./run tools/virsh snapshot-revert dhb snap1 > > $ > $ sudo ./run tools/virsh dumpxml --inactive dhb > <domain type='kvm'> >   <name>dhb</name> >   <uuid>d3fe0169-2be6-431c-85a5-cc178601bb69</uuid> >   <memory unit='KiB'>67108864</memory> >   <currentMemory unit='KiB'>67108864</currentMemory> >   <vcpu placement='static' current='8'>16</vcpu> >   <vcpus> > > The inactive XML got overwritten by the vcpu hotplug, which is not > intended. > > > After the patch, the problem isn't reproduced: the inactive VM XML was > preserved after the snapshot-revert. > > > Maxiwell, you (or the commiter) can add the following in the commit-msg: > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1689216 > > > Since the bug you're fixing here also fixes this RH bugzilla. > > > Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; > Tested-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; 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. Michal