[libvirt] [PATCH 3/4] Allow multiple parameters for schedinfo

[Eric Blake]

On 03/14/2013 03:27 AM, 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
> 
> 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 at redhat.com>
> ---
>  tools/virsh-domain.c | 55 ++++++++++++++++++++++++++--------------------------
>  1 file changed, 27 insertions(+), 28 deletions(-)

No change to tools/virsh.pod?  I would have expected something like:

=item B<schedinfo> I<domain> [[I<--config>] [I<--live>] | [I<--current>]]
[I<--set> B<parameter=value>]...

> +++ b/tools/virsh-domain.c
> @@ -4026,36 +4026,33 @@ static const vshCmdOptDef opts_schedinfo[] = {
>       .flags = VSH_OFLAG_REQ,
>       .help = N_("domain name, id or uuid")
>      },
> -    {.name = "set",
> -     .type = VSH_OT_STRING,
> -     .flags = VSH_OFLAG_NONE,
> -     .help = N_("parameter=value")
> -    },
>      {.name = "weight",
>       .type = VSH_OT_INT,
> -     .flags = VSH_OFLAG_NONE,
> +     .flags = VSH_OFLAG_REQ_OPT,
>       .help = N_("weight for XEN_CREDIT")

Previously, 'schedinfo domain 1' was parsed as --set=1, but then errored
out because there was no '=' in the argument to set; a user doing weight
in isolation had to do an explicit --weight=1 to skip the --set field.
Now that you have re-ordered parameters, but used VSH_OFLAG_REQ_OPT on
all parameters that got moved before set, a single argument still parses
as --set, and the user still has to do an explicit --weight=1 to use the
weight option instead.  That's good - no semantic change for the
single-argument case.

For the multi-argument case: previously, 'schedinfo domain foo=bar 1'
was parsed as --set=foo=bar --weight=1, now it will parse as
--set=foo=bar --set=1 and error out.  But I don't think that anyone was
relying on mixing old and new syntax (the man page called out --weight
on a different line than --set), so I can live with that change.

Thus, even though I see a difference in parse, that difference is only
on a case that users should not have been doing, and I'm happy with your
patch.

ACK, if you touch up virsh.pod to match.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130314/f6b575df/attachment-0001.sig>