Skip to contentSkip to conversation

slirp: Read host DNS config on demand

PATCH In review Ed Swierk 4 participants 6 messages Aug 1, 2009 – Aug 3, 2009

lore
Share & tools
lore

Ed Swierk

author
#0 lore raw

PATCH 1/1 slirp: Read host DNS config on demand

Currently the qemu user-mode networking stack reads the host DNS configuration (/etc/resolv.conf or the Windows equivalent) only once when qemu starts. This causes name lookups in the guest to fail if the host is moved to a different network from which the original DNS servers are unreachable, a common occurrence when the host is a laptop.

This patch changes the slirp code to read the host DNS configuration on demand, caching the results for 10 seconds to avoid unnecessary overhead if name lookups occur in rapid succession.

---

slirp/ip_icmp.c +2 -1
index 95a4b39..751a8e2 100644
@@ -127,7 +127,8 @@ icmp_input(struct mbuf *m, int hlen)
           slirp->vnetwork_addr.s_addr) {
 	/* It's an alias */
 	if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
-	  addr.sin_addr = dns_addr;
+	  if (get_dns_addr(&addr.sin_addr) < 0)
+	    addr.sin_addr = loopback_addr;
 	} else {
 	  addr.sin_addr = loopback_addr;
 	}
slirp/libslirp.h +2 -0
index 93087ed..67c70e3 100644
@@ -8,6 +8,8 @@
 struct Slirp;
 typedef struct Slirp Slirp;
 
