From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MXWKQ-00081R-88 for qemu-devel@nongnu.org; Sun, 02 Aug 2009 04:22:02 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MXWKL-0007w1-8m for qemu-devel@nongnu.org; Sun, 02 Aug 2009 04:22:01 -0400 Received: from [199.232.76.173] (port=48197 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MXWKK-0007vo-WE for qemu-devel@nongnu.org; Sun, 02 Aug 2009 04:21:57 -0400 Received: from mx2.redhat.com ([66.187.237.31]:44254) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MXWKK-0006Rr-F9 for qemu-devel@nongnu.org; Sun, 02 Aug 2009 04:21:56 -0400 Date: Sun, 2 Aug 2009 11:20:03 +0300 From: "Michael S. Tsirkin" Subject: Re: [Qemu-devel] [PATCH] slirp: Read host DNS config on demand Message-ID: <20090802082003.GB4654@redhat.com> References: <1249089022.20690.11.camel@localhost.localdomain> <20090802074352.GB3339@redhat.com> <4A75483E.8000200@web.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4A75483E.8000200@web.de> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: qemu-devel@nongnu.org, Ed Swierk On Sun, Aug 02, 2009 at 10:03:10AM +0200, Jan Kiszka wrote: > Michael S. Tsirkin wrote: > > On Fri, Jul 31, 2009 at 06:10:22PM -0700, Ed Swierk wrote: > >> 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. > >> > >> Signed-off-by: Ed Swierk > >> Acked-by: Jan Kiszka > > > > 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 Maybe you are right ... OTOH it is at least consistently infinite :) > > > >> --- > >> diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c > >> index 95a4b39..751a8e2 100644 > >> --- a/slirp/ip_icmp.c > >> +++ b/slirp/ip_icmp.c > >> @@ -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; > >> } > >> diff --git a/slirp/libslirp.h b/slirp/libslirp.h > >> index 93087ed..67c70e3 100644 > >> --- a/slirp/libslirp.h > >> +++ b/slirp/libslirp.h > >> @@ -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, > >> diff --git a/slirp/main.h b/slirp/main.h > >> index 28d92d8..e87b068 100644 > >> --- a/slirp/main.h > >> +++ b/slirp/main.h > >> @@ -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; > >> diff --git a/slirp/slirp.c b/slirp/slirp.c > >> index e883f82..987aad9 100644 > >> --- a/slirp/slirp.c > >> +++ b/slirp/slirp.c > >> @@ -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); > >> diff --git a/slirp/socket.c b/slirp/socket.c > >> index d8fbe89..207109c 100644 > >> --- a/slirp/socket.c > >> +++ b/slirp/socket.c > >> @@ -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; > >> } > >> diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c > >> index 51b3834..0417345 100644 > >> --- a/slirp/tcp_subr.c > >> +++ b/slirp/tcp_subr.c > >> @@ -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; > >> } > >> > >> > >> > >