Re: [PATCH v16 5/8] virtio-net: support local migration of backend

[Peter Xu <peterx@redhat.com>]

On Wed, May 27, 2026 at 11:32:24PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 27.05.26 22:41, Peter Xu wrote:
> > On Wed, May 27, 2026 at 08:19:53PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > > On 27.05.26 14:35, Vladimir Sementsov-Ogievskiy wrote:
> > > > On 26.05.26 14:23, Vladimir Sementsov-Ogievskiy wrote:
> > > > > +
> > > > > +type_init(tap_register_types)
> > > > > diff --git a/qapi/net.json b/qapi/net.json
> > > > > index 82ddbb51cd7..029f5a4532c 100644
> > > > > --- a/qapi/net.json
> > > > > +++ b/qapi/net.json
> > > > > @@ -429,9 +429,18 @@
> > > > 
> > > > a bit more context:
> > > > 
> > > >    # @incoming-fds: do not open or create any TAP devices.  Prepare for
> > > > 
> > > > 
> > > > >    #     getting TAP file descriptors from incoming migration stream.
> > > > >    #     The option is incompatible with any of @fd, @fds, @helper, @br,
> > > > >    #     @ifname, @sndbuf and @vnet_hdr options, and requires @script and
> > > > > -#     @downscript be explicitly set to nothing (empty string or "no")
> > > > > +#     @downscript be explicitly set to nothing (empty string or "no"),
> > > > > +#     and requires also @local-migration to be set an "local"
> > > > > +#     migration parameter be set as well.
> > > > >    #     (Since 11.1)
> > > > >    #
> > > > > +# @local-migration: enable local migration for this TAP backend.
> > > > > +#     When set, local migration is enabled/disabled by "local"
> > > > > +#     migration parameter for this TAP backend.  When unset, "local"
> > > > > +#     migration parameter is ignored for this TAP backend.
> > > > > +#     (Since 11.1.  Defaults to true for MT >= 11.1,
> > > > > +#     and to false for MT < 11.1)
> > > > > +#
> > > > >    # Since: 1.2
> > > > >    ##
> > > > >    { 'struct': 'NetdevTapOptions',
> > > > > @@ -451,7 +460,8 @@
> > > > >        '*vhostforce': 'bool',
> > > > >        '*queues':     'uint32',
> > > > >        '*poll-us':    'uint32',
> > > > > -    '*incoming-fds': 'bool' } }
> > > > > +    '*incoming-fds': 'bool',
> > > > > +    '*local-migration': 'bool' } }
> > > > 
> > > > 
> > > > Having both "incoming-fds" and "local-migration" (or rename
> > > > it to "support-local-migration") seems too much.
> > > > 
> > > > But, we can't simply drop "incoming-fds" and rely only on
> > > > "local-migration", a this make logic a lot more complicated,
> > > > as "local" migration parameter starts to "decide" how tap
> > > > initialization works:
> > > > 
> > > > We have to postpone opening any files up to "pre-incoming"
> > > > point, when all migration parameters are known, to decide:
> > > > 
> > > >     if "local-migration"=true and "local"=true - do not open
> > > >        any files, wait for incoming FDs
> > > > 
> > > >     if "local-migration"=true and "local"-false - open files
> > > > 
> > > > In this picture, the fact that migration parameter influence
> > > > device initialization - seems rather bad precedent, which is
> > > > better to avoid.
> > > > 
> > > > On the other hand, with "incoming-fds" parameter, everything is
> > > > explicit. We can simply detect and error-out incorrect
> > > > combinations (like incoming-fds=true, but incoming migration
> > > > started with local=false, etc.), but user is sure, that target
> > > > QEMU process will not open any files with this parameter set.
> > > > That's a safe way.
> > > > 
> > > > 
> > > > Still, I have alternative idea:
> > > > 
> > > > Instead of combination "-incoming defer" + "incoming-fds=true",
> > > > make a new "-incoming local".
> > > > 
> > > > "-incoming local" would be equal to "-incoming defer", but also
> > > > implies local=true (i.e. starting incoming migration with
> > > > local=false will simply fail).
> > > > 
> > > > This way, we know that local=True from the start of the process,
> > > > and don't have to wait for some pre-incoming point to be sure
> > > > in value of "local" parameter.
> > > > 
> > > > So, we simply check "local-migration == True  +  incoming == local"
> > > > instead of "incoming-fds == True" in tap code, and we are done.
> > > > 
> > > > What do you think?
> > > > 
> > > > 
> > > 
> > > Hmm. Bad idea. It may have same problem if use commandline options
> > > to create devices, as ordering of "-incoming local" and "-netdev"
> > > may affect how tap device will be initialized. It can be solved (or,
> > > maybe, it works as expected even now, if "-incoming" always handled
> > > before "-netdev" options... But anyway that's much more shaky than
> > > simple incoming-fds=True.
> > 
> > Personally I liked part of your proposal.. normally I would rather leave it
> > for later, but then it means we will be stuck with this incoming-fds API.
> > So I'll still leave the thoughts below for consideration.
> > 
> > In general, to me this is another requirement to specify migration
> > parameters during early boot.
> > 
> > We used to tackle with this problem a few times, in many cases by
> > reordering of initialization of the migration object, which can be error
> > prone.
> > 
> > We almost moved to another model where we moved some special migration
> > parameters out of the migration object, into some global variables instead,
> > then they're available since the start of QEMU.
> > 
> > Two examples I am aware of while looking at this (maybe more):
> > 
> > - The -only-migratable option
> > 
> >    This one used to be a "parameter-like" option, but then things broke, we
> >    moved to a global only_migratable variable, to make it available during
> >    early boot.
> > 
> >    Currently, it consumes a special option, QEMU_OPTION_only_migratable.
> > 
> > - The "mode" parameter (used by CPR-transfer)
> > 
> >    This one is literally a parameter even now, CPR-transfer needs it to be
> >    set even before loading of CPR channel, which is very, very early..
> > 
> >    It's done by quite a few complex steps:
> > 
> >    - hijacking -incoming parameter, in incoming_option_parse() we first
> >      allow setup of CPR channels,
> > 
> >    - then at cpr_state_load(), it consumes that channel when set, invoke
> >      cpr_set_incoming_mode(), which sets a magical global var ( :( ) called
> >      incoming_mode,
> > 
> >    - then, further hijack migrate_mode(), which normally should just fetch
> >      "mode" parameter from migration object, peek at incoming_mode first,
> >      when set, overwrite the "mode" parameter...
> > 
> > Now this is the third one: we actually want to have "local" parameter
> > available.
> > 
> > I wonder if we should just have some migration parameters to be special to
> > be set early, maintained in a single global place, then when creating
> > migration objects we should apply these to migration object, making sure
> > it's consistent.
> > 
> > We could actually reuse -incoming, so far it supprts:
> > 
> >    - "defer", as a string
> >    - URI, in form of (SOME_WORD:.*)
> >    - then we assume it's a channel
> > 
> > Since we already have defer, we could make a special case for it, say,
> >    -incoming config:key1=value1,key2=value2,...
> > 
> > With that, migration options can be set at boot and can be referenced
> > anytime, we don't need to worry on migration object init ordering.  We
> > could obsolete -only-migratable but allow:
> > 
> >    -incoming config:only-migrate=on
> > 
> > For CPR, to keep compatibility we still need to set mode=cpr-* silently,
> > but then we should be able to be able to reference any migration parameters
> > anytime, including local= now, removing incoming-fds TAP option.
> > 
> > I'm not sure if this is a good approach, but we can think about it.  I do
> > worry we have future demands on similar things, then we need to tackle it
> > sooner or later (we will want to stop introducing special parameters at
> > some point, like incoming-fds=on).
> > 
> > Thanks,
> > 
> 
> What make me doubt is asymmetry between outgoing and incoming config:
> 
> If we add early "local" parameter, which is set by
> "-incoming config:local=on", what about normal "local" parameter?

When creating the migration object, we should apply all those config:* to
it to keep it consistent.  Then the global can be released.

> 
> If we keep it as is, user may think, that config:local=on and setting
> local parameter are the same, and chose setting the parameter through
> QMP (which is bad for us)..

Yeah, in documents we need to mark -incoming config:* to be required for
use cases.  We can also mention that in the manual about the difference,
but I agree it's not easy to catch.

> 
> So, seems, local migration parameter and local early parameter should
> be different things.
> 
> Maybe, rename "local" parameter to "outgoing-local", and ignore it for
> incoming migration. And add "-incoming config:incoming-local=on", which
> produces "incoming-local" parameter, which may be set _only_ through
> "-incoming" option and cannot be set by migrate-set-parameters, to avoid
> any kind of misuse.
> 
> Doesn't look ideal :/ And it breaks the rule that migration parameters
> should be the same for source and target for migration to work properly.
> 
> 
> OK, the workaround is: keep "local" name for both "early" and "normal"
> versions of parameter, and add some checks:
> 
> - If user tries to set "local" parameter during incoming migration,
>   but "early local" is not set - fail
> 
> - If user tries to run incoming migration, when "early local" is set
>   but "normal local" is not - fail too
> 
> This way, the only possible and correct way to turn on local migration
> on target is to set both "early" and "normal" local parameter. Still,
> not ideal.

My goal is to make it really the same thing, no need to two "local"
parameters, no need to differenciate src/dst "local" parameters. They
should be the same. Just to provide a slightly cleaner way to let us not
worry about "whether migration object is created, and what's the order of
creation" kind of problems.

-- 
Peter Xu