Re: [PATCH] resolvconf support (Re: NM 0.7 Debian Lenny - strangest issue...)

[Alexander Sack <asac jwsdot com>]

On Tue, Jul 08, 2008 at 04:42:39PM +0200, Michael Biebl wrote:
>
> I wouldn't necessarily add a configure flag and #ifdef WITH_RESOLVCONF.
>
> I'd simply check at runtime, if the resolvconf binary is available (  
> g_file_test (RESOLVCONF, G_FILE_TEST_IS_EXECUTABLE)) { .. } )

OK, I didnt do that because I assumed that this is a debian/ubuntu
only thing. However, I could easily change it obviously.

(However, the fallbacks will still be necessary in case someone trashed
resolvconf by replacing the /etc/resolv.conf link with a file.)

>
> Compiling NM with resolvconf support doesn't necessarily mean, that the  
> machine NM is installed on, has resolvconf installed.
>
>> Comments?
>>
>>
>> --- src/named-manager/nm-named-manager.c	2008-06-26 19:33:13 +0000
>> +++ src/named-manager/nm-named-manager.c	2008-07-08 10:07:08 +0000
>> @@ -29,25 +29,30 @@
>>  #include <glib.h>
>>   #include <glib/gi18n.h>
>>   #include "nm-named-manager.h"
>>  #include "nm-ip4-config.h"
>>  #include "nm-utils.h"
>>  #include "NetworkManagerSystem.h"
>> +#include "NetworkManagerUtils.h"
>>   #ifdef HAVE_SELINUX
>>  #include <selinux/selinux.h>
>>  #endif
>>   #ifndef RESOLV_CONF
>>  #define RESOLV_CONF "/etc/resolv.conf"
>>  #endif
>>  +#ifndef RESOLV_CONF_PATH
>> +#define RESOLV_CONF_PATH "/sbin/resolvconf"
>> +#endif
>
>
> I'd name that simply RESOLVCONF
>
>>  G_DEFINE_TYPE(NMNamedManager, nm_named_manager, G_TYPE_OBJECT)
>>   #define NM_NAMED_MANAGER_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE 
>> ((o), \
>>                                           NM_TYPE_NAMED_MANAGER, \
>>                                           NMNamedManagerPrivate))
>>    struct NMNamedManagerPrivate {
>> @@ -291,23 +296,51 @@ rewrite_resolv_conf (NMNamedManager *mgr
>>  				     g_strerror (errno));
>>  		}
>>  	}
>>   	g_free (domain);
>>  	g_free (searches);
>>   	if (*error == NULL) {
>> -		if (rename (tmp_resolv_conf, RESOLV_CONF) < 0) {
>> +		gboolean do_rewrite = TRUE;
>> +
>> +#ifdef WITH_RESOLVCONF
>> +		gint exit_code;
>> +		char *cmd;
>> +
>> +		// delete NM resolvconf
>> +		nm_spawn_process(RESOLV_CONF_PATH " -d NetworkManager");
>> +		cmd = g_strconcat (RESOLV_CONF_PATH " -a nm1 ",
>> +					 tmp_resolv_conf,
>> +					 NULL);
>
> Don't you think, we should use the real interface name here?
> (And is that a typo: Networkmanager vs. nm1?)

yeah its a typo. As explained in the other mail, we dont have a _real_
interface name because the NMIP4Config used is a _merged_ one
(possibly from multiple interfaces).


>
> I haven't actually tested the code yet.

You would need the resolvconf package from network-manager ppa [1] which
adds the ability to pass the resolv.conf file as an argument (vs. just
as stdin). If we agree on a fix we can either send it to stdin or
consider to apply the resolvconf changes to debian as well (i would
vote for the latter).

[1] - https://edge.launchpad.net/~network-manager/+archive


 - Alexander