[libvirt] [PATCH 5/6] tools: fix the wrong check when use virsh setvcpus --maximum

[Pavel Hrdina]

On Tue, Mar 24, 2015 at 05:34:31PM +0100, Peter Krempa wrote:
> On Fri, Mar 20, 2015 at 15:39:03 +0100, Pavel Hrdina wrote:
> > From: Luyao Huang <lhuang at redhat.com>
> > 
> > We will ignore --maximum option when only use setvcpus with
> > this option, like this (this error is another issue):
> > 
> >  # virsh setvcpus test3 --maximum 10
> >  error: Failed to create controller cpu for group: No such file or directory
> > 
> > this is because we do not set it in flags before we check if there is
> > a flags set.
> > 
> > Refactor these code and fix the logic.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1204033
> > 
> > Signed-off-by: Luyao Huang <lhuang at redhat.com>
> > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> > ---
> >  tools/virsh-domain.c | 30 ++++++------------------------
> >  1 file changed, 6 insertions(+), 24 deletions(-)
> > 
> > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > index 1d8225c..9430ad9 100644
> > --- a/tools/virsh-domain.c
> > +++ b/tools/virsh-domain.c
> > @@ -6735,6 +6735,8 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
> >      VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
> >      VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
> >      VSH_EXCLUSIVE_OPTIONS_VAR(guest, config);
> > +    VSH_EXCLUSIVE_OPTIONS_VAR(guest, maximum);
> > +    VSH_EXCLUSIVE_OPTIONS_VAR(config, maximum);
> 
> As Yanbing pointed out, you want to make live and maximum exclusive.
> 
> Additionally this changes semantics, as currently --maximum was possible
> if and only if --config was specified, which would make it exclusive
> with --current too. This is also implied in the man page.
> 
> We have the following options:
> 
> 1) Make --maximum imply --config and document that properly
> 2) Make --maximum mutualy exclusive with --current too
> 3) Allow --maximum and --current and document that it will fail for
>    online domains
> 
> I'm fine with either of those options

There is also 4th option: specify that maximum requires config instead of make
it mutually exclusive with current.  This flag requirements are used by some of
the other libvirt APIs.  I'll create another set of macros to tell that some
flags are required and send v2.

Thanks for review.

Pavel

> 
> Peter