On 06.05.20 15:11, Kevin Wolf wrote:
> Am 06.05.2020 um 12:37 hat Kevin Wolf geschrieben:
>> Am 18.02.2020 um 13:42 hat Max Reitz geschrieben:
>>> After the series this patch belongs to, we want to have a common
>>> BdrvChildClass that encompasses all of child_file, child_format, and
>>> child_backing. Such a single class needs a single .inherit_options()
>>> implementation, and this patch introduces it.
>>>
>>> The next patch will show how the existing implementations can fall back
>>> to it just by passing appropriate BdrvChildRole and parent_is_format
>>> values.
>>>
>>> Signed-off-by: Max Reitz <mre...@redhat.com>
>>> Reviewed-by: Eric Blake <ebl...@redhat.com>
>>> ---
>>> block.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 84 insertions(+)
>>>
>>> diff --git a/block.c b/block.c
>>> index c33f0e9b42..9179b9b604 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -998,6 +998,90 @@ static void bdrv_temp_snapshot_options(int
>>> *child_flags, QDict *child_options,
>>> *child_flags &= ~BDRV_O_NATIVE_AIO;
>>> }
>>>
>>> +/*
>>> + * Returns the options and flags that a generic child of a BDS should
>>> + * get, based on the given options and flags for the parent BDS.
>>> + */
>>> +static void __attribute__((unused))
>>> + bdrv_inherited_options(BdrvChildRole role, bool parent_is_format,
>>> + int *child_flags, QDict *child_options,
>>> + int parent_flags, QDict *parent_options)
>>> +{
>>> + int flags = parent_flags;
>>> +
>>> + /*
>>> + * First, decide whether to set, clear, or leave BDRV_O_PROTOCOL.
>>> + * Generally, the question to answer is: Should this child be
>>> + * format-probed by default?
>>> + */
>
> Just for clarity: Do you know a good reason to ever leave it (i.e.
> inherit it from the parent), except that that's what we have always been
> doing for backing files? Though of course, only formats have backing
> files, so the flag would never be set in practice in this case.
It seems correct for filters.
[...]
>>> + if (parent_is_format && !(role & BDRV_CHILD_COW)) {
>>> + /*
>>> + * Our format drivers take care to send flushes and respect
>>> + * unmap policy, so we can default to enable both on lower
>>> + * layers regardless of the corresponding parent options.
>>> + */
>>> + qdict_set_default_str(child_options, BDRV_OPT_DISCARD, "unmap");
>>> + }
>>
>> Why the restriction to format here? Don't we break "unmap" propagation
>> through filters with this?
>>
>> It would probably also be a good question why we don't propagate it to
>> the backing file, but this is preexisting.
>
> Some patches later, I think the fix is an else branch that copies the
> flag from parent_options.
I thought about the same thing, but is that really necessary if
bdrv_co_pdiscard() will already suppress discards on the parent if unmap
is false?
Max
signature.asc
Description: OpenPGP digital signature
