On 18/03/2019 17:42, Alberto Garcia wrote:
> On Tue 26 Feb 2019 05:39:41 PM CET, Andrey Shinkevich wrote:
>> On 26/02/2019 16:33, Alberto Garcia wrote:
>>> On Mon 25 Feb 2019 05:39:14 PM CET, Vladimir Sementsov-Ogievskiy wrote:
>>>>> --- a/block/stream.c
>>>>> +++ b/block/stream.c
>>>>> @@ -259,6 +259,15 @@ void stream_start(const char *job_id,
>>>>> BlockDriverState *bs,
>>>>> &error_abort);
>>>>> }
>>>>>
>>>>> + if (base) {
>>>>> + /*
>>>>> + * The base node should not disappear during the job.
>>>>> + */
>>>>> + block_job_add_bdrv(&s->common, "base", base, 0,
>>>>> + BLK_PERM_ALL & ~BLK_PERM_GRAPH_MOD,
>>>>> + &error_abort);
>>>>> + }
>>>>> +
>>>>> s->base = base;
>>>>> s->backing_file_str = g_strdup(backing_file_str);
>>>>> s->bs_read_only = bs_read_only;
>>>>>
>>>>
>>>>
>>>> Hmm, intersting, is BLK_PERM_GRAPH_MOD a good way to lock relations in
>>>> node graph?
>>>>
>>>> bdrv_replace_node don't check this permission. So, I don't understand,
>>>> how this permission works.. Graph modification operation in difference
>>>> with read or write are not done through BdrvChild at all.
>>>>
>>>> Are there any ideas or work started for some another mechanism of
>>>> "freezing" a subgraph during an operation?
>>>
>>> See this discussion:
>>>
>>> https://lists.gnu.org/archive/html/qemu-block/2018-11/msg00387.html
>>>
>>> And these patches (currently under review):
>>>
>>> https://lists.gnu.org/archive/html/qemu-block/2019-01/msg00622.html
>>
>> The bdrv_freeze_backing_chain() excludes the base BlockDriverState.
>> And we would like to protect the base as well when running the
>> block-stream.
>
> I was just looking into this again. The bdrv_freeze_backing_chain() (now
> in master) freezes all links between bs and base, both included, so base
> cannot go away anymore.
>
> Is block_job_add_bdrv() still necessary in this scenario?
>
> Unless I'm wrong I think that what we can do now is to start getting rid
> of the op blockers, shouldn't it be possible to get the same
> functionality with the permission system plus the BdrvChild.frozen
> attribute ?
>
> Berto
> I have got rid of using the block_job_add_bdrv() for the 'base'.
But, as for the "intermediate nodes", I will want to add the
BLK_PERM_WRITE shared flag to them for the 'discard block' feature
in future. So, I have to check if we can get 'write' permission for
the block discard operation without invoking block_job_add_bdrv()
for them...
Andrey
