On 19.09.25 20:13, Fabiano Rosas wrote:
Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru> writes:
On 19.09.25 18:20, Fabiano Rosas wrote:
Vladimir Sementsov-Ogievskiy <vsement...@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
