From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39104) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bXTBC-0000kq-Ib for qemu-devel@nongnu.org; Wed, 10 Aug 2016 09:04:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bXTB6-0000Og-H0 for qemu-devel@nongnu.org; Wed, 10 Aug 2016 09:04:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50196) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bXTB6-0000Ob-9m for qemu-devel@nongnu.org; Wed, 10 Aug 2016 09:04:12 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 74F026687E for ; Wed, 10 Aug 2016 13:04:11 +0000 (UTC) Date: Wed, 10 Aug 2016 09:04:09 -0400 From: Jeff Cody Message-ID: <20160810130409.GG5270@localhost.localdomain> References: <1470736094-19194-1-git-send-email-prasanna.kalever@redhat.com> <87shud3wab.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87shud3wab.fsf@dusky.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH v1 1/1] block/gluster: fix port type in the QAPI options list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Prasanna Kumar Kalever , qemu-devel@nongnu.org, vbellur@redhat.com, areis@redhat.com On Wed, Aug 10, 2016 at 09:42:04AM +0200, Markus Armbruster wrote: > Prasanna Kumar Kalever writes: > > > After introduction of qapi schema in gluster block driver code, the port > > type is now string as per InetSocketAddress > > > > { 'struct': 'InetSocketAddress', > > 'data': { > > 'host': 'str', > > 'port': 'str', > > '*to': 'uint16', > > '*ipv4': 'bool', > > '*ipv6': 'bool' } } > > > > but the current code still treats it as QEMU_OPT_NUMBER, hence fixing port > > to accept QEMU_OPT_STRING. > > > > Credits: Markus Armbruster > > Commonly written as > Suggested-by: Markus Armbruster > > > Signed-off-by: Prasanna Kumar Kalever > > --- > > block/gluster.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/block/gluster.c b/block/gluster.c > > index edde1ad..e6afa48 100644 > > --- a/block/gluster.c > > +++ b/block/gluster.c > > @@ -161,7 +161,7 @@ static QemuOptsList runtime_tcp_opts = { > > }, > > { > > .name = GLUSTER_OPT_PORT, > > - .type = QEMU_OPT_NUMBER, > > + .type = QEMU_OPT_STRING, > > .help = "port number on which glusterd is listening (default 24007)", > > }, > > { > > The difference between QEMU_OPT_NUMBER and QEMU_OPT_STRING: > > * The string value is stored for both. For QEMU_OPT_NUMBER, we > additionally parse the string as decimal number (this can fail, > obviously), and store the result as uint64_t. See qemu_opt_parse(). > > * qemu_opt_get() & friends return the stored string for both. > > * qemu_opt_get_number() & friends require QEMU_OPT_NUMBER and return the > stored number. > > * qemu_opts_print() prints the stored string (with comma doubled) for > QEMU_OPT_STRING, and the stored number for QEMU_OPT_NUMBER. > > Your patch works, because: > > * We get the value only with qemu_opt_get(). The only effect we get > from QEMU_OPT_NUMBER is qemu_opt_parse() failure. > > * "[PATCH v2 1/1] block/gluster: improve defense over string to int > conversion" fixes the conversion port string to port number to detect > errors. With QEMU_OPT_NUMBER, this can't actually fail, because > qemu_opt_parse() fails first. With QEMU_OPT_STRING, it can. > > The commit message should explain this. > > I'd squash the two patches together, because a decent commit message for > the squash will probably be simpler than separate ones. Are these two patches intended for 2.7?