qapi-schema: mark InetSocketAddress as mandatory again
PATCH 1/1 qapi-schema: mark InetSocketAddress as mandatory again
Revert the qapi-schema.json change done in:
commit 0983f5e6af76d5df8c6346cbdfff9d8305fb6da0
Author: Daniel P. Berrange <[email protected]>
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.
--- qapi-schema.json | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
qapi-schema.json +2 -4
@@ -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 # # @to: highest port to try #
@@ -2631,7 +2629,7 @@ { 'struct': 'InetSocketAddress', 'data': { 'host': 'str', - '*port': 'str', + 'port': 'str', '*to': 'uint16', '*ipv4': 'bool', '*ipv6': 'bool' } }
Signature · 1 line
2.4.3
Quoted 14 lines · Daniel P. Berrange
Revert the qapi-schema.json change done in:
commit
0983f5e6af76d5df8c6346cbdfff9d8305fb6da0
Author: Daniel P. Berrange <[email protected]>
Date: Tue Sep 1 14:46:50 2015 +0100sockets: 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.
Quoted 5 lines
Reported-by Knut Omang <[email protected]>Signed-off-by Daniel P. Berrange <[email protected]>--- 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.
qapi-schema.json line 2617
Quoted 13 lines
qapi-schema.json +1
-3@@ -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?
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Quoted 33 lines · Eric Blake
Quoted 14 lines · Daniel P. Berrange
Revert the qapi-schema.json change done in:
commit
0983f5e6af76d5df8c6346cbdfff9d8305fb6da0
Author: Daniel P. Berrange <[email protected]>
Date: Tue Sep 1 14:46:50 2015 +0100sockets: 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.Quoted 5 lines
Reported-by Knut Omang <[email protected]>Signed-off-by Daniel P. Berrange <[email protected]>--- 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.
Quoted 1 line
Quoted 15 lines
qapi-schema.json line 2617
Quoted 12 lines
qapi-schema.json +1
-3@@ -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 presentDon'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
Signature · 4 lines
|: 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 :|
Eric Blake
Reviewed-byQuoted 13 lines · Daniel P. Berrange
Quoted 4 lines
Quoted 4 lines
+++
b/qapi-schema.jsonqapi-schema.json +0
-0@@ -2614,9 +2614,7 @@ # # @host: host part of the addressQuoted 8 lines
qapi-schema.json line 2617
Quoted 5 lines
qapi-schema.json +1
-3# -# @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 presentDon't you still want to allow an empty string as the way to allow kernel port selection?
Quoted 4 lines
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
Fair enough; then you can add:
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Quoted 8 lines · Daniel P. Berrange
Revert the qapi-schema.json change done in:
commit
0983f5e6af76d5df8c6346cbdfff9d8305fb6da0
Author: Daniel P. Berrange <[email protected]>
Date: Tue Sep 1 14:46:50 2015 +0100sockets: allow port to be NULL when listening on IP address
Quoted 8 lines
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 <[email protected]>Signed-off-by Daniel P. Berrange <[email protected]>
I'll take this through my tree before even more people get bitten.