[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] slirp: Read host DNS config on demand
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH] slirp: Read host DNS config on demand |
Date: |
Sun, 2 Aug 2009 11:20:03 +0300 |
User-agent: |
Mutt/1.5.19 (2009-01-05) |
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 <address@hidden>
> >> Acked-by: Jan Kiszka <address@hidden>
> >
> > 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;
> >> }
> >>
> >>
> >>
>
>