Re: [Qemu-devel] [PATCH 3/4] net: split hostname and service by last colon
Am 02.03.2012 20:54, schrieb Laine Stump: On 03/02/2012 05:35 AM, Kevin Wolf wrote: Am 02.03.2012 10:58, schrieb Amos Kong: On 02/03/12 11:38, Amos Kong wrote: --- a/net.c +++ b/net.c @@ -84,7 +84,7 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep) const char *p, *p1; int len; p = *pp; -p1 = strchr(p, sep); +p1 = strrchr(p, sep); if (!p1) return -1; len = p1 - p; And what if the port isn't specified? I think you would erroneously interpret the last part of the IP address as port. Hi Kevin, port must be specified in '-incoming' parameters and migrate monitor cmd. qemu-kvm ... -incoming tcp:$host:$port (qemu) migrate -d tcp:$host:$port If use boot up guest by wrong cmdline, qemu will report an error msg. # ./x86_64-softmmu/qemu-system-x86_64 --enable-kvm -boot n -incoming tcp:2312::8272 -monitor stdio qemu-system-x86_64: qemu: getaddrinfo: Name or service not known tcp_server_start: Invalid argument Migration failed. Exit code tcp:2312::8272(-22), exiting. Which is because 2312: isn't a valid IP address, right? But what if you have something like 2312::1234:8272? If you misinterpret the 8272 as a port number, the remaining address is still a valid IPv6 address. This is made irrelevant by PATCH 4/4, which allows for the IP address to be placed inside brackets: [2312::8272]:port (at least it's irrelevant if your documentation *requires* brackets for all numeric ipv6-address:port pairs, which is strongly recommended by RFC 5952). It really is impossible to disambiguate the meaning of the final : unless you require these brackets (or 1) require full specification of all potential colons in the IPv6 address or require that the port *always* be specified, neither of which seem acceptable to me). Here you're actually explaining why it's not irrelevant. You don't want to enforce port numbers, so 2312::1234:8272 must be interpreted as an IPv6 address without a port. This code however would take 8727 as the port and 2312::1234 as the IPv6 address, which is not what you expected (even after brackets are allowed - they don't make a difference because the example doesn't use brackets). Kevin -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/4] net: split hostname and service by last colon
- Original Message - Am 02.03.2012 20:54, schrieb Laine Stump: On 03/02/2012 05:35 AM, Kevin Wolf wrote: Am 02.03.2012 10:58, schrieb Amos Kong: On 02/03/12 11:38, Amos Kong wrote: --- a/net.c +++ b/net.c @@ -84,7 +84,7 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep) const char *p, *p1; int len; p = *pp; -p1 = strchr(p, sep); +p1 = strrchr(p, sep); if (!p1) return -1; len = p1 - p; And what if the port isn't specified? I think you would erroneously interpret the last part of the IP address as port. Hi Kevin, port must be specified in '-incoming' parameters and migrate monitor cmd. qemu-kvm ... -incoming tcp:$host:$port (qemu) migrate -d tcp:$host:$port If use boot up guest by wrong cmdline, qemu will report an error msg. # ./x86_64-softmmu/qemu-system-x86_64 --enable-kvm -boot n -incoming tcp:2312::8272 -monitor stdio qemu-system-x86_64: qemu: getaddrinfo: Name or service not known tcp_server_start: Invalid argument Migration failed. Exit code tcp:2312::8272(-22), exiting. Which is because 2312: isn't a valid IP address, right? But what if you have something like 2312::1234:8272? If you misinterpret the 8272 as a port number, the remaining address is still a valid IPv6 address. This is made irrelevant by PATCH 4/4, which allows for the IP address to be placed inside brackets: [2312::8272]:port (at least it's irrelevant if your documentation *requires* brackets for all numeric ipv6-address:port pairs, which is strongly recommended by RFC 5952). It really is impossible to disambiguate the meaning of the final : unless you require these brackets (or 1) require full specification of all potential colons in the IPv6 address or require that the port *always* be specified, neither of which seem acceptable to me). Here you're actually explaining why it's not irrelevant. You don't want to enforce port numbers, so 2312::1234:8272 must be interpreted as an IPv6 address without a port. This code however would take 8727 as the port and 2312::1234 as the IPv6 address, which is not what you expected (even after brackets are allowed - they don't make a difference because the example doesn't use brackets). In the migration context, host/port are all necessary, so it's right to parse 8272 to a port. However, for IPv6 brackets must be mandatory if you require a port. BTW, the DNS delay issue existed in the past (gethostbyname()), it should be fixed by another patchset. I will post my V2 (without fix of DNS delay) later. Kevin -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/4] net: split hostname and service by last colon
Am 05.03.2012 09:59, schrieb Amos Kong: - Original Message - Am 02.03.2012 20:54, schrieb Laine Stump: On 03/02/2012 05:35 AM, Kevin Wolf wrote: Am 02.03.2012 10:58, schrieb Amos Kong: On 02/03/12 11:38, Amos Kong wrote: --- a/net.c +++ b/net.c @@ -84,7 +84,7 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep) const char *p, *p1; int len; p = *pp; -p1 = strchr(p, sep); +p1 = strrchr(p, sep); if (!p1) return -1; len = p1 - p; And what if the port isn't specified? I think you would erroneously interpret the last part of the IP address as port. Hi Kevin, port must be specified in '-incoming' parameters and migrate monitor cmd. qemu-kvm ... -incoming tcp:$host:$port (qemu) migrate -d tcp:$host:$port If use boot up guest by wrong cmdline, qemu will report an error msg. # ./x86_64-softmmu/qemu-system-x86_64 --enable-kvm -boot n -incoming tcp:2312::8272 -monitor stdio qemu-system-x86_64: qemu: getaddrinfo: Name or service not known tcp_server_start: Invalid argument Migration failed. Exit code tcp:2312::8272(-22), exiting. Which is because 2312: isn't a valid IP address, right? But what if you have something like 2312::1234:8272? If you misinterpret the 8272 as a port number, the remaining address is still a valid IPv6 address. This is made irrelevant by PATCH 4/4, which allows for the IP address to be placed inside brackets: [2312::8272]:port (at least it's irrelevant if your documentation *requires* brackets for all numeric ipv6-address:port pairs, which is strongly recommended by RFC 5952). It really is impossible to disambiguate the meaning of the final : unless you require these brackets (or 1) require full specification of all potential colons in the IPv6 address or require that the port *always* be specified, neither of which seem acceptable to me). Here you're actually explaining why it's not irrelevant. You don't want to enforce port numbers, so 2312::1234:8272 must be interpreted as an IPv6 address without a port. This code however would take 8727 as the port and 2312::1234 as the IPv6 address, which is not what you expected (even after brackets are allowed - they don't make a difference because the example doesn't use brackets). In the migration context, host/port are all necessary, so it's right to parse 8272 to a port. However, for IPv6 brackets must be mandatory if you require a port. Makes sense. BTW, the DNS delay issue existed in the past (gethostbyname()), it should be fixed by another patchset. I will post my V2 (without fix of DNS delay) later. Yes, I agree. Kevin -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/4] net: split hostname and service by last colon
On 02/03/12 11:38, Amos Kong wrote: --- a/net.c +++ b/net.c @@ -84,7 +84,7 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep) const char *p, *p1; int len; p = *pp; -p1 = strchr(p, sep); +p1 = strrchr(p, sep); if (!p1) return -1; len = p1 - p; And what if the port isn't specified? I think you would erroneously interpret the last part of the IP address as port. Hi Kevin, port must be specified in '-incoming' parameters and migrate monitor cmd. qemu-kvm ... -incoming tcp:$host:$port (qemu) migrate -d tcp:$host:$port If use boot up guest by wrong cmdline, qemu will report an error msg. # ./x86_64-softmmu/qemu-system-x86_64 --enable-kvm -boot n -incoming tcp:2312::8272 -monitor stdio qemu-system-x86_64: qemu: getaddrinfo: Name or service not known tcp_server_start: Invalid argument Migration failed. Exit code tcp:2312::8272(-22), exiting. IPv6 address have paired colons, need more precision check. -- parse_host_port() are used in four functions in net/socket.c: tcp_start_outgoing_migration() tcp_start_incoming_migration() net_socket_mcast_init() net_socket_udp_init() The argument type of parse_host_port() needs to be changed if we replace inet_aton/gethostbyname by getaddrinfo. So I will not change parse_host_port(), and verify Ipv6 addr in tcp_start_common without parse_host_port. -- Amos. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/4] net: split hostname and service by last colon
Am 02.03.2012 10:58, schrieb Amos Kong: On 02/03/12 11:38, Amos Kong wrote: --- a/net.c +++ b/net.c @@ -84,7 +84,7 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep) const char *p, *p1; int len; p = *pp; -p1 = strchr(p, sep); +p1 = strrchr(p, sep); if (!p1) return -1; len = p1 - p; And what if the port isn't specified? I think you would erroneously interpret the last part of the IP address as port. Hi Kevin, port must be specified in '-incoming' parameters and migrate monitor cmd. qemu-kvm ... -incoming tcp:$host:$port (qemu) migrate -d tcp:$host:$port If use boot up guest by wrong cmdline, qemu will report an error msg. # ./x86_64-softmmu/qemu-system-x86_64 --enable-kvm -boot n -incoming tcp:2312::8272 -monitor stdio qemu-system-x86_64: qemu: getaddrinfo: Name or service not known tcp_server_start: Invalid argument Migration failed. Exit code tcp:2312::8272(-22), exiting. Which is because 2312: isn't a valid IP address, right? But what if you have something like 2312::1234:8272? If you misinterpret the 8272 as a port number, the remaining address is still a valid IPv6 address. parse_host_port() are used in four functions in net/socket.c: tcp_start_outgoing_migration() tcp_start_incoming_migration() net_socket_mcast_init() net_socket_udp_init() The argument type of parse_host_port() needs to be changed if we replace inet_aton/gethostbyname by getaddrinfo. So I will not change parse_host_port(), and verify Ipv6 addr in tcp_start_common without parse_host_port. Why not change the prototype of parse_host_port() and modify all callers? Kevin -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/4] net: split hostname and service by last colon
On 03/02/2012 05:35 AM, Kevin Wolf wrote: Am 02.03.2012 10:58, schrieb Amos Kong: On 02/03/12 11:38, Amos Kong wrote: --- a/net.c +++ b/net.c @@ -84,7 +84,7 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep) const char *p, *p1; int len; p = *pp; -p1 = strchr(p, sep); +p1 = strrchr(p, sep); if (!p1) return -1; len = p1 - p; And what if the port isn't specified? I think you would erroneously interpret the last part of the IP address as port. Hi Kevin, port must be specified in '-incoming' parameters and migrate monitor cmd. qemu-kvm ... -incoming tcp:$host:$port (qemu) migrate -d tcp:$host:$port If use boot up guest by wrong cmdline, qemu will report an error msg. # ./x86_64-softmmu/qemu-system-x86_64 --enable-kvm -boot n -incoming tcp:2312::8272 -monitor stdio qemu-system-x86_64: qemu: getaddrinfo: Name or service not known tcp_server_start: Invalid argument Migration failed. Exit code tcp:2312::8272(-22), exiting. Which is because 2312: isn't a valid IP address, right? But what if you have something like 2312::1234:8272? If you misinterpret the 8272 as a port number, the remaining address is still a valid IPv6 address. This is made irrelevant by PATCH 4/4, which allows for the IP address to be placed inside brackets: [2312::8272]:port (at least it's irrelevant if your documentation *requires* brackets for all numeric ipv6-address:port pairs, which is strongly recommended by RFC 5952). It really is impossible to disambiguate the meaning of the final : unless you require these brackets (or 1) require full specification of all potential colons in the IPv6 address or require that the port *always* be specified, neither of which seem acceptable to me). -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/4] net: split hostname and service by last colon
On 24/02/12 17:29, Kevin Wolf wrote: Am 10.02.2012 07:27, schrieb Amos Kong: IPv6 address contains colons, parse will be wrong. [2312::8274]:5200 Signed-off-by: Amos Kongak...@redhat.com --- net.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/net.c b/net.c index f63014c..9e1ef9e 100644 --- a/net.c +++ b/net.c @@ -84,7 +84,7 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep) const char *p, *p1; int len; p = *pp; -p1 = strchr(p, sep); +p1 = strrchr(p, sep); if (!p1) return -1; len = p1 - p; And what if the port isn't specified? I think you would erroneously interpret the last part of the IP address as port. IPv6 address have paired colons, need more precision check. -- Amos. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/4] net: split hostname and service by last colon
Am 10.02.2012 07:27, schrieb Amos Kong: IPv6 address contains colons, parse will be wrong. [2312::8274]:5200 Signed-off-by: Amos Kong ak...@redhat.com --- net.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/net.c b/net.c index f63014c..9e1ef9e 100644 --- a/net.c +++ b/net.c @@ -84,7 +84,7 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep) const char *p, *p1; int len; p = *pp; -p1 = strchr(p, sep); +p1 = strrchr(p, sep); if (!p1) return -1; len = p1 - p; And what if the port isn't specified? I think you would erroneously interpret the last part of the IP address as port. Kevin -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
