[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 5/9] migration: control whether snapshots are ovewritten
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v4 5/9] migration: control whether snapshots are ovewritten |
Date: |
Wed, 16 Sep 2020 09:25:22 +0100 |
User-agent: |
Mutt/1.14.6 (2020-07-11) |
On Wed, Sep 16, 2020 at 09:47:32AM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> 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é <berrange@redhat.com>
> > ---
> > 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 :|
- [PATCH v4 0/9] migration: bring improved savevm/loadvm/delvm to QMP, Daniel P . Berrangé, 2020/09/15
- [PATCH v4 1/9] block: push error reporting into bdrv_all_*_snapshot functions, Daniel P . Berrangé, 2020/09/15
- [PATCH v4 2/9] migration: stop returning errno from load_snapshot(), Daniel P . Berrangé, 2020/09/15
- [PATCH v4 3/9] block: add ability to specify list of blockdevs during snapshot, Daniel P . Berrangé, 2020/09/15
- [PATCH v4 4/9] block: allow specifying name of block device for vmstate storage, Daniel P . Berrangé, 2020/09/15
- [PATCH v4 5/9] migration: control whether snapshots are ovewritten, Daniel P . Berrangé, 2020/09/15
- [PATCH v4 6/9] migration: wire up support for snapshot device selection, Daniel P . Berrangé, 2020/09/15
- [PATCH v4 7/9] migration: introduce a delete_snapshot wrapper, Daniel P . Berrangé, 2020/09/15
- [PATCH v4 8/9] iotests: add support for capturing and matching QMP events, Daniel P . Berrangé, 2020/09/15
- [PATCH v4 9/9] migration: introduce snapshot-{save, load, delete} QMP commands, Daniel P . Berrangé, 2020/09/15