On 2023/2/7 23:36, Jiri Denemark wrote:
> On Fri, Jan 20, 2023 at 16:47:40 +0800, Jiang Jiacheng wrote:
>> Add description for VIR_MIGRATE_PARAM_COMPRESSION, it will
>> be reused in choosing compression method during multifd migration.
>> Add public API VIR_MIGRATE_PARAM_COMPRESSION_ZLIB_LEVEL,
>> VIR_MIGRATE_PARAM_COMPRESSION_ZSTD_LEVEL for migration APIs
>> to support set compress level during multifd migration.
>>
>> Signed-off-by: Jiang Jiacheng <jiangjiach...@huawei.com>
>> ---
>> include/libvirt/libvirt-domain.h | 24 +++++++++++++++++++++++-
>> 1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/libvirt/libvirt-domain.h
>> b/include/libvirt/libvirt-domain.h
>> index 5152ed4551..deac3687ed 100644
>> --- a/include/libvirt/libvirt-domain.h
>> +++ b/include/libvirt/libvirt-domain.h
>> @@ -1271,7 +1271,9 @@ typedef enum {
>> * virDomainMigrate* params multiple field: name of the method used to
>> * compress migration traffic. Supported compression methods: xbzrle, mt.
>> * The parameter may be specified multiple times if more than one method
>> - * should be used.
>> + * should be used. Support compression methods for multifd migration: zlib,
>> + * zstd. When using with multifd migration, only one supported compression
>> + * method can be specified.
>
> I don't think we should separate the new methods this way. And we use
> "parallel" instead of "multifd" migration. So how about:
>
> virDomainMigrate* params multiple field: name of the method used to
> compress migration traffic. Supported compression methods: xbzrle,
> mt, zlib, zstd. The parameter may be specified multiple times if more
> than one method should be used. Not all combinations of compression
> methods and migration options may be allowed. Parallel migration of
> QEMU domains is only compatible with either zlib or zstd method.
>
Thank you for your corrections and suggestions, and I will improve these
descriptions (and those in patch 4) to make them clearer and more
consistent with other descriptions.
Thanks, Jiangjiacheng
>> *
>> * Since: 1.3.4
>> */
>> @@ -1351,6 +1353,26 @@ typedef enum {
>> */
>> # define VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS "parallel.connections"
>>
>> +/**
>> + * VIR_MIGRATE_PARAM_COMPRESSION_ZLIB_LEVEL:
>> + *
>> + * virDomainMigrate* params field: zlib compress level used during parallel
>> + * migration. As VIR_TYPED_PARAM_INT.
>
> I don't think we need to explicitly mention parallel migration here. On
> the other hand, we should mention allowed values and their semantics.
> See how compression.mt.level is described:
>
> "Accepted values are in range 0-9. 0 is no compression, 1 is maximum
> speed and 9 is maximum compression."
>
>> + *
>> + * Since: 9.1.0
>> + */
>> +# define VIR_MIGRATE_PARAM_COMPRESSION_ZLIB_LEVEL
>> "compression.zlib.level"
>> +
>> +/**
>> + * VIR_MIGRATE_PARAM_COMPRESSION_ZSTD_LEVEL:
>> + *
>> + * virDomainMigrate* params field: zstd compress level used during parallel
>> + * migration. As VIR_TYPED_PARAM_INT.
>
> The same comment as above.
>
>> + *
>> + * Since: 9.1.0
>> + */
>> +# define VIR_MIGRATE_PARAM_COMPRESSION_ZSTD_LEVEL
>> "compression.zstd.level"
>> +
>
> And I think documenting them next to the other
> VIR_MIGRATE_PARAM_COMPRESSION parameters would make more sense.
>
>> /**
>> * VIR_MIGRATE_PARAM_TLS_DESTINATION:
>> *
>
> Jirka
>
