On Mon, Aug 08, 2022 at 23:41:27 +0800, ~hyman wrote:
> From: Hyman Huang(黄勇) <yong.huang@xxxxxxxxxx>
>
> Introduce limit-dirty-page-rate virsh api to set dirty page
> rate upper limit for virtual CPUs:
>
> The following is the usage:
> $ virsh limit-dirty-page-rate <domain> <rate> [--vcpu <number>]
>
> Set the specified index of vcpu if 'vcpu' option is specified,
> set all virtual CPUs of a VM otherwise.
>
> Signed-off-by: Hyman Huang(黄勇) <yong.huang@xxxxxxxxxx>
> ---
> tools/virsh-domain.c | 92 ++++++++++++++++++++++++++++++++++++++++++++
missing addition to docs/manpages/virsh.rst
> 1 file changed, 92 insertions(+)
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index fb54562eb6..ba2309eb3c 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -13817,6 +13817,92 @@ cmdDomDirtyRateCalc(vshControl *ctl, const vshCmd *cmd)
> return true;
> }
>
> +#define IGNORED_CPU_INDEX -1
Too generic name. Also no need for the definition most likely.
> +
> +/*
> + * "limit-dirty-page-rate" command
> + */
> +static const vshCmdInfo info_limit_dirty_page_rate[] = {
> + {.name = "help",
> + .data = N_("Set dirty page rate upper limit")
> + },
> + {.name = "desc",
> + .data = N_("Set dirty page rate upper limit, "
> + "require dirty-ring size configured")
This limitation is not documented anywhere else.
> + },
> + {.name = NULL}
> +};
> +
> +static const vshCmdOptDef opts_limit_dirty_page_rate[] = {
> + VIRSH_COMMON_OPT_DOMAIN_FULL(0),
> + {.name = "rate",
> + .type = VSH_OT_INT,
> + .flags = VSH_OFLAG_REQ,
> + .help = N_("Upper limit of dirty page rate (MB/s) for "
> + "virtual CPUs")
> + },
> + {.name = "vcpu",
> + .type = VSH_OT_INT,
> + .help = N_("Index of a virtual CPU")
> + },
> + {.name = NULL}
> +};
> +
> +static bool
> +cmdLimitDirtyPageRate(vshControl *ctl, const vshCmd *cmd)
> +{
> + g_autoptr(virshDomain) dom = NULL;
> + int vcpu_idx = -1;
> + unsigned long long rate = 0;
> + bool vcpu = vshCommandOptBool(cmd, "vcpu");
> + unsigned int flags = 0;
> +
> + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
> + return false;
> +
> + if (vcpu) {
> + if (vshCommandOptInt(ctl, cmd, "vcpu", &vcpu_idx) < 0)
> + return false;
> +
> + if (vcpu_idx < 0) {
> + vshError(ctl, "%s", _("Invalid vcpu index, using --vcpu "
> + "to specify cpu index"));
> + return false;
> + }
> + }
> +
> + if (vshCommandOptULongLong(ctl, cmd, "rate", &rate) < 0)
> + return false;
> +
> + if (!rate) {
Checks for 0 with integers that can have arbitrary numeric value should
be explicit "if (rate == 0)"...
> + vshError(ctl, "%s", _("Invalid dirty page rate limit"));
> + return false;
> + }
> +
> + if (vcpu) {
> + /* set specified vcpu dirty page rate limit of vm */
> + if (virDomainSetVcpuDirtyLimit(dom, vcpu_idx,
> + rate, flags | VIR_DOMAIN_DIRTYLIMIT_VCPU) < 0)
> + return false;
> + g_autofree char *info =
> + g_strdup_printf("Set vcpu[%d] dirty page rate upper "
> + "limit %lld(MB/s) successfully",
> + vcpu_idx, rate);
This is not translatable. Make sure to always construct the message
within vshPRintExtra's second argument which is wrapped with the gettext
macro.
> + vshPrintExtra(ctl, _("%1$s\n"), info);
> + } else {
> + /* set all vcpu dirty page rate limit of vm */
> + if (virDomainSetVcpuDirtyLimit(dom, IGNORED_CPU_INDEX,
> + rate, flags | VIR_DOMAIN_DIRTYLIMIT_ALL) < 0)
> + return false;
> + g_autofree char *info =
> + g_strdup_printf("Set dirty page rate limit %lld(MB/s) "
> + "on all virtual CPUs successfully",
> + rate);
This is not translatable.
> + vshPrintExtra(ctl, _("%1$s\n"), info);
> + }
> +
> + return true;
> +}
>
> const vshCmdDef domManagementCmds[] = {
> {.name = "attach-device",
> @@ -14481,5 +14567,11 @@ const vshCmdDef domManagementCmds[] = {
> .info = info_dom_fd_associate,
> .flags = 0
> },
> + {.name = "limit-dirty-page-rate",
> + .handler = cmdLimitDirtyPageRate,
> + .opts = opts_limit_dirty_page_rate,
> + .info = info_limit_dirty_page_rate,
> + .flags = 0
> + },
> {.name = NULL}
> };
> --
> 2.38.5
>
