Re: [Qemu-devel] [PATCH 2/2] block-stream: include base into job node list

[Andrey Shinkevich]

On 08/03/2019 17:00, Alberto Garcia wrote:
> On Thu 21 Feb 2019 04:26:39 PM CET, Andrey Shinkevich wrote:
>> The block-stream job needs to own the base node as the limiter
>> of the copy-on-read operation. So, the base node is included in
>> the job node list (block_job_add_bdrv).
>> Also, the block-stream job would not allow the base node to go
>> away due to the graph modification, e.g. when a filter node is
>> inserted between the bottom node and the base node.
>> For that reason, the flag BLK_PERM_GRAPH_MOD is unset in the
>> shared permission bit mask of the base node.
>>
>> Signed-off-by: Andrey Shinkevich <address@hidden>
>> ---
>>   block/stream.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/block/stream.c b/block/stream.c
>> index 7a49ac0..c8f93d4 100644
>> --- 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);
> 
> I'm not sure if I understand the ~BLK_PERM_GRAPH_MOD bit, what's the
> consequence of not removing that permission?
> 
> Berto
> 
Alberto,
Thank you for your question.
There will be neither a bad consequence, nor a good one.
That means the block_job_add_bdrv() does not help to protect the graph
from modification. So, "freezing BdrvChild links" may be a better
solution if the chain includes the 'base' node.
-- 
With the best regards,
Andrey Shinkevich