From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=56045 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1ORSb7-0007AG-En
	for qemu-devel@nongnu.org; Wed, 23 Jun 2010 12:14:46 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69)
	(envelope-from <lcapitulino@redhat.com>) id 1ORSb6-0005oM-C9
	for qemu-devel@nongnu.org; Wed, 23 Jun 2010 12:14:45 -0400
Received: from mx1.redhat.com ([209.132.183.28]:29289)
	by eggs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <lcapitulino@redhat.com>) id 1ORSb6-0005oE-4t
	for qemu-devel@nongnu.org; Wed, 23 Jun 2010 12:14:44 -0400
Received: from int-mx04.intmail.prod.int.phx2.redhat.com
	(int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.17])
	by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o5NGEgRO025463
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK)
	for <qemu-devel@nongnu.org>; Wed, 23 Jun 2010 12:14:43 -0400
Date: Wed, 23 Jun 2010 13:14:31 -0300
From: Luiz Capitulino <lcapitulino@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 07/13] Monitor: handle optional '-' arg as
	a bool
Message-ID: <20100623131431.69ab6ed2@redhat.com>
In-Reply-To: <m34ogtzt5t.fsf@blackfin.pond.sub.org>
References: <1277228451-7741-1-git-send-email-lcapitulino@redhat.com>
	<1277228451-7741-8-git-send-email-lcapitulino@redhat.com>
	<m34ogtzt5t.fsf@blackfin.pond.sub.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org

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.