[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: bitmap migration bug with -drive while block mirror runs
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: bitmap migration bug with -drive while block mirror runs |
Date: |
Tue, 1 Oct 2019 16:22:51 +0000 |
01.10.2019 19:17, Max Reitz wrote:
> On 01.10.19 18:12, Kevin Wolf wrote:
>> Am 01.10.2019 um 17:27 hat Max Reitz geschrieben:
>>> On 01.10.19 17:09, Kevin Wolf wrote:
>>>> Am 01.10.2019 um 16:34 hat Max Reitz geschrieben:
>>>>> On 01.10.19 16:27, Vladimir Sementsov-Ogievskiy wrote:
>>>>>> 01.10.2019 17:13, Max Reitz wrote:
>>>>>>> On 01.10.19 16:00, Vladimir Sementsov-Ogievskiy wrote:
>>>>>>>> 01.10.2019 3:09, John Snow wrote:
>>>>>>>>> Hi folks, I identified a problem with the migration code that Red Hat
>>>>>>>>> QE
>>>>>>>>> found and thought you'd like to see it:
>>>>>>>>>
>>>>>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1652424#c20
>>>>>>>>>
>>>>>>>>> Very, very briefly: drive-mirror inserts a filter node that changes
>>>>>>>>> what
>>>>>>>>> bdrv_get_device_or_node_name() returns, which causes a migration
>>>>>>>>> problem.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Ignorant question #1: Can we multi-parent the filter node and
>>>>>>>>> source-node? It looks like at the moment both consider their only
>>>>>>>>> parent
>>>>>>>>> to be the block-job and don't have a link back to their parents
>>>>>>>>> otherwise.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 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
>>>>>>>>
>>>>>>>>>
>>>>>>>>> A simple way to do this might be a "child_unfiltered" BdrvChild role
>>>>>>>>> that simply bypasses the filter that was inserted and serves no real
>>>>>>>>> purpose other than to allow the child to have a parent link and find
>>>>>>>>> who
>>>>>>>>> it's """real""" parent is.
>>>>>>>>>
>>>>>>>>> Because of flushing, reopen, sync, drain &c &c &c I'm not sure how
>>>>>>>>> feasible this quick idea might be, though.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> - Corollary fix #1: call error_setg if the bitmap node name that's
>>>>>>>>> about
>>>>>>>>> to go over the wire is an autogenerated node: this is never correct!
>>>>>>>>>
>>>>>>>>> (Why not? because the target is incapable of matching the node-name
>>>>>>>>> because they are randomly generated AND you cannot specify node-names
>>>>>>>>> with # prefixes as they are especially reserved!
>>>>>>>>>
>>>>>>>>> (This raises a related problem: if you explicitly add bitmaps to nodes
>>>>>>>>> with autogenerated names, you will be unable to migrate them.))
>>>>>>>>>
>>>>>>>>> --js
>>>>>>>>>
>>>>>>>>
>>>>>>>> What about the following:
>>>>>>>>
>>>>>>>> diff --git a/block.c b/block.c
>>>>>>>> index 5944124845..6739c19be9 100644
>>>>>>>> --- a/block.c
>>>>>>>> +++ b/block.c
>>>>>>>> @@ -1009,8 +1009,20 @@ static void bdrv_inherited_options(int
>>>>>>>> *child_flags, QDict *child_options,
>>>>>>>> *child_flags = flags;
>>>>>>>> }
>>>>>>>>
>>>>>>>> +static const char *bdrv_child_get_name(BdrvChild *child)
>>>>>>>> +{
>>>>>>>> + BlockDriverState *parent = child->opaque;
>>>>>>>> +
>>>>>>>> + if (parent->drv && parent->drv->is_filter) {
>>>>>>>> + return bdrv_get_parent_name(parent);
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + return NULL;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>
>>>>>>> Why would we skip filters explicitly added by the user?
>>>>>>>
>>>>>>
>>>>>> Why not? Otherwise migration of bitmaps will not work: we may have
>>>>>> different set
>>>>>> of filters on source and destination, and we still should map nodes with
>>>>>> bitmaps
>>>>>> automatically.
>>>>>
>>>>> Why would we have a different set of explicitly added filters on source
>>>>> and destination and allow them to be automatically changed during
>>>>> migration? Shouldn’t users only change them pre or post migration?
>>>>
>>>> We never made a requirement that the backend must be the same on the
>>>> source and the destination. Basically, migration copies the state of
>>>> frontends and the user is responsible for having these frontends created
>>>> and connected to the right backends on the destination.
>>>>
>>>> Using different paths on the destination is a very obvious requirement
>>>> for block devices. It's less obvious for the graph structure, but I
>>>> don't see a reason why it couldn't change on migration. Say we were
>>>> using local storage on the source, but now we did storage migration to
>>>> some network storage, access to which should be throttled.
>>>
>>> I don’t quite see why we couldn’t add such filters before or after
>>> migration.
>>
>> Possibly. But why would we when the source doesn't need the filter? We
>> don't change the image path before migration either.
>>
>> I think the tricky part is coming up with rules and "keep the frontend
>> the same, the backend can change arbitrarily" is a very easy rule.
>
> OK, indeed.
>
>>> And it was my impression that bitmap migration was a problem now
>>> precisely because it is bound to the graph structure.
>>
>> So apparently I wasn't completely wrong when I preferred just writing
>> bitmaps back to the image instead of transferring them in the migration
>> stream...
>>
>> It's not really bound to the graph structure per se, but to node names
>> and for non-anonymous BlockBackends to the link between the BB and its
>> root node. The latter is part of the graph structure, but only a very
>> small part, and it exists only for legacy (non-blockdev) configurations.
>>
>>> But anyway. I’ll gladly remove myself from this discussion because I
>>> don’t know much about migration and actually I’d prefer to keep it that
>>> way. (Sorry.)
>>
>> Good idea, let's have the migration maintainers handle this.
>
> :-)
>
> That’s always how it goes, isn’t it? Migration maintainers don’t know
> the block side, and we don’t know the migration side...
Haha, luckily I'm not a maintainer :)
>
> Anyway. It’s just a fact that I don’t have much to add to the
> discussion, whereas there seems to be a productive discussion without me
> already.
>
> Max
>
--
Best regards,
Vladimir
- Re: bitmap migration bug with -drive while block mirror runs, (continued)
- Re: bitmap migration bug with -drive while block mirror runs, Max Reitz, 2019/10/01
- Re: bitmap migration bug with -drive while block mirror runs, Vladimir Sementsov-Ogievskiy, 2019/10/01
- Re: bitmap migration bug with -drive while block mirror runs, Max Reitz, 2019/10/01
- Re: bitmap migration bug with -drive while block mirror runs, Vladimir Sementsov-Ogievskiy, 2019/10/01
- Re: bitmap migration bug with -drive while block mirror runs, Max Reitz, 2019/10/01
- Re: bitmap migration bug with -drive while block mirror runs, Peter Krempa, 2019/10/02
- Re: bitmap migration bug with -drive while block mirror runs, Kevin Wolf, 2019/10/01
- Re: bitmap migration bug with -drive while block mirror runs, Max Reitz, 2019/10/01
- Re: bitmap migration bug with -drive while block mirror runs, Kevin Wolf, 2019/10/01
- Re: bitmap migration bug with -drive while block mirror runs, Max Reitz, 2019/10/01
- Re: bitmap migration bug with -drive while block mirror runs,
Vladimir Sementsov-Ogievskiy <=