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