Re: [Qemu-devel] [PATCH v2 03/16] sockets: allow port to be NULL when listening on IP address

["Daniel P. Berrange" <berrange@redhat.com>]

On Wed, Oct 21, 2015 at 07:52:58PM +0200, Knut Omang wrote:
> On Mon, 2015-10-12 at 12:14 +0100, Daniel P. Berrange wrote:
> > If the port in the SocketAddress struct is NULL, it can allow
> > the kernel to automatically select a free port. This is useful
> > in particular in unit tests to avoid a race trying to find a
> > free port to run a test case on.
> 
> This patch seems to do a bit more than it claims ;-)
> 
> I just noticed that after a rebase a few minutes ago, all options I
> have that uses this type of port syntax:
> 
> -serial telnet:ip:port
> -serial tcp:ip:port
> -qmp ip:port
> 
> started to ignore the port argument and instead provide a dynamic port.
> Rebasing without this patch fixes the issue,

It is the change to qapi-schema.json that causes this. When 'port'
is marked as optional in the schema for InetSocketAddress, then
even if the 'port' field is set to a non-NULL string, the
qapi_copy_SocketAddress function  will not copy that field, so
the valid port setting gets discarded.

For reasons I don't understand, it appears that even string
fields which are marked as optional get a 'has_XXX' flag,
to distinguish betweeen a NULL string and an unset string.
I struggle to imagine why we need this. It makes sense for
integer/boolean types, but I would have thought just checking
for NULL was sufficient for string types.

Auditing the code to add in 'has_port = true' everywhere
that we set 'port = <a non-NULL string>' is a bit of work,
so simplest is probably to revert the change which marks
port as optional in qapi-schema.json by doing

diff --git a/qapi-schema.json b/qapi-schema.json
index f60be29..c613dfd 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2615,8 +2615,6 @@
 # @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
 #
 # @to: highest port to try
 #
@@ -2631,7 +2629,7 @@
 { 'struct': 'InetSocketAddress',
   'data': {
     'host': 'str',
-    '*port': 'str',
+    'port': 'str',
     '*to': 'uint16',
     '*ipv4': 'bool',
     '*ipv6': 'bool' } }


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 :|