On Wed, Sep 16, 2020 at 09:47:32AM +0200, Markus Armbruster wrote: > Daniel P. Berrangé <[email protected]> writes: > > > The traditional HMP "savevm" command will overwrite an existing snapshot > > if it already exists with the requested name. This new flag allows this > > to be controlled allowing for safer behaviour with a future QMP command. > > > > Signed-off-by: Daniel P. Berrangé <[email protected]> > > --- > > include/migration/snapshot.h | 2 +- > > migration/savevm.c | 4 ++-- > > monitor/hmp-cmds.c | 2 +- > > replay/replay-snapshot.c | 2 +- > > 4 files changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/include/migration/snapshot.h b/include/migration/snapshot.h > > index c85b6ec75b..d7db1174ef 100644 > > --- a/include/migration/snapshot.h > > +++ b/include/migration/snapshot.h > > @@ -15,7 +15,7 @@ > > #ifndef QEMU_MIGRATION_SNAPSHOT_H > > #define QEMU_MIGRATION_SNAPSHOT_H > > > > -int save_snapshot(const char *name, Error **errp); > > +int save_snapshot(const char *name, bool overwrite, Error **errp); > > int load_snapshot(const char *name, Error **errp); > > > > #endif > > diff --git a/migration/savevm.c b/migration/savevm.c > > index 76972d69b0..2025e3e579 100644 > > --- a/migration/savevm.c > > +++ b/migration/savevm.c > > @@ -2658,7 +2658,7 @@ int qemu_load_device_state(QEMUFile *f) > > return 0; > > } > > > > -int save_snapshot(const char *name, Error **errp) > > +int save_snapshot(const char *name, bool overwrite, Error **errp) > > { > > BlockDriverState *bs; > > QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1; > > @@ -2685,7 +2685,7 @@ int save_snapshot(const char *name, Error **errp) > > } > > > > /* Delete old snapshots of the same name */ > > - if (name) { > > + if (name && overwrite) { > > if (bdrv_all_delete_snapshot(name, false, NULL, errp) < 0) { > > return ret; > > } > > Are you sure this is sane? > > To see what happens, I set a breakpoint on this function, set overwrite > to false. I got a *second* snapshot with the same ID.
Sigh. No, it doesn't do what I was meaning it to, and I forgot to add a test case for this scenario in the last patch. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
