Kevin Wolf <kw...@redhat.com> writes:
> Am 08.09.2022 um 19:36 hat Claudio Fontana geschrieben:
>> On 9/8/22 19:10, Claudio Fontana wrote:
>> > On 9/8/22 18:03, Richard Henderson wrote:
>> >> On 9/8/22 15:53, Claudio Fontana wrote:
>> >>> @@ -446,8 +447,13 @@ static int dmg_open(BlockDriverState *bs, QDict
>> >>> *options, int flags,
>> >>> return -EINVAL;
>> >>> }
>> >>>
>> >>> - block_module_load_one("dmg-bz2");
>> >>> - block_module_load_one("dmg-lzfse");
>> >>> + if (!block_module_load_one("dmg-bz2", &local_err) && local_err) {
>> >>> + error_report_err(local_err);
>> >>> + }
>> >>> + local_err = NULL;
>> >>> + if (!block_module_load_one("dmg-lzfse", &local_err) && local_err) {
>> >>> + error_report_err(local_err);
>> >>> + }
>> >>>
>> >>> s->n_chunks = 0;
>> >>> s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
>> >>
>> >> I wonder if these shouldn't fail hard if the modules don't exist?
>> >> Or at least pass back the error.
>> >>
>> >> Kevin?
>>
>> is "dmg-bz" _required_ for dmg open to work? I suspect if the dmg
>> image is not compressed, "dmg" can function even if the extra dmg-bz
>> module is not loaded right?
>
> Indeed. The code seems to consider that the modules may not be present.
> The behaviour in these cases is questionable (it seems to silently leave
> the buffers as they are and return success), but the modules are clearly
> optional.
>
>> I'd suspect we should then do:
>>
>> if (!block_module_load_one("dmg-bz2", &local_err)) {
>> if (local_err) {
>> error_report_err(local_err);
>> return -EINVAL;
>> }
>> warn_report("dmg-bz2 is not present, dmg will skip bz2-compressed chunks */
>> }
>>
>> and same for dmg-lzfse...?
>
> Actually, I think during initialisation, we should just pass NULL as
> errp and ignore any errors.
>
> When a request would access a block that can't be uncompressed because
> of the missing module, that's where we can have a warn_report_once() and
> arguably should fail the I/O request.
Seems like asking for data corruption. To avoid it, the complete stack
needs to handle I/O errors correctly.
Can we detect presence of compressed blocks on open?