+int get_dns_addr(struct in_addr *pdns_addr);
+
 Slirp *slirp_init(int restricted, struct in_addr vnetwork,
                   struct in_addr vnetmask, struct in_addr vhost,
                   const char *vhostname, const char *tftp_path,
slirp/main.h +0 -1
index 28d92d8..e87b068 100644
@@ -32,7 +32,6 @@ extern u_int curtime;
 extern fd_set *global_readfds, *global_writefds, *global_xfds;
 extern struct in_addr our_addr;
 extern struct in_addr loopback_addr;
-extern struct in_addr dns_addr;
 extern char *username;
 extern char *socket_path;
 extern int towrite_max;
slirp/slirp.c +21 -10
index e883f82..987aad9 100644
@@ -29,8 +29,6 @@
 
 /* host address */
 struct in_addr our_addr;
-/* host dns address */
-struct in_addr dns_addr;
 /* host loopback address */
 struct in_addr loopback_addr;
 
@@ -51,9 +49,12 @@ static int do_slowtimo;
 TAILQ_HEAD(slirp_instances, Slirp) slirp_instances =
     TAILQ_HEAD_INITIALIZER(slirp_instances);
 
+struct in_addr dns_addr = { 0 };
+u_int dns_addr_time = 0;
+
 #ifdef _WIN32
 
-static int get_dns_addr(struct in_addr *pdns_addr)
+int get_dns_addr(struct in_addr *pdns_addr)
 {
     FIXED_INFO *FixedInfo=NULL;
     ULONG    BufLen;
@@ -61,6 +62,11 @@ static int get_dns_addr(struct in_addr *pdns_addr)
     IP_ADDR_STRING *pIPAddr;
     struct in_addr tmp_addr;
 
+    if (dns_addr.s_addr != 0 && (curtime - dns_addr_time) < 10000) {
+        *pdns_addr = dns_addr;
+        return 0;
+    }
+
     FixedInfo = (FIXED_INFO *)GlobalAlloc(GPTR, sizeof(FIXED_INFO));
     BufLen = sizeof(FIXED_INFO);
 
@@ -84,6 +90,8 @@ static int get_dns_addr(struct in_addr *pdns_addr)
     pIPAddr = &(FixedInfo->DnsServerList);
     inet_aton(pIPAddr->IpAddress.String, &tmp_addr);
     *pdns_addr = tmp_addr;
+    dns_addr = tmp_addr;
+    dns_addr_time = curtime;
     if (FixedInfo) {
         GlobalFree(FixedInfo);
         FixedInfo = NULL;
@@ -98,7 +106,7 @@ static void winsock_cleanup(void)
 
 #else
 
-static int get_dns_addr(struct in_addr *pdns_addr)
+int get_dns_addr(struct in_addr *pdns_addr)
 {
     char buff[512];
     char buff2[257];
@@ -106,6 +114,11 @@ static int get_dns_addr(struct in_addr *pdns_addr)
     int found = 0;
     struct in_addr tmp_addr;
 
+    if (dns_addr.s_addr != 0 && (curtime - dns_addr_time) < 10000) {
+        *pdns_addr = dns_addr;
+        return 0;
+    }
+
     f = fopen("/etc/resolv.conf", "r");
     if (!f)
         return -1;
@@ -120,8 +133,11 @@ static int get_dns_addr(struct in_addr *pdns_addr)
             if (tmp_addr.s_addr == loopback_addr.s_addr)
                 tmp_addr = our_addr;
             /* If it's the first one, set it to dns_addr */
-            if (!found)
+            if (!found) {
                 *pdns_addr = tmp_addr;
+                dns_addr = tmp_addr;
+                dns_addr_time = curtime;
+            }
 #ifdef DEBUG
             else
                 lprint(", ");
@@ -177,11 +193,6 @@ static void slirp_init_once(void)
     if (our_addr.s_addr == 0) {
         our_addr = loopback_addr;
     }
-
-    /* FIXME: This address may change during runtime */
-    if (get_dns_addr(&dns_addr) < 0) {
-        dns_addr = loopback_addr;
-    }
 }
 
 static void slirp_state_save(QEMUFile *f, void *opaque);
slirp/socket.c +2 -1
index d8fbe89..207109c 100644
@@ -552,7 +552,8 @@ sosendto(struct socket *so, struct mbuf *m)
 	    slirp->vnetwork_addr.s_addr) {
 	  /* It's an alias */
 	  if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
-	    addr.sin_addr = dns_addr;
+	    if (get_dns_addr(&addr.sin_addr) < 0)
+	      addr.sin_addr = loopback_addr;
 	  } else {
 	    addr.sin_addr = loopback_addr;
 	  }
slirp/tcp_subr.c +2 -1
index 51b3834..0417345 100644
@@ -340,7 +340,8 @@ int tcp_fconnect(struct socket *so)
         slirp->vnetwork_addr.s_addr) {
       /* It's an alias */
       if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
-	addr.sin_addr = dns_addr;
+	if (get_dns_addr(&addr.sin_addr) < 0)
+	  addr.sin_addr = loopback_addr;
       } else {
 	addr.sin_addr = loopback_addr;
       }
#1 lore raw
Quoted 6 lines · Ed Swierk

Currently the qemu user-mode networking stack reads the host DNS configuration (/etc/resolv.conf or the Windows equivalent) only once when qemu starts. This causes name lookups in the guest to fail if the host is moved to a different network from which the original DNS servers are unreachable, a common occurrence when the host is a laptop.

Quoted 6 lines

This patch changes the slirp code to read the host DNS configuration on demand, caching the results for 10 seconds to avoid unnecessary overhead if name lookups occur in rapid succession.

Introducing a random delay until update might surprise users. Would it make sense to use something like inotify instead?

Quoted 165 lines

---

slirp/ip_icmp.c +2 -1
index 95a4b39..751a8e2 100644
@@ -127,7 +127,8 @@ icmp_input(struct mbuf *m, int hlen)
           slirp->vnetwork_addr.s_addr) {
 	/* It's an alias */
 	if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
-	  addr.sin_addr = dns_addr;
+	  if (get_dns_addr(&addr.sin_addr) < 0)
+	    addr.sin_addr = loopback_addr;
 	} else {
 	  addr.sin_addr = loopback_addr;
 	}
slirp/libslirp.h +2 -0
index 93087ed..67c70e3 100644
@@ -8,6 +8,8 @@
 struct Slirp;
 typedef struct Slirp Slirp;
 
