Re: [libvirt] [PATCH 0/7] Keep non-persistent changes alive in snapshot

On Mon, 2017-10-30 at 14:21 +0530, Kothapally Madhu Pavan wrote: > Restoring to a snapshot should not overwrite the persistent XML > configuration > of a snapshot as a side effect. This patchset fixes the same. > Currently, > virDomainSnapshotDef only saves active domain definition of the > guest. > And on restore the active domain definition is used as both active > and > inactive domain definitions. This will make the non-persistent > changes > persistent in snapshot image. This patchset allows to save inactive > domain > definition as well and on snapshot-revert non-persistent > configuration is > restored as is. > > Currently, snapshot-revert is making non-presistent changes as > persistent. > Here are the steps to reproduce. > Step1: virsh define $dom > Step2: virsh attach-device $dom $memory-device.xml --live > Step3: virsh snapshot-create $dom > Step4: virsh destroy $dom > Step5: virsh snapshot-revert $dom $snapshot-name > Step6: virsh destroy $dom > Step7: virsh start $dom > Here we still have $memory-device attached in Step2. > > This patchset is attempting to solve this issue. This patchset will > also > allow user to dump and edit inactive XML configuration of a snapshot. > Dumping inactive domain definition of a snapshot is important as > --redefine uses snapshot-dumpxml output to redefine a snapshot. > > Kothapally Madhu Pavan (7): >   qemu: Store inactive domain configuration in snapshot >   qemu: Use active and inactive snapshot configuration on restore >   conf: Allow editing inactive snapshot configuration >   virsh: Dump inactive XML configuration of snapshot using >     snapshot-dumpxml >   virsh: Edit inactive XML configuration of snapshot using snapshot- > edit >   virsh: Allow restoring snapshot with non-persistent configuration >   tests: docs: Add schema and testcase for domainsnapshot > >  docs/schemas/domainsnapshot.rng                    | 19 +++++ >  include/libvirt/libvirt-domain-snapshot.h          | 10 ++- >  include/libvirt/libvirt-domain.h                   |  1 + >  src/conf/domain_conf.c                             |  6 +- >  src/conf/domain_conf.h                             |  2 + >  src/conf/snapshot_conf.c                           | 48 > ++++++++++++- >  src/conf/snapshot_conf.h                           |  1 + >  src/qemu/qemu_driver.c                             | 33 ++++++++- >  .../full_domain_withinactive.xml                   | 83 > ++++++++++++++++++++++ >  tests/domainsnapshotxml2xmltest.c                  |  1 + >  tools/virsh-snapshot.c                             | 20 ++++++ >  tools/virsh.pod                                    | 37 +++++++++- >  12 files changed, 251 insertions(+), 10 deletions(-) >  create mode 100644 > tests/domainsnapshotxml2xmlout/full_domain_withinactive.xml Could someone please review the patch set? Jiri mentioned in early December that it was on his list; perhaps this fell through the cracks or got displaced by the (admittedly more urgent) Spectre/Meltdown