To migrate virtio-net TAP device backend (including open fds) locally,
user should simply set migration parameter
fds = [virtio-net]
Why not simple boolean? To simplify migration to further versions,
when more devices will support fds migration.
Alternatively, we may add per-device option to disable fds-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 capabilites and parameters are fully define the resulting
migration stream. We'll break this if add in future more fds-passing
support in devices under same fds=true parameter.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
include/qapi/util.h | 17 +++++++++++++++++
migration/options.c | 30 +++++++++++++++++++++++++++++
migration/options.h | 2 ++
qapi/migration.json | 46 ++++++++++++++++++++++++++++++++++++---------
4 files changed, 86 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; \
+ })
+
#endif
diff --git a/migration/options.c b/migration/options.c
index 4e923a2e07..061a1b8eaf 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,13 @@ bool migrate_mapped_ram(void)
return s->capabilities[MIGRATION_CAPABILITY_MAPPED_RAM];
}
+bool migrate_fds_virtio_net(void)
+{
+ MigrationState *s = migrate_get_current();
+
+ return QAPI_LIST_CONTAINS(s->parameters.fds, FDS_TARGET_VIRTIO_NET);
+}
+
bool migrate_ignore_shared(void)
{
MigrationState *s = migrate_get_current();
@@ -960,6 +968,11 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
params->has_direct_io = true;
params->direct_io = s->parameters.direct_io;
+ if (s->parameters.has_fds) {
+ params->has_fds = true;
+ params->fds = QAPI_CLONE(FdsTargetList, s->parameters.fds);
+ }
+
return params;
}
@@ -1179,6 +1192,11 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
return false;
}
+ if (params->has_fds) {
+ error_setg(errp, "Not implemented");
+ return false;
+ }
+
return true;
}
@@ -1297,6 +1315,11 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
if (params->has_direct_io) {
dest->direct_io = params->direct_io;
}
+
+ if (params->has_fds) {
+ dest->has_fds = true;
+ dest->fds = params->fds;
+ }
}
static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
@@ -1429,6 +1452,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_fds) {
+ qapi_free_FdsTargetList(s->parameters.fds);
+
+ s->parameters.has_fds = true;
+ s->parameters.fds = QAPI_CLONE(FdsTargetList, params->fds);
+ }
}
void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
diff --git a/migration/options.h b/migration/options.h
index 82d839709e..a79472a235 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_fds_virtio_net(void);
+
/* parameters helpers */
bool migrate_params_check(MigrationParameters *params, Error **errp);
diff --git a/qapi/migration.json b/qapi/migration.json
index 2387c21e9c..6ef9629c6d 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -747,6 +747,19 @@
'*transform': 'BitmapMigrationBitmapAliasTransform'
} }
+##
+# @FdsTarget:
+#
+# @virtio-net: Enable live backend migration for virtio-net.
+# The only supported backend is TAP device. When enabled, TAP fds
+# and all related state is passed to target QEMU through migration
+# channel (which should be unix socket).
+#
+# Since: 10.2
+##
+{ 'enum': 'FdsTarget',
+ 'data': [ 'virtio-net' ] }
+
##
# @BitmapMigrationNodeAlias:
#
@@ -924,10 +937,14 @@
# only has effect if the @mapped-ram capability is enabled.
# (Since 9.1)
#
+# @fds: List of targets to enable live-backend 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 @fds are experimental.
#
# Since: 2.4
##
@@ -950,7 +967,8 @@
'vcpu-dirty-limit',
'mode',
'zero-page-detection',
- 'direct-io'] }
+ 'direct-io',
+ 'fds' ] }
##
# @MigrateSetParameters:
@@ -1105,10 +1123,14 @@
# only has effect if the @mapped-ram capability is enabled.
# (Since 9.1)
#
+# @fds: List of targets to enable live-backend 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 @fds are experimental.
#
# TODO: either fuse back into `MigrationParameters`, or make
# `MigrationParameters` members mandatory
@@ -1146,7 +1168,8 @@
'*vcpu-dirty-limit': 'uint64',
'*mode': 'MigMode',
'*zero-page-detection': 'ZeroPageDetection',
- '*direct-io': 'bool' } }
+ '*direct-io': 'bool',
+ '*fds': { 'type': [ 'FdsTarget' ], 'features': [ 'unstable' ] } } }
##
# @migrate-set-parameters:
@@ -1315,10 +1338,14 @@
# only has effect if the @mapped-ram capability is enabled.
# (Since 9.1)
#
+# @fds: List of targets to enable live-backend 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 @fds are experimental.
#
# Since: 2.4
##
@@ -1353,7 +1380,8 @@
'*vcpu-dirty-limit': 'uint64',
'*mode': 'MigMode',
'*zero-page-detection': 'ZeroPageDetection',
- '*direct-io': 'bool' } }
+ '*direct-io': 'bool',
+ '*fds': { 'type': [ 'FdsTarget' ], 'features': [ 'unstable' ] } } }
##
# @query-migrate-parameters:
--
2.48.1Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
Hi Vladimir, as usual with "qapi:" patches, some comments about
language:
> To migrate virtio-net TAP device backend (including open fds) locally,
> user should simply set migration parameter
>
> fds = [virtio-net]
>
> Why not simple boolean? To simplify migration to further versions,
> when more devices will support fds migration.
>
> Alternatively, we may add per-device option to disable fds-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 capabilites and parameters are fully define the resulting
> migration stream. We'll break this if add in future more fds-passing
> support in devices under same fds=true parameter.
>
These arguments look convincing to me.
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> include/qapi/util.h | 17 +++++++++++++++++
> migration/options.c | 30 +++++++++++++++++++++++++++++
> migration/options.h | 2 ++
> qapi/migration.json | 46 ++++++++++++++++++++++++++++++++++++---------
> 4 files changed, 86 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; \
> + })
> +
> #endif
> diff --git a/migration/options.c b/migration/options.c
> index 4e923a2e07..061a1b8eaf 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,13 @@ bool migrate_mapped_ram(void)
> return s->capabilities[MIGRATION_CAPABILITY_MAPPED_RAM];
> }
>
> +bool migrate_fds_virtio_net(void)
> +{
> + MigrationState *s = migrate_get_current();
> +
> + return QAPI_LIST_CONTAINS(s->parameters.fds, FDS_TARGET_VIRTIO_NET);
> +}
> +
> bool migrate_ignore_shared(void)
> {
> MigrationState *s = migrate_get_current();
> @@ -960,6 +968,11 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
> params->has_direct_io = true;
> params->direct_io = s->parameters.direct_io;
>
> + if (s->parameters.has_fds) {
> + params->has_fds = true;
> + params->fds = QAPI_CLONE(FdsTargetList, s->parameters.fds);
> + }
> +
> return params;
> }
>
> @@ -1179,6 +1192,11 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
> return false;
> }
>
> + if (params->has_fds) {
> + error_setg(errp, "Not implemented");
> + return false;
> + }
> +
> return true;
> }
>
> @@ -1297,6 +1315,11 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
> if (params->has_direct_io) {
> dest->direct_io = params->direct_io;
> }
> +
> + if (params->has_fds) {
> + dest->has_fds = true;
I think what you want is to set this in
migrate_params_init(). block_bitmap_mapping is a bit of an outlier in
that it takes an empty *input* as valid.
> + dest->fds = params->fds;
> + }
> }
>
> static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
> @@ -1429,6 +1452,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_fds) {
> + qapi_free_FdsTargetList(s->parameters.fds);
> +
> + s->parameters.has_fds = true;
> + s->parameters.fds = QAPI_CLONE(FdsTargetList, params->fds);
Same here, has_fds should always be true in s->parameters.
> + }
> }
>
> void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
> diff --git a/migration/options.h b/migration/options.h
> index 82d839709e..a79472a235 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_fds_virtio_net(void);
> +
> /* parameters helpers */
>
> bool migrate_params_check(MigrationParameters *params, Error **errp);
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 2387c21e9c..6ef9629c6d 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -747,6 +747,19 @@
> '*transform': 'BitmapMigrationBitmapAliasTransform'
> } }
>
> +##
> +# @FdsTarget:
> +#
> +# @virtio-net: Enable live backend migration for virtio-net.
So you're assuming normal migration is "non-live backend migration"
because the backend is stopped and started again on the destination. I
think it makes sense.
> +# The only supported backend is TAP device. When enabled, TAP fds
> +# and all related state is passed to target QEMU through migration
> +# channel (which should be unix socket).
> +#
> +# Since: 10.2
> +##
> +{ 'enum': 'FdsTarget',
> + 'data': [ 'virtio-net' ] }
> +
> ##
> # @BitmapMigrationNodeAlias:
> #
> @@ -924,10 +937,14 @@
> # only has effect if the @mapped-ram capability is enabled.
> # (Since 9.1)
> #
> +# @fds: List of targets to enable live-backend migration for. This
> +# requires migration channel to be a unix socket (to pass fds
> +# through). (Since 10.2)
I think I prefer live-backend as written here rather than the non
hyphenated version above. This clears up the ambiguity and can be used
in variable names, as a name for the feature and ... _thinks really
hard_ ... as the parameter name! (maybe)
On that note, fds is just too broad, I'm sure you know that, but to be
specific, we already have fd migration, multifd, fdset (as argument of
file: migration), etc. Talking just about the user interface here, of
course.
Also: "@fds: List of targets..." is super confusing.
And although "to pass fds through" is helping clarify the meaning of
@fds, it exposes implementation details on the QAPI, I don't think it's
relevant in the parameter description.
> +#
> # 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 @fds are experimental.
> #
> # Since: 2.4
> ##
> @@ -950,7 +967,8 @@
> 'vcpu-dirty-limit',
> 'mode',
> 'zero-page-detection',
> - 'direct-io'] }
> + 'direct-io',
> + 'fds' ] }
>
> ##
> # @MigrateSetParameters:
> @@ -1105,10 +1123,14 @@
> # only has effect if the @mapped-ram capability is enabled.
> # (Since 9.1)
> #
> +# @fds: List of targets to enable live-backend 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 @fds are experimental.
> #
> # TODO: either fuse back into `MigrationParameters`, or make
> # `MigrationParameters` members mandatory
> @@ -1146,7 +1168,8 @@
> '*vcpu-dirty-limit': 'uint64',
> '*mode': 'MigMode',
> '*zero-page-detection': 'ZeroPageDetection',
> - '*direct-io': 'bool' } }
> + '*direct-io': 'bool',
> + '*fds': { 'type': [ 'FdsTarget' ], 'features': [ 'unstable' ] } } }
>
> ##
> # @migrate-set-parameters:
> @@ -1315,10 +1338,14 @@
> # only has effect if the @mapped-ram capability is enabled.
> # (Since 9.1)
> #
> +# @fds: List of targets to enable live-backend 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 @fds are experimental.
> #
> # Since: 2.4
> ##
> @@ -1353,7 +1380,8 @@
> '*vcpu-dirty-limit': 'uint64',
> '*mode': 'MigMode',
> '*zero-page-detection': 'ZeroPageDetection',
> - '*direct-io': 'bool' } }
> + '*direct-io': 'bool',
> + '*fds': { 'type': [ 'FdsTarget' ], 'features': [ 'unstable' ] } } }
>
> ##
> # @query-migrate-parameters:
On 19.09.25 18:20, Fabiano Rosas wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>
> Hi Vladimir, as usual with "qapi:" patches, some comments about
> language:
>
>> To migrate virtio-net TAP device backend (including open fds) locally,
>> user should simply set migration parameter
>>
>> fds = [virtio-net]
>>
>> Why not simple boolean? To simplify migration to further versions,
>> when more devices will support fds migration.
>>
>> Alternatively, we may add per-device option to disable fds-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 capabilites and parameters are fully define the resulting
>> migration stream. We'll break this if add in future more fds-passing
>> support in devices under same fds=true parameter.
>>
>
> These arguments look convincing to me.
>
[..]
>> return true;
>> }
>>
>> @@ -1297,6 +1315,11 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
>> if (params->has_direct_io) {
>> dest->direct_io = params->direct_io;
>> }
>> +
>> + if (params->has_fds) {
>> + dest->has_fds = true;
>
> I think what you want is to set this in
> migrate_params_init(). block_bitmap_mapping is a bit of an outlier in
> that it takes an empty *input* as valid.
Hmm I made it behave like block_bitmap_mapping because it also a list.
As I understand, for list we have to treat empty list not like absent parameter: we should have a way
to clear previously set list by subsequent migrate-set-parameters call.
>
>> + dest->fds = params->fds;
>> + }
>> }
>>
>> static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
>> @@ -1429,6 +1452,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_fds) {
>> + qapi_free_FdsTargetList(s->parameters.fds);
>> +
>> + s->parameters.has_fds = true;
>> + s->parameters.fds = QAPI_CLONE(FdsTargetList, params->fds);
>
> Same here, has_fds should always be true in s->parameters.
>
>> + }
>> }
>>
>> void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
>> diff --git a/migration/options.h b/migration/options.h
>> index 82d839709e..a79472a235 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_fds_virtio_net(void);
>> +
>> /* parameters helpers */
>>
>> bool migrate_params_check(MigrationParameters *params, Error **errp);
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 2387c21e9c..6ef9629c6d 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -747,6 +747,19 @@
>> '*transform': 'BitmapMigrationBitmapAliasTransform'
>> } }
>>
>> +##
>> +# @FdsTarget:
>> +#
>> +# @virtio-net: Enable live backend migration for virtio-net.
>
> So you're assuming normal migration is "non-live backend migration"
> because the backend is stopped and started again on the destination. I
> think it makes sense.
>
>> +# The only supported backend is TAP device. When enabled, TAP fds
>> +# and all related state is passed to target QEMU through migration
>> +# channel (which should be unix socket).
>> +#
>> +# Since: 10.2
>> +##
>> +{ 'enum': 'FdsTarget',
>> + 'data': [ 'virtio-net' ] }
>> +
>> ##
>> # @BitmapMigrationNodeAlias:
>> #
>> @@ -924,10 +937,14 @@
>> # only has effect if the @mapped-ram capability is enabled.
>> # (Since 9.1)
>> #
>> +# @fds: List of targets to enable live-backend migration for. This
>> +# requires migration channel to be a unix socket (to pass fds
>> +# through). (Since 10.2)
>
> I think I prefer live-backend as written here rather than the non
> hyphenated version above. This clears up the ambiguity and can be used
> in variable names, as a name for the feature and ... _thinks really
> hard_ ... as the parameter name! (maybe)
>
> On that note, fds is just too broad, I'm sure you know that, but to be
> specific, we already have fd migration, multifd, fdset (as argument of
> file: migration), etc. Talking just about the user interface here, of
> course.
>
> Also: "@fds: List of targets..." is super confusing.
>
> And although "to pass fds through" is helping clarify the meaning of
> @fds, it exposes implementation details on the QAPI, I don't think it's
> relevant in the parameter description.
Agree. I see all this mess, each time I come with some new name:
live-tap, live-backend, fds-passing, fds migration, local tap migration, fds...
Finally, only one should be chosen for all names and in documentation.
With your comments, "live-backend" really looks the best one.
Still, we can't say live-backends: [virtio-net], as virtio-net is not
a backed.
Maybe
live-backends: [virtio-net-tap]
to show that it's about virtio-net+TAP pair.
>
>> +#
>> # Features:
>> #
>> -# @unstable: Members @x-checkpoint-delay and
>> -# @x-vcpu-dirty-limit-period are experimental.
Thanks!
--
Best regards,
Vladimir
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> On 19.09.25 18:20, Fabiano Rosas wrote:
>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>>
>> Hi Vladimir, as usual with "qapi:" patches, some comments about
>> language:
>>
>>> To migrate virtio-net TAP device backend (including open fds) locally,
>>> user should simply set migration parameter
>>>
>>> fds = [virtio-net]
>>>
>>> Why not simple boolean? To simplify migration to further versions,
>>> when more devices will support fds migration.
>>>
>>> Alternatively, we may add per-device option to disable fds-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 capabilites and parameters are fully define the resulting
>>> migration stream. We'll break this if add in future more fds-passing
>>> support in devices under same fds=true parameter.
>>>
>>
>> These arguments look convincing to me.
>>
>
> [..]
>
>>> return true;
>>> }
>>>
>>> @@ -1297,6 +1315,11 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
>>> if (params->has_direct_io) {
>>> dest->direct_io = params->direct_io;
>>> }
>>> +
>>> + if (params->has_fds) {
>>> + dest->has_fds = true;
>>
>> I think what you want is to set this in
>> migrate_params_init(). block_bitmap_mapping is a bit of an outlier in
>> that it takes an empty *input* as valid.
>
> Hmm I made it behave like block_bitmap_mapping because it also a list.
>
> As I understand, for list we have to treat empty list not like absent parameter: we should have a way
> to clear previously set list by subsequent migrate-set-parameters call.
>
Sorry, I explained myself poorly. Yes, the empty list is valid and it
clears a previously set value. But that's just:
migrate_params_init(MigrationParameters *params):
...
params->has_fds = true;
migrate_params_test_apply(MigrateSetParameters *params,
MigrationParameters *dest):
...
/* means user provided it */
if (params->has_fds) {
dest->fds = params->fds;
}
migrate_params_check(MigrationParameters *params):
...
if (params->has_fds) {
/* empty list ok */
}
migrate_params_apply(MigrateSetParameters *params):
...
if (params->has_fds) {
qapi_free_...(s->parameters.fds);
/* clones the full list or the empty list, no difference */
s->parameters.fds = QAPI_CLONE(..., params->fds);
}
The block_bitmap_mapping does the has_ part a bit differently because it
wants to carry that flag over to the rest of the code. See 3cba22c9ad
("migration: Fix block_bitmap_mapping migration"). In that case, even if
the list is empty, it needs to know when it was made empty on purpose to
carry on with the removal of the mappings. In your case, it doesn't
matter, empty is empty, whether forcefully, or because it was never
set. Right?
(this migrate_params code is tricky, FYI I'm reworking that in:
https://lore.kernel.org/r/20250603013810.4772-1-farosas@suse.de)
>>
>>> + dest->fds = params->fds;
>>> + }
>>> }
>>>
>>> static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
>>> @@ -1429,6 +1452,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_fds) {
>>> + qapi_free_FdsTargetList(s->parameters.fds);
>>> +
>>> + s->parameters.has_fds = true;
>>> + s->parameters.fds = QAPI_CLONE(FdsTargetList, params->fds);
>>
>> Same here, has_fds should always be true in s->parameters.
>>
>>> + }
>>> }
>>>
>>> void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
>>> diff --git a/migration/options.h b/migration/options.h
>>> index 82d839709e..a79472a235 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_fds_virtio_net(void);
>>> +
>>> /* parameters helpers */
>>>
>>> bool migrate_params_check(MigrationParameters *params, Error **errp);
>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>> index 2387c21e9c..6ef9629c6d 100644
>>> --- a/qapi/migration.json
>>> +++ b/qapi/migration.json
>>> @@ -747,6 +747,19 @@
>>> '*transform': 'BitmapMigrationBitmapAliasTransform'
>>> } }
>>>
>>> +##
>>> +# @FdsTarget:
>>> +#
>>> +# @virtio-net: Enable live backend migration for virtio-net.
>>
>> So you're assuming normal migration is "non-live backend migration"
>> because the backend is stopped and started again on the destination. I
>> think it makes sense.
>>
>>> +# The only supported backend is TAP device. When enabled, TAP fds
>>> +# and all related state is passed to target QEMU through migration
>>> +# channel (which should be unix socket).
>>> +#
>>> +# Since: 10.2
>>> +##
>>> +{ 'enum': 'FdsTarget',
>>> + 'data': [ 'virtio-net' ] }
>>> +
>>> ##
>>> # @BitmapMigrationNodeAlias:
>>> #
>>> @@ -924,10 +937,14 @@
>>> # only has effect if the @mapped-ram capability is enabled.
>>> # (Since 9.1)
>>> #
>>> +# @fds: List of targets to enable live-backend migration for. This
>>> +# requires migration channel to be a unix socket (to pass fds
>>> +# through). (Since 10.2)
>>
>> I think I prefer live-backend as written here rather than the non
>> hyphenated version above. This clears up the ambiguity and can be used
>> in variable names, as a name for the feature and ... _thinks really
>> hard_ ... as the parameter name! (maybe)
>>
>> On that note, fds is just too broad, I'm sure you know that, but to be
>> specific, we already have fd migration, multifd, fdset (as argument of
>> file: migration), etc. Talking just about the user interface here, of
>> course.
>>
>> Also: "@fds: List of targets..." is super confusing.
>>
>> And although "to pass fds through" is helping clarify the meaning of
>> @fds, it exposes implementation details on the QAPI, I don't think it's
>> relevant in the parameter description.
>
> Agree. I see all this mess, each time I come with some new name:
> live-tap, live-backend, fds-passing, fds migration, local tap migration, fds...
> Finally, only one should be chosen for all names and in documentation.
>
Yeah, not to mention the similarities with CPR. I thought of borrowing
the "transfer" term to underline that's a common concept between the two
approaches, but I'm not sure it's worth the trouble.
[live-]backend-transfer
> With your comments, "live-backend" really looks the best one.
>
> Still, we can't say live-backends: [virtio-net], as virtio-net is not
> a backed.
>
> Maybe
>
> live-backends: [virtio-net-tap]
>
> to show that it's about virtio-net+TAP pair.
>
Works for me.
>>
>>> +#
>>> # Features:
>>> #
>>> -# @unstable: Members @x-checkpoint-delay and
>>> -# @x-vcpu-dirty-limit-period are experimental.
>
>
> Thanks!
On 19.09.25 20:13, Fabiano Rosas wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>
>> On 19.09.25 18:20, Fabiano Rosas wrote:
>>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>>>
>>> Hi Vladimir, as usual with "qapi:" patches, some comments about
>>> language:
>>>
>>>> To migrate virtio-net TAP device backend (including open fds) locally,
>>>> user should simply set migration parameter
>>>>
>>>> fds = [virtio-net]
>>>>
>>>> Why not simple boolean? To simplify migration to further versions,
>>>> when more devices will support fds migration.
>>>>
>>>> Alternatively, we may add per-device option to disable fds-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 capabilites and parameters are fully define the resulting
>>>> migration stream. We'll break this if add in future more fds-passing
>>>> support in devices under same fds=true parameter.
>>>>
>>>
>>> These arguments look convincing to me.
>>>
>>
>> [..]
>>
>>>> return true;
>>>> }
>>>>
>>>> @@ -1297,6 +1315,11 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
>>>> if (params->has_direct_io) {
>>>> dest->direct_io = params->direct_io;
>>>> }
>>>> +
>>>> + if (params->has_fds) {
>>>> + dest->has_fds = true;
>>>
>>> I think what you want is to set this in
>>> migrate_params_init(). block_bitmap_mapping is a bit of an outlier in
>>> that it takes an empty *input* as valid.
>>
>> Hmm I made it behave like block_bitmap_mapping because it also a list.
>>
>> As I understand, for list we have to treat empty list not like absent parameter: we should have a way
>> to clear previously set list by subsequent migrate-set-parameters call.
>>
>
> Sorry, I explained myself poorly. Yes, the empty list is valid and it
> clears a previously set value. But that's just:
>
> migrate_params_init(MigrationParameters *params):
> ...
> params->has_fds = true;
>
> migrate_params_test_apply(MigrateSetParameters *params,
> MigrationParameters *dest):
> ...
> /* means user provided it */
> if (params->has_fds) {
> dest->fds = params->fds;
> }
>
> migrate_params_check(MigrationParameters *params):
> ...
> if (params->has_fds) {
> /* empty list ok */
> }
>
> migrate_params_apply(MigrateSetParameters *params):
> ...
> if (params->has_fds) {
> qapi_free_...(s->parameters.fds);
> /* clones the full list or the empty list, no difference */
> s->parameters.fds = QAPI_CLONE(..., params->fds);
> }
>
> The block_bitmap_mapping does the has_ part a bit differently because it
> wants to carry that flag over to the rest of the code. See 3cba22c9ad
> ("migration: Fix block_bitmap_mapping migration"). In that case, even if
> the list is empty, it needs to know when it was made empty on purpose to
> carry on with the removal of the mappings. In your case, it doesn't
> matter, empty is empty, whether forcefully, or because it was never
> set. Right?
Oh, yes, understand now, thanks.
--
Best regards,
Vladimir
© 2016 - 2025 Red Hat, Inc.