Re: [Qemu-stable] [Qemu-devel] [PATCH 01/12] block: qcow2 driver may not be found

[Max Reitz]

On 2014-11-26 at 08:23, Markus Armbruster wrote:
> Max Reitz <address@hidden> writes:
>
> > Albeit absolutely impossible right now, bdrv_find_format("qcow2") may
> > fail. bdrv_append_temp_snapshot() should heed that case.
> Impossible because we always compile in bdrv_qcow2.

Right now we do, right.

> > Cc: address@hidden
> > Signed-off-by: Max Reitz <address@hidden>
> > ---
> >   block.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> >
> > diff --git a/block.c b/block.c
> > index 866c8b4..b31fb67 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -1320,6 +1320,12 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int 
> > flags, Error **errp)
> >       }
> >   
> >       bdrv_qcow2 = bdrv_find_format("qcow2");
> > +    if (!bdrv_qcow2) {
> > +        error_setg(errp, "Failed to locate qcow2 driver");
> > +        ret = -ENOENT;
> > +        goto out;
> > +    }
> > +
> >       opts = qemu_opts_create(bdrv_qcow2->create_opts, NULL, 0,
> >                               &error_abort);
> >       qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size);
> This dynamic qcow2 driver lookup business is silly.  Compiling without
> qcow2 would be a massive loss of functionality, and I wouldn't bet a
> nickel on the error paths it would pring to live.

True.

> Even sillier lookups: "file" in bdrv_find_protocol(), and "raw" in
> find_image_format().
>
> Statically linking to them would be simpler and more honest.
>
> Aside: similar silliness exists around QemuOpts.
>
> Patch looks correct.

Well, at least it will silence Coverity...

Max