Vladimir Sementsov-Ogievskiy <[email protected]> writes:
> To migrate virtio-net TAP device backend (including open fds) locally,
> user should simply set migration parameter
>
> backend-transfer = ["virtio-net-tap"]
>
> Why not simple boolean? To simplify migration to further versions,
> when more devices will support backend-transfer migration.
>
> Alternatively, we may add per-device option to disable backend-transfer
> migration, but still:
>
> 1. It's more comfortable to set same capabilities/parameters on both
> source and target QEMU, than care about each device.
>
> 2. To not break the design, that machine-type + device options +
> migration capabilities and parameters are fully define the resulting
> migration stream. We'll break this if add in future more
> backend-transfer support in devices under same backend-transfer=true
> parameter.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
> ---
> include/qapi/util.h | 17 ++++++++++++++++
> migration/options.c | 32 ++++++++++++++++++++++++++++++
> migration/options.h | 2 ++
> qapi/migration.json | 47 ++++++++++++++++++++++++++++++++++++---------
> 4 files changed, 89 insertions(+), 9 deletions(-)
>
> diff --git a/include/qapi/util.h b/include/qapi/util.h
> index 29bc4eb865..b953402416 100644
> --- a/include/qapi/util.h
> +++ b/include/qapi/util.h
> @@ -69,4 +69,21 @@ int parse_qapi_name(const char *name, bool complete);
> _len; \
> })
>
> +/*
> + * For any GenericList @list, return true if it contains specified
> + * element.
> + */
> +#define QAPI_LIST_CONTAINS(list, el) \
> + ({ \
> + bool _found = false; \
> + typeof_strip_qual(list) _tail; \
> + for (_tail = list; _tail != NULL; _tail = _tail->next) { \
> + if (_tail->value == el) { \
> + _found = true; \
> + break; \
> + } \
> + } \
> + _found; \
> + })
> +
Not a fan of lengthy macros.
There's a single use below: migrate_virtio_net_tap(). I can't see
potential uses for such a search in existing code.
Open-code it in migrate_virtio_net_tap()?
> #endif
> diff --git a/migration/options.c b/migration/options.c
> index 4e923a2e07..137ca2147e 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -13,6 +13,7 @@
>
> #include "qemu/osdep.h"
> #include "qemu/error-report.h"
> +#include "qapi/util.h"
> #include "exec/target_page.h"
> #include "qapi/clone-visitor.h"
> #include "qapi/error.h"
> @@ -262,6 +263,14 @@ bool migrate_mapped_ram(void)
> return s->capabilities[MIGRATION_CAPABILITY_MAPPED_RAM];
> }
>
> +bool migrate_virtio_net_tap(void)
> +{
> + MigrationState *s = migrate_get_current();
> +
> + return QAPI_LIST_CONTAINS(s->parameters.backend_transfer,
> + BACKEND_TRANSFER_VIRTIO_NET_TAP);
> +}
> +
> bool migrate_ignore_shared(void)
> {
> MigrationState *s = migrate_get_current();
> @@ -960,6 +969,12 @@ MigrationParameters *qmp_query_migrate_parameters(Error
> **errp)
> params->has_direct_io = true;
> params->direct_io = s->parameters.direct_io;
>
> + if (s->parameters.backend_transfer) {
> + params->has_backend_transfer = true;
> + params->backend_transfer = QAPI_CLONE(BackendTransferList,
> +
> s->parameters.backend_transfer);
> + }
> +
> return params;
> }
>
> @@ -993,6 +1008,7 @@ void migrate_params_init(MigrationParameters *params)
> params->has_mode = true;
> params->has_zero_page_detection = true;
> params->has_direct_io = true;
> + params->has_backend_transfer = true;
> }
>
> /*
> @@ -1179,6 +1195,11 @@ bool migrate_params_check(MigrationParameters *params,
> Error **errp)
> return false;
> }
>
> + if (params->has_backend_transfer) {
> + error_setg(errp, "Not implemented");
> + return false;
> + }
> +
This goes away in the next patch. Fine, but mentioning the gap in the
commit message can save reviewer a bit of work.
> return true;
> }
>
> @@ -1297,6 +1318,10 @@ static void
> migrate_params_test_apply(MigrateSetParameters *params,
> if (params->has_direct_io) {
> dest->direct_io = params->direct_io;
> }
> +
> + if (params->has_backend_transfer) {
> + dest->backend_transfer = params->backend_transfer;
> + }
> }
>
> static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
> @@ -1429,6 +1454,13 @@ static void migrate_params_apply(MigrateSetParameters
> *params, Error **errp)
> if (params->has_direct_io) {
> s->parameters.direct_io = params->direct_io;
> }
> +
> + if (params->has_backend_transfer) {
> + qapi_free_BackendTransferList(s->parameters.backend_transfer);
> +
> + s->parameters.backend_transfer = QAPI_CLONE(BackendTransferList,
> +
> params->backend_transfer);
> + }
> }
>
> void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
> diff --git a/migration/options.h b/migration/options.h
> index 82d839709e..55c0345433 100644
> --- a/migration/options.h
> +++ b/migration/options.h
> @@ -87,6 +87,8 @@ const char *migrate_tls_hostname(void);
> uint64_t migrate_xbzrle_cache_size(void);
> ZeroPageDetection migrate_zero_page_detection(void);
>
> +bool migrate_virtio_net_tap(void);
> +
> /* parameters helpers */
>
> bool migrate_params_check(MigrationParameters *params, Error **errp);
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 2387c21e9c..e39785dc07 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -747,6 +747,18 @@
> '*transform': 'BitmapMigrationBitmapAliasTransform'
> } }
>
> +##
> +# @BackendTransfer:
> +#
> +# @virtio-net-tap: Enable backend-transfer migration for virtio-net/tap. When
> +# enabled, TAP fds and all related state is passed to target QEMU through
> +# migration channel (which should be unix socket).
Suggest "are passed to the destination in the migration channel" and
"should be a UNIX domain socket".
docs/devel/qapi-code-gen.rst:
For legibility, wrap text paragraphs so every line is at most 70
characters long.
Separate sentences with two spaces.
> +#
> +# Since: 10.2
> +##
> +{ 'enum': 'BackendTransfer',
> + 'data': [ 'virtio-net-tap' ] }
> +
> ##
> # @BitmapMigrationNodeAlias:
> #
> @@ -924,10 +936,14 @@
> # only has effect if the @mapped-ram capability is enabled.
> # (Since 9.1)
> #
> +# @backend-transfer: List of targets to enable backend-transfer
> +# migration for. This requires migration channel to be a unix
> +# socket (to pass fds through). (Since 10.2)
Elsewhere, we describe the same restriction like this:
This CPR channel must support
# file descriptor transfer with SCM_RIGHTS, i.e. it must be a
# UNIX domain socket.
> +#
> # Features:
> #
> -# @unstable: Members @x-checkpoint-delay and
> -# @x-vcpu-dirty-limit-period are experimental.
> +# @unstable: Members @x-checkpoint-delay,
> +# @x-vcpu-dirty-limit-period and @backend-transfer are experimental.
List members in alphabetical order, please.
> #
> # Since: 2.4
> ##
> @@ -950,7 +966,8 @@
> 'vcpu-dirty-limit',
> 'mode',
> 'zero-page-detection',
> - 'direct-io'] }
> + 'direct-io',
> + 'backend-transfer' ] }
Forgot feature 'unstable'?
>
> ##
> # @MigrateSetParameters:
> @@ -1105,10 +1122,14 @@
> # only has effect if the @mapped-ram capability is enabled.
> # (Since 9.1)
> #
> +# @backend-transfer: List of targets to enable backend-transfer
> +# migration for. This requires migration channel to be a unix
> +# socket (to pass fds through). (Since 10.2)
> +#
> # Features:
> #
> -# @unstable: Members @x-checkpoint-delay and
> -# @x-vcpu-dirty-limit-period are experimental.
> +# @unstable: Members @x-checkpoint-delay,
> +# @x-vcpu-dirty-limit-period and @backend-transfer are experimental.
> #
> # TODO: either fuse back into `MigrationParameters`, or make
> # `MigrationParameters` members mandatory
> @@ -1146,7 +1167,9 @@
> '*vcpu-dirty-limit': 'uint64',
> '*mode': 'MigMode',
> '*zero-page-detection': 'ZeroPageDetection',
> - '*direct-io': 'bool' } }
> + '*direct-io': 'bool',
> + '*backend-transfer': { 'type': [ 'BackendTransfer' ],
> + 'features': [ 'unstable' ] } } }
>
> ##
> # @migrate-set-parameters:
> @@ -1315,10 +1338,14 @@
> # only has effect if the @mapped-ram capability is enabled.
> # (Since 9.1)
> #
> +# @backend-transfer: List of targets to enable backend-transfer
> +# migration for. This requires migration channel to be a unix
> +# socket (to pass fds through). (Since 10.2)
> +#
> # Features:
> #
> -# @unstable: Members @x-checkpoint-delay and
> -# @x-vcpu-dirty-limit-period are experimental.
> +# @unstable: Members @x-checkpoint-delay,
> +# @x-vcpu-dirty-limit-period and @backend-transfer are experimental.
> #
> # Since: 2.4
> ##
> @@ -1353,7 +1380,9 @@
> '*vcpu-dirty-limit': 'uint64',
> '*mode': 'MigMode',
> '*zero-page-detection': 'ZeroPageDetection',
> - '*direct-io': 'bool' } }
> + '*direct-io': 'bool',
> + '*backend-transfer': { 'type': [ 'BackendTransfer' ],
> + 'features': [ 'unstable' ] } } }
>
> ##
> # @query-migrate-parameters:
