Re: [Qemu-devel] [PATCH 07/13] Monitor: handle optional '-' arg as a bool

[Luiz Capitulino <lcapitulino@redhat.com>]

On Wed, 23 Jun 2010 18:31:53 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > On Wed, 23 Jun 2010 17:05:02 +0200
> > Markus Armbruster <armbru@redhat.com> wrote:
> >
> >> Luiz Capitulino <lcapitulino@redhat.com> writes:
> >> 
> >> > Historically, user monitor arguments beginning with '-' (eg. '-f')
> >> > were passed as integers down to handlers.
> >> >
> >> > I've maintained this behavior in the new monitor because we didn't
> >> > have a boolean type at the very beginning of QMP. Today we have it
> >> > and this behavior is causing trouble to QMP's argument checker.
> >> >
> >> > This commit fixes the problem by doing the following changes:
> >> >
> >> > 1. User Monitor
> >> >
> >> >    Before: the optional arg was represented as a QInt, we'd pass 1
> >> >            down to handlers if the user specified the argument or
> >> >            0 otherwise
> >> >
> >> >    This commit: the optional arg is represented as a QBool, we pass
> >> >                 true down to handlers if the user specified the
> >> >                 argument, otherwise _nothing_ is passed
> >> >
> >> > 2. QMP
> >> >
> >> >    Before: the client was required to pass the arg as QBool, but we'd
> >> >            convert it to QInt internally. If the argument wasn't passed,
> >> >            we'd pass 0 down
> >> >
> >> >    This commit: still require a QBool, but doesn't do any conversion and
> >> >                 doesn't pass any default value
> >> >
> >> > 3. Convert existing handlers (do_eject()/do_migrate()) to the new way
> >> >
> >> >    Before: Both handlers would expect a QInt value, either 0 or 1
> >> >
> >> >    This commit: Change the handlers to accept a QBool, they handle the
> >> >                 following cases:
> >> >
> >> >                    A) true is passed: the option is enabled
> >> >                    B) false is passed: the option is disabled
> >> >                    C) nothing is passed: option not specified, use
> >> >                                          default behavior
> >> 
> >> Because the user monitor can't pass false, the only sensible default
> >> behavior is "disabled".
> >
> > Yes, but I think we shouldn't impose it. I mean, handlers are still free
> > to choose an 'enabled' default state.
> 
> They are.  Renders the option entirely useless in the user monitor,
> though.
> 
> I don't want you to change anything, I just wanted to point out that the
> human monitor can't yet support boolean arguments defaulting to true.
> QMP can, of course.

Exactly, that was my point too.