Re: [Qemu-devel] [PATCH V7 2/8] snapshot: distinguish id and name in snapshot delete

[Wenchao Xia]

于 2013/9/10 20:36, Kevin Wolf 写道:
> Am 07.08.2013 um 05:00 hat Wenchao Xia geschrieben:
> > Snapshot creation actually already distinguish id and name since it take
> > a structured parameter *sn, but delete can't. Later an accurate delete
> > is needed in qmp_transaction abort and blockdev-snapshot-delete-sync,
> > so change its prototype. Also *errp is added to tip error, but return
> > value is kepted to let caller check what kind of error happens. Existing
> > caller for it are savevm, delvm and qemu-img, they are not impacted by
> > introducing a new function bdrv_snapshot_delete_by_id_or_name(), which
> > check the return value and do the operation again.
> >
> > Before this patch:
> >    For qcow2, it search id first then name to find the one to delete.
> >    For rbd, it search name.
> >    For sheepdog, it does nothing.
> >
> > After this patch:
> >    For qcow2, logic is the same by call it twice in caller.
> >    For rbd, it always fails in delete with id, but still search for name
> > in second try, no change to user.
> >
> > Some code for *errp is based on Pavel's patch.
> >
> > Signed-off-by: Wenchao Xia<address@hidden>
> > Signed-off-by: Pavel Hrdina<address@hidden>
> > diff --git a/savevm.c b/savevm.c
> > index 03fc4d9..0808414 100644
> > --- a/savevm.c
> > +++ b/savevm.c
> > @@ -2325,18 +2325,21 @@ static int del_existing_snapshots(Monitor *mon, const 
> > char *name)
> >   {
> >       BlockDriverState *bs;
> >       QEMUSnapshotInfo sn1, *snapshot =&sn1;
> > -    int ret;
> > +    Error *err = NULL;
> >
> >       bs = NULL;
> >       while ((bs = bdrv_next(bs))) {
> >           if (bdrv_can_snapshot(bs)&&
> >               bdrv_snapshot_find(bs, snapshot, name)>= 0)
> >           {
> > -            ret = bdrv_snapshot_delete(bs, name);
> > -            if (ret<  0) {
> > +            bdrv_snapshot_delete_by_id_or_name(bs, name,&err);
> > +            if (error_is_set(&err)) {
> >                   monitor_printf(mon,
> > -                               "Error while deleting snapshot on '%s'\n",
> > -                               bdrv_get_device_name(bs));
> > +                               "Error while deleting snapshot on device '%s', "
> > +                               "reason: %s\n",
> More commonly, error messages just use a colon before the detailed
> error code instead of saying ", reason:"
>
> > +                               bdrv_get_device_name(bs),
> > +                               error_get_pretty(err));
> > +                error_free(err);
> >                   return -1;
> >               }
> >           }
> Kevin
Thanks for reviewing, will fix and rebase.