Re: [Qemu-devel] [PATCH V3 07/11] block: export function bdrv_find_snapshot()

[Wenchao Xia]

于 2013-1-15 7:39, Eric Blake 写道:
> On 01/14/2013 12:09 AM, Wenchao Xia wrote:
> >    This patch move it from savevm.c to block.c and export it.
> >
> > Signed-off-by: Wenchao Xia <address@hidden>
> > ---
> >   block.c               |   23 +++++++++++++++++++++++
> >   include/block/block.h |    2 ++
> >   savevm.c              |   22 ----------------------
> >   3 files changed, 25 insertions(+), 22 deletions(-)
> >
> > diff --git a/block.c b/block.c
> > index 8192d8e..b7d2f03 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -3351,6 +3351,29 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
> >       return -ENOTSUP;
> >   }
> >
> > +int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
> > +                       const char *name)
> > +{
> > +    QEMUSnapshotInfo *sn_tab, *sn;
> > +    int nb_sns, i, ret;
> > +
> > +    ret = -ENOENT;
> > +    nb_sns = bdrv_snapshot_list(bs, &sn_tab);
> > +    if (nb_sns < 0) {
> > +        return ret;
> > +    }
> > +    for (i = 0; i < nb_sns; i++) {
> > +        sn = &sn_tab[i];
> > +        if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) {
>
> It is possible (albeit probably stupid) to create a qcow2 file where
> snapshot names are merely numeric strings.  In fact, just to see what
> would happen, I once[1] created a file where:
>
> snapshot id '1' was named '2'
> snapshot id '2' was named 'foo'
>
> This code comparison favors ids over names; so if I request to delvm 2,
> I end up removing the second snapshot, not the first.  This is okay, but
> probably worth documenting, and probably worth making sure that all code
> that looks up a snapshot by name or id goes through this function so
> that we get the same behavior everywhere.  My experiment was done
> several months ago, but my recollection was that at the time, there was
> an inconsistency where 'qemu-img snapshot' picked a different snapshot
> for the request of '2' than the online 'delvm' monitor command of qemu;
> making it unsafe to rely on either behavior in that version of qemu
> source code.
>
> [1]https://bugzilla.redhat.com/show_bug.cgi?id=733143

how about:
/*  if id is not NULL, try find it with id, if not exist, return NULL
 *  if id is NULL and name is not NULL, try find it with name.
 */ if id and name is NULL, direct return fail.
int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
                       const char *id, const char *name)

--
Best Regards

Wenchao Xia