qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] util: socket: Add missing localaddr and localport option for DGRAM socket
@ 2015-05-13 14:44 Peter Krempa
  2015-05-13 16:55 ` Eric Blake
  2015-05-13 18:23 ` Markus Armbruster
  0 siblings, 2 replies; 3+ messages in thread
From: Peter Krempa @ 2015-05-13 14:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Krempa, Markus Armbruster

The 'socket_optslist' structure does not contain the 'localaddr' and
'localport' options that are parsed in case you are creating a
'connect' type UDP character device. This causes abort of qemu after
commit:

commit f43e47dbf6de24db20ec9b588bb6cc762093dd69
Author: Markus Armbruster <armbru@redhat.com>
Date:   Thu Feb 12 17:52:20 2015 +0100

    QemuOpts: Drop qemu_opt_set(), rename qemu_opt_set_err(), fix use

Add the two fields so that the options can again be parsed correctly.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1220252

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
Cc: Eric Blake <eblake@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>

 util/qemu-sockets.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 87c9bc6..72066be 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -45,6 +45,12 @@ QemuOptsList socket_optslist = {
             .name = "port",
             .type = QEMU_OPT_STRING,
         },{
+            .name = "localaddr",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "localport",
+            .type = QEMU_OPT_STRING,
+        },{
             .name = "to",
             .type = QEMU_OPT_NUMBER,
         },{
-- 
2.3.5

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] util: socket: Add missing localaddr and localport option for DGRAM socket
  2015-05-13 14:44 [Qemu-devel] [PATCH] util: socket: Add missing localaddr and localport option for DGRAM socket Peter Krempa
@ 2015-05-13 16:55 ` Eric Blake
  2015-05-13 18:23 ` Markus Armbruster
  1 sibling, 0 replies; 3+ messages in thread
From: Eric Blake @ 2015-05-13 16:55 UTC (permalink / raw)
  To: Peter Krempa, qemu-devel; +Cc: Markus Armbruster

[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]

On 05/13/2015 08:44 AM, Peter Krempa wrote:
> The 'socket_optslist' structure does not contain the 'localaddr' and
> 'localport' options that are parsed in case you are creating a
> 'connect' type UDP character device. This causes abort of qemu after
> commit:
> 
> commit f43e47dbf6de24db20ec9b588bb6cc762093dd69
> Author: Markus Armbruster <armbru@redhat.com>
> Date:   Thu Feb 12 17:52:20 2015 +0100
> 
>     QemuOpts: Drop qemu_opt_set(), rename qemu_opt_set_err(), fix use
> 
> Add the two fields so that the options can again be parsed correctly.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1220252
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> 
>  util/qemu-sockets.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] util: socket: Add missing localaddr and localport option for DGRAM socket
  2015-05-13 14:44 [Qemu-devel] [PATCH] util: socket: Add missing localaddr and localport option for DGRAM socket Peter Krempa
  2015-05-13 16:55 ` Eric Blake
@ 2015-05-13 18:23 ` Markus Armbruster
  1 sibling, 0 replies; 3+ messages in thread
From: Markus Armbruster @ 2015-05-13 18:23 UTC (permalink / raw)
  To: Peter Krempa; +Cc: qemu-devel

Peter Krempa <pkrempa@redhat.com> writes:

> The 'socket_optslist' structure does not contain the 'localaddr' and
> 'localport' options that are parsed in case you are creating a
> 'connect' type UDP character device. This causes abort of qemu after
> commit:
>
> commit f43e47dbf6de24db20ec9b588bb6cc762093dd69
> Author: Markus Armbruster <armbru@redhat.com>
> Date:   Thu Feb 12 17:52:20 2015 +0100
>
>     QemuOpts: Drop qemu_opt_set(), rename qemu_opt_set_err(), fix use
>
> Add the two fields so that the options can again be parsed correctly.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1220252
>
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>

I fished the reproducer out of Bugzilla:

    $ qemu-system-x86_64 -chardev udp,id=charrng0,host=127.0.0.1,port=1234,localaddr=,localport=1234
    qemu-system-x86_64: -chardev udp,id=charrng0,host=127.0.0.1,port=1234,localaddr=,localport=1234: Invalid parameter 'localaddr'
    Aborted (core dumped)

Please include it in the commit message next time.

Before commit f43e47d, errors are reported and ignored instead:

    qemu-system-x86_64: -chardev udp,id=charrng0,host=127.0.0.1,port=1234,localaddr=,localport=1234: Invalid parameter 'localaddr'
    qemu-system-x86_64: -chardev udp,id=charrng0,host=127.0.0.1,port=1234,localaddr=,localport=1234: Invalid parameter 'localport'

When inet_dgram_opts() tries to retrieve them, it gets NULL, and duly
assumes default values: wildcard address, port 0.  bind() then binds a
port chosen by the kernel on all interfaces.

Specifying a localaddr other than "" or a localport other than 0 does
not work.

The commit you quoted didn't break it, it only made existing breakage
more obvious.

Do you have an idea when this was broken?  Assuming it ever worked...

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-05-13 18:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-13 14:44 [Qemu-devel] [PATCH] util: socket: Add missing localaddr and localport option for DGRAM socket Peter Krempa
2015-05-13 16:55 ` Eric Blake
2015-05-13 18:23 ` Markus Armbruster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).