Re: [PATCH v2] Allow multiple parameters for schedinfo

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

 



ping, still applicable on master :)

On 03/21/2013 05:22 PM, Martin Kletzander wrote:
> virsh schedinfo was able to set only one parameter at a time (not
> counting the deprecated options), but it is useful to set more at
> once, so this patch adds the possibility to do stuff like this:
> 
> virsh schedinfo <domain> cpu_shares=0 vcpu_period=0 vcpu_quota=0 \
> emulator_period=0 emulator_quota=0
> 
> Invalid scheduler options are reported as well.  These were previously
> reported only if the command hadn't updated any values (when
> cmdSchedInfoUpdate returned 0).
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=810078
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=919372
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=919375
> 
> Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> ---
> v2:
>  - correctly report unsupported options
>  - man page updated
> 
>  tests/virsh-schedinfo |   4 +-
>  tools/virsh-domain.c  | 119 ++++++++++++++++++++++++++++----------------------
>  tools/virsh.pod       |   4 +-
>  3 files changed, 72 insertions(+), 55 deletions(-)
> 
> diff --git a/tests/virsh-schedinfo b/tests/virsh-schedinfo
> index 4f462f8..37f7bd3 100755
> --- a/tests/virsh-schedinfo
> +++ b/tests/virsh-schedinfo
> @@ -1,7 +1,7 @@
>  #!/bin/sh
>  # Ensure that virsh schedinfo --set invalid=val fails
> 
> -# Copyright (C) 2010-2011 Red Hat, Inc.
> +# Copyright (C) 2010-2011, 2013 Red Hat, Inc.
> 
>  # This program is free software: you can redistribute it and/or modify
>  # it under the terms of the GNU General Public License as published by
> @@ -37,7 +37,7 @@ fi
>  . "$srcdir/test-lib.sh"
> 
>  printf 'Scheduler      : fair\n\n' > exp-out || framework_failure
> -printf 'error: invalid scheduler option: j=k\n' > exp-err || framework_failure
> +printf 'error: invalid scheduler option: j\n' > exp-err || framework_failure
> 
>  fail=0
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 128e516..cc2eddc 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -3918,16 +3918,14 @@ static const vshCmdOptDef opts_schedinfo[] = {
>       .flags = VSH_OFLAG_REQ,
>       .help = N_("domain name, id or uuid")
>      },
> -    {.name = "set",
> -     .type = VSH_OT_STRING,
> -     .help = N_("parameter=value")
> -    },
>      {.name = "weight",
>       .type = VSH_OT_INT,
> +     .flags = VSH_OFLAG_REQ_OPT,
>       .help = N_("weight for XEN_CREDIT")
>      },
>      {.name = "cap",
>       .type = VSH_OT_INT,
> +     .flags = VSH_OFLAG_REQ_OPT,
>       .help = N_("cap for XEN_CREDIT")
>      },
>      {.name = "current",
> @@ -3942,72 +3940,100 @@ static const vshCmdOptDef opts_schedinfo[] = {
>       .type = VSH_OT_BOOL,
>       .help = N_("get/set value from running domain")
>      },
> +    {.name = "set",
> +     .type = VSH_OT_ARGV,
> +     .flags = VSH_OFLAG_NONE,
> +     .help = N_("parameter=value")
> +    },
>      {.name = NULL}
>  };
> 
>  static int
> +cmdSchedInfoUpdateOne(vshControl *ctl,
> +                      virTypedParameterPtr src_params, int nsrc_params,
> +                      virTypedParameterPtr *params,
> +                      int *nparams, int *maxparams,
> +                      const char *field, const char *value)
> +{
> +    virTypedParameterPtr param;
> +    int ret = -1;
> +    int i;
> +
> +    for (i = 0; i < nsrc_params; i++) {
> +        param = &(src_params[i]);
> +
> +        if (STRNEQ(field, param->field))
> +            continue;
> +
> +        if (virTypedParamsAddFromString(params, nparams, maxparams,
> +                                        field, param->type,
> +                                        value) < 0) {
> +            vshSaveLibvirtError();
> +            goto cleanup;
> +        }
> +        ret = 0;
> +        break;
> +    }
> +
> +    if (ret < 0)
> +        vshError(ctl, _("invalid scheduler option: %s"), field);
> +
> + cleanup:
> +    return ret;
> +}
> +
> +static int
>  cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd,
>                     virTypedParameterPtr src_params, int nsrc_params,
>                     virTypedParameterPtr *update_params)
>  {
> -    const char *set_arg;
>      char *set_field = NULL;
>      char *set_val = NULL;
> -    virTypedParameterPtr param;
> +    const char *val = NULL;
> +    const vshCmdOpt *opt = NULL;
>      virTypedParameterPtr params = NULL;
>      int nparams = 0;
>      int maxparams = 0;
>      int ret = -1;
>      int rv;
> -    int val;
> -    int i;
> 
> -    if (vshCommandOptString(cmd, "set", &set_arg) > 0) {
> -        set_field = vshStrdup(ctl, set_arg);
> +    while ((opt = vshCommandOptArgv(cmd, opt))) {
> +        set_field = vshStrdup(ctl, opt->data);
>          if (!(set_val = strchr(set_field, '='))) {
> -            vshError(ctl, "%s", _("Invalid syntax for --set, expecting name=value"));
> +            vshError(ctl, "%s", _("Invalid syntax for --set, "
> +                                  "expecting name=value"));
>              goto cleanup;
>          }
> 
>          *set_val = '\0';
>          set_val++;
> -    }
> 
> -    for (i = 0; i < nsrc_params; i++) {
> -        param = &(src_params[i]);
> -
> -        /* Legacy 'weight' and 'cap'  parameter */
> -        if (param->type == VIR_TYPED_PARAM_UINT &&
> -            (STREQ(param->field, "weight") || STREQ(param->field, "cap")) &&
> -            (rv = vshCommandOptInt(cmd, param->field, &val)) != 0) {
> -            if (rv < 0) {
> -                vshError(ctl, _("Invalid value of %s"), param->field);
> -                goto cleanup;
> -            }
> -
> -            if (virTypedParamsAddUInt(&params, &nparams, &maxparams,
> -                                      param->field, val) < 0) {
> -                vshSaveLibvirtError();
> -                goto cleanup;
> -            }
> +        if (cmdSchedInfoUpdateOne(ctl, src_params, nsrc_params,
> +                                  &params, &nparams, &maxparams,
> +                                  set_field, set_val) < 0)
> +            goto cleanup;
> 
> -            continue;
> -        }
> +        VIR_FREE(set_field);
> +    }
> 
> -        if (set_field && STREQ(set_field, param->field)) {
> -            if (virTypedParamsAddFromString(&params, &nparams, &maxparams,
> -                                            set_field, param->type,
> -                                            set_val) < 0) {
> -                vshSaveLibvirtError();
> -                goto cleanup;
> -            }
> +    rv = vshCommandOptStringReq(ctl, cmd, "cap", &val);
> +    if (rv < 0 ||
> +        (val &&
> +         cmdSchedInfoUpdateOne(ctl, src_params, nsrc_params,
> +                               &params, &nparams, &maxparams,
> +                               "cap", val) < 0))
> +        goto cleanup;
> 
> -            continue;
> -        }
> -    }
> +    rv = vshCommandOptStringReq(ctl, cmd, "weight", &val);
> +    if (rv < 0 ||
> +        (val &&
> +         cmdSchedInfoUpdateOne(ctl, src_params, nsrc_params,
> +                               &params, &nparams, &maxparams,
> +                               "weight", val) < 0))
> +        goto cleanup;
> 
> -    *update_params = params;
>      ret = nparams;
> +    *update_params = params;
>      params = NULL;
> 
>  cleanup:
> @@ -4102,15 +4128,6 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd)
>              if (ret == -1)
>                  goto cleanup;
>          } else {
> -            /* See if we've tried to --set var=val.  If so, the fact that
> -               we reach this point (with update == 0) means that "var" did
> -               not match any of the settable parameters.  Report the error.  */
> -            const char *var_value_pair = NULL;
> -            if (vshCommandOptString(cmd, "set", &var_value_pair) > 0) {
> -                vshError(ctl, _("invalid scheduler option: %s"),
> -                         var_value_pair);
> -                goto cleanup;
> -            }
>              /* When not doing --set, --live and --config do not mix.  */
>              if (live && config) {
>                  vshError(ctl, "%s",
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index b5e632e..6111e58 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -1281,8 +1281,8 @@ except that it does some error checking.
>  The editor used can be supplied by the C<$VISUAL> or C<$EDITOR> environment
>  variables, and defaults to C<vi>.
> 
> -=item B<schedinfo> [I<--set> B<parameter=value>] I<domain> [[I<--config>]
> -[I<--live>] | [I<--current>]]
> +=item B<schedinfo> I<domain> [[I<--config>] [I<--live>] | [I<--current>]]
> +[[I<--set>] B<parameter=value>]...
> 
>  =item B<schedinfo> [I<--weight> B<number>] [I<--cap> B<number>]
>  I<domain>
> --
> 1.8.1.5
> 
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]