Re: [PATCH 24/33] chardev-add: support local migration

[Markus Armbruster <armbru@redhat.com>]

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

> This commit introduces a possibility to migrate open chardev
> socket fd through migration channel without reconnecting.
>
> For this, user should:
>  - enable new migration capability local-char-socket
>  - mark the socket by an option support-local-migration=true
>  - on target add local-incoming=true option to the socket
>
> Motivation for the API:
>
> 1. We don't want to migrate all sockets. For example, QMP-connection is
>    bad candidate, as it is separate on source and target. So, we need
>    @support-local-migration option to mark sockets, which we want to
>    migrate (after this series, we'll want to migrate chardev used to
>    connect with vhost-user-server).
>
> 2. Still, for remote migration, we can't migrate any sockets, so, we
>    need a capability, to enable/disable the whole feature.
>
> 3. And finally, we need a sign for the socket to not open a connection
>    on initialization, but wait for incoming migration. We can't use
>    @support-local-migration option for it, as it may be enabled, but we
>    are in incoming-remote migration. Also, we can't rely on the
>    migration capability, as user is free to setup capabilities before or
>    after chardev creation, and it would be a bad precedent to create
>    relations here.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

[...]

> diff --git a/qapi/char.json b/qapi/char.json
> index f0a53f742c..5b535c196a 100644
> --- a/qapi/char.json
> +++ b/qapi/char.json
> @@ -280,11 +280,23 @@
>  #     mutually exclusive with @reconnect.
>  #     (default: 0) (Since: 9.2)
>  #
> +# @support-local-migration: The socket open file descriptor will
> +#     migrate if this field is true and local-char-socket migration
> +#     capability enabled (default: false) (Since: 10.2)
> +#
> +# @local-incoming: Do load open file descriptor for the socket
> +#     on incoming migration. May be used only if QEMU is started
> +#     for incoming migration and only together with local-char-socket
> +#     migration capability (default: false) (Since: 10.2)
> +#
>  # Features:
>  #
>  # @deprecated: Member @reconnect is deprecated.  Use @reconnect-ms
>  #     instead.
>  #
> +# @unstable: Members @support-local-migration and @local-incoming
> +#            are experimental
> +#
>  # Since: 1.4
>  ##
>  { 'struct': 'ChardevSocket',
> @@ -298,7 +310,9 @@
>              '*tn3270': 'bool',
>              '*websocket': 'bool',
>              '*reconnect': { 'type': 'int', 'features': [ 'deprecated' ] },
> -            '*reconnect-ms': 'int' },
> +            '*reconnect-ms': 'int',
> +            '*support-local-migration': { 'type': 'bool', 'features': [ 'unstable' ] },
> +            '*local-incoming': { 'type': 'bool', 'features': [ 'unstable' ] } },
>    'base': 'ChardevCommon' }
>  
>  ##
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 2387c21e9c..4f282d168e 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -517,6 +517,11 @@
>  #     each RAM page.  Requires a migration URI that supports seeking,
>  #     such as a file.  (since 9.0)
>  #
> +# @local-char-socket: Migrate socket chardevs open file descriptors.
> +#     Only may be used when migration channel is unix socket. Only
> +#     involves socket chardevs with "support-local-migration" option
> +#     enabled.  (since 10.2)
> +#
>  # Features:
>  #
>  # @unstable: Members @x-colo and @x-ignore-shared are experimental.
> @@ -536,7 +541,8 @@
>             { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
>             'validate-uuid', 'background-snapshot',
>             'zero-copy-send', 'postcopy-preempt', 'switchover-ack',
> -           'dirty-limit', 'mapped-ram'] }
> +           'dirty-limit', 'mapped-ram',
> +           { 'name': 'local-char-socket', 'features': [ 'unstable' ] } ] }
>  
>  ##
>  # @MigrationCapabilityStatus:

I understand why we need a knob to enable the feature.  A
MigrationCapability looks fine to me.  We could perhaps come up with a
better name, but let's leave that for later.

I'm unsure about making users mark the sockets (really: the sockets
wrapped in a character device backend) to be migrated that way.

Which sockets are users supposed to mark, and how would they know?

What happens when a user marks the QMP socket?  You called that a "bad
candidate".

Doesn't feel like good user interface design.

Could QEMU decide (in principle) which sockets are suitable for
sending down the migration channel?

If yes, could we make it do the right thing automatically?  Or at least
a check that stops the user from doing the wrong thing?

[...]