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

[Max Reitz <mreitz@redhat.com>]

On 2014-11-26 at 08:23, Markus Armbruster wrote:
> Max Reitz <mreitz@redhat.com> 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: qemu-stable@nongnu.org
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   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