[PATCH V3 0/3] migration: add qemu parallel migration options

Jiang Jiacheng posted 3 patches 1 year, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20230224092712.3033504-1-jiangjiacheng@huawei.com
docs/manpages/virsh.rst          | 29 ++++++++----
include/libvirt/libvirt-domain.h | 30 ++++++++++--
src/qemu/qemu_migration.h        |  2 +
src/qemu/qemu_migration_params.c | 80 +++++++++++++++++++++++++++++++-
src/qemu/qemu_migration_params.h |  3 ++
tools/virsh-domain.c             | 26 +++++++++++
6 files changed, 156 insertions(+), 14 deletions(-)
[PATCH V3 0/3] migration: add qemu parallel migration options
Posted by Jiang Jiacheng 1 year, 2 months ago
Add compress method zlib and zstd for parallel migration and new
migration options to set qemu's parameter related with parallel
migration(multifd-compression, multifd-zlib-level and multifd-zstd-level).
These parameters has been supported by QEMU since 5.0.

v3 of:
https://listman.redhat.com/archives/libvir-list/2023-February/237604.html

diff to v2:
* merge the processing of new method into 'qemuMigrationParamsSetCompression'
* improve descriptions for the new options.


Jiang Jiacheng (3):
  Add public API for parallel compression method
  virsh: Add migrate options to set parallel compress level
  qemu: support set parallel migration compression method

 docs/manpages/virsh.rst          | 29 ++++++++----
 include/libvirt/libvirt-domain.h | 30 ++++++++++--
 src/qemu/qemu_migration.h        |  2 +
 src/qemu/qemu_migration_params.c | 80 +++++++++++++++++++++++++++++++-
 src/qemu/qemu_migration_params.h |  3 ++
 tools/virsh-domain.c             | 26 +++++++++++
 6 files changed, 156 insertions(+), 14 deletions(-)

-- 
2.33.0
Re: [PATCH V3 0/3] migration: add qemu parallel migration options
Posted by Jiri Denemark 1 year ago
On Fri, Feb 24, 2023 at 17:27:09 +0800, Jiang Jiacheng wrote:
> Add compress method zlib and zstd for parallel migration and new
> migration options to set qemu's parameter related with parallel
> migration(multifd-compression, multifd-zlib-level and multifd-zstd-level).
> These parameters has been supported by QEMU since 5.0.

Oh, I apologize for the delay reviewing this series. It got lost among
all the other emails somehow. And since this is my fault, I will reply
with suggested changes (mostly minor ones) and if you agree, I will
squash them in and push the series.

Jirka
Re: [PATCH V3 0/3] migration: add qemu parallel migration options
Posted by Jiang Jiacheng 1 year ago
Thank you for your reply and review, I'd appreciate for you to do that.

And I'd also like to confirm that we have the following usages after the
modification.
for non-parallel migration, we can use

    --compressed
    default to use XBZRLE for migration

    --compressed --comp-methods ...
    use the specificed methods for migration

    --comp-methods ...
    use the specificed methods for migration, and set the corresponding cap

and those are forbidden since the method isn't supported with
non-parallel migration

    [--compressed] --comp-methods=zlib
    [--compressed] --comp-methods=zstd

for parallel migration, we have to enable the cap using "--parallel",
and can use like:

    --parallel
    default to NONE compression method for parallel migration

    --parallel --comp-methods ...
    use the specificed methods for parallel migration

    --parallel --compressed --comp-methods ...
    use the specificed methods for parallel migration, it's same as the
above

and those are forbidden since the method isn't supported with parallel
migration

    --parallel [--compressed] --comp-methods=mt
    --parallel [--compressed] --comp-methods=xbzrle

In particular,

    --parallel --compressed

is forbiddene because it leads to a result that NONE compression method
is chosen but compressed flag is set.
And I test it on libvirt-6.2.0 with qemu-6.2.0, '--parallel
--compressed' will set cap 'xbzrle' and 'multifd' both to true, which I
think is unreasonable though the migration is succeed.

Thanks,
Jiang Jiacheng

On 2023/4/21 1:45, Jiri Denemark wrote:
> On Fri, Feb 24, 2023 at 17:27:09 +0800, Jiang Jiacheng wrote:
>> Add compress method zlib and zstd for parallel migration and new
>> migration options to set qemu's parameter related with parallel
>> migration(multifd-compression, multifd-zlib-level and multifd-zstd-level).
>> These parameters has been supported by QEMU since 5.0.
> 
> Oh, I apologize for the delay reviewing this series. It got lost among
> all the other emails somehow. And since this is my fault, I will reply
> with suggested changes (mostly minor ones) and if you agree, I will
> squash them in and push the series.
> 
> Jirka
>
Re: [PATCH V3 0/3] migration: add qemu parallel migration options
Posted by Jiri Denemark 1 year ago
On Fri, Apr 21, 2023 at 17:39:12 +0800, Jiang Jiacheng wrote:
> 
> Thank you for your reply and review, I'd appreciate for you to do that.
> 
> And I'd also like to confirm that we have the following usages after the
> modification.
> for non-parallel migration, we can use
> 
>     --compressed
>     default to use XBZRLE for migration
> 
>     --compressed --comp-methods ...
>     use the specificed methods for migration
> 
>     --comp-methods ...
>     use the specificed methods for migration, and set the corresponding cap
> 
> and those are forbidden since the method isn't supported with
> non-parallel migration
> 
>     [--compressed] --comp-methods=zlib
>     [--compressed] --comp-methods=zstd

Correct.

> for parallel migration, we have to enable the cap using "--parallel",
> and can use like:
> 
>     --parallel
>     default to NONE compression method for parallel migration
> 
>     --parallel --comp-methods ...
>     use the specificed methods for parallel migration
> 
>     --parallel --compressed --comp-methods ...
>     use the specificed methods for parallel migration, it's same as the
> above
> 
> and those are forbidden since the method isn't supported with parallel
> migration
> 
>     --parallel [--compressed] --comp-methods=mt
>     --parallel [--compressed] --comp-methods=xbzrle

Right, but see below...

> In particular,
> 
>     --parallel --compressed
> 
> is forbiddene because it leads to a result that NONE compression method
> is chosen but compressed flag is set.
> And I test it on libvirt-6.2.0 with qemu-6.2.0, '--parallel
> --compressed' will set cap 'xbzrle' and 'multifd' both to true, which I
> think is unreasonable though the migration is succeed.

Hmm, that would mean xbzrle is supported even for parallel migrations
and we should not break this functionality. I experimented a bit and
both mt and xbzrle compression methods seem to work with parallel
migration. Even query-migrate reports compression statistics that
suggest it actually works since they are similar for both normal and
--parallel migrations.

On the other hand according to Juan from QEMU none of them is compatible
with parallel migration. I will try to discuss this a bit more with him,
but so far I think we should allow both xbzrle and mt with parallel
migration to avoid breaking things people might be using.

Jirka
Re: [PATCH V3 0/3] migration: add qemu parallel migration options
Posted by Jiang Jiacheng 1 year ago
On 2023/4/24 18:57, Jiri Denemark wrote:
> On Fri, Apr 21, 2023 at 17:39:12 +0800, Jiang Jiacheng wrote:
>>
>> Thank you for your reply and review, I'd appreciate for you to do that.
>>
>> And I'd also like to confirm that we have the following usages after the
>> modification.
>> for non-parallel migration, we can use
>>
>>     --compressed
>>     default to use XBZRLE for migration
>>
>>     --compressed --comp-methods ...
>>     use the specificed methods for migration
>>
>>     --comp-methods ...
>>     use the specificed methods for migration, and set the corresponding cap
>>
>> and those are forbidden since the method isn't supported with
>> non-parallel migration
>>
>>     [--compressed] --comp-methods=zlib
>>     [--compressed] --comp-methods=zstd
> 
> Correct.
> 
>> for parallel migration, we have to enable the cap using "--parallel",
>> and can use like:
>>
>>     --parallel
>>     default to NONE compression method for parallel migration
>>
>>     --parallel --comp-methods ...
>>     use the specificed methods for parallel migration
>>
>>     --parallel --compressed --comp-methods ...
>>     use the specificed methods for parallel migration, it's same as the
>> above
>>
>> and those are forbidden since the method isn't supported with parallel
>> migration
>>
>>     --parallel [--compressed] --comp-methods=mt
>>     --parallel [--compressed] --comp-methods=xbzrle
> 
> Right, but see below...
> 
>> In particular,
>>
>>     --parallel --compressed
>>
>> is forbiddene because it leads to a result that NONE compression method
>> is chosen but compressed flag is set.
>> And I test it on libvirt-6.2.0 with qemu-6.2.0, '--parallel
>> --compressed' will set cap 'xbzrle' and 'multifd' both to true, which I
>> think is unreasonable though the migration is succeed.
> 
> Hmm, that would mean xbzrle is supported even for parallel migrations
> and we should not break this functionality. I experimented a bit and
> both mt and xbzrle compression methods seem to work with parallel
> migration. Even query-migrate reports compression statistics that
> suggest it actually works since they are similar for both normal and
> --parallel migrations.
> 
> On the other hand according to Juan from QEMU none of them is compatible
> with parallel migration. I will try to discuss this a bit more with him,
> but so far I think we should allow both xbzrle and mt with parallel
> migration to avoid breaking things people might be using.
>

I found that there is a patch about this in qemu, which disables multifd
capability explicitly when compression=on. Maybe xbzrle have the same
problem as mt?

ref:https://github.com/qemu/qemu/commit/6f39c90b86b9d3772779f873ed88aaa75a220aba

