Vladimir Sementsov-Ogievskiy <[email protected]> writes:
> On 06.10.25 16:23, Markus Armbruster wrote:
>> 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]>
[...]
>>> 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.
>
> Ok. We do lack this check in checkpatch
Would be nice, yes.
>>> +#
>>> +# 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.
>>
>
> Thanks, I'll copy this phrasing to be consistent.
>
>>> +#
>>> # 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'?
>
> Opps. Interesting, how it compiles? Usually, inconsistencies between
> QAPI comments and definitions are hardly checked.
You found a gap in the checking.
[...]
