Re: [Qemu-devel] [PATCH v3 9/9] net: support to include ipv6 address by brackets

[Michael Roth]

On Wed, Mar 14, 2012 at 05:58:20PM +0800, Amos Kong wrote:
> On 14/03/12 03:47, Michael Roth wrote:
> >On Wed, Mar 07, 2012 at 06:48:57AM +0800, Amos Kong wrote:
> >>That method of representing an IPv6 address with a port is
> >
> >I'm not sure what "that" is referencing.
> 
> 2312::8274:5200  (5200 is a port)
> 
> >I assumed the previous patch
> >but the representation seems to be the same?
> 
> 2312::8274:5200
> 
> [2312::8274]:5200
> 
> The second one is better.

The commit message for the previous patch also uses "[2312::8274]:5200"

If that format wasn't supported till this patch you should fix up the
example in patch 8 to be "2312::8274:5200". Small nit, but it gives the
impression [host]:port was already supported in some form, which is all
the more confusing because [host]:port *is* already supported in some
form, depending on where you are in QEMU :)

> 
> >>discouraged because of its ambiguity. Referencing to RFC5952,
> >>the recommended format is:
> >>
> >>      [2312::8274]:5200
> >>
> >>For IPv6 brackets must be mandatory if you require a port.
> >>
> >>test status: Successed
> >>listen side: qemu-kvm .... -incoming tcp:[2312::8274]:5200
> >>client side: qemu-kvm ...
> >>              (qemu) migrate -d tcp:[2312::8274]:5200
> >>
> >>Signed-off-by: Amos Kong<address@hidden>
> >>---
> >>  net.c |    6 ++++++
> >>  1 files changed, 6 insertions(+), 0 deletions(-)
> >>
> >>diff --git a/net.c b/net.c
> >>index d6ce1fa..499ed1d 100644
> >>--- a/net.c
> >>+++ b/net.c
> >>@@ -88,6 +88,12 @@ static int get_str_sep(char *buf, int buf_size, const 
> >>char **pp, int sep)
> >>      if (!p1)
> >>          return -1;
> >>      len = p1 - p;
> >>+    /* remove brackets which includes hostname */
> >>+    if (*p == '['&&  *(p1-1) == ']') {
> >>+        p += 1;
> >>+        len -= 2;
> >>+    }
> >
> >Sorry, looking again I guess net/slirp.c actually has it's own copy of
> >get_str_sep(), so modifying this doesn't look like it would break
> >anything currently.
> 
> >It might cause some confusion though :). And I think
> >the special handling for brackets should be done in
> >parse_host_port_info() since get_str_sep() is pretty generically named.
> 
> agree.
> 
> >>+
> >>      p1++;
> >>      if (buf_size>  0) {
> >>          if (len>  buf_size - 1)
> >>
> >>
> 
> -- 
>                       Amos.
>