Thanks,
Jiang Jiacheng
Re: [PATCH V3 0/3] migration: add qemu parallel migration options
Posted by Jiri Denemark 11 months, 2 weeks ago
On Wed, Apr 26, 2023 at 09:56:30 +0800, Jiang Jiacheng wrote:
> On 2023/4/24 18:57, Jiri Denemark wrote:
> > On Fri, Apr 21, 2023 at 17:39:12 +0800, Jiang Jiacheng wrote:
> >>
> >> Thank you for your reply and review, I'd appreciate for you to do that.
> >>
> >> And I'd also like to confirm that we have the following usages after the
> >> modification.
> >> for non-parallel migration, we can use
> >>
> >>     --compressed
> >>     default to use XBZRLE for migration
> >>
> >>     --compressed --comp-methods ...
> >>     use the specificed methods for migration
> >>
> >>     --comp-methods ...
> >>     use the specificed methods for migration, and set the corresponding cap
> >>
> >> and those are forbidden since the method isn't supported with
> >> non-parallel migration
> >>
> >>     [--compressed] --comp-methods=zlib
> >>     [--compressed] --comp-methods=zstd
> > 
> > Correct.
> > 
> >> for parallel migration, we have to enable the cap using "--parallel",
> >> and can use like:
> >>
> >>     --parallel
> >>     default to NONE compression method for parallel migration
> >>
> >>     --parallel --comp-methods ...
> >>     use the specificed methods for parallel migration
> >>
> >>     --parallel --compressed --comp-methods ...
> >>     use the specificed methods for parallel migration, it's same as the
> >> above
> >>
> >> and those are forbidden since the method isn't supported with parallel
> >> migration
> >>
> >>     --parallel [--compressed] --comp-methods=mt
> >>     --parallel [--compressed] --comp-methods=xbzrle
> > 
> > Right, but see below...
> > 
> >> In particular,
> >>
> >>     --parallel --compressed
> >>
> >> is forbiddene because it leads to a result that NONE compression method
> >> is chosen but compressed flag is set.
> >> And I test it on libvirt-6.2.0 with qemu-6.2.0, '--parallel
> >> --compressed' will set cap 'xbzrle' and 'multifd' both to true, which I
> >> think is unreasonable though the migration is succeed.
> > 
> > Hmm, that would mean xbzrle is supported even for parallel migrations
> > and we should not break this functionality. I experimented a bit and
> > both mt and xbzrle compression methods seem to work with parallel
> > migration. Even query-migrate reports compression statistics that
> > suggest it actually works since they are similar for both normal and
> > --parallel migrations.
> > 
> > On the other hand according to Juan from QEMU none of them is compatible
> > with parallel migration. I will try to discuss this a bit more with him,
> > but so far I think we should allow both xbzrle and mt with parallel
> > migration to avoid breaking things people might be using.
> >
> 
> I found that there is a patch about this in qemu, which disables multifd
> capability explicitly when compression=on. Maybe xbzrle have the same
> problem as mt?
> 
> ref:https://github.com/qemu/qemu/commit/6f39c90b86b9d3772779f873ed88aaa75a220aba

Yeah, I discussed the situation with Juan and he said both mt and xbzrle
compressions are incompatible with multifd. QEMU would just ignore
either multifd or compression when asked for both. The mt case will
already report an error since QEMU 7.2.0 and xbzrle case may eventually
do similar thing. So apparently allowing these two compression methods
with --parallel is actually a bug and we can fix it properly in this
series.

That said, I will push this with the suggested changes squashed in.

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Re: [PATCH V3 0/3] migration: add qemu parallel migration options
Posted by Jiang Jiacheng 1 year, 1 month ago
Ping...

On 2023/2/24 17:27, Jiang Jiacheng wrote:
> Add compress method zlib and zstd for parallel migration and new
> migration options to set qemu's parameter related with parallel
> migration(multifd-compression, multifd-zlib-level and multifd-zstd-level).
> These parameters has been supported by QEMU since 5.0.
> 
> v3 of:
> https://listman.redhat.com/archives/libvir-list/2023-February/237604.html
> 
> diff to v2:
> * merge the processing of new method into 'qemuMigrationParamsSetCompression'
> * improve descriptions for the new options.
> 
> 
> Jiang Jiacheng (3):
>   Add public API for parallel compression method
>   virsh: Add migrate options to set parallel compress level
>   qemu: support set parallel migration compression method
> 
>  docs/manpages/virsh.rst          | 29 ++++++++----
>  include/libvirt/libvirt-domain.h | 30 ++++++++++--
>  src/qemu/qemu_migration.h        |  2 +
>  src/qemu/qemu_migration_params.c | 80 +++++++++++++++++++++++++++++++-
>  src/qemu/qemu_migration_params.h |  3 ++
>  tools/virsh-domain.c             | 26 +++++++++++
>  6 files changed, 156 insertions(+), 14 deletions(-)
>