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

[Markus Armbruster]

Kevin Wolf <address@hidden> writes:

> Am 26.11.2014 um 16:20 hat Max Reitz geschrieben:
>> On 2014-11-26 at 16:19, Eric Blake wrote:
>> >On 11/26/2014 02:13 AM, Max Reitz wrote:
>> >>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.
>> >>>>+++ 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) {
>> >Would it be shorter to 'assert(bdrv_qcow2);' to still silence Coverity?
>> 
>> Sounds like a good compromise. Will do.
>
> I think it's better to have either proper error handling for the case
> that someone compiles it out, like implemented by this patch, or to
> reference the symbol so that compiling it out already breaks the build.
> The assert() would potentially be a crash of a running VM, which is not
> as nice.

Ways to bind to a well-known block driver, and how the binding fails
when the driver isn't around, in decreasing order of preference:

1. Static binding

   Fails at compile time.  Me like.

2. Dynamic binding, assert the required code is there

   Fails at run time in a catastrophic, but locally obvious way.

3. Dynamic binding without error checking

   Fails at run time in a catastrophic way, when the pointer gets
   dereferenced.  Less predictable than 2.

4. Dynamic binding, error out if the required code is there

   Fails at run time in a way that could be recoverable, but probably
   isn't, not least because nobody ever tests it.  Moreover, code
   clutter.

Use of dynamic binding to reference a well-known block driver feels like
"Look ma, dynamic binding in C!" to me.