Re: bitmap migration bug with -drive while block mirror runs

[John Snow <jsnow@redhat.com>]

On 10/1/19 11:57 AM, Vladimir Sementsov-Ogievskiy wrote:
> 01.10.2019 17:10, John Snow wrote:
>>
>>
>> On 10/1/19 10:00 AM, Vladimir Sementsov-Ogievskiy wrote:
>>>> Otherwise: I have a lot of cloudy ideas on how to solve this, but
>>>> ultimately what we want is to be able to find the "addressable" name for
>>>> the node the bitmap is attached to, which would be the name of the first
>>>> ancestor node that isn't a filter. (OR, the name of the block-backend
>>>> above that node.)
>>> Not the name of ancestor node, it will break mapping: it must be name of the
>>> node itself or name of parent (may be through several filters) block-backend
>>>
>>
>> Ah, you are right of course -- because block-backends are the only
>> "nodes" for which we actually descend the graph and add the bitmap to
>> its child.
>>
>> So the real back-resolution mechanism is:
>>

Amendment:
   - If our local node-name N is well-formed, use this.
   - Otherwise:
>> - Find the first non-filter ancestor, A
>> - if A is not a block-backend, we must use our node-local name.
     Amendment: If it's not a BB, we have no addressable name
                for the bitmap and this is an error.
>> - if A's name is empty, we must use our node-local name.
     Amendment: If it's empty, we have no addressable name
                for the bitmap and this is an error.
>> - If the name we have chosen is not id_wellformed, we have no
>> migration-stable addressable name for this bitmap and the migration must
>> fail!
     (Handled by above amendments.)

The reason for the change is to prefer user-defined node names whenever
possible; only trying to find a "device" or "backend" name whenever we
fail to find one.

>>
>>
>> For resolving bitmap addresses via QMP (node, name) pairs; the
>> resolution method would be this:
>>
>> - if the node-name N is a block-backend, descend the tree until we find
>> the first non-filter node V.
>> - if the node-name N is a BlockDriverState, use this node directly.
>>
> 
> Looks good for me.
> 

I hope the increased leeway of the name resolution doesn't make things
worse with respect to filters. I specified this to match the
back-resolution process.

I suppose if you want to add bitmaps to filters, you are required to add
them by node-name specifically.

> I also think if on destination we have both block-backend with name N and
> block-node with name N and the latter is not (filtered) child of the former,
> we should fail migration of at least that bitmap. (Hope, nobody reuse
> block-backend names as node-names in practice.. (should we restrict it
> explicitly ?))
> 

:(

>>
>> (I don't have the time to investigate the code snippet right now; my
>> attention is being pulled to a different project. sorry!)
>>
> 
> So, you are not working on this? Then I'll make patches.
> 

Right; I'm not prototyping a fix currently.

--js