+int get_dns_addr(struct in_addr *pdns_addr);
+
 Slirp *slirp_init(int restricted, struct in_addr vnetwork,
                   struct in_addr vnetmask, struct in_addr vhost,
                   const char *vhostname, const char *tftp_path,
slirp/main.h +0 -1
index 28d92d8..e87b068 100644
@@ -32,7 +32,6 @@ extern u_int curtime;
 extern fd_set *global_readfds, *global_writefds, *global_xfds;
 extern struct in_addr our_addr;
 extern struct in_addr loopback_addr;
-extern struct in_addr dns_addr;
 extern char *username;
 extern char *socket_path;
 extern int towrite_max;
slirp/slirp.c +21 -10
index e883f82..987aad9 100644
@@ -29,8 +29,6 @@
 
 /* host address */
 struct in_addr our_addr;
-/* host dns address */
-struct in_addr dns_addr;
 /* host loopback address */
 struct in_addr loopback_addr;
 
@@ -51,9 +49,12 @@ static int do_slowtimo;
 TAILQ_HEAD(slirp_instances, Slirp) slirp_instances =
     TAILQ_HEAD_INITIALIZER(slirp_instances);
 
+struct in_addr dns_addr = { 0 };
+u_int dns_addr_time = 0;
+
 #ifdef _WIN32
 
-static int get_dns_addr(struct in_addr *pdns_addr)
+int get_dns_addr(struct in_addr *pdns_addr)
 {
     FIXED_INFO *FixedInfo=NULL;
     ULONG    BufLen;
@@ -61,6 +62,11 @@ static int get_dns_addr(struct in_addr *pdns_addr)
     IP_ADDR_STRING *pIPAddr;
     struct in_addr tmp_addr;
 
+    if (dns_addr.s_addr != 0 && (curtime - dns_addr_time) < 10000) {
+        *pdns_addr = dns_addr;
+        return 0;
+    }
+
     FixedInfo = (FIXED_INFO *)GlobalAlloc(GPTR, sizeof(FIXED_INFO));
     BufLen = sizeof(FIXED_INFO);
 
@@ -84,6 +90,8 @@ static int get_dns_addr(struct in_addr *pdns_addr)
     pIPAddr = &(FixedInfo->DnsServerList);
     inet_aton(pIPAddr->IpAddress.String, &tmp_addr);
     *pdns_addr = tmp_addr;
+    dns_addr = tmp_addr;
+    dns_addr_time = curtime;
     if (FixedInfo) {
         GlobalFree(FixedInfo);
         FixedInfo = NULL;
@@ -98,7 +106,7 @@ static void winsock_cleanup(void)
 
 #else
 
-static int get_dns_addr(struct in_addr *pdns_addr)
+int get_dns_addr(struct in_addr *pdns_addr)
 {
     char buff[512];
     char buff2[257];
@@ -106,6 +114,11 @@ static int get_dns_addr(struct in_addr *pdns_addr)
     int found = 0;
     struct in_addr tmp_addr;
 
+    if (dns_addr.s_addr != 0 && (curtime - dns_addr_time) < 10000) {
+        *pdns_addr = dns_addr;
+        return 0;
+    }
+
     f = fopen("/etc/resolv.conf", "r");
     if (!f)
         return -1;
@@ -120,8 +133,11 @@ static int get_dns_addr(struct in_addr *pdns_addr)
             if (tmp_addr.s_addr == loopback_addr.s_addr)
                 tmp_addr = our_addr;
             /* If it's the first one, set it to dns_addr */
-            if (!found)
+            if (!found) {
                 *pdns_addr = tmp_addr;
+                dns_addr = tmp_addr;
+                dns_addr_time = curtime;
+            }
 #ifdef DEBUG
             else
                 lprint(", ");
@@ -177,11 +193,6 @@ static void slirp_init_once(void)
     if (our_addr.s_addr == 0) {
         our_addr = loopback_addr;
     }
-
-    /* FIXME: This address may change during runtime */
-    if (get_dns_addr(&dns_addr) < 0) {
-        dns_addr = loopback_addr;
-    }
 }
 
 static void slirp_state_save(QEMUFile *f, void *opaque);
slirp/socket.c +2 -1
index d8fbe89..207109c 100644
@@ -552,7 +552,8 @@ sosendto(struct socket *so, struct mbuf *m)
 	    slirp->vnetwork_addr.s_addr) {
 	  /* It's an alias */
 	  if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
-	    addr.sin_addr = dns_addr;
+	    if (get_dns_addr(&addr.sin_addr) < 0)
+	      addr.sin_addr = loopback_addr;
 	  } else {
 	    addr.sin_addr = loopback_addr;
 	  }
slirp/tcp_subr.c +2 -1
index 51b3834..0417345 100644
@@ -340,7 +340,8 @@ int tcp_fconnect(struct socket *so)
         slirp->vnetwork_addr.s_addr) {
       /* It's an alias */
       if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
-	addr.sin_addr = dns_addr;
+	if (get_dns_addr(&addr.sin_addr) < 0)
+	  addr.sin_addr = loopback_addr;
       } else {
 	addr.sin_addr = loopback_addr;
       }
#2 lore raw
Quoted 11 lines · Michael S. Tsirkin
Quoted 10 lines · Ed Swierk

Currently the qemu user-mode networking stack reads the host DNS configuration (/etc/resolv.conf or the Windows equivalent) only once when qemu starts. This causes name lookups in the guest to fail if the host is moved to a different network from which the original DNS servers are unreachable, a common occurrence when the host is a laptop.

This patch changes the slirp code to read the host DNS configuration on demand, caching the results for 10 seconds to avoid unnecessary overhead if name lookups occur in rapid succession.

Quoted 5 lines
Quoted 2 lines

Introducing a random delay until update might surprise users. Would it make sense to use something like inotify instead?

IMHO, 10 s is below the user surprise threshold for a dynamically changing network link. Moreover, inotify does not exist on all platforms.

Let's keep things simple for the first step, we could still further improve it later on. 10 s are already an improvement over the current infinite timeout.

Jan

Quoted 166 lines
Quoted 166 lines

---

slirp/ip_icmp.c +2 -1
index 95a4b39..751a8e2 100644
@@ -127,7 +127,8 @@ icmp_input(struct mbuf *m, int hlen)
           slirp->vnetwork_addr.s_addr) {
 	/* It's an alias */
 	if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
-	  addr.sin_addr = dns_addr;
+	  if (get_dns_addr(&addr.sin_addr) < 0)
+	    addr.sin_addr = loopback_addr;
 	} else {
 	  addr.sin_addr = loopback_addr;
 	}
