[PATCH]: core files with make testenv

Michael Adam obnox at samba.org
Mon May 6 16:12:32 MDT 2013


Hi,

I think these are good.

(i.e. "Reviewed-by: Michael Adam <obnox at samba.org>")

Anyone else?

I have analyzed the use of dump_core_setup() some more.
And I think we can do much better in the daemon start code.

Attached find a patchset that makes smbd,nmbd, and winbindd
all behave the same, so that they call dump_core_setup() four
times during startup to adapt to potential changes of the
logfile base:

- right at the start
- after parsing the command line
- after initial config loading (without registry)
- after loading the full config
  (Or should we rather re-setup the core as part of the functions
   reload_services/reload_nmbd_services/reload_services_file ?)

I still think Gregor's patch to hand in the logdir to the daemons via
command line in test setup is good, so that we are able to dump
core as early as possible.

Comments or push welcome.

Cheers - Michael

On 2013-04-22 at 14:18 +0200, Gregor Beck wrote:
> Hi,
> 
> with make testenv SELFTEST_TESTENV=s3dc calls to dump_core_setup() usually 
> fail so no core files where generated on failure.
> 
> 
> best regards,
> Gregor
> 

> >From 30785c42a03e851061f4a332999a0c1ea6c40d99 Mon Sep 17 00:00:00 2001
> From: Gregor Beck <gbeck at sernet.de>
> Date: Mon, 22 Apr 2013 12:35:01 +0200
> Subject: [PATCH 1/2] selftest:Samba3: pass -l logdir to daemon processes.
> 
> The daemons try to set up the corefile path within logdir before the config
> file was read.
> ---
>  selftest/target/Samba3.pm |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
> index 85ef64b..6a5c9a0 100755
> --- a/selftest/target/Samba3.pm
> +++ b/selftest/target/Samba3.pm
> @@ -653,7 +653,8 @@ sub check_or_start($$$$$) {
>  		close($env_vars->{STDIN_PIPE});
>  		open STDIN, ">&", \*STDIN_READER or die "can't dup STDIN_READER to STDIN: $!";
>  
> -		exec(@preargs, Samba::bindir_path($self, "nmbd"), "-F", "--no-process-group", "--log-stdout", "-s", $env_vars->{SERVERCONFFILE}, @optargs) or die("Unable to start nmbd: $!");
> +		exec(@preargs, Samba::bindir_path($self, "nmbd"), "-F", "--no-process-group", "--log-stdout", "-s", $env_vars->{SERVERCONFFILE},
> +		     "-l", $env_vars->{LOGDIR}, @optargs) or die("Unable to start nmbd: $!");
>  	}
>  	$env_vars->{NMBD_TL_PID} = $pid;
>  	write_pid($env_vars, "nmbd", $pid);
> @@ -707,7 +708,8 @@ sub check_or_start($$$$$) {
>  		close($env_vars->{STDIN_PIPE});
>  		open STDIN, ">&", \*STDIN_READER or die "can't dup STDIN_READER to STDIN: $!";
>  
> -		exec(@preargs, Samba::bindir_path($self, "winbindd"), "-F", "--no-process-group", "--stdout", "-s", $env_vars->{SERVERCONFFILE}, @optargs) or die("Unable to start winbindd: $!");
> +		exec(@preargs, Samba::bindir_path($self, "winbindd"), "-F", "--no-process-group", "--stdout", "-s", $env_vars->{SERVERCONFFILE},
> +		     "-l", $env_vars->{LOGDIR}, @optargs) or die("Unable to start winbindd: $!");
>  	}
>  	$env_vars->{WINBINDD_TL_PID} = $pid;
>  	write_pid($env_vars, "winbindd", $pid);
> @@ -757,7 +759,8 @@ sub check_or_start($$$$$) {
>  		close($env_vars->{STDIN_PIPE});
>  		open STDIN, ">&", \*STDIN_READER or die "can't dup STDIN_READER to STDIN: $!";
>  
> -		exec(@preargs, Samba::bindir_path($self, "smbd"), "-F", "--no-process-group", "--log-stdout", "-s", $env_vars->{SERVERCONFFILE}, @optargs) or die("Unable to start smbd: $!");
> +		exec(@preargs, Samba::bindir_path($self, "smbd"), "-F", "--no-process-group", "--log-stdout", "-s", $env_vars->{SERVERCONFFILE},
> +		     "-l", $env_vars->{LOGDIR},  @optargs) or die("Unable to start smbd: $!");
>  	}
>  	$env_vars->{SMBD_TL_PID} = $pid;
>  	write_pid($env_vars, "smbd", $pid);
> @@ -1194,6 +1197,7 @@ domadmins:X:$gid_domadmins:
>  	$ret{NSS_WRAPPER_GROUP} = $nss_wrapper_group;
>  	$ret{NSS_WRAPPER_WINBIND_SO_PATH} = Samba::nss_wrapper_winbind_so_path($self);
>  	$ret{LOCAL_PATH} = "$shrdir";
> +        $ret{LOGDIR} = $logdir;
>  
>  	return \%ret;
>  }
> -- 
> 1.7.9.5
> 

