> However, since we already have VIR_MIGRATE_COMPRESSED flag and I can imagine various other hypervisors could support
> their own compression methods, I think using flags for selecting the compression method is wrong. So what if we keep just
> VIR_MIGRATE_COMPRESSED flag and introduce a new migration parameter to let the user select what compression method
> they want to use (XBZRLE, multithreaded compression, ...) and each of them could be further configurable with additional
> parameters. Each hypervisor would also advertise a list of supported compression methods via
> virConnectGetDomainCapabilities. QEMU would have XBZRLE method selected by default for backward compatibility (it would
> have to be advertised as the default method in virConnectGetDomainCapabilities too).
>
Hi, Jiri.
I have check the domcapabilities.
There is no any compression info from domcapabilities.
Do you means we need to add a new element of domainCapabilities as follow:
<domainCapabilities>
<migration>
< XBZRLE supported='yes'>
</ XBZRLE >
< mutil-thread supported='yes'>
<method >
<value>xz</value>
</method >
<level >
<value>8</value>
</ level >
<compress-counter>
<value>4</value>
</ compress-counter >
<decompress-counter>
<value>2</value>
</decompress-counter >
</ mutil-thread >
</ migration >
</domainCapabilities>
virsh # domcapabilities
<domainCapabilities>
<path>/home/shaohe/qemu/bin/debug/native/x86_64-softmmu/qemu-system-x86_64</path>
<domain>qemu</domain>
<machine>pc-i440fx-2.4</machine>
<arch>x86_64</arch>
<vcpu max='255'/>
<os supported='yes'>
<loader supported='yes'>
<enum name='type'>
<value>rom</value>
<value>pflash</value>
</enum>
<enum name='readonly'>
<value>yes</value>
<value>no</value>
</enum>
</loader>
</os>
<devices>
<disk supported='yes'>
<enum name='diskDevice'>
<value>disk</value>
<value>cdrom</value>
<value>floppy</value>
<value>lun</value>
</enum>
<enum name='bus'>
<value>ide</value>
<value>fdc</value>
<value>scsi</value>
<value>virtio</value>
<value>usb</value>
</enum>
</disk>
<hostdev supported='yes'>
<enum name='mode'>
<value>subsystem</value>
</enum>
<enum name='startupPolicy'>
<value>default</value>
<value>mandatory</value>
<value>requisite</value>
<value>optional</value>
</enum>
<enum name='subsysType'>
<value>usb</value>
<value>pci</value>
<value>scsi</value>
</enum>
<enum name='capsType'/>
<enum name='pciBackend'/>
</hostdev>
</devices>
</domainCapabilities>
> -----Original Message-----
> From: Jiri Denemark [mailto:jdenemar@xxxxxxxxxx]
> Sent: Thursday, October 15, 2015 5:15 PM
> To: Feng, Shaohe
> Cc: libvir-list@xxxxxxxxxx; Li, Liang Z; Qiao, Liyong
> Subject: Re: [PATCH V2 1/9] qemu_migration: Add support for mutil-thread compressed migration enable
>
> On Thu, Jul 09, 2015 at 21:01:49 +0800, ShaoHe Feng wrote:
> > We need to set the mutil-thread compress capability as true to enable it.
> >
> > Signed-off-by: Eli Qiao <liyong.qiao@xxxxxxxxx>
> > Signed-off-by: ShaoHe Feng <shaohe.feng@xxxxxxxxx>
> > ---
> > .gnulib | 2 +-
> > src/qemu/qemu_migration.c | 56
> > +++++++++++++++++++++++++++++++++++++++++++++++
> > src/qemu/qemu_migration.h | 9 +++++++-
> > src/qemu/qemu_monitor.c | 2 +-
> > src/qemu/qemu_monitor.h | 1 +
> > 5 files changed, 67 insertions(+), 3 deletions(-)
> >
> > diff --git a/.gnulib b/.gnulib
> > index f39477d..875ec93 160000
> > --- a/.gnulib
> > +++ b/.gnulib
> > @@ -1 +1 @@
> > -Subproject commit f39477dba778e99392948dd3dd19ec0d46aee932
> > +Subproject commit 875ec93e1501d2d2a8bab1b64fa66b8ceb51dc67
>
> Drop this change to .gnulib
>
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 7257182..891ddb6 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
> > @@ -2343,6 +2343,52 @@ qemuMigrationSetCompression(virQEMUDriverPtr driver,
> > return ret;
> > }
> >
> > +int
> > +qemuMigrationSetMultiThreadCompression(virQEMUDriverPtr driver,
> > + virDomainObjPtr vm,
> > + bool state,
> > + qemuDomainAsyncJob job) {
> > + qemuDomainObjPrivatePtr priv = vm->privateData;
> > + int ret = -1;
> > +
> > + if (qemuDomainObjEnterMonitorAsync(driver, vm, job) < 0)
> > + return -1;
> > +
> > + ret = qemuMonitorGetMigrationCapability(
> > + priv->mon,
> > + QEMU_MONITOR_MIGRATION_CAPS_MT_COMPRESS);
>
> Some people don't like this formatting and prefer
>
> ret = qemuMonitorGetMigrationCapability(priv->mon,
> QEMU_MONITOR_MIGRATION_CAPS_MT_COMPRESS);
>
> even though the result is longer than 80 characters. I don't mind either way so it's up to you if you want to change it or not :-)
> [1]
>
> > +
> > + if (ret < 0) {
> > + goto cleanup;
> > + } else if (ret == 0 && !state) {
> > + /* Unsupported but we want it off anyway */
> > + goto cleanup;
> > + } else if (ret == 0) {
> > + if (job == QEMU_ASYNC_JOB_MIGRATION_IN) {
> > + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> > + _("Multi-thread compressed migration is not supported by "
> > + "target QEMU binary"));
>
> Split the error message in two lines earlier so that both lines fit within 80 columns.
>
> > + } else {
> > + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> > + _("Multi-thread compressed migration is not supported by "
> > + "source QEMU binary"));
>
> And here as well.
>
> > + }
> > + ret = -1;
> > + goto cleanup;
> > + }
> > +
> > + ret = qemuMonitorSetMigrationCapability(
> > + priv->mon,
> > + QEMU_MONITOR_MIGRATION_CAPS_MT_COMPRESS,
> > + state);
>
> [1] applies here too.
>
> > +
> > + cleanup:
> > + if (qemuDomainObjExitMonitor(driver, vm) < 0)
> > + ret = -1;
> > + return ret;
> > +}
> > +
> > static int
> > qemuMigrationSetAutoConverge(virQEMUDriverPtr driver,
> > virDomainObjPtr vm, @@ -3374,6 +3420,11
> > @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
> > QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
> > goto stop;
> >
> > + if (qemuMigrationSetMultiThreadCompression(driver, vm,
> > + flags &
> > + VIR_MIGRATE_MT_COMPRESSED,
>
> This won't compile because the public flag is introduced later in patch 6/9. The introduction of this flag must either be moved
> here or to an earlier standalone patch. Remember, "make all check syntax-check" should pass after every single patch in a
> series.
>
> However, since we already have VIR_MIGRATE_COMPRESSED flag and I can imagine various other hypervisors could support
> their own compression methods, I think using flags for selecting the compression method is wrong. So what if we keep just
> VIR_MIGRATE_COMPRESSED flag and introduce a new migration parameter to let the user select what compression method
> they want to use (XBZRLE, multithreaded compression, ...) and each of them could be further configurable with additional
> parameters. Each hypervisor would also advertise a list of supported compression methods via
> virConnectGetDomainCapabilities. QEMU would have XBZRLE method selected by default for backward compatibility (it would
> have to be advertised as the default method in virConnectGetDomainCapabilities too).
>
> So what about something like
>
> /* compression method */
> #define VIR_MIGRATE_PARAM_COMPRESSION "compression"
>
> /* XBZRLE parameters */
> #define VIR_MIGRATE_PARAM_COMPRESSION_XBZRLE_CACHE "compression.xbzrle.cache"
>
> /* multithreaded compression parameters */ #define VIR_MIGRATE_PARAM_COMPRESSION_MT_LEVEL
> "compression.mt.level"
> #define VIR_MIGRATE_PARAM_COMPRESSION_MT_THREADS "compression.mt.threads"
> #define VIR_MIGRATE_PARAM_COMPRESSION_MT_DTHREADS "compression.mt.dthreads"
>
> Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list