Re: [PATCH v4 08/14] migration/multifd: Add new migration option for multifd DSA offloading.

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Apr 25, 2024 at 02:21:11AM +0000, Hao Xiang wrote:
>> Intel DSA offloading is an optional feature that turns on if
>> proper hardware and software stack is available. To turn on
>> DSA offloading in multifd live migration:
>> 
>> multifd-dsa-accel="[dsa_dev_path1] [dsa_dev_path2] ... [dsa_dev_pathX]"
>> 
>> This feature is turned off by default.
>> 
>> Signed-off-by: Hao Xiang <hao.xiang@linux.dev>
>> ---
>>  migration/migration-hmp-cmds.c |  8 ++++++++
>>  migration/options.c            | 30 ++++++++++++++++++++++++++++++
>>  migration/options.h            |  1 +
>>  qapi/migration.json            | 26 +++++++++++++++++++++++---
>>  4 files changed, 62 insertions(+), 3 deletions(-)
>
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 8c65b90328..934fa8839e 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -914,6 +914,12 @@
>>  #     See description in @ZeroPageDetection.  Default is 'multifd'.
>>  #     (since 9.0)
>>  #
>> +# @multifd-dsa-accel: If enabled, use DSA accelerator offloading for
>> +#     certain memory operations. Enable DSA accelerator offloading by
>> +#     setting this string to a list of DSA device path separated by space
>> +#     characters. Setting this string to an empty string means disabling
>> +#     DSA accelerator offloading. Defaults to an empty string. (since 9.2)
>
> Passing a list of paths as a single space separate string is a
> design anti-pattern. This needs to use a list type at the QAPI
> level.

Yup.

> Also I don't think we need add 'multifd' on the name - all
> new features are for multifd.
>
> Overall it should be called 'dsa-accel-path' I thjink

Moreover, 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.

[...]