Re: [Qemu-devel] [PATCH V7 05/14] block: add snapshot info query function bdrv_query_snapshot_infolist()

[Wenchao Xia]

于 2013-2-28 23:36, Stefan Hajnoczi 写道:
> On Tue, Feb 26, 2013 at 06:40:19PM +0800, Wenchao Xia wrote:
> > diff --git a/include/block/block.h b/include/block/block.h
> > index e6d915c..51a14f2 100644
> > --- a/include/block/block.h
> > +++ b/include/block/block.h
> > @@ -319,6 +319,13 @@ void bdrv_get_backing_filename(BlockDriverState *bs,
> >                                  char *filename, int filename_size);
> >   void bdrv_get_full_backing_filename(BlockDriverState *bs,
> >                                       char *dest, size_t sz);
> > +
> > +typedef int (*SnapshotFilterFunc)(const QEMUSnapshotInfo *sn, void *opaque);
> > +/* assume bs is already opened, use qapi_free_* to free returned value. */
> > +SnapshotInfoList *bdrv_query_snapshot_infolist(BlockDriverState *bs,
> > +                                               SnapshotFilterFunc filter,
> > +                                               void *opaque,
> > +                                               Error **errp);
>
> bdrv_query_snapshot_infolist() uses SnapshotInfoList, therefore it makes
> sense to pass a SnapshotInfo argument to SnapshotFilterFunc() instead of
> a QEMUSnapshotInfo.  The QEMUSnapshotInfo type is an implementation
> detail that should not be part of the API.
  OK.

> > diff --git a/qemu-img.c b/qemu-img.c
> > index b650d17..1034cc5 100644
> > --- a/qemu-img.c
> > +++ b/qemu-img.c
> > @@ -1280,7 +1280,13 @@ static ImageInfoList *collect_image_info_list(const char 
> > *filename,
> >
> >           info = g_new0(ImageInfo, 1);
> >           collect_image_info(bs, info, filename);
> > -        collect_snapshots(bs, info);
> > +        if (bdrv_can_read_snapshot(bs)) {
> > +            info->snapshots = bdrv_query_snapshot_infolist(bs, NULL,
> > +                                                           NULL, NULL);
> > +            if (info->snapshots) {
> > +                info->has_snapshots = true;
> > +            }
> > +        }
>
> bdrv_can_read_snapshot() is not needed.  The code works fine without the
> if check.
  OK.

--
Best Regards

Wenchao Xia