Re: [libvirt] [PATCH] qemuDomainSnapshotCreateActiveExternal: Remove memory snapshot on failure less frequently

On 08/30/2018 12:57 PM, Peter Krempa wrote: > On Thu, Aug 30, 2018 at 11:22:35 +0200, Michal Privoznik wrote: >> https://bugzilla.redhat.com/show_bug.cgi?id=1589115 >> >> When creating a memory snapshot the domain is suspended and qemu >> is told to dump memory into the desired file. After that we set a >> flag so that the file is not left behind if a failure occurs at >> some later point (e.g. when creating disk snapshot fails). >> However, the way we currently handle the memory snapshot file is >> way too aggressive. For instance, if resuming domain vCPUs fails >> after all snapshots were done successfully, we still remove the >> memory snapshot (even though we leave disk snapshot be). >> >> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; >> --- >> src/qemu/qemu_driver.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c >> index a0f7c71675..2eae3dd49d 100644 >> --- a/src/qemu/qemu_driver.c >> +++ b/src/qemu/qemu_driver.c >> @@ -15248,6 +15248,7 @@ qemuDomainSnapshotCreateActiveExternal(virQEMUDriverPtr driver, >> virObjectEventStateQueue(driver->domainEventState, event); >> } >> >> + memory_unlink = false; >> ret = 0; > > This does not make sense. The snapshot metadata is not saved at this > point so this file would be left over and could not be used (once we > implement external snapshot reversion). > > The problem is that you can't get rid of the disk images once the > transaction is executed, but you can dispose of the memory image. > > The failure points after the disk snapshot is taken are very hard to > roll back. > So what are you saying is that this is expected behaviour? Okay. Thanks anyway.