Re: [PATCH v2 01/33] block: Add BlockDriver.is_format

[Max Reitz]

On 05.02.20 14:51, Eric Blake wrote:
> On 2/4/20 11:08 AM, Max Reitz wrote:
>> We want to unify child_format and child_file at some point.  One of the
>> important things that set format drivers apart from other drivers is
>> that they do not expect other format nodes under them (except in the
>> backing chain).  That means we need something on which to distinguish
>> format drivers from others, and hence this flag.
> 
> It _is_ possible to set up a format node on top of another;

Sure, but format nodes don’t “expect” that.  That is, it isn’t what we
should strive to do by default.

(Trying to do format probing inside of an already probed format would be
a security risk, actually.)

> in fact, our
> testsuite does that in at least iotest 072.  I agree that setups like
> 'qcow2 - qcow2 - file' are uncommon, but the setup 'qcow2 - raw - file'
> may be useful for extracting a partition of a raw disk when it is known
> that the partition in that disk itself contains qcow2 data.
> 
>>
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
> 
>> +++ b/include/block/block_int.h
>> @@ -94,6 +94,13 @@ struct BlockDriver {
>>        * must implement them and return -ENOTSUP.
>>        */
>>       bool is_filter;
>> +    /*
>> +     * Set to true if the BlockDriver is a format driver.  Format nodes
>> +     * generally do not expect their children to be other format nodes
>> +     * (except for backing files), and so format probing is disabled
>> +     * on those children.
> 
> Aha - nested formats ARE still allowed when you explicitly request it
> (which is what iotest 72 does) - what you are stating here is that
> implicit probing of is forbidden for a parent declared as a format
> driver.  That makes more sense.
> 
> I'm not sure if the commit message needs a tweak, but the patch itself
> is sane as-is.

I’m not sure either.  I think “expect” is OK as-is.  I suppose I could
add a sentence like “... they do not expect other format nodes under
them (...).  That is, we must not probe formats inside of formats.”

> Reviewed-by: Eric Blake <address@hidden>

Thanks for reviewing!

Max

signature.asc
Description: OpenPGP digital signature