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
