Re: [Qemu-devel] [PATCH v2 2/2] blockjob: use blk_new_pinned in block_job_create

[Vladimir Sementsov-Ogievskiy]

06.06.2019 13:05, Kevin Wolf wrote:
> Am 05.06.2019 um 19:16 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> 05.06.2019 20:11, Kevin Wolf wrote:
>>> Am 05.06.2019 um 14:32 hat Vladimir Sementsov-Ogievskiy geschrieben:
>>>> child_role job already has .stay_at_node=true, so on bdrv_replace_node
>>>> operation these child are unchanged. Make block job blk behave in same
>>>> manner, to avoid inconsistent intermediate graph states and workarounds
>>>> like in mirror.
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>>>
>>> This feels dangerous. It does what you want it to do if the only graph
>>> change below the BlockBackend is the one in mirror_exit_common. But the
>>> user could also take a snapshot, or in the future hopefully insert a
>>> filter node, and you would then want the BlockBackend to move.
>>>
>>> To be honest, even BdrvChildRole.stay_at_node is a bit of a hack. But at
>>> least it's only used for permissions and not for the actual data flow.
>>
>> Hmm. Than, may be just add a parameter to bdrv_replace_node, which parents
>> to ignore? Would it work?
> 
> I would have to think a bit more about it, but it does sound safer.
> 
> Or we take a step back and ask why it's even a problem for the mirror
> block job if the BlockBackend is moved to a different node. The main
> reason I see is because of bs->job that is set for the root node of the
> BlockBackend and needs to be unset for the same node.
> 
> Maybe we can just finally get rid of bs->job? It doesn't have many users
> any more.
> 

Hmm, looked at it. Not sure what should be refactored around job to get rid
of "main node" concept.. Which seems to be in a bad relation with starting
job on implicit filters as a main node..

But about just removing bs->job pointer, I don't know at least what to do with
blk_iostatus_reset and blockdev_mark_auto_del..


-- 
Best regards,
Vladimir