Re: [PATCH v19 13/15] net/tap: support local migration with virtio-net

[Markus Armbruster <armbru@redhat.com>]

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> 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 whether to call open/connect in
> TAP initialization code, i.e. we need to know the value of migration
> parameter "local" when creating the TAP device.  For incoming migration,
> we can know only for TAP devices created with QMP after setting the
> migration parameter with QMP.
>
> 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>
> ---
>  net/tap.c     | 165 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  qapi/net.json |  22 ++++++-
>  2 files changed, 180 insertions(+), 7 deletions(-)
>
> diff --git a/net/tap.c b/net/tap.c
> index 2bd4b089573..4162820bcee 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -38,12 +38,17 @@
>  #include "monitor/monitor.h"
>  #include "system/runstate.h"
>  #include "system/system.h"
> +#include "migration/misc.h"
>  #include "qapi/error.h"
>  #include "qemu/cutils.h"
>  #include "qemu/error-report.h"
>  #include "qemu/main-loop.h"
>  #include "qemu/sockets.h"
>  #include "hw/virtio/vhost.h"
> +#include "hw/core/vmstate-if.h"
> +#include "migration/vmstate.h"
> +#include "qom/object.h"
> +#include "qom/compat-properties.h"
>  
>  #include "net/tap.h"
>  #include "net/util.h"
> @@ -71,6 +76,8 @@ static const int kernel_feature_bits[] = {
>  
>  OBJECT_DECLARE_SIMPLE_TYPE(TAPState, TAP_NETDEV)
>  
> +static const VMStateDescription vmstate_tap;
> +
>  struct TAPState {
>      Object parent_obj;
>  
> @@ -95,6 +102,7 @@ struct TAPState {
>      int queue_index;
>      bool read_poll_detached;
>      VMChangeStateEntry *vmstate;
> +    bool local_migration_supported;
>  };
>  
>  static void launch_script(const char *setup_script, const char *ifname,
> @@ -409,6 +417,8 @@ static void tap_cleanup(NetClientState *nc)
>      tap_write_poll(s, false);
>      close(s->fd);
>      s->fd = -1;
> +
> +    vmstate_unregister(VMSTATE_IF(s), &vmstate_tap, s);
>  }
>  
>  static void tap_poll(NetClientState *nc, bool enable)
> @@ -445,6 +455,76 @@ static VHostNetState *tap_get_vhost_net(NetClientState *nc)
>      return s->vhost_net;
>  }
>  
> +static bool tap_is_wait_incoming(NetClientState *nc)
> +{
> +    TAPState *s = container_of(nc, TAPState, nc);
> +    assert(nc->info->type == NET_CLIENT_DRIVER_TAP);
> +    return s->fd == -1;
> +}
> +
> +static bool tap_pre_load(void *opaque, Error **errp)
> +{
> +    TAPState *s = opaque;
> +
> +    if (s->fd != -1) {
> +        error_setg(errp,
> +                   "TAP is already initialized and cannot receive "
> +                   "incoming fd.  For local migration, 'local' "
> +                   "migration parameter must be set _before_ "
> +                   "creating TAP device.");

From error_setg()'s contract:

 * The resulting message should be a single phrase, with no newline or
 * trailing punctuation.

Suggest something like

           error_setg(errp,
                      "TAP is already initialized and cannot receive "
                      "incoming fd");
           error_append_hint(errp,
                             "Migration parameter 'local' must be set"
                             " _before_ creating the TAP device.");

Requires ERRP_GUARD().

> +        return false;
> +    }
> +
> +    return true;
> +}

[...]

> diff --git a/qapi/net.json b/qapi/net.json
> index ada0329ef9d..d0d6e303b0d 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -435,6 +435,25 @@
>  # @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 a 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,
> +#     the following options are not supported and must not be set:
> +#     @fd, @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)

Default?

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