Re: [Qemu-devel] [PATCH v3 3/3] Document -incoming options

[Markus Armbruster]

"Dr. David Alan Gilbert (git)" <address@hidden> writes:

> From: "Dr. David Alan Gilbert" <address@hidden>
>
> Document the various URI formats for -incoming, the previous
> manpage and help text was wrong (out of date?)

Thanks a lot for updating the docs.

>
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> Reviewed-by: Juan Quintela <address@hidden>
> ---
>  qemu-options.hx | 29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 85ca3ad..6d6d2a8 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3169,12 +3169,35 @@ Set TB size.
>  ETEXI
>  
>  DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \
> -    "-incoming p     prepare for incoming migration, listen on port p\n",
> +    "-incoming uri   prepare for incoming migration, specifying source:\n" \
> +    "                exec:command    Execute 'command' use the stdout as\n" \
> +    "                                the migration stream\n" \
> +    "                fd:num          listen on the given fd\n" \
> +    "                defer           wait for the URI to be specified by\n" \
> +    "                                the monitor (migrate_incoming)\n" \
> +    "                rdma:addr:port  Listen on RDMA port on given address\n" 
> \
> +    "                tcp:addr:port   listen on TCP port (optional 
> address)\n" \
> +    "                unix:path       listen on the UNIX socket 'path'\n", \
>      QEMU_ARCH_ALL)
>  STEXI
> address@hidden -incoming @var{port}
> address@hidden -incoming @var{uri}
>  @findex -incoming
> -Prepare for incoming migration, listen on @var{port}.
> +Prepare for incoming migration, specifying the source of the migration stream
> address@hidden @option
> address@hidden exec:@var{command}
> +Execute 'command' and use the stdout as the migration stream.

Suggest:

    Execute 'command' and use its standard output as migration stream
    source.

> address@hidden fd:@var{num}
> +listen on the given fd

Well, we're not listening in usual the sense of listen(2).  Suggest

    Use file descriptor @var{num}.

> address@hidden defer
> +wait for the URI to be specified by the monitor (migrate_incoming)

"Wait", since the other items start with a capital letter.

> address@hidden rdma:@var{addr}:@var{port}
> +Listen on RDMA port on given address

Let's call the thing between the colons "host" instead of "äddr".  The
address consists of host and port.

rdma_start_incoming_migration() passes everything after rdma: to
qemu_rdma_data_init(), which parses it with inet_parse(), then uses only
host and port.  The other members of InetSocketAddress are silently
ignored.  Wonderful.

qemu_rdma_data_init() errors out if you omit host.

inet_parse() accepts one of

    :PORT
    [V6ADDR]:PORT
    V4ADDR:PORT
    HOST:PORT

followed by an options string.  The options string isn't really parsed,
instead substrings matching one of these patterns are recognized:

    ,to=PORT
    ,ipv4
    ,ipv6

Note: you can specify ipv4,ipv6 or combine V6ADDR with ipv4, or V4ADDR
with ipv6 to get both flags set in InetSocketAddress.

> address@hidden tcp:@var{addr}:@var{port}[,ipv4][,ipv6][,to=to]

address@hidden instead of @var{addr}, and [,address@hidden instead of
[,to=to], please.

> +Listen on TCP port @var{port} (optional @var{addr} to specify address to 
> listen on).
> +The options ,ipv4, ipv6 and ,to are used in the same manner as chardev TCP 
> options.

The reference to chardev options makes sense, but the reader needs to
make the connection from "chardev TCP options" to "-chardev socket"
himself.  Maybe "in the same manner as in -chardev socket"?

> address@hidden unix:@var{path}
> +listen on the UNIX socket @var{path}
> address@hidden table
>  ETEXI
>  
>  DEF("nodefaults", 0, QEMU_OPTION_nodefaults, \

Don't get fooled by my many comments, this series is good stuff!