[PATCHv2,3/6] util: Enable hostname resolution of tcp address

Submitted by Radostin Stoyanov on Sept. 3, 2018, 8:35 p.m.

Details

Message ID 20180903203514.32423-4-rstoyanov1@gmail.com
State New
Series "remote: Refactor TCP client/server setup"
Headers show

Commit Message

Radostin Stoyanov Sept. 3, 2018, 8:35 p.m.
Add hostname resolution in setup_tcp_client(). This change allows a
valid hostname to be provided as value for the --address option when
connecting to page server.

Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
---
 Documentation/criu.txt |  2 +-
 criu/include/util.h    |  2 +-
 criu/util.c            | 56 +++++++++++++++++++++++++++++++-----------
 3 files changed, 44 insertions(+), 16 deletions(-)

Patch hide | download patch | download mbox

diff --git a/Documentation/criu.txt b/Documentation/criu.txt
index 6bb3f0f5..04e7a610 100644
--- a/Documentation/criu.txt
+++ b/Documentation/criu.txt
@@ -550,7 +550,7 @@  Launches *criu* in page server mode.
     It isn't supposed to use --daemon and --status-fd together.
 
 *--address* 'address'::
-    Page server IP address.
+    Page server IP address or hostname.
 
 *--port* 'number'::
     Page server port number.
diff --git a/criu/include/util.h b/criu/include/util.h
index b286b3e9..aba16818 100644
--- a/criu/include/util.h
+++ b/criu/include/util.h
@@ -293,7 +293,7 @@  void print_data(unsigned long addr, unsigned char *data, size_t size);
 
 int setup_tcp_server(char *type, char *addr, unsigned short *port);
 int run_tcp_server(bool daemon_mode, int *ask, int cfd, int sk);
-int setup_tcp_client(char *addr);
+int setup_tcp_client(char *hostname);
 
 #define LAST_PID_PATH		"sys/kernel/ns_last_pid"
 #define PID_MAX_PATH		"sys/kernel/pid_max"
diff --git a/criu/util.c b/criu/util.c
index 71bb2f6e..5c9fb306 100644
--- a/criu/util.c
+++ b/criu/util.c
@@ -30,6 +30,7 @@ 
 #include <sys/resource.h>
 #include <sys/wait.h>
 #include <sys/socket.h>
+#include <netdb.h>
 #include <netinet/in.h>
 #include <netinet/tcp.h>
 #include <sched.h>
@@ -1384,28 +1385,55 @@  out:
 	return -1;
 }
 
-int setup_tcp_client(char *addr)
+int setup_tcp_client(char *hostname)
 {
 	struct sockaddr_storage saddr;
-	int sk;
-
-	pr_info("Connecting to server %s:%u\n", addr, opts.port);
+	struct addrinfo addr_criteria, *addr_list, *p;
+	char ipstr[INET6_ADDRSTRLEN];
+	int sk = -1;
+	void *ip;
 
-	if (get_sockaddr_in(&saddr, addr, opts.port))
-		return -1;
+	memset(&addr_criteria, 0, sizeof(addr_criteria));
+	addr_criteria.ai_family = AF_UNSPEC;
+	addr_criteria.ai_socktype = SOCK_STREAM;
+	addr_criteria.ai_protocol = IPPROTO_TCP;
 
-	sk = socket(saddr.ss_family, SOCK_STREAM, IPPROTO_TCP);
-	if (sk < 0) {
-		pr_perror("Can't create socket");
-		return -1;
+	if (getaddrinfo(hostname, NULL, &addr_criteria, &addr_list)) {
+		pr_perror("Failed to resolve hostname: %s", hostname);
+		goto out;
 	}
 
-	if (connect(sk, (struct sockaddr *)&saddr, sizeof(saddr)) < 0) {
-		pr_perror("Can't connect to server");
-		close(sk);
-		return -1;
+	for(p = addr_list; p != NULL; p = p->ai_next) {
+
+		if (p->ai_family == AF_INET) {
+			struct sockaddr_in *ipv4 = (struct sockaddr_in *)p->ai_addr;
+			ip = &(ipv4->sin_addr);
+		} else {
+			struct sockaddr_in6 *ipv6 = (struct sockaddr_in6 *)p->ai_addr;
+			ip = &(ipv6->sin6_addr);
+		}
+
+		inet_ntop(p->ai_family, ip, ipstr, sizeof(ipstr));
+		pr_info("Connecting to server %s:%u\n", ipstr, opts.port);
+
+		if (get_sockaddr_in(&saddr, ipstr, opts.port))
+			goto out;
+
+		sk = socket(saddr.ss_family, SOCK_STREAM, IPPROTO_TCP);
+		if (sk < 0) {
+			pr_perror("Can't create socket");
+			goto out;
+		}
+
+		if (connect(sk, (struct sockaddr *)&saddr, sizeof(saddr)) < 0) {
+			pr_info("Can't connect to server %s:%u\n", ipstr, opts.port);
+			close(sk);
+			sk = -1;
+		}
 	}
 
+out:
+	freeaddrinfo(addr_list);
 	return sk;
 }
 

Comments

Adrian Reber Sept. 4, 2018, 8:20 a.m.
I like this change as I often used a hostname with '--address' to only
find out that it has to be an IP address.

Looking at travis I see, however, errors which might be related to this
change (https://api.travis-ci.org/v3/job/424078990/log.txt):

(00.019563) uffd: 36-7: Found 16 VMAs in image
(00.019579) uffd: 36-7: Found 6 pages to be handled by UFFD
(00.019595) Error (criu/util.c:1402): Failed to resolve hostname: (null): No such file or directory

		Adrian

On Mon, Sep 03, 2018 at 09:35:11PM +0100, Radostin Stoyanov wrote:
> Add hostname resolution in setup_tcp_client(). This change allows a
> valid hostname to be provided as value for the --address option when
> connecting to page server.
> 
> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
> ---
>  Documentation/criu.txt |  2 +-
>  criu/include/util.h    |  2 +-
>  criu/util.c            | 56 +++++++++++++++++++++++++++++++-----------
>  3 files changed, 44 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/criu.txt b/Documentation/criu.txt
> index 6bb3f0f5..04e7a610 100644
> --- a/Documentation/criu.txt
> +++ b/Documentation/criu.txt
> @@ -550,7 +550,7 @@ Launches *criu* in page server mode.
>      It isn't supposed to use --daemon and --status-fd together.
>  
>  *--address* 'address'::
> -    Page server IP address.
> +    Page server IP address or hostname.
>  
>  *--port* 'number'::
>      Page server port number.
> diff --git a/criu/include/util.h b/criu/include/util.h
> index b286b3e9..aba16818 100644
> --- a/criu/include/util.h
> +++ b/criu/include/util.h
> @@ -293,7 +293,7 @@ void print_data(unsigned long addr, unsigned char *data, size_t size);
>  
>  int setup_tcp_server(char *type, char *addr, unsigned short *port);
>  int run_tcp_server(bool daemon_mode, int *ask, int cfd, int sk);
> -int setup_tcp_client(char *addr);
> +int setup_tcp_client(char *hostname);
>  
>  #define LAST_PID_PATH		"sys/kernel/ns_last_pid"
>  #define PID_MAX_PATH		"sys/kernel/pid_max"
> diff --git a/criu/util.c b/criu/util.c
> index 71bb2f6e..5c9fb306 100644
> --- a/criu/util.c
> +++ b/criu/util.c
> @@ -30,6 +30,7 @@
>  #include <sys/resource.h>
>  #include <sys/wait.h>
>  #include <sys/socket.h>
> +#include <netdb.h>
>  #include <netinet/in.h>
>  #include <netinet/tcp.h>
>  #include <sched.h>
> @@ -1384,28 +1385,55 @@ out:
>  	return -1;
>  }
>  
> -int setup_tcp_client(char *addr)
> +int setup_tcp_client(char *hostname)
>  {
>  	struct sockaddr_storage saddr;
> -	int sk;
> -
> -	pr_info("Connecting to server %s:%u\n", addr, opts.port);
> +	struct addrinfo addr_criteria, *addr_list, *p;
> +	char ipstr[INET6_ADDRSTRLEN];
> +	int sk = -1;
> +	void *ip;
>  
> -	if (get_sockaddr_in(&saddr, addr, opts.port))
> -		return -1;
> +	memset(&addr_criteria, 0, sizeof(addr_criteria));
> +	addr_criteria.ai_family = AF_UNSPEC;
> +	addr_criteria.ai_socktype = SOCK_STREAM;
> +	addr_criteria.ai_protocol = IPPROTO_TCP;
>  
> -	sk = socket(saddr.ss_family, SOCK_STREAM, IPPROTO_TCP);
> -	if (sk < 0) {
> -		pr_perror("Can't create socket");
> -		return -1;
> +	if (getaddrinfo(hostname, NULL, &addr_criteria, &addr_list)) {
> +		pr_perror("Failed to resolve hostname: %s", hostname);
> +		goto out;
>  	}
>  
> -	if (connect(sk, (struct sockaddr *)&saddr, sizeof(saddr)) < 0) {
> -		pr_perror("Can't connect to server");
> -		close(sk);
> -		return -1;
> +	for(p = addr_list; p != NULL; p = p->ai_next) {
> +
> +		if (p->ai_family == AF_INET) {
> +			struct sockaddr_in *ipv4 = (struct sockaddr_in *)p->ai_addr;
> +			ip = &(ipv4->sin_addr);
> +		} else {
> +			struct sockaddr_in6 *ipv6 = (struct sockaddr_in6 *)p->ai_addr;
> +			ip = &(ipv6->sin6_addr);
> +		}
> +
> +		inet_ntop(p->ai_family, ip, ipstr, sizeof(ipstr));
> +		pr_info("Connecting to server %s:%u\n", ipstr, opts.port);
> +
> +		if (get_sockaddr_in(&saddr, ipstr, opts.port))
> +			goto out;
> +
> +		sk = socket(saddr.ss_family, SOCK_STREAM, IPPROTO_TCP);
> +		if (sk < 0) {
> +			pr_perror("Can't create socket");
> +			goto out;
> +		}
> +
> +		if (connect(sk, (struct sockaddr *)&saddr, sizeof(saddr)) < 0) {
> +			pr_info("Can't connect to server %s:%u\n", ipstr, opts.port);
> +			close(sk);
> +			sk = -1;
> +		}
>  	}
>  
> +out:
> +	freeaddrinfo(addr_list);
>  	return sk;
>  }
>  
> -- 
> 2.17.1
> 
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
Radostin Stoyanov Sept. 4, 2018, 9:18 p.m.
Hi Adrian,

On 04/09/18 09:20, Adrian Reber wrote:
> I like this change as I often used a hostname with '--address' to only
> find out that it has to be an IP address.
>
> Looking at travis I see, however, errors which might be related to this
> change (https://api.travis-ci.org/v3/job/424078990/log.txt):
>
> (00.019563) uffd: 36-7: Found 16 VMAs in image
> (00.019579) uffd: 36-7: Found 6 pages to be handled by UFFD
> (00.019595) Error (criu/util.c:1402): Failed to resolve hostname: (null): No such file or directory

Thank you for the code review and that you point out the 'failed to
resolve hostname' issue.
It looks like in zdtm.py when the --remote-lazy-pages option is
specified, only --port (12345) is being set, and --address was omitted.
This is causing the failure. I will resend the patch series with this fixed.

Radostin