[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 09/10] vcpubandwidth: Implement virsh support



On Thu, 30 Jun 2011 11:19:14 +0800
Wen Congyang <wency cn fujitsu com> wrote:

> Introduce new command vcpu-bandwidth to change and query bandwidth for each vcpu.
> 
> Usage:
> 1. query bandwidth for all vcpus:
>    # virsh vcpu-bandwidth <domain>
> 
> 2. query bandwidth for a vcpu:
>    # virsh vcpu-bandwidth <domain> <vcpuid>
> 
> 3. change bandwidth for a vcpu:
>    # virsh vcpu-bandwidth <domain> <vcpuid> <period> <quota>
>    You can omit period or quota.
> 
> The option --live, --config, and --current is the same as the other commands.


I try the following:

# virsh domstate VM
shut off

# virsh vcpu-bandwidth VM --config
vcpu: period quota
----------------------------------
   0: 100000 50000
   1: 100000 50000
   2: 100000 50000
   3: 100000 50000

# virsh start VM
Domain VM started

# virsh vcpu-bandwidth VM --live  
vcpu: period quota
----------------------------------
   0: 100000    -1
   1: 100000    -1
   2: 100000    -1
   3: 100000    -1
   

This behavior doesn't make sense to me.
Is this what you intend?
Or, do I make a mistake?


