Re: [Qemu-devel] [PATCH 04/10] snapshot: create bdrv_all_goto_snapshot helper

[Denis V. Lunev]

On 11/12/2015 11:38 AM, Fam Zheng wrote:
> On Tue, 11/10 17:25, Denis V. Lunev wrote:
> > to switch to snapshot on all loaded block drivers.
> >
> > The patch also ensures proper locking.
> >
> > Signed-off-by: Denis V. Lunev <address@hidden>
> > Reviewed-by: Greg Kurz <address@hidden>
> > CC: Juan Quintela <address@hidden>
> > CC: Stefan Hajnoczi <address@hidden>
> > CC: Kevin Wolf <address@hidden>
> > ---
> >   block/snapshot.c         | 20 ++++++++++++++++++++
> >   include/block/snapshot.h |  1 +
> >   migration/savevm.c       | 15 +++++----------
> >   3 files changed, 26 insertions(+), 10 deletions(-)
> >
> > diff --git a/block/snapshot.c b/block/snapshot.c
> > index 61a6ad1..9f07a63 100644
> > --- a/block/snapshot.c
> > +++ b/block/snapshot.c
> > @@ -403,3 +403,23 @@ int bdrv_all_delete_snapshot(const char *name, 
> > BlockDriverState **first_bad_bs,
> >       *first_bad_bs = bs;
> >       return ret;
> >   }
> > +
> > +
> > +int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
> > +{
> > +    int err = 0;
> > +    BlockDriverState *bs = NULL;
> > +
> > +    while (err == 0 && (bs = bdrv_next(bs))) {
> > +        AioContext *ctx = bdrv_get_aio_context(bs);
> > +
> > +        aio_context_acquire(ctx);
> > +        if (bdrv_can_snapshot(bs)) {
> > +            err = bdrv_snapshot_goto(bs, name);
> > +        }
> > +        aio_context_release(ctx);
> > +    }
> > +
> > +    *first_bad_bs = bs;
> > +    return err;
> > +}
> > diff --git a/include/block/snapshot.h b/include/block/snapshot.h
> > index d02d2b1..0a176c7 100644
> > --- a/include/block/snapshot.h
> > +++ b/include/block/snapshot.h
> > @@ -84,5 +84,6 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
> >   bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs);
> >   int bdrv_all_delete_snapshot(const char *name, BlockDriverState 
> > **first_bsd_bs,
> >                                Error **err);
> > +int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bsd_bs);
> >   
> >   #endif
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index 1157a6f..d18ff13 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -1425,16 +1425,11 @@ int load_vmstate(const char *name)
> >       /* Flush all IO requests so they don't interfere with the new state.  */
> >       bdrv_drain_all();
> >   
> > -    bs = NULL;
> > -    while ((bs = bdrv_next(bs))) {
> > -        if (bdrv_can_snapshot(bs)) {
> > -            ret = bdrv_snapshot_goto(bs, name);
> > -            if (ret < 0) {
> > -                error_report("Error %d while activating snapshot '%s' on '%s'",
> > -                             ret, name, bdrv_get_device_name(bs));
> > -                return ret;
> > -            }
> > -        }
> > +    ret = bdrv_all_goto_snapshot(name, &bs);
> > +    if (ret < 0) {
> > +        error_report("Error %d while activating snapshot '%s' on '%s'",
> > +                     ret, name, bdrv_get_device_name(bs));
> > +        return ret;
> Maybe more friendlily strerror(ret)?
>
> >       }
> >   
> >       /* restore the VM state */
> > --
> > 2.5.0
we can. I think that this could be done for all such functions
as follow up patch. I'll do that separately and will send
with the next iteration if applicable.

Den