Re: [Qemu-devel] [PATCH 2/4] sockets: helper functions for qemu.

[Anthony Liguori]

Gerd Hoffmann wrote:
> This patch creates a new source and header file qemu-sockets.[ch] with
> a bunch of helper functions to create listening and connected sockets.
>
> New features of this code are (a) support for searching for a free port
> in a given range and (b) support for IPv6.
>
> The following patches put that code into use.
>   

I like this patch, but we already have a qemu_socket.h.  Please remove 
qemu_socket.h if you're going to introduce qemu-sockets.h.

> +
> +listen:
> +    if (0 != listen(slisten,1)) {
>   

Please try to avoid this style of if().

> diff --git a/vl.c b/vl.c
> index a9fae17..e9fe478 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -37,6 +37,7 @@
>  #include "gdbstub.h"
>  #include "qemu-timer.h"
>  #include "qemu-char.h"
> +#include "qemu-sockets.h"
>  #include "block.h"
>  #include "audio/audio.h"
>  #include "migration.h"
> @@ -8280,6 +8281,9 @@ static void help(int exitcode)
>             "-redir [tcp|udp]:host-port:[guest-host]:guest-port\n"
>             "                redirect TCP or UDP connections from host to guest 
> [-net user]\n"
>  #endif
> +           "-ipv4           Use IPv4 only.\n"
> +           "-ipv6           Use UPv6 only.\n"
> +           "                By default qemu uses what is available on your 
> machine.\n"
>             "\n"
>             "Linux boot specific:\n"
>             "-kernel bzImage use 'bzImage' as kernel image\n"
> @@ -8386,6 +8390,8 @@ enum {
>      QEMU_OPTION_bootp,
>      QEMU_OPTION_smb,
>      QEMU_OPTION_redir,
> +    QEMU_OPTION_ipv4,
> +    QEMU_OPTION_ipv6,
>  
>      QEMU_OPTION_kernel,
>      QEMU_OPTION_append,
> @@ -8489,6 +8495,10 @@ static const QEMUOption qemu_options[] = {
>  #endif
>      { "redir", HAS_ARG, QEMU_OPTION_redir },
>  #endif
> +    { "4", 0, QEMU_OPTION_ipv4 },
> +    { "6", 0, QEMU_OPTION_ipv6 },
> +    { "ipv4", 0, QEMU_OPTION_ipv4 },
> +    { "ipv6", 0, QEMU_OPTION_ipv6 },

I don't like the idea of aliasing these options.  Please just stick with 
one set of options.

And do we really need to have options for this?  Can't we just do the 
right thing?  I can't believe that every application has to have an ipv6 
switch to be ipv6 enabled.

Regards,

Anthony Liguori