> 
> ---
>  tools/virsh.c   |  142 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  tools/virsh.pod |   16 ++++++
>  2 files changed, 157 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index d15d206..b5a49dd 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -3049,7 +3049,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
>              flags |= VIR_DOMAIN_AFFECT_LIVE;
>          /* neither option is specified */
>          if (!live && !config)
> -            flags = -1;
> +            flags = VIR_DOMAIN_AFFECT_CURRENT;
>      }
>  
>      if (!vshConnectionUsability(ctl, ctl->conn))
> @@ -3239,6 +3239,144 @@ parse_error:
>  }
>  
>  /*
> + * "vcpu-bandwidth" command
> + */
> +static const vshCmdInfo info_vcpu_bandwidth[] = {
> +    {"help", N_("control or query domain vcpu bandwidth")},
> +    {"desc", N_("Change or query the bandwidth of virtual CPUs in the guest domain.")},
> +    {NULL, NULL}
> +};
> +
> +static const vshCmdOptDef opts_vcpu_bandwidth[] = {
> +    {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
> +    {"vcpu", VSH_OT_INT, 0, N_("vcpu number")},
> +    {"period", VSH_OT_INT, VSH_OFLAG_EMPTY_OK, N_("bandwidth period in usecs")},
> +    {"quota", VSH_OT_INT, VSH_OFLAG_EMPTY_OK,
> +     N_("cpu bandwidth (in usecs) that this vcpu will be allowed to consume over period")},
> +    {"config", VSH_OT_BOOL, 0, N_("affect next boot")},
> +    {"live", VSH_OT_BOOL, 0, N_("affect running domain")},
> +    {"current", VSH_OT_BOOL, 0, N_("affect current domain")},
> +    {NULL, 0, 0, NULL}
> +};
> +
> +static bool
> +cmdVcpuBandwidth(vshControl *ctl, const vshCmd *cmd)
> +{
> +    virDomainPtr dom;
> +    int vcpu = -1;
> +    int max_vcpus, nvcpus = 0;
> +    unsigned long long period = 0;
> +    long long quota = 0;
> +    int period_found, quota_found;
> +    virDomainVcpuBWDefPtr vcpubw_list = NULL;
> +    virDomainVcpuBWDefPtr vcpubw = NULL;
> +    bool ret = false;
> +    bool config = vshCommandOptBool(cmd, "config");
> +    bool live = vshCommandOptBool(cmd, "live");
> +    bool current = vshCommandOptBool(cmd, "current");
> +    bool query = false; /* Query mode if no period and no quota */
> +    int flags = 0;
> +    int rc;
> +    int i;
> +
> +    if (current) {
> +        if (live || config) {
> +            vshError(ctl, "%s", _("--current must be specified exclusively"));
> +            return false;
> +        }
> +        flags = VIR_DOMAIN_AFFECT_CURRENT;
> +    } else {
> +        if (config)
> +            flags |= VIR_DOMAIN_AFFECT_CONFIG;
> +        if (live)
> +            flags |= VIR_DOMAIN_AFFECT_LIVE;
> +        /* neither option is specified */
> +        if (!live && !config)
> +            flags = VIR_DOMAIN_AFFECT_CURRENT;
> +    }
> +
> +    if (!vshConnectionUsability(ctl, ctl->conn))
> +        return false;
> +
> +    if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
> +        return false;
> +
> +    if ((period_found = vshCommandOptULongLong(cmd, "period", &period)) < 0) {
> +        vshError(ctl, "%s", _("Invalid value of period"));
> +        goto cleanup;
> +    }
> +
> +    if ((quota_found = vshCommandOptLongLong(cmd, "quota", &quota)) < 0) {
> +        vshError(ctl, "%s", _("Invalid value of quota"));
> +        goto cleanup;
> +    }
> +
> +    query = quota_found == 0 && period_found == 0;
> +
> +    /* In query mode, "vcpu" is optional */
> +    if (vshCommandOptInt(cmd, "vcpu", &vcpu) < !query) {
> +        vshError(ctl, "%s", _("Invalid or missing VCPU number."));
> +        goto cleanup;
> +    }
> +
> +    max_vcpus = virDomainGetVcpusFlags(dom, flags | VIR_DOMAIN_VCPU_MAXIMUM);
> +    if (max_vcpus < 0)
> +        goto cleanup;

   Is the following case OK?  VCPU#4-7 are not present.

     <vcpu current='4'>8</vcpu>

> +
> +    /* In query mode, we need to know current setting. In change mode, if user
> +     * does not specify period or quota, we need to know its origin setting.
> +     */
> +    if (quota_found == 0 || period_found == 0) {
> +        nvcpus = max_vcpus;
> +        vcpubw_list = vshMalloc(ctl, nvcpus * sizeof(*vcpubw_list));
> +        rc = virDomainGetVcpuBW(dom, vcpubw_list, &nvcpus, flags);
> +        if (rc < 0)
> +            goto cleanup;
> +    }
> +
> +    /* Query mode: show CPU bandwidth information then exit.*/
> +    if (query) {
> +        vshPrint(ctl, "%s %s %s\n", _("vcpu:"), _("period"), _("quota"));
> +        vshPrint(ctl, "----------------------------------\n");
> +        for (i = 0; i < nvcpus; i++) {
> +            if (vcpu != -1 && i != vcpu)
> +                continue;
> +            vshPrint(ctl, "%4d: ", i);
> +            vshPrint(ctl, "%6llu ", vcpubw_list[i].period);
> +            vshPrint(ctl, "%5lld\n", vcpubw_list[i].quota);
> +        }
> +        goto cleanup;
> +    }
> +
> +    if (vcpu >= max_vcpus) {
> +        vshError(ctl, _("Virtual CPU %d doesn't exist."), vcpu);
> +        goto cleanup;
> +    }

  This check should be done before querying?


> +
> +    vcpubw = vshMalloc(ctl, sizeof(*vcpubw));
> +    vcpubw[0].vcpuid = vcpu;
> +    if (period_found)
> +        vcpubw[0].period = period;
> +    else
> +        vcpubw[0].period = vcpubw_list[vcpu].period;
> +    if (quota_found)
> +        vcpubw[0].quota = quota;
> +    else
> +        vcpubw[0].quota = vcpubw_list[vcpu].quota;
> +    rc = virDomainSetVcpuBW(dom, vcpubw, 1, flags);
> +    if (rc < 0)
> +        goto cleanup;
> +
> +    ret = true;
> +
> +cleanup:
> +    VIR_FREE(vcpubw_list);
> +    VIR_FREE(vcpubw);
> +    virDomainFree(dom);
> +    return ret;
> +}
> +
> +/*
>   * "setvcpus" command
>   */
>  static const vshCmdInfo info_setvcpus[] = {
> @@ -11675,6 +11813,8 @@ static const vshCmdDef domManagementCmds[] = {
>      {"vcpucount", cmdVcpucount, opts_vcpucount, info_vcpucount, 0},
>      {"vcpuinfo", cmdVcpuinfo, opts_vcpuinfo, info_vcpuinfo, 0},
>      {"vcpupin", cmdVcpuPin, opts_vcpupin, info_vcpupin, 0},
> +    {"vcpu-bandwidth", cmdVcpuBandwidth, opts_vcpu_bandwidth,
> +     info_vcpu_bandwidth, 0},
>      {"version", cmdVersion, opts_version, info_version, 0},
>      {"vncdisplay", cmdVNCDisplay, opts_vncdisplay, info_vncdisplay, 0},
>      {NULL, NULL, NULL, NULL, 0}
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 736b919..686d8ba 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -849,6 +849,22 @@ If no flag is specified, behavior is different depending on hypervisor.
>  B<Note>: The expression is sequentially evaluated, so "0-15,^8" is
>  identical to "9-14,0-7,15" but not identical to "^8,0-15".
>  
> +=item B<vcpu-bandwidth> I<domain-id> optional I<vcpu> I<period> I<quota>
> +I<--live> I<--config> I<--current>
> +
> +Change or query the bandwidth of domain vcpus. To change a single I<vcpu>,
> +specify I<period> or I<quota>; otherwise, you can query one I<vcpu> or omit
> +I<vcpu> to list all at once.
> +
> +I<period> is bandwidth period in usecs, and I<quota> is cpu bandwidth (in usecs)
> +that this vcpu will be allowed to consume over period.
> +If I<--live> is specified, affect a running guest.
> +If I<--config> is specified, affect the next boot of a persistent guest.
> +If I<--current> is specified, affect the current guest state.
> +Both I<--live> and I<--config> flags may be given if I<period> or I<quota> is
> +present, but I<--current> is exclusive.
> +If no flag is specified, it is identical to I<--current>.
> +
>  =item B<vncdisplay> I<domain-id>
>  
>  Output the IP address and port number for the VNC display. If the information
> -- 
> 1.7.1
> 
> --
> libvir-list mailing list
> libvir-list redhat com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 


-- 
Taku Izumi <izumi taku jp fujitsu com>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]