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@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