[libvirt] [PATCH 3/4] Allow multiple parameters for schedinfo
Martin Kletzander
mkletzan at redhat.com
Thu Mar 21 08:12:33 UTC 2013
On 03/14/2013 09:53 PM, Eric Blake wrote:
> 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.
>
Thanks for that, but I've found out one more inconsistency which is
bothering me a bit (even though it was present there even before this
patch), so I'll be sending a v2 for this one. This time with the manual
fixed as well.
Martin
More information about the libvir-list
mailing list