Re: [PATCH 1/4] Add public API for parallel compression method

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 <jiangjiacheng(a)huawei.com&gt; > --- > 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.

> * > * 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