Re: [PATCH v2 0/5] fix migration with bitmaps and mirror

[Vladimir Sementsov-Ogievskiy]

03.04.2020 18:05, Dr. David Alan Gilbert wrote:
> * Vladimir Sementsov-Ogievskiy (address@hidden) wrote:
> > 03.04.2020 14:29, Vladimir Sementsov-Ogievskiy wrote:
> > > 03.04.2020 14:23, Peter Krempa wrote:
> > > > On Fri, Apr 03, 2020 at 14:02:47 +0300, Vladimir Sementsov-Ogievskiy wrote:
> > > > > 19.12.2019 13:36, Peter Krempa wrote:
> > > > > > On Thu, Dec 19, 2019 at 11:51:01 +0300, Vladimir Sementsov-Ogievskiy wrote:
> > > > > > > Hi all!
> > > > > > >
> > > > > > > It's a continuation for
> > > > > > > "bitmap migration bug with -drive while block mirror runs"
> > > > > > > <address@hidden>
> > > > > > > https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg07241.html
> > > > > > >
> > > > > > > The problem is that bitmaps migrated to node with same node-name or
> > > > > > > blk-parent name. And currently only the latter actually work in libvirt.
> > > > > > > And with mirror-top filter it doesn't work, because
> > > > > > > bdrv_get_device_or_node_name don't go through filters.
> > > > > >
> > > > > > I want to point out that since libvirt-5.10 we use -blockdev to
> > > > > > configure the backend of storage devices with qemu-4.2 and later. This
> > > > > > means unfortunately that the BlockBackend of the drive does not have a
> > > > > > name any more and thus the above will not work even if you make the
> > > > > > lookup code to see through filters.
> > > > >
> > > > > Not that this series doesn't make things worse, as it loops through named
> > > > > block backends when trying to use their name for migration. So with these
> > > > > patches applied, qemu will just work in more possible scenarios.
> > > >
> > > > Okay, if that's so it's fair enough in this case.
> > > >
> > > > I'm just very firmly against baking in the assumption that
> > > > node names mean the same thing accross migration, because that will
> > > > create a precedent situation and more stuff may be baked in on top of
> > > > this in the future. It seems that it has already happened though and
> > > > it's wrong. And the worst part is that it's never mentioned that this
> > > > might occur. But again, don't do that and preferrably remove the
> > > > matching of node names for bitmaps altogether until we can control it
> > > > arbitrarily.
> > > >
> > > > We've also seen this already before with the backend name of memory
> > > > devices being baked in to the migration stream which creates an unwanted
> > > > dependancy.
> > >
> > > Hmm. Actually, matching by node-name never worked. May be just drop it now, and 
> > > allow only matching by blk-name?
> > >
> > > And then (in 5.1) implement special qmp commands for precise mapping.
> >
> > Hmm, it may break someones setup... Bad idea. Probably we can forbid 
> > auto-generated node-names.
>
> If we want to remove it I guess we have to go through a proper
> deprecation; but that's OK.
>
> The thing to keep in mind is that when people say 'the commandline
> should match' on source/destination - that's just not true;
> so we have to define what actually needs to stay the same for bitmap
> migration to work.

Hmm. Let's add two qmp commands

1. migrate-set-outgoing-bitmap-mapping, which can set mapping (node-name, 
bitmap-name) -> (migration-node-name, migration-bitmap-name)
2. migrate-set-incoming-bitmap-mapping, which can set mapping 
(migration-node-name, migration-bitmap-name) -> (node-name, bitmap-name)

So, if we want to migrate bitmap B1 from node N1 on source to node M1 on 
target, we'll have three possibilities:

1. call on source migrate-set-outgoing-bitmap-mapping, to set mapping (N1, B1) 
-> (M1, B1) (and target will use 'M1' from migration stream to search the node)

2. call on destination migrate-set-incoming-bitmap-mapping, to set mapping (N1, 
B1) -> (M1, B1) (source will just put 'N1' to migration stream, and target will 
made transformation)

or even
3. Set mapping both on source and target, to make migration stream abstract, for example, 
(N1, B1) -> ('_abstract_bitmap_migration_node_', <bitmap-id>) -> (M1, B1)

Using 1 and 2, it is possible to make any migration to/from older Qemu version..

And what should be deprecated is dirty-bitmaps migration capability, which is 
associated with old behavior. So, newer libvirt will call set-mapping commands 
both on source and target, instead of enabling capability.


--
Best regards,
Vladimir