> >From 031160dc983e92925b344961eccd596672727859 Mon Sep 17 00:00:00 2001
> From: Gregor Beck <gbeck at sernet.de>
> Date: Mon, 22 Apr 2013 13:33:00 +0200
> Subject: [PATCH 2/2] s3: let the effective user of the parent smbd own the
>  core dump dir.
> 
> This prevents the chown to fail in testenv and seems to do the right thing in
> normal operation.
> ---
>  source3/lib/dumpcore.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/source3/lib/dumpcore.c b/source3/lib/dumpcore.c
> index 90acc16..d41f258 100644
> --- a/source3/lib/dumpcore.c
> +++ b/source3/lib/dumpcore.c
> @@ -66,7 +66,7 @@ static char *get_default_corepath(const char *logbase, const char *progname)
>  	if (mkdir(tmp_corepath, 0700) == -1 && errno != EEXIST)
>  		goto err_out;
>  
> -	if (chown(tmp_corepath, getuid(), getgid()) == -1)
> +	if (chown(tmp_corepath, geteuid(), getegid()) == -1)
>  		goto err_out;
>  
>  	if (chmod(tmp_corepath, 0700) == -1)
> -- 
> 1.7.9.5
> 

-------------- next part --------------
From b8686e78d0753c98b32a4f74407964a32ece7953 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Mon, 6 May 2013 23:13:05 +0200
Subject: [PATCH 1/3] s3:winbindd: setup the core path again after loading the
 full configuration

With registry configuration, this can change the logfile base,
which is used as the core path, once more, compared to the result of
lp_load_initial_only().

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/winbindd/winbindd.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
index 7a0700d..d70ec46 100644
--- a/source3/winbindd/winbindd.c
+++ b/source3/winbindd/winbindd.c
@@ -1455,6 +1455,14 @@ int main(int argc, char **argv, char **envp)
 		exit(1);
 	}
 
+	/*
+	 * Setup the core path one more time after loading the full
+	 * configuration. With registry configuration, this can change
+	 * the logfile base once more, compared to the result of
+	 * lp_load_initial_only().
+	 */
+	dump_core_setup("winbindd", lp_logfile(talloc_tos()));
+
 	ok = directory_create_or_exist(lp_lockdir(), geteuid(), 0755);
 	if (!ok) {
 		DEBUG(0, ("Failed to create directory %s for lock files - %s\n",
-- 
1.7.9.5


From 89f0759d5b4af39dfbcb5dad9ccb20b135359e4d Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Mon, 6 May 2013 23:50:16 +0200
Subject: [PATCH 2/3] s3:smbd: fix setting up of core path - setup core path
 four times.

As with winbindd, set up the core path four times:
- right at the start
- after the command line has been parsed
- after initial loading of the configuration
- after loading the full configuration

This way, the core setup is adapted each time the logfile
base may have been changed during the startup process.

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/smbd/server.c |   18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index f07bd28..3c664b4 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -1076,6 +1076,9 @@ extern void build_options(bool screen);
 
 	setup_logging(argv[0], DEBUG_DEFAULT_STDOUT);
 
+	fault_setup();
+	dump_core_setup("smbd", lp_logfile(talloc_tos()));
+
 	load_case_tables();
 
 	set_smbd_shim(&smbd_shim_fns);
@@ -1162,7 +1165,6 @@ extern void build_options(bool screen);
 	gain_root_privilege();
 	gain_root_group_privilege();
 
-	fault_setup();
 	dump_core_setup("smbd", lp_logfile(talloc_tos()));
 
 	/* we are never interested in SIGPIPE */
@@ -1214,6 +1216,13 @@ extern void build_options(bool screen);
 		exit(1);
 	}
 
+	/*
+	 * Setup the core path again, since loading the configuration
+	 * may have changed the logfile base, which is used as the
+	 * core path.
+	 */
+	dump_core_setup("smbd", lp_logfile(talloc_tos()));
+
 	/* Init the security context and global current_user */
 	init_sec_ctx();
 
@@ -1245,6 +1254,13 @@ extern void build_options(bool screen);
 		exit(1);
 	}
 
+	/*
+	 * Setup the core path again, since loading the full configuration
+	 * including the registry may have changed the logfile base again,
+	 * compared to the result of lp_load_initial_only().
+	 */
+	dump_core_setup("smbd", lp_logfile(talloc_tos()));
+
 	if (lp_server_role() == ROLE_ACTIVE_DIRECTORY_DC
 	    && !lp_parm_bool(-1, "server role check", "inhibit", false)) {
 		DEBUG(0, ("server role = 'active directory domain controller' not compatible with running smbd standalone. \n"));
-- 
1.7.9.5


From edbc772f29bb254814ff65d5da44a6c6a61fe5ac Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Mon, 6 May 2013 23:57:25 +0200
Subject: [PATCH 3/3] s3:nmbd: fix setting up of core path - setup core path
 four times.

As with winbindd and smbd, set up the core path four times:
- right at the start
- after the command line has been parsed
- after initial loading of the configuration
- after loading the full configuration

This way, the core setup is adapted each time the logfile
base may have been changed during the startup process.

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/nmbd/nmbd.c |   18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/source3/nmbd/nmbd.c b/source3/nmbd/nmbd.c
index 12afb00..b3b74e9 100644
--- a/source3/nmbd/nmbd.c
+++ b/source3/nmbd/nmbd.c
@@ -798,6 +798,9 @@ static bool open_sockets(bool isdaemon, int port)
 
 	setup_logging(argv[0], DEBUG_DEFAULT_STDOUT);
 
+	fault_setup();
+	dump_core_setup("nmbd", lp_logfile(talloc_tos()));
+
 	load_case_tables();
 
 	global_nmb_port = NMB_PORT;
@@ -844,7 +847,6 @@ static bool open_sockets(bool isdaemon, int port)
 		SAFE_FREE(lfile);
 	}
 
-	fault_setup();
 	dump_core_setup("nmbd", lp_logfile(talloc_tos()));
 
 	/* POSIX demands that signals are inherited. If the invoking process has
@@ -889,6 +891,13 @@ static bool open_sockets(bool isdaemon, int port)
 		exit(1);
 	}
 
+	/*
+	 * Setup the core path again, since loading the configuration
+	 * may have changed the logfile base, which is used as the
+	 * core path.
+	 */
+	dump_core_setup("nmbd", lp_logfile(talloc_tos()));
+
 	if (lp_server_role() == ROLE_ACTIVE_DIRECTORY_DC
 	    && !lp_parm_bool(-1, "server role check", "inhibit", false)) {
 		/* TODO: when we have a merged set of defaults for
@@ -912,6 +921,13 @@ static bool open_sockets(bool isdaemon, int port)
 
 	reload_nmbd_services( True );
 
+	/*
+	 * Setup the core path again, since loading the full configuration
+	 * including the registry may have changed the logfile base again,
+	 * compared to the result of lp_load_initial_only().
+	 */
+	dump_core_setup("nmbd", lp_logfile(talloc_tos()));
+
 	if (strequal(lp_workgroup(),"*")) {
 		DEBUG(0,("ERROR: a workgroup name of * is no longer supported\n"));
 		exit(1);
-- 
1.7.9.5

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 206 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20130507/7b00bb3b/attachment.pgp>


More information about the samba-technical mailing list