From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57884) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpFCJ-0008CR-2V for qemu-devel@nongnu.org; Thu, 22 Oct 2015 08:42:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZpFCF-0002j6-R2 for qemu-devel@nongnu.org; Thu, 22 Oct 2015 08:42:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33062) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpFCF-0002iw-M5 for qemu-devel@nongnu.org; Thu, 22 Oct 2015 08:42:19 -0400 Date: Thu, 22 Oct 2015 13:42:15 +0100 From: "Daniel P. Berrange" Message-ID: <20151022124215.GF9079@redhat.com> References: <1445509543-30679-1-git-send-email-berrange@redhat.com> <5628D505.20203@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <5628D505.20203@redhat.com> Subject: Re: [Qemu-devel] [PATCH] qapi-schema: mark InetSocketAddress as mandatory again Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Paolo Bonzini , qemu-devel@nongnu.org, Knut Omang On Thu, Oct 22, 2015 at 06:22:29AM -0600, Eric Blake wrote: > On 10/22/2015 04:25 AM, Daniel P. Berrange wrote: > > Revert the qapi-schema.json change done in: > > > > commit 0983f5e6af76d5df8c6346cbdfff9d8305fb6da0 > > Author: Daniel P. Berrange > > Date: Tue Sep 1 14:46:50 2015 +0100 > > > > sockets: allow port to be NULL when listening on IP address > > > > Switching "port" from mandatory to optional causes the QAPI > > code generator to add a 'has_port' field to the InetSocketAddress > > struct. No code that created InetSocketAddress objects was updated > > to set 'has_port = true', which caused the non-NULL port strings > > to be silently dropped when copying InetSocketAddress objects. > > > > Reported-by: Knut Omang > > Signed-off-by: Daniel P. Berrange > > --- > > qapi-schema.json | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > Yes, I have plans for a qapi improvement that will let us mark strings > where we want the convention of memb == NULL rather than has_memb == > false (flipping the switch globally would touch too much code in one > commit, so a marker saying which structs are okay with the idea will > make it possible to convert smaller portions at a time - although we may > want to do the cleanup over the entire tree and then drop the marker). > But since I haven't posted patches on that front yet, it's missed > softfreeze and will be something for 2.6. > > So this patch is necessary for 2.5. > > > > > diff --git a/qapi-schema.json b/qapi-schema.json > > index f60be29..702b7b5 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -2614,9 +2614,7 @@ > > # > > # @host: host part of the address > > # > > -# @port: port part of the address, or lowest port if @to is present. > > -# Kernel selects a free port if omitted for listener addresses. > > -# #optional > > +# @port: port part of the address, or lowest port if @to is present > > Don't you still want to allow an empty string as the way to allow kernel > port selection? I only needed the support for empty port for the sake of the unit tests, which doesn't use QAPI parsing code. Enabling it via the command line / monitor was just a happy accident, so I figure this is fine for 2.5. We can re-revert it in 2.6 when you change the QAPI generator Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|