All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Cc: jasowang@redhat.com, mst@redhat.com, armbru@redhat.com,
	peterx@redhat.com, farosas@suse.de, raphael.s.norwitz@gmail.com,
	bchaney@akamai.com, qemu-devel@nongnu.org, pbonzini@redhat.com,
	yc-core@yandex-team.ru, mark.caveayland@nutanix.com,
	"Philippe Mathieu-Daudé" <philmd@mailo.com>,
	"Zhao Liu" <zhao1.liu@intel.com>,
	"Jason Wang" <jasowangio@gmail.com>,
	"Eric Blake" <eblake@redhat.com>
Subject: Re: [PATCH v18 12/14] net/tap: support local migration with virtio-net
Date: Mon, 13 Jul 2026 12:17:00 +0100	[thread overview]
Message-ID: <alTJLI3EWU96Ks61@redhat.com> (raw)
In-Reply-To: <20260710191809.721154-13-vsementsov@yandex-team.ru>

On Fri, Jul 10, 2026 at 10:18:01PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Support transferring of TAP state (including open fd).
> 
> Add new property "local-migration-supported", which defines
> whether local-migration is actually supported for this TAP device.
> Starting from 11.2 QEMU Machine Types it's enabled by default.
> 
> Note, that local-migration (including migrating opened FDs
> through migration channel, which must be UNIX socket), is
> enabled by global "local" migration parameters. But individual
> devices may have additional options to enable/disable it
> per device.
> 
> The tricky thing, is that we need to know, should we do
> call open/connect in TAP initialization code. That means,
> that on incoming migration we want to know status of
> "local" migration parameter at time of creating TAP device.
> So, currently, for incoming local migration only TAP device
> created through QMP is supported, as you need to set migration
> parameter first (through QMP as well).
> 
> So the full picture is:
> 
> On source, to start outgoing "local" migration you need:
> 
>  - migration parameter "local" set to true
>  - "local-migration-supported" TAP option set to true (the
>    default, starting from 11.2 QEMU Machine Types)
> 
> If at least one of these options is not set, TAP backend
> doesn't participate in migration.
> 
> On target, things are more difficult:
> 
> Same, you need both "local" and "local-migration-supported"
> be set. And same, if one of them is not set, TAP backend
> is initialized as usual, and doesn't accept any incoming
> state.
> 
> Additionally, if you are going to set "local", it must be
> set before creating the TAP device. If TAP device created
> with "local" unset, it initializes as usual. If you enable
> "local" after it and start incoming migration, it will fail
> in .pre_load handler of TAP backend.
> 
> Moreover, there are interface restrictions: if you create TAP
> device when QEMU is in INCOMING state, and both "local"
> and "local-migration-supported" set, most of TAP options are
> not allowed, and script/downscript are required to be explicitly
> unset (set to "" or "no").
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> 
> In v17 thread we considered making new qapi parameters "unstable".
> Still, it was because of doubtful "incoming-fds" parameter. In this
> v18 it's completely removed, and we may live only with "local"
> migration parameter + "local-migration-supported" TAP option.
> This way the series doesn't conflict with suggested by Peter
>   [PATCH RFC 0/2] migration/vl: new -incoming config:* for early migration parameters
> So, I think, worth making one more iteration as "stable candidate".
> If still some doubts, not problem for me to resend with "unstable" features.
> 
>  hw/core/machine.c |   7 ++
>  net/tap.c         | 162 ++++++++++++++++++++++++++++++++++++++++++++--
>  qapi/net.json     |  23 ++++++-
>  3 files changed, 185 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 15886a56b19..2a1abb29041 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -36,10 +36,16 @@
>  #include "hw/virtio/virtio-pci.h"
>  #include "hw/virtio/virtio-net.h"
>  #include "hw/virtio/virtio-iommu.h"
> +#include "net/tap.h"
>  #include "hw/acpi/generic_event_device.h"
>  #include "qemu/audio.h"
>  #include "hw/arm/smmuv3.h"
>  
> +/*
> + * TODO: When hw_compat_11_1 appears, local-migration-supported=false
> + * for TYPE_TAP_NETDEV should be move to it from hw_compat_11_0.
> + */
> +
>  GlobalProperty hw_compat_11_0[] = {
>      { "chardev-vc", "encoding", "cp437" },
>      { "tpm-crb", "cap-chunk", "off" },
> @@ -49,6 +55,7 @@ GlobalProperty hw_compat_11_0[] = {
>      { TYPE_ARM_SMMUV3, "ssidsize", "0" },
>      { TYPE_ARM_SMMUV3, "oas", "44" },
>      { "migration", "switchover-ack-legacy", "on" },
> +    { TYPE_TAP_NETDEV, "local-migration-supported", "false" },

So disabled by default for historical machines type versions, but....

> +static void tap_instance_init(Object *obj)
> +{
> +    TAPState *s = TAP_NETDEV(obj);
> +    s->local_migration_supported = true;
> +}

..enabled by default for all future machine type versions, but...


> @@ -1005,6 +1123,23 @@ int net_init_tap(const Netdev *netdev, const char *name,
>          return -1;
>      }
>  
> +    incoming_fds = tap->local_migration_supported && migrate_local() &&
> +                   runstate_check(RUN_STATE_INMIGRATE);
> +
> +    if (incoming_fds &&
> +        (tap->fd || tap->fds || tap->helper || tap->br || tap->ifname ||
> +         tap->has_sndbuf || tap->has_vnet_hdr ||
> +         !tap_is_explicit_no_script(tap->script) ||
> +         !tap_is_explicit_no_script(tap->downscript))) {
> +        error_setg(errp, "Local incoming migration of TAP device (-incoming, "
> +                   "migration parameter @local is set, "
> +                   "TAP parameter @local-migration-supported is set) "
> +                   "is incompatible with "
> +                   "fd=, fds=, helper=, br=, ifname=, sndbuf= and vnet_hdr=, "
> +                   "and requires explicit empty script= and downscript=");
> +        return -1;
> +    }

...not usable by default unless the mgmt app knows to avoid these settings,
but the user won't find out about this limit until they try to migrate and
it fails at runtime, months or years after the VM was first configured &
launched :-(

AFAICT, there's also no way for mgmt apps to determine which parameters
are forbidden, because it is merely exposed in API docs text, no way to
introspect

IMHO the sane way to do this is to not have any "local-migration-supported"
device flag at all, and also no "local" migration parameter. The src & dst
QEMU's so badly need to be able to auto-negotiate and just do the right
thing, otherwise this local migration optimization becomes a mgmt nightmare
long term :-(

And yet we don't have any auto-negotiate mechanism, and I don't want to
block your work :-(

I'm annoyed that we keep creating more and more pain for both ourselves
and our mgmt apps in migration for as long as we don't address this
fundamental migration protocol design flaw.

> diff --git a/qapi/net.json b/qapi/net.json
> index ada0329ef9d..2070979f20e 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -435,6 +435,26 @@
>  # @poll-us: maximum number of microseconds that could be spent on busy
>  #     polling for tap (since 2.7)
>  #
> +# @local-migration-supported: enable local migration for this TAP
> +#     backend.  When set, local migration is enabled/disabled by
> +#     migration parameter @local for this TAP backend.  When unset,
> +#     migration parameter @local is ignored for this TAP backend.
> +#     To be able to do incoming local migration of TAP backend,
> +#     migration parameter @local must be set _before_ creating the
> +#     TAP backend.  Otherwise, TAP backend is initialized as usual,
> +#     opening/creating TAP devices in kernel.  In this case further
> +#     local incoming migration (with migration parameter @local set
> +#     after creating TAP backend with @local-migration-supporeted
> +#     parameter set) will simply fail.
> +#     Moreover, when QEMU is in incoming migration state, migration
> +#     parameter @local is set and @local-migration-supported is set,
> +#     following options are not supported and must not be set:
> +#     @ds, @fds, @helper, @br, @ifname, @sndbuf, @vnet_hdr.
> +#     Additionally in this case @script and @downscipt must be
> +#     explicitly disabled (empty strings or "no").
> +#     (Since 11.2.  Defaults to true for QEMU Machine Types >= 11.2,
> +#     and to false for QEMU Machine Types < 11.2)



> +#
>  # Since: 1.2
>  ##
>  { 'struct': 'NetdevTapOptions',
> @@ -453,7 +473,8 @@
>      '*vhostfds':   'str',
>      '*vhostforce': 'bool',
>      '*queues':     'uint32',
> -    '*poll-us':    'uint32'} }
> +    '*poll-us':    'uint32',
> +    '*local-migration-supported': 'bool' } }
>  
>  ##
>  # @NetdevSocketOptions:
> -- 
> 2.43.0
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|



  parent reply	other threads:[~2026-07-13 11:17 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-10 19:17 [PATCH v18 00/14] virtio-net: live-TAP local migration Vladimir Sementsov-Ogievskiy
2026-07-10 19:17 ` [PATCH v18 01/14] net/tap: rework tap_parse_script Vladimir Sementsov-Ogievskiy
2026-07-10 19:17 ` [PATCH v18 02/14] net/tap: improve script/downscript options documentation Vladimir Sementsov-Ogievskiy
2026-07-13  7:33   ` Markus Armbruster
2026-07-10 19:17 ` [PATCH v18 03/14] net/tap: deprecate "no" as special value for script/downscript Vladimir Sementsov-Ogievskiy
2026-07-13  7:45   ` Markus Armbruster
2026-07-13  9:37     ` Vladimir Sementsov-Ogievskiy
2026-07-13  9:47       ` Daniel P. Berrangé
2026-07-13 10:25         ` Vladimir Sementsov-Ogievskiy
2026-07-10 19:17 ` [PATCH v18 04/14] net/tap: move vhost-net open() calls to tap_parse_vhost_fds() Vladimir Sementsov-Ogievskiy
2026-07-10 19:17 ` [PATCH v18 05/14] net/tap: move vhost initialization to tap_setup_vhost() Vladimir Sementsov-Ogievskiy
2026-07-10 19:17 ` [PATCH v18 06/14] net/tap: use container_of instead of DO_UPCAST Vladimir Sementsov-Ogievskiy
2026-07-10 19:17 ` [PATCH v18 07/14] net/tap: QOMify tap backend Vladimir Sementsov-Ogievskiy
2026-07-10 19:17 ` [PATCH v18 08/14] net/tap: add TYPE_VMSTATE_IF interface Vladimir Sementsov-Ogievskiy
2026-07-10 19:17 ` [PATCH v18 09/14] qapi: add local migration parameter Vladimir Sementsov-Ogievskiy
2026-07-13  9:51   ` Daniel P. Berrangé
2026-07-13 10:55     ` Vladimir Sementsov-Ogievskiy
2026-07-13 11:21       ` Daniel P. Berrangé
2026-07-13 17:55         ` Vladimir Sementsov-Ogievskiy
2026-07-13 18:37           ` Daniel P. Berrangé
2026-07-13 20:22             ` Michael S. Tsirkin
2026-07-14  6:50               ` Vladimir Sementsov-Ogievskiy
2026-07-14  8:03                 ` Michael S. Tsirkin
2026-07-14  8:04                   ` Vladimir Sementsov-Ogievskiy
2026-07-14  7:55               ` Daniel P. Berrangé
2026-07-14  8:02                 ` Michael S. Tsirkin
2026-07-14  8:18                   ` Daniel P. Berrangé
2026-07-14  6:45             ` Vladimir Sementsov-Ogievskiy
2026-07-10 19:17 ` [PATCH v18 10/14] virtio-net: support local migration of backend Vladimir Sementsov-Ogievskiy
2026-07-10 19:18 ` [PATCH v18 11/14] net/tap: disable read polling for stopped VM Vladimir Sementsov-Ogievskiy
2026-07-10 19:18 ` [PATCH v18 12/14] net/tap: support local migration with virtio-net Vladimir Sementsov-Ogievskiy
2026-07-13  7:57   ` Markus Armbruster
2026-07-13 11:51     ` Vladimir Sementsov-Ogievskiy
2026-07-13 13:29       ` Markus Armbruster
2026-07-13 15:08         ` Vladimir Sementsov-Ogievskiy
2026-07-14 13:19           ` Michael S. Tsirkin
2026-07-13 11:17   ` Daniel P. Berrangé [this message]
2026-07-13 12:35     ` Vladimir Sementsov-Ogievskiy
2026-07-13 15:05       ` Daniel P. Berrangé
2026-07-13 15:19         ` Vladimir Sementsov-Ogievskiy
2026-07-13 19:36   ` Michael S. Tsirkin
2026-07-14  6:43     ` Vladimir Sementsov-Ogievskiy
2026-07-10 19:18 ` [PATCH v18 13/14] tests/functional: add skipWithoutSudo() decorator Vladimir Sementsov-Ogievskiy
2026-07-10 19:18 ` [PATCH v18 14/14] tests/functional: add test_tap_migration Vladimir Sementsov-Ogievskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alTJLI3EWU96Ks61@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=bchaney@akamai.com \
    --cc=eblake@redhat.com \
    --cc=farosas@suse.de \
    --cc=jasowang@redhat.com \
    --cc=jasowangio@gmail.com \
    --cc=mark.caveayland@nutanix.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=philmd@mailo.com \
    --cc=qemu-devel@nongnu.org \
    --cc=raphael.s.norwitz@gmail.com \
    --cc=vsementsov@yandex-team.ru \
    --cc=yc-core@yandex-team.ru \
    --cc=zhao1.liu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.