Re: [PATCH v2 3/4] virsh: Add support for VIR_MIGRATE_DIRTY_LIMIT flag

On 11/30/23 01:25, Hyman Huang wrote: > Introudce option to enable dirty-limit convergence algorithim > during live migration. > > Signed-off-by: Hyman Huang <yong.huang(a)smartx.com&gt; > --- > docs/manpages/virsh.rst | 10 +++++++++- > src/libvirt-domain.c | 8 ++++++++ > tools/virsh-domain.c | 10 ++++++++++ > 3 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst > index 3e7a4c6c22..d1d20cb5ba 100644 > --- a/docs/manpages/virsh.rst > +++ b/docs/manpages/virsh.rst > @@ -3376,7 +3376,7 @@ migrate > [--compressed] [--comp-methods method-list] > [--comp-mt-level] [--comp-mt-threads] [--comp-mt-dthreads] > [--comp-xbzrle-cache] [--comp-zlib-level] [--comp-zstd-level] > - [--auto-converge] [auto-converge-initial] > + [--dirty-limit] [--auto-converge] [auto-converge-initial] > [auto-converge-increment] [--persistent-xml file] [--tls] > [--postcopy-bandwidth bandwidth] > [--parallel [--parallel-connections connections]] > @@ -3432,6 +3432,14 @@ source or destination host and the ``migrate`` command will report an error > leaving the domain active on both hosts. To recover from such situation repeat > the original ``migrate`` command with an additional *--postcopy-resume* flag. > > +*--dirty-limit* forces convergence using dirty-limit algorithms during live > +migration. For QEMU/KVM, this means migration will throttle vCPUs as needed to > +keep their dirty page rate within the migration parameter ``vcpu-dirty-limit`` > +(1 megabits/s by default). This can improve the responsiveness of large guests So how are users supposed to set this dirty limit value? I wonder whether we should make --dirty-limit accept a value and then have the QEMU driver call 'set-vcpu-dirty-limit' monitor command with desired value.

Except, the monitor command allows setting different value for each vCPU. I'm not sure our APIs are flexible enough to express that. Jirka? > +during live migration and result in more stable read performance. It requires > +dirty-ring size configuration and conflicts with the traditional > +``auto-converge`` algorithm. > + > *--auto-converge* forces convergence during live migration. The initial > guest CPU throttling rate can be set with *auto-converge-initial*. If the > initial throttling rate is not enough to ensure convergence, the rate is > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c > index 77a9682ecb..dde60ab05b 100644 > --- a/src/libvirt-domain.c > +++ b/src/libvirt-domain.c > @@ -4227,6 +4227,10 @@ virDomainMigrate3(virDomainPtr domain, > /* Now checkout the destination */ > virCheckReadOnlyGoto(dconn->flags, error); > > + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_AUTO_CONVERGE, > + VIR_MIGRATE_DIRTY_LIMIT, > + error); > + > VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK, > VIR_MIGRATE_NON_SHARED_INC, > error); > @@ -4652,6 +4656,10 @@ virDomainMigrateToURI3(virDomainPtr domain, > virCheckDomainReturn(domain, -1); > virCheckReadOnlyGoto(domain->conn->flags, error); > > + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_AUTO_CONVERGE, > + VIR_MIGRATE_DIRTY_LIMIT, > + error); > + > VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_TUNNELLED, > VIR_MIGRATE_PARALLEL, > error); > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c > index 66f933dead..3f34230e6b 100644 > --- a/tools/virsh-domain.c > +++ b/tools/virsh-domain.c > @@ -10981,6 +10981,10 @@ static const vshCmdOptDef opts_migrate[] = { > .type = VSH_OT_BOOL, > .help = N_("compress repeated pages during live migration") > }, > + {.name = "dirty-limit", > + .type = VSH_OT_BOOL, > + .help = N_("force convergence with dirty-limit algorithim during live migration") > + }, > {.name = "auto-converge", > .type = VSH_OT_BOOL, > .help = N_("force convergence during live migration") > @@ -11167,6 +11171,7 @@ doMigrate(void *opaque) > { "change-protection", VIR_MIGRATE_CHANGE_PROTECTION }, > { "unsafe", VIR_MIGRATE_UNSAFE }, > { "compressed", VIR_MIGRATE_COMPRESSED }, > + { "dirty-limit", VIR_MIGRATE_DIRTY_LIMIT }, > { "auto-converge", VIR_MIGRATE_AUTO_CONVERGE }, > { "rdma-pin-all", VIR_MIGRATE_RDMA_PIN_ALL }, > { "offline", VIR_MIGRATE_OFFLINE }, > @@ -11193,6 +11198,11 @@ doMigrate(void *opaque) > flags |= flagmap[i].migflag; > } > > + if (flags & VIR_MIGRATE_DIRTY_LIMIT && flags & VIR_MIGRATE_AUTO_CONVERGE) { > + vshError(ctl, "'--dirty-limit' conflicts with '--auto-converge'"); > + goto out; > + } > + This doesn't belong here, but into the caller: cmdMigrate(). And it can be simplified too: VSH_EXCLUSIVE_OPTIONS("dirty-limit", "auto-converge");