slirp/libslirp.h +2 -0
index 93087ed..67c70e3 100644
@@ -8,6 +8,8 @@
 struct Slirp;
 typedef struct Slirp Slirp;
 
+int get_dns_addr(struct in_addr *pdns_addr);
+
 Slirp *slirp_init(int restricted, struct in_addr vnetwork,
                   struct in_addr vnetmask, struct in_addr vhost,
                   const char *vhostname, const char *tftp_path,
slirp/main.h +0 -1
index 28d92d8..e87b068 100644
@@ -32,7 +32,6 @@ extern u_int curtime;
 extern fd_set *global_readfds, *global_writefds, *global_xfds;
 extern struct in_addr our_addr;
 extern struct in_addr loopback_addr;
-extern struct in_addr dns_addr;
 extern char *username;
 extern char *socket_path;
 extern int towrite_max;
slirp/slirp.c +21 -10
index e883f82..987aad9 100644
@@ -29,8 +29,6 @@
 
 /* host address */
 struct in_addr our_addr;
-/* host dns address */
-struct in_addr dns_addr;
 /* host loopback address */
 struct in_addr loopback_addr;
 
@@ -51,9 +49,12 @@ static int do_slowtimo;
 TAILQ_HEAD(slirp_instances, Slirp) slirp_instances =
     TAILQ_HEAD_INITIALIZER(slirp_instances);
 
+struct in_addr dns_addr = { 0 };
+u_int dns_addr_time = 0;
+
 #ifdef _WIN32
 
-static int get_dns_addr(struct in_addr *pdns_addr)
+int get_dns_addr(struct in_addr *pdns_addr)
 {
     FIXED_INFO *FixedInfo=NULL;
     ULONG    BufLen;
@@ -61,6 +62,11 @@ static int get_dns_addr(struct in_addr *pdns_addr)
     IP_ADDR_STRING *pIPAddr;
     struct in_addr tmp_addr;
 
+    if (dns_addr.s_addr != 0 && (curtime - dns_addr_time) < 10000) {
+        *pdns_addr = dns_addr;
+        return 0;
+    }
+
     FixedInfo = (FIXED_INFO *)GlobalAlloc(GPTR, sizeof(FIXED_INFO));
     BufLen = sizeof(FIXED_INFO);
 
@@ -84,6 +90,8 @@ static int get_dns_addr(struct in_addr *pdns_addr)
     pIPAddr = &(FixedInfo->DnsServerList);
     inet_aton(pIPAddr->IpAddress.String, &tmp_addr);
     *pdns_addr = tmp_addr;
+    dns_addr = tmp_addr;
+    dns_addr_time = curtime;
     if (FixedInfo) {
         GlobalFree(FixedInfo);
         FixedInfo = NULL;
@@ -98,7 +106,7 @@ static void winsock_cleanup(void)
 
 #else
 
-static int get_dns_addr(struct in_addr *pdns_addr)
+int get_dns_addr(struct in_addr *pdns_addr)
 {
     char buff[512];
     char buff2[257];
@@ -106,6 +114,11 @@ static int get_dns_addr(struct in_addr *pdns_addr)
     int found = 0;
     struct in_addr tmp_addr;
 
+    if (dns_addr.s_addr != 0 && (curtime - dns_addr_time) < 10000) {
+        *pdns_addr = dns_addr;
+        return 0;
+    }
+
     f = fopen("/etc/resolv.conf", "r");
     if (!f)
         return -1;
@@ -120,8 +133,11 @@ static int get_dns_addr(struct in_addr *pdns_addr)
             if (tmp_addr.s_addr == loopback_addr.s_addr)
                 tmp_addr = our_addr;
             /* If it's the first one, set it to dns_addr */
-            if (!found)
+            if (!found) {
                 *pdns_addr = tmp_addr;
+                dns_addr = tmp_addr;
+                dns_addr_time = curtime;
+            }
 #ifdef DEBUG
             else
                 lprint(", ");
@@ -177,11 +193,6 @@ static void slirp_init_once(void)
     if (our_addr.s_addr == 0) {
         our_addr = loopback_addr;
     }
-
-    /* FIXME: This address may change during runtime */
-    if (get_dns_addr(&dns_addr) < 0) {
-        dns_addr = loopback_addr;
-    }
 }
 
 static void slirp_state_save(QEMUFile *f, void *opaque);
slirp/socket.c +2 -1
index d8fbe89..207109c 100644
@@ -552,7 +552,8 @@ sosendto(struct socket *so, struct mbuf *m)
 	    slirp->vnetwork_addr.s_addr) {
 	  /* It's an alias */
 	  if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
-	    addr.sin_addr = dns_addr;
+	    if (get_dns_addr(&addr.sin_addr) < 0)
+	      addr.sin_addr = loopback_addr;
 	  } else {
 	    addr.sin_addr = loopback_addr;
 	  }
slirp/tcp_subr.c +2 -1
index 51b3834..0417345 100644
@@ -340,7 +340,8 @@ int tcp_fconnect(struct socket *so)
         slirp->vnetwork_addr.s_addr) {
       /* It's an alias */
       if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
-	addr.sin_addr = dns_addr;
+	if (get_dns_addr(&addr.sin_addr) < 0)
+	  addr.sin_addr = loopback_addr;
       } else {
 	addr.sin_addr = loopback_addr;
       }
#3 lore raw
Quoted 18 lines · Jan Kiszka
Quoted 11 lines · Michael S. Tsirkin
Quoted 10 lines · Ed Swierk

Currently the qemu user-mode networking stack reads the host DNS configuration (/etc/resolv.conf or the Windows equivalent) only once when qemu starts. This causes name lookups in the guest to fail if the host is moved to a different network from which the original DNS servers are unreachable, a common occurrence when the host is a laptop.

This patch changes the slirp code to read the host DNS configuration on demand, caching the results for 10 seconds to avoid unnecessary overhead if name lookups occur in rapid succession.

Quoted 5 lines
Quoted 2 lines

Introducing a random delay until update might surprise users. Would it make sense to use something like inotify instead?

Quoted 8 lines

IMHO, 10 s is below the user surprise threshold for a dynamically changing network link. Moreover, inotify does not exist on all platforms.

Let's keep things simple for the first step, we could still further improve it later on. 10 s are already an improvement over the current infinite timeout.

Jan

Maybe you are right ... OTOH it is at least consistently infinite :)

