[Qemu-devel] [PATCH] qemu-sockets: fix conversion of ipv4/ipv6 JSON to QemuOpts

[Qemu-devel] [PATCH] qemu-sockets: fix conversion of ipv4/ipv6 JSON to QemuOpts

[Paolo Bonzini]

The QemuOpts-based code treats "option not set" and "option set
to false" the same way for the ipv4 and ipv6 options, because it
is meant to handle only the ",ipv4" and ",ipv6" substrings in
hand-crafted option parsers.

When converting InetSocketAddress to QemuOpts, however, it is
necessary to handle all three cases (not set, set to true, set
to false).  Currently we are not handling all cases correctly.
The rules are:

* if none or both options are absent, leave things as is

* if the single present option is Y, the other should be N.
This can be implemented by leaving things as is, or by setting
the other option to N as done in this patch.

* if the single present option is N, the other should be Y.
This is handled by the "else if" branch of this patch.

This ensures that the ipv4 option has an effect on Windows,
where creating the socket with PF_UNSPEC makes an ipv6
socket.  With this patch, ",ipv4" will result in a PF_INET
socket instead.

Reported-by: Sair, Umair <Umair_Sair@mentor.com>
Tested-by: Sair, Umair <Umair_Sair@mentor.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 util/qemu-sockets.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 2add83a..0a041a9 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -586,12 +586,15 @@ fail:
 
 static void inet_addr_to_opts(QemuOpts *opts, const InetSocketAddress *addr)
 {
-    bool ipv4 = addr->ipv4 || !addr->has_ipv4;
-    bool ipv6 = addr->ipv6 || !addr->has_ipv6;
+    bool ipv4 = addr->has_ipv4 && addr->ipv4;
+    bool ipv6 = addr->has_ipv6 && addr->ipv6;
 
-    if (!ipv4 || !ipv6) {
+    if (ipv4 || ipv6) {
         qemu_opt_set_bool(opts, "ipv4", ipv4, &error_abort);
         qemu_opt_set_bool(opts, "ipv6", ipv6, &error_abort);
+    } else if (addr->has_ipv4 || addr->has_ipv6) {
+        qemu_opt_set_bool(opts, "ipv4", !addr->has_ipv4, &error_abort);
+        qemu_opt_set_bool(opts, "ipv6", !addr->has_ipv6, &error_abort);
     }
     if (addr->has_to) {
         qemu_opt_set_number(opts, "to", addr->to, &error_abort);
-- 
2.5.0

Re: [Qemu-devel] [PATCH] qemu-sockets: fix conversion of ipv4/ipv6 JSON to QemuOpts

[Daniel P. Berrange]

On Mon, Oct 12, 2015 at 03:35:41PM +0200, Paolo Bonzini wrote:
> The QemuOpts-based code treats "option not set" and "option set
> to false" the same way for the ipv4 and ipv6 options, because it
> is meant to handle only the ",ipv4" and ",ipv6" substrings in
> hand-crafted option parsers.
> 
> When converting InetSocketAddress to QemuOpts, however, it is
> necessary to handle all three cases (not set, set to true, set
> to false).  Currently we are not handling all cases correctly.
> The rules are:
> 
> * if none or both options are absent, leave things as is
> 
> * if the single present option is Y, the other should be N.
> This can be implemented by leaving things as is, or by setting
> the other option to N as done in this patch.
> 
> * if the single present option is N, the other should be Y.
> This is handled by the "else if" branch of this patch.
> 
> This ensures that the ipv4 option has an effect on Windows,
> where creating the socket with PF_UNSPEC makes an ipv6
> socket.  With this patch, ",ipv4" will result in a PF_INET
> socket instead.
> 
> Reported-by: Sair, Umair <Umair_Sair@mentor.com>
> Tested-by: Sair, Umair <Umair_Sair@mentor.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  util/qemu-sockets.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>

I look forward to formally submitting the remainder of my patches to kill
off QemuOpts from this qemu-sockets.c code :-)

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