From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Extend domdirtyrate-calc virsh api with mode option, either
of these three options "page-sampling,dirty-bitmap,dirty-ring"
can be specified when calculating dirty page rate.
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
docs/manpages/virsh.rst | 7 +++++--
src/libvirt-domain.c | 12 +++++++++++-
src/qemu/qemu_driver.c | 24 +++++++++++++++++++++++-
tools/virsh-domain.c | 28 +++++++++++++++++++++++++++-
4 files changed, 66 insertions(+), 5 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index e28927e..e09703c 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -1714,13 +1714,16 @@ domdirtyrate-calc
::
domdirtyrate-calc <domain> [--seconds <sec>]
+ [{--page-sampling | --dirty-bitmap | --dirty-ring}]
Calculate an active domain's memory dirty rate which may be expected by
user in order to decide whether it's proper to be migrated out or not.
The ``seconds`` parameter can be used to calculate dirty rate in a
specific time which allows 60s at most now and would be default to 1s
-if missing. The calculated dirty rate information is available by calling
-'domstats --dirtyrate'.
+if missing. These three *--page-sampling, --dirty-bitmap, --dirty-ring*
+paremeters are mutually exclusive and used to specify calculation mode,
+*--page-sampling* is the default mode if missing. The calculated dirty
+rate information is available by calling 'domstats --dirtyrate'.
domdisplay
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 5912551..4caa740 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -13298,7 +13298,7 @@ virDomainGetMessages(virDomainPtr domain,
* virDomainStartDirtyRateCalc:
* @domain: a domain object
* @seconds: specified calculating time in seconds
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of supported virDomainDirtyRateCalcFlags
*
* Calculate the current domain's memory dirty rate in next @seconds.
* The calculated dirty rate information is available by calling
@@ -13322,6 +13322,16 @@ virDomainStartDirtyRateCalc(virDomainPtr domain,
virCheckReadOnlyGoto(conn->flags, error);
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_DIRTYRATE_MODE_PAGE_SAMPLING,
+ VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_BITMAP,
+ error);
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_DIRTYRATE_MODE_PAGE_SAMPLING,
+ VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_RING,
+ error);
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_BITMAP,
+ VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_RING,
+ error);
+
if (conn->driver->domainStartDirtyRateCalc) {
int ret;
ret = conn->driver->domainStartDirtyRateCalc(domain, seconds, flags);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cb2af61..5c2f289 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20651,7 +20651,9 @@ qemuDomainStartDirtyRateCalc(virDomainPtr dom,
VIR_DOMAIN_DIRTYRATE_CALC_MODE_PAGE_SAMPLING;
int ret = -1;
- virCheckFlags(0, -1);
+ virCheckFlags(VIR_DOMAIN_DIRTYRATE_MODE_PAGE_SAMPLING |
+ VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_BITMAP |
+ VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_RING, -1);
if (seconds < MIN_DIRTYRATE_CALC_PERIOD ||
seconds > MAX_DIRTYRATE_CALC_PERIOD) {
@@ -20687,6 +20689,26 @@ qemuDomainStartDirtyRateCalc(virDomainPtr dom,
goto endjob;
}
+ if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DIRTYRATE_MODE)) {
+ /* libvirt-domain.c already guaranteed these two flags are exclusive. */
+ if (flags & VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_BITMAP) {
+ mode = VIR_DOMAIN_DIRTYRATE_CALC_MODE_DIRTY_BITMAP;
+ } else if (flags & VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_RING) {
+ if (vm->def->features[VIR_DOMAIN_FEATURE_KVM] != VIR_TRISTATE_SWITCH_ON ||
+ vm->def->kvm_features->features[VIR_DOMAIN_KVM_DIRTY_RING] != VIR_TRISTATE_SWITCH_ON) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("dirty-ring calculation mode requires dirty-ring feature enabled."));
+ goto endjob;
+ }
+ mode = VIR_DOMAIN_DIRTYRATE_CALC_MODE_DIRTY_RING;
+ }
+ } else if ((flags & VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_BITMAP ||
+ (flags & VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_RING))) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("QEMU does not support dirty page rate calculation mode"));
+ goto endjob;
+ }
+
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorStartDirtyRateCalc(priv->mon, seconds, mode);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b56f6a9..29d8f9a 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -14465,6 +14465,20 @@ static const vshCmdOptDef opts_domdirtyrate_calc[] = {
.help = N_("calculate memory dirty rate within specified seconds, "
"the supported value range from 1 to 60, default to 1.")
},
+ {.name = "page-sampling",
+ .type = VSH_OT_BOOL,
+ .help = N_("dirty page rate is calculated by sampling memory.")
+ },
+ {.name = "dirty-bitmap",
+ .type = VSH_OT_BOOL,
+ .help = N_("dirty page rate is calculated by recording dirty bitmap "
+ "during calculation period.")
+ },
+ {.name = "dirty-ring",
+ .type = VSH_OT_BOOL,
+ .help = N_("dirty page rate is calculated by recording dirty pages "
+ "for a virtual CPU when dirty-ring feature enabled.")
+ },
{.name = NULL}
};
@@ -14473,6 +14487,7 @@ cmdDomDirtyRateCalc(vshControl *ctl, const vshCmd *cmd)
{
g_autoptr(virshDomain) dom = NULL;
int seconds = 1; /* the default value is 1 */
+ unsigned int flags = 0;
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -14480,7 +14495,18 @@ cmdDomDirtyRateCalc(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptInt(ctl, cmd, "seconds", &seconds) < 0)
return false;
- if (virDomainStartDirtyRateCalc(dom, seconds, 0) < 0)
+ VSH_EXCLUSIVE_OPTIONS("page-sampling", "dirty-bitmap");
+ VSH_EXCLUSIVE_OPTIONS("page-sampling", "dirty-ring");
+ VSH_EXCLUSIVE_OPTIONS("dirty-bitmap", "dirty-ring");
+
+ if (vshCommandOptBool(cmd, "page-sampling"))
+ flags |= VIR_DOMAIN_DIRTYRATE_MODE_PAGE_SAMPLING;
+ if (vshCommandOptBool(cmd, "dirty-bitmap"))
+ flags |= VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_BITMAP;;
+ if (vshCommandOptBool(cmd, "dirty-ring"))
+ flags |= VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_RING;;
+
+ if (virDomainStartDirtyRateCalc(dom, seconds, flags) < 0)
return false;
vshPrintExtra(ctl, _("Start to calculate domain's memory "
--
1.8.3.1
On 1/28/22 08:35, huangy81@chinatelecom.cn wrote:
> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>
> Extend domdirtyrate-calc virsh api with mode option, either
> of these three options "page-sampling,dirty-bitmap,dirty-ring"
> can be specified when calculating dirty page rate.
>
> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> ---
> docs/manpages/virsh.rst | 7 +++++--
> src/libvirt-domain.c | 12 +++++++++++-
> src/qemu/qemu_driver.c | 24 +++++++++++++++++++++++-
> tools/virsh-domain.c | 28 +++++++++++++++++++++++++++-
> 4 files changed, 66 insertions(+), 5 deletions(-)
These changes need to be broken into smaller patches. Modifying virsh is
one patch, qemu driver is the other.
>
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index e28927e..e09703c 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -1714,13 +1714,16 @@ domdirtyrate-calc
> ::
>
> domdirtyrate-calc <domain> [--seconds <sec>]
> + [{--page-sampling | --dirty-bitmap | --dirty-ring}]
I'm wondering whether we should go with domdirtyrate-calc $dom
--mode=[page-samling|dirty-bitmap|dirty-ring] instead. My reasoning is
that it's more user friendly, IMO. But I don't have strong opinion, your
version is good too.
>
> Calculate an active domain's memory dirty rate which may be expected by
> user in order to decide whether it's proper to be migrated out or not.
> The ``seconds`` parameter can be used to calculate dirty rate in a
> specific time which allows 60s at most now and would be default to 1s
> -if missing. The calculated dirty rate information is available by calling
> -'domstats --dirtyrate'.
> +if missing. These three *--page-sampling, --dirty-bitmap, --dirty-ring*
> +paremeters are mutually exclusive and used to specify calculation mode,
> +*--page-sampling* is the default mode if missing. The calculated dirty
> +rate information is available by calling 'domstats --dirtyrate'.
>
>
> domdisplay
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 5912551..4caa740 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -13298,7 +13298,7 @@ virDomainGetMessages(virDomainPtr domain,
> * virDomainStartDirtyRateCalc:
> * @domain: a domain object
> * @seconds: specified calculating time in seconds
> - * @flags: extra flags; not used yet, so callers should always pass 0
> + * @flags: bitwise-OR of supported virDomainDirtyRateCalcFlags
> *
> * Calculate the current domain's memory dirty rate in next @seconds.
> * The calculated dirty rate information is available by calling
> @@ -13322,6 +13322,16 @@ virDomainStartDirtyRateCalc(virDomainPtr domain,
>
> virCheckReadOnlyGoto(conn->flags, error);
>
> + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_DIRTYRATE_MODE_PAGE_SAMPLING,
> + VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_BITMAP,
> + error);
> + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_DIRTYRATE_MODE_PAGE_SAMPLING,
> + VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_RING,
> + error);
> + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_BITMAP,
> + VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_RING,
> + error);
> +
> if (conn->driver->domainStartDirtyRateCalc) {
> int ret;
> ret = conn->driver->domainStartDirtyRateCalc(domain, seconds, flags);
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index cb2af61..5c2f289 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -20651,7 +20651,9 @@ qemuDomainStartDirtyRateCalc(virDomainPtr dom,
> VIR_DOMAIN_DIRTYRATE_CALC_MODE_PAGE_SAMPLING;
> int ret = -1;
>
> - virCheckFlags(0, -1);
> + virCheckFlags(VIR_DOMAIN_DIRTYRATE_MODE_PAGE_SAMPLING |
> + VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_BITMAP |
> + VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_RING, -1);
>
> if (seconds < MIN_DIRTYRATE_CALC_PERIOD ||
> seconds > MAX_DIRTYRATE_CALC_PERIOD) {
> @@ -20687,6 +20689,26 @@ qemuDomainStartDirtyRateCalc(virDomainPtr dom,
> goto endjob;
> }
>
> + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DIRTYRATE_MODE)) {
> + /* libvirt-domain.c already guaranteed these two flags are exclusive. */
> + if (flags & VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_BITMAP) {
> + mode = VIR_DOMAIN_DIRTYRATE_CALC_MODE_DIRTY_BITMAP;
> + } else if (flags & VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_RING) {
> + if (vm->def->features[VIR_DOMAIN_FEATURE_KVM] != VIR_TRISTATE_SWITCH_ON ||
> + vm->def->kvm_features->features[VIR_DOMAIN_KVM_DIRTY_RING] != VIR_TRISTATE_SWITCH_ON) {
> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> + _("dirty-ring calculation mode requires dirty-ring feature enabled."));
> + goto endjob;
> + }
> + mode = VIR_DOMAIN_DIRTYRATE_CALC_MODE_DIRTY_RING;
> + }
> + } else if ((flags & VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_BITMAP ||
> + (flags & VIR_DOMAIN_DIRTYRATE_MODE_DIRTY_RING))) {
> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> + _("QEMU does not support dirty page rate calculation mode"));
> + goto endjob;
> + }
> +
Again, this is something that could be done before acquiring domain job.
And if I may suggest slightly different implementation: have the code
that sets @mode first, without checking for qemuCaps and save that bit
for the end:
if (mode != VIR_DOMAIN_DIRTYRATE_MODE_PAGE_SAMPLING &&
virQEMUCapsGet(priv->qemuCaps)) {
error
}
That allows you to drop one level of nesting, which again improves
readability.
> qemuDomainObjEnterMonitor(driver, vm);
> ret = qemuMonitorStartDirtyRateCalc(priv->mon, seconds, mode);
Michal
在 2022/2/14 18:13, Michal Prívozník 写道:
> On 1/28/22 08:35, huangy81@chinatelecom.cn wrote:
>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>
>> Extend domdirtyrate-calc virsh api with mode option, either
>> of these three options "page-sampling,dirty-bitmap,dirty-ring"
>> can be specified when calculating dirty page rate.
>>
>> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>> ---
>> docs/manpages/virsh.rst | 7 +++++--
>> src/libvirt-domain.c | 12 +++++++++++-
>> src/qemu/qemu_driver.c | 24 +++++++++++++++++++++++-
>> tools/virsh-domain.c | 28 +++++++++++++++++++++++++++-
>> 4 files changed, 66 insertions(+), 5 deletions(-)
>
> These changes need to be broken into smaller patches. Modifying virsh is
> one patch, qemu driver is the other.
>
Ok.
>>
>> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
>> index e28927e..e09703c 100644
>> --- a/docs/manpages/virsh.rst
>> +++ b/docs/manpages/virsh.rst
>> @@ -1714,13 +1714,16 @@ domdirtyrate-calc
>> ::
>>
>> domdirtyrate-calc <domain> [--seconds <sec>]
>> + [{--page-sampling | --dirty-bitmap | --dirty-ring}]
>
> I'm wondering whether we should go with domdirtyrate-calc $dom
> --mode=[page-samling|dirty-bitmap|dirty-ring] instead. My reasoning is
> that it's more user friendly, IMO. But I don't have strong opinion, your
> version is good too.
>
Indeed, these 3 modes are mutually exclusive, it's more sensible to use
the --mode=[page-samling|dirty-bitmap|dirty-ring] instead.
I'll do the modification next verison.
>>
>> Calculate an active domain's memory dirty rate which may be expected by
>> user in order to decide whether it's proper to be migrated out or not.
>> The ``seconds`` parameter can be used to calculate dirty rate in a
>> specific time which allows 60s at most now and would be default to 1s
>> -if missing. The calculated dirty rate information is available by calling
>> -'domstats --dirtyrate'.
>> +if missing. These three *--page-sampling, --dirty-bitmap, --dirty-ring*
>> +paremeters are mutually exclusive and used to specify calculation mode,
>> +*--page-sampling* is the default mode if missing. The calculated dirty
>> +rate information is available by calling 'domstats --dirtyrate'.
>>
>>
>> domdisplay
>> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
>> index 5912551..4caa740 100644
>> --- a/src/libvirt-domain.c
>> +++ b/src/libvirt-domain.c
>> @@ -13298,7 +13298,7 @@ virDomainGetMessages(virDomainPtr domain,
>> * virDomainStartDirtyRateCalc:
>> * @domain: a domain object
>> * @seconds: specified calculating time in seconds
>> - * @flags: extra flags; not used yet, so callers should always pass 0
>> + * @flags: bitwise-OR of supported virDomainDirtyRateCalcFlags
>> *
>> * Calculate the current domain's memory dirty rate in next @seconds
© 2016 - 2026 Red Hat, Inc.