Quoted 168 lines
Quoted 166 lines
Quoted 166 lines

---

slirp/ip_icmp.c +2 -1
index 95a4b39..751a8e2 100644
@@ -127,7 +127,8 @@ icmp_input(struct mbuf *m, int hlen)
           slirp->vnetwork_addr.s_addr) {
 	/* It's an alias */
 	if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
-	  addr.sin_addr = dns_addr;
+	  if (get_dns_addr(&addr.sin_addr) < 0)
+	    addr.sin_addr = loopback_addr;
 	} else {
 	  addr.sin_addr = loopback_addr;
 	}
slirp/libslirp.h +2 -0
index 93087ed..67c70e3 100644
@@ -8,6 +8,8 @@
 struct Slirp;
 typedef struct Slirp Slirp;
 
+int get_dns_addr(struct in_addr *pdns_addr);
+
 Slirp *slirp_init(int restricted, struct in_addr vnetwork,
                   struct in_addr vnetmask, struct in_addr vhost,
                   const char *vhostname, const char *tftp_path,
slirp/main.h +0 -1
index 28d92d8..e87b068 100644
@@ -32,7 +32,6 @@ extern u_int curtime;
 extern fd_set *global_readfds, *global_writefds, *global_xfds;
 extern struct in_addr our_addr;
 extern struct in_addr loopback_addr;
-extern struct in_addr dns_addr;
 extern char *username;
 extern char *socket_path;
 extern int towrite_max;
slirp/slirp.c +21 -10
index e883f82..987aad9 100644
@@ -29,8 +29,6 @@
 
 /* host address */
 struct in_addr our_addr;
-/* host dns address */
-struct in_addr dns_addr;
 /* host loopback address */
 struct in_addr loopback_addr;
 
@@ -51,9 +49,12 @@ static int do_slowtimo;
 TAILQ_HEAD(slirp_instances, Slirp) slirp_instances =
     TAILQ_HEAD_INITIALIZER(slirp_instances);
 
+struct in_addr dns_addr = { 0 };
+u_int dns_addr_time = 0;
+
 #ifdef _WIN32
 
-static int get_dns_addr(struct in_addr *pdns_addr)
+int get_dns_addr(struct in_addr *pdns_addr)
 {
     FIXED_INFO *FixedInfo=NULL;
     ULONG    BufLen;
@@ -61,6 +62,11 @@ static int get_dns_addr(struct in_addr *pdns_addr)
     IP_ADDR_STRING *pIPAddr;
     struct in_addr tmp_addr;
 
+    if (dns_addr.s_addr != 0 && (curtime - dns_addr_time) < 10000) {
+        *pdns_addr = dns_addr;
+        return 0;
+    }
+
     FixedInfo = (FIXED_INFO *)GlobalAlloc(GPTR, sizeof(FIXED_INFO));
     BufLen = sizeof(FIXED_INFO);
 
@@ -84,6 +90,8 @@ static int get_dns_addr(struct in_addr *pdns_addr)
     pIPAddr = &(FixedInfo->DnsServerList);
     inet_aton(pIPAddr->IpAddress.String, &tmp_addr);
     *pdns_addr = tmp_addr;
+    dns_addr = tmp_addr;
+    dns_addr_time = curtime;
     if (FixedInfo) {
         GlobalFree(FixedInfo);
         FixedInfo = NULL;
@@ -98,7 +106,7 @@ static void winsock_cleanup(void)
 
 #else
 
-static int get_dns_addr(struct in_addr *pdns_addr)
+int get_dns_addr(struct in_addr *pdns_addr)
 {
     char buff[512];
     char buff2[257];
@@ -106,6 +114,11 @@ static int get_dns_addr(struct in_addr *pdns_addr)
     int found = 0;
     struct in_addr tmp_addr;
 
+    if (dns_addr.s_addr != 0 && (curtime - dns_addr_time) < 10000) {
+        *pdns_addr = dns_addr;
+        return 0;
+    }
+
     f = fopen("/etc/resolv.conf", "r");
     if (!f)
         return -1;
@@ -120,8 +133,11 @@ static int get_dns_addr(struct in_addr *pdns_addr)
             if (tmp_addr.s_addr == loopback_addr.s_addr)
                 tmp_addr = our_addr;
             /* If it's the first one, set it to dns_addr */
-            if (!found)
+            if (!found) {
                 *pdns_addr = tmp_addr;
+                dns_addr = tmp_addr;
+                dns_addr_time = curtime;
+            }
 #ifdef DEBUG
             else
                 lprint(", ");
@@ -177,11 +193,6 @@ static void slirp_init_once(void)
     if (our_addr.s_addr == 0) {
         our_addr = loopback_addr;
     }
-
-    /* FIXME: This address may change during runtime */
-    if (get_dns_addr(&dns_addr) < 0) {
-        dns_addr = loopback_addr;
-    }
 }
 
 static void slirp_state_save(QEMUFile *f, void *opaque);
slirp/socket.c +2 -1
index d8fbe89..207109c 100644
@@ -552,7 +552,8 @@ sosendto(struct socket *so, struct mbuf *m)
 	    slirp->vnetwork_addr.s_addr) {
 	  /* It's an alias */
 	  if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
-	    addr.sin_addr = dns_addr;
+	    if (get_dns_addr(&addr.sin_addr) < 0)
+	      addr.sin_addr = loopback_addr;
 	  } else {
 	    addr.sin_addr = loopback_addr;
 	  }
slirp/tcp_subr.c +2 -1
index 51b3834..0417345 100644
@@ -340,7 +340,8 @@ int tcp_fconnect(struct socket *so)
         slirp->vnetwork_addr.s_addr) {
       /* It's an alias */
       if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
-	addr.sin_addr = dns_addr;
+	if (get_dns_addr(&addr.sin_addr) < 0)
+	  addr.sin_addr = loopback_addr;
       } else {
 	addr.sin_addr = loopback_addr;
       }
author
#4 lore raw
In reply to Jan Kiszka
Quoted 6 lines · Jan Kiszka

IMHO, 10 s is below the user surprise threshold for a dynamically changing network link. Moreover, inotify does not exist on all platforms.

Let's keep things simple for the first step, we could still further improve it later on. 10 s are already an improvement over the current infinite timeout.

Right. The most common situation in which the DNS server address changes is after you wake up your laptop and wait for it to connect to a new network. DHCP itself can take several seconds to do its thing, so you're unlikely to notice the up-to-10-second delay before full network connectivity is restored in a VM.

Of course there's nothing magical about 10 seconds. A shorter timeout would be even nicer, and I doubt anyone would really miss the CPU cycles lost to re-reading a tiny text file, but I don't think instantaneous response is required here.

--Ed

#5 lore raw
Quoted 8 lines · Ed Swierk
Quoted 6 lines · Jan Kiszka

IMHO, 10 s is below the user surprise threshold for a dynamically changing network link. Moreover, inotify does not exist on all platforms.

Let's keep things simple for the first step, we could still further improve it later on. 10 s are already an improvement over the current infinite timeout.

Quoted 5 lines

Right. The most common situation in which the DNS server address changes is after you wake up your laptop and wait for it to connect to a new network. DHCP itself can take several seconds to do its thing, so you're unlikely to notice the up-to-10-second delay before full network connectivity is restored in a VM.

I strongly disagree. Usual practice is to watch the NetworkManager icon until it indicates that DHCP is complete with a green light, finger poised over the clicking button, and then expect web browsing and SSH to work immediately after the green light appears. A 10 second delay at that point would be most irritating.

Ideally, that's what RTNETLINK events are for: you can register for asynchronous events whenever a network interface is reconfigured, and trigger things like rereading /etc/resolv.conf from that. Windows has a similar mechanism for watching interface changes.

But a 1/2 second delay would be fine.

Quoted 4 lines

Of course there's nothing magical about 10 seconds. A shorter timeout would be even nicer, and I doubt anyone would really miss the CPU cycles lost to re-reading a tiny text file, but I don't think instantaneous response is required here.

All you need is to call stat() to confirm that the file hasn't changed. Just check the dev/ino/mtime/size.

If that were a periodic 1 second timer, the biggest cost on a laptop might be power drain due to CPU wakeups. But since the resolv.conf check should only should be done in reaction to the VM actively doing a DNS query (plus 1 second has passed since the last resolve.conf check), the CPU is already woken up, so it's cheap.

-- Jamie