[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: bitmap migration bug with -drive while block mirror runs
From: |
Max Reitz |
Subject: |
Re: bitmap migration bug with -drive while block mirror runs |
Date: |
Tue, 1 Oct 2019 18:17:24 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.0 |
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...
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
signature.asc
Description: OpenPGP digital signature
- 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 <=
- Re: bitmap migration bug with -drive while block mirror runs, Vladimir Sementsov-Ogievskiy, 2019/10/01