Re: [PATCH 1/1] nm-dns-dnsmasq: clear cache when dns server changes

[Beniamino Galvani <bgalvani redhat com>]

On Tue, Nov 29, 2016 at 10:10:21PM +0100, Thomas Haller wrote:
> On Tue, 2016-11-29 at 20:23 +0100, Christian Hesse wrote:
> > Thomas Haller <thaller redhat com> on Tue, 2016/11/29 17:51:
> > > On Tue, 2016-11-29 at 15:03 +0100, Christian Hesse wrote:
> > > > From: Christian Hesse <mail eworm de>
> > > >
> > > > A new DNS may have different data for us. So tell dnsmasq to
> > > > clear
> > > > the
> > > > cache when servers change.
> > >
> > > we currently always do a ClearCache call after SetServerEx.
> > >
> > > How does this relate to your patch? Does --clear-on-reload make the
> > > ClearCache call unnecessary? Why is ClearCache not sufficient?
> >
> > Oops, missed that in my logs (and did not find the code)... You are
> > right,
> > cache is cleared.
> >
> > Either of both is just fine and it works as-is. So ignore my patch.
>
> Hi,
>
> clear-on-reload might be better:
>
>   - it saves the explicit call for ClearCache (less code, and possibly
>     less runtime-overhead).
>   - depending on how --clear-on-reload is implemented, it might be
>     race-free and clear the cache atomically when setting new servers.

I agree and wish I had noticed such option before :)

> IMO, i would apply the patch, and possibly a follow-up to drop the
> ClearCache call. But I don't know dnsmasq well enough to be sure that
> that is indeed correct...

I tried the patch with the ClearCache call reverted and the behavior
seems equivalent to master. How about branch bg/dnsmasq-clear-cache-option?

Beniamino

Attachment: signature.asc
Description: PGP signature