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

[Dr. David Alan Gilbert]

* Markus Armbruster (address@hidden) wrote:
> "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.

Yes OK, can do that.

> > 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}.

OK.

> > 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 "addr".  The
> address consists of host and port.

Yes I thought about doing that, but it really isn't a host, it's the
identifier for the network cards you're going to listen on; saying
'host' suggests thats that you need to specify the source host of the
destination which is certainly wrong.

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

Yes, there are also some weird dependencies in the RDMA code on
IPv6 availability I've not quite figured out yet.

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

OK.

> > +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"?

Yep OK.

> > 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!
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK