Re: [Qemu-devel] [PATCH 09/10] block/qapi: Ignore filters on top for format name

[Max Reitz]

On 05.03.2014 17:20, Benoît Canet wrote:
> The Monday 03 Mar 2014 à 16:28:52 (+0100), Max Reitz wrote :
> > bdrv_query_image_info() currently deduces the image filename and the
> > format name from the top BDS. However, it is probably more reasonable to
> > ignore as many filters as possible on top of the BDS chain since those
> > neither change the type nor the filename of the underlying image.
> >
> > Filters like quorum which have multiple underlying BDS should not be
> > removed, however, since the underlying BDS chains may lead to different
> > image formats and most certainly to different filenames. Therefore, only
> > simple filters with a single underlying BDS may be skipped.
> >
> > In addition, any "raw" BDS on top of such a simple filter should be
> > removed, since they have probably been automatically created by
> > bdrv_open() but basically function as a simple filter as well.
> >
> > Signed-off-by: Max Reitz <address@hidden>
> > ---
> >   block/qapi.c | 20 ++++++++++++++++++--
> >   1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/block/qapi.c b/block/qapi.c
> > index 8f2b4db..c4ddced 100644
> > --- a/block/qapi.c
> > +++ b/block/qapi.c
> > @@ -171,11 +171,27 @@ void bdrv_query_image_info(BlockDriverState *bs,
> >       int ret;
> >       Error *err = NULL;
> >       ImageInfo *info = g_new0(ImageInfo, 1);
> > +    BlockDriverState *ubs;
> >   
> >       bdrv_get_geometry(bs, &total_sectors);
> >   
> > -    info->filename        = g_strdup(bs->filename);
> > -    info->format          = g_strdup(bdrv_get_format_name(bs));
> > +    /* Remove the top layer of filters; that is, remove every "raw" BDS on top
> > +     * of a simple filter and every simple filter itself until a BDS is
> > +     * encountered which cannot be removed through these rules; a simple filter
> > +     * is a filter BDS which has its child in .file (that is,
> > +     * bdrv_recurse_is_first_non_filter is NULL). */
> > +    ubs = bs;
> > +    while ((ubs->drv && ubs->drv->authorizations[BS_IS_A_FILTER]) ||
> > +           (ubs->drv && !strcmp(ubs->drv->format_name, "raw") && ubs->file &&
> > +            ubs->file->drv && ubs->file->drv->authorizations[BS_IS_A_FILTER] &&
> > +            !ubs->file->drv->bdrv_recurse_is_first_non_filter))
> > +    {
> > +        ubs = ubs->file;
> > +    }
> The rewrite Paolo asked me to do don't allow to do it anymore: there is no
> distinction between simple filter driver and quorum like filter drivers.
>
> Maybe this rewrite was not so nice.

I'll try to think of something. *g*


Max

> Best regards
>
> Benoît
>
> > +
> > +    info->filename        = g_strdup(ubs->filename);
> > +    info->format          = g_strdup(bdrv_get_format_name(ubs));
> > +
> >       info->virtual_size    = total_sectors * 512;
> >       info->actual_size     = bdrv_get_allocated_file_size(bs);
> >       info->has_actual_size = info->actual_size >= 0;
> > --
> > 1.9.0