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

2018-02-08 Thread Jiri Denemark
On Thu, Jan 11, 2018 at 13:41:39 -0600, Scott Garfinkle wrote:
> 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

Yeah, more urgent stuff appeared at that time. However, John replied
with several comments and the author didn't address any of them by
either sending a v2 or replying to John's emails or (preferably) both.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


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

2018-01-11 Thread Scott Garfinkle
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
patches.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list