On 2023/1/18 0:13, Jiri Denemark wrote: > On Tue, Jan 17, 2023 at 21:58:39 +0800, Jiang Jiacheng wrote: >> >> >> On 2023/1/17 16:44, Claudio Fontana wrote: >>> Hi, >>> >>> On 1/16/23 14:42, Jiang Jiacheng wrote: >>>> Add public API VIR_MIGRATE_PARAM_PARALLEL_COMPRESSION, >>>> VIR_MIGRATE_PARAM_PARALLEL_ZLIB_LEVEL, VIR_MIGRATE_PARAM_PARALLEL_ZSTD_LEVEL >>>> for migration APIs to support set compression method >>>> and compress level used during migration. >>>> >>>> Signed-off-by: Jiang Jiacheng <jiangjiacheng@xxxxxxxxxx> >>> >>> we don't know what drivers other than QEMU are going to implement in terms of algorithms, >>> and we also don't know what new algorithms QEMU will come up with in the future, >>> >> >> Indeed, what you're talking about is better and more scalable. These >> patches only adapt to the three parameters of the current version >> QEMU，and some commits on the QEMU side may be required to support this >> mode. >> I should fix that we don't need commits in QEMU to implement in this way. >>> so I was told when I was working on something similar (but not identical), >>> for the "multifd save restore prototype" to try to make it more general. >>> >>> First of all, maybe we don't need two different _LEVEL parameters, >>> just a VIR_MIGRATE_PARAM_PARALLEL_COMPRESSION_LEVEL, which is then mapped in the qemu driver to the specific correct QEMU parameter according to the chosen algo. >>> >> >> Since QEMU sets different parameters for different compression methods, >> I've added two corresponding parameters here. If the qemu side does not >> change, we can use the same virsh option to input the compression level >> and pass the expected QEMU parameters according to the input compression >> method on the libvirt side. >> same here. >>> Then the VIR_MIGRATE_PARAM_PARALLEL_COMPRESSION could be a freeform string, that is then interpreted by each driver, for example "zlib", or "zstd" in this case. >>> So other drivers can pass different strings, and QEMU future new compression algorithms will not need changes to libvirt at all. >> >> I think what you mean is we can input any string to >> VIR_MIGRATE_PARAM_PARALLEL_COMPRESSION and whether it is available is >> judged by QEMU or other emulators. >> It is supported with these patches, we can use any string for this >> parameter, though QEMU only support 'none', 'zlib','zstd' for it, and >> other input will return with an error. > > I think we should just be able to use the existing > VIR_MIGRATE_PARAM_COMPRESSION to specify the method. We know whether > we're asking for parallel migration from VIR_MIGRATE_PARALLEL flag. We > might need to add checks for which compression methods are only allowed > with/without VIR_MIGRATE_PARALLEL. > I see, I will try this way and add necessary check for them in next version. > On the other hand, for consistency reasons I think we should add > separate VIR_MIGRATE_PARAM_COMPRESSION_ZLIB_LEVEL and > VIR_MIGRATE_PARAM_COMPRESSION_ZSTD_LEVEL even though they are the same. > The other two compressions methods we currently support (mt and > xbzrle) have each their own set of parameters. > If so, shall we keeping using separate virsh migrate options(like --comp-zlib-level and --comp-zstd-level)? Or just use one option (--parallel-comp-level) to pass compression level, according to the compression method we have chosen? Thanks Jiang Jiacheng > Jirka >
