Kevin Wolf <kw...@redhat.com> 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 <mre...@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.
>> >>>>+++ 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.
