[PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them

[PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them

[Ian Campbell]

Refactor the $kvp_replace helper in ts-xen-install into a generic
helper (which requires using ::EO and ::EI for namespacing) for use
with target_editfile and use it to edit /etc/sysctl.conf to set
kernel.core_pattern on boot.

Tested in standalone mode by installing and running a C program
containing "*(int *)0 = 1;" which, after running "ulimit -c unlimited"
produces the expected core file. ts-logs-capture when run in
standalone mode then picks them up.

I've not yet figured out how to make the desired rlimit take affect
for all processes (including e.g. daemons spawned on boot). Likely
this will involve some combination of pam_limits.so PAM module and
adding explicit ulimit calls to the initscripts which we care about
(primarily xencommons and libvirt initscripts).

I did debate making the presence of cores in /var/core fail the test
(somehow), but I decided that would be annoying for standalone mode or
shared host scenarios where the core files might be stale or related
to another job.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 Osstest/TestSupport.pm | 22 ++++++++++++++++++++++
 ts-host-install        |  9 +++++++++
 ts-logs-capture        |  2 ++
 ts-xen-install         | 19 ++-----------------
 4 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 8754e22..ece2282 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -57,6 +57,7 @@ BEGIN {
                       target_put_guest_image target_editfile
                       target_editfile_cancel
                       target_editfile_root target_file_exists
+                      target_editfile_kvp_replace
                       target_run_apt
                       target_install_packages target_install_packages_norec
                       target_jobdir target_extract_jobdistpath_subdir
@@ -542,6 +543,27 @@ sub teditfileex {
 	if $install;
 }
 
+# Replace a Key=Value style line in a config file.
+#
+# To be used as 3rd argument to target_editfile(_root) as:
+#    target_editfile_root($ho, "/path/to/a/file",
+#			 sub { target_editfile_kvp_replace($key, $value) });
+sub target_editfile_kvp_replace ($$)
+{
+    my ($key,$value) = @_;
+    my $prnow;
+    $prnow= sub {
+	print ::EO "$key=$value\n" or die $!;
+	$prnow= sub { };
+    };
+    while (<::EI>) {
+	print ::EO or die $! unless m/^$key\b/;
+	$prnow->() if m/^#$key/;
+    }
+    print ::EO "\n" or die $!;
+    $prnow->();
+};
+
 sub target_editfile_root ($$$;$$) { teditfileex('root',@_); }
 sub target_editfile      ($$$;$$) { teditfileex('osstest',@_); }
     # my $code= pop @_;
diff --git a/ts-host-install b/ts-host-install
index 9656079..b60abae 100755
--- a/ts-host-install
+++ b/ts-host-install
@@ -139,6 +139,15 @@ END
 	});
     }
 
+    target_cmd_root($ho, 'mkdir -p /var/core');
+
+    target_editfile_root($ho, '/etc/sysctl.conf',
+	sub { target_editfile_kvp_replace(
+		  "kernel.core_pattern",
+		  # %p==pid,%e==executable name,%t==timestamp
+		  "/var/core/%t.%p.%e.core") });
+    target_cmd_root($ho, "sysctl --load /etc/sysctl.conf");
+
     target_cmd_root($ho, "update-rc.d osstest-confirm-booted start 99 2 .");
 
     logm('OK: install completed');
diff --git a/ts-logs-capture b/ts-logs-capture
index 453b03d..45b0a38 100755
--- a/ts-logs-capture
+++ b/ts-logs-capture
@@ -136,6 +136,8 @@ sub fetch_logs_host_guests () {
 
                   /home/osstest/osstest-confirm-booted.log
 
+                  /var/core/*.core
+
                   )];
     if (!try_fetch_logs($ho, $logs)) {
         logm("log fetching failed, trying hard host reboot...");
diff --git a/ts-xen-install b/ts-xen-install
index 5282f0a..da64a90 100755
--- a/ts-xen-install
+++ b/ts-xen-install
@@ -114,26 +114,11 @@ sub adjustconfig () {
     }
     die unless defined $trace_config_file;
 
-    my $kvp_replace = sub($$) {
-	my ($key,$value) = @_;
-        my $prnow;
-        $prnow= sub {
-            print EO "$key=$value\n" or die $!;
-            $prnow= sub { };
-        };
-        while (<EI>) {
-            print EO or die $! unless m/^$key\b/;
-            $prnow->() if m/^#$key/;
-        }
-        print EO "\n" or die $!;
-        $prnow->();
-    };
-
     target_editfile_root($ho, $trace_config_file,
-			 sub { $kvp_replace->("XENCONSOLED_TRACE", "guest") });
+	sub { target_editfile_kvp_replace("XENCONSOLED_TRACE", "guest") });
 
     target_editfile_root($ho, '/etc/libvirt/libvirtd.conf',
-			 sub { $kvp_replace->("log_level", "1") })
+		sub { target_editfile_kvp_replace("log_level", "1") })
 	if toolstack($ho)->{Name} eq "libvirt";
 
     target_cmd_root($ho, 'mkdir -p /var/log/xen/console');
-- 
2.1.4

Re: [PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them

[Don Slutz]

On 03/09/15 11:40, Ian Campbell wrote:
> Refactor the $kvp_replace helper in ts-xen-install into a generic
> helper (which requires using ::EO and ::EI for namespacing) for use
> with target_editfile and use it to edit /etc/sysctl.conf to set
> kernel.core_pattern on boot.
> 
> Tested in standalone mode by installing and running a C program
> containing "*(int *)0 = 1;" which, after running "ulimit -c unlimited"
> produces the expected core file. ts-logs-capture when run in
> standalone mode then picks them up.
> 
> I've not yet figured out how to make the desired rlimit take affect
> for all processes (including e.g. daemons spawned on boot). Likely
> this will involve some combination of pam_limits.so PAM module and
> adding explicit ulimit calls to the initscripts which we care about
> (primarily xencommons and libvirt initscripts).

I am not sure about debian, but for fedora the places are:


/etc/security/limits.conf:
*  soft  core  unlimited

/etc/profile:
ulimit -S -c unlimited > /dev/null 2>&1

/etc/sysctl.conf
fs.suid_dumpable = 1

/etc/sysconfig/init:
DAEMON_COREFILE_LIMIT='unlimited'

Note: The last depends on:
  /etc/init.d/functions:
  ulimit -S -c ${DAEMON_COREFILE_LIMIT:-0} >/dev/null 2>&1

Hope this helps.
   -Don Slutz

> 
> I did debate making the presence of cores in /var/core fail the test
> (somehow), but I decided that would be annoying for standalone mode or
> shared host scenarios where the core files might be stale or related
> to another job.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  Osstest/TestSupport.pm | 22 ++++++++++++++++++++++
>  ts-host-install        |  9 +++++++++
>  ts-logs-capture        |  2 ++
>  ts-xen-install         | 19 ++-----------------
>  4 files changed, 35 insertions(+), 17 deletions(-)
> 
> diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
> index 8754e22..ece2282 100644
> --- a/Osstest/TestSupport.pm
> +++ b/Osstest/TestSupport.pm
> @@ -57,6 +57,7 @@ BEGIN {
>                        target_put_guest_image target_editfile
>                        target_editfile_cancel
>                        target_editfile_root target_file_exists
> +                      target_editfile_kvp_replace
>                        target_run_apt
>                        target_install_packages target_install_packages_norec
>                        target_jobdir target_extract_jobdistpath_subdir
> @@ -542,6 +543,27 @@ sub teditfileex {
>  	if $install;
>  }
>  
> +# Replace a Key=Value style line in a config file.
> +#
> +# To be used as 3rd argument to target_editfile(_root) as:
> +#    target_editfile_root($ho, "/path/to/a/file",
> +#			 sub { target_editfile_kvp_replace($key, $value) });
> +sub target_editfile_kvp_replace ($$)
> +{
> +    my ($key,$value) = @_;
> +    my $prnow;
> +    $prnow= sub {
> +	print ::EO "$key=$value\n" or die $!;
> +	$prnow= sub { };
> +    };
> +    while (<::EI>) {
> +	print ::EO or die $! unless m/^$key\b/;
> +	$prnow->() if m/^#$key/;
> +    }
> +    print ::EO "\n" or die $!;
> +    $prnow->();
> +};
> +
>  sub target_editfile_root ($$$;$$) { teditfileex('root',@_); }
>  sub target_editfile      ($$$;$$) { teditfileex('osstest',@_); }
>      # my $code= pop @_;
> diff --git a/ts-host-install b/ts-host-install
> index 9656079..b60abae 100755
> --- a/ts-host-install
> +++ b/ts-host-install
> @@ -139,6 +139,15 @@ END
>  	});
>      }
>  
> +    target_cmd_root($ho, 'mkdir -p /var/core');
> +
> +    target_editfile_root($ho, '/etc/sysctl.conf',
> +	sub { target_editfile_kvp_replace(
> +		  "kernel.core_pattern",
> +		  # %p==pid,%e==executable name,%t==timestamp
> +		  "/var/core/%t.%p.%e.core") });
> +    target_cmd_root($ho, "sysctl --load /etc/sysctl.conf");
> +
>      target_cmd_root($ho, "update-rc.d osstest-confirm-booted start 99 2 .");
>  
>      logm('OK: install completed');
> diff --git a/ts-logs-capture b/ts-logs-capture
> index 453b03d..45b0a38 100755
> --- a/ts-logs-capture
> +++ b/ts-logs-capture
> @@ -136,6 +136,8 @@ sub fetch_logs_host_guests () {
>  
>                    /home/osstest/osstest-confirm-booted.log
>  
> +                  /var/core/*.core
> +
>                    )];
>      if (!try_fetch_logs($ho, $logs)) {
>          logm("log fetching failed, trying hard host reboot...");
> diff --git a/ts-xen-install b/ts-xen-install
> index 5282f0a..da64a90 100755
> --- a/ts-xen-install
> +++ b/ts-xen-install
> @@ -114,26 +114,11 @@ sub adjustconfig () {
>      }
>      die unless defined $trace_config_file;
>  
> -    my $kvp_replace = sub($$) {
> -	my ($key,$value) = @_;
> -        my $prnow;
> -        $prnow= sub {
> -            print EO "$key=$value\n" or die $!;
> -            $prnow= sub { };
> -        };
> -        while (<EI>) {
> -            print EO or die $! unless m/^$key\b/;
> -            $prnow->() if m/^#$key/;
> -        }
> -        print EO "\n" or die $!;
> -        $prnow->();
> -    };
> -
>      target_editfile_root($ho, $trace_config_file,
> -			 sub { $kvp_replace->("XENCONSOLED_TRACE", "guest") });
> +	sub { target_editfile_kvp_replace("XENCONSOLED_TRACE", "guest") });
>  
>      target_editfile_root($ho, '/etc/libvirt/libvirtd.conf',
> -			 sub { $kvp_replace->("log_level", "1") })
> +		sub { target_editfile_kvp_replace("log_level", "1") })
>  	if toolstack($ho)->{Name} eq "libvirt";
>  
>      target_cmd_root($ho, 'mkdir -p /var/log/xen/console');
> 

Re: [PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them

[Ian Campbell]

On Mon, 2015-03-09 at 17:54 -0400, Don Slutz wrote:
> 
> On 03/09/15 11:40, Ian Campbell wrote:
> > Refactor the $kvp_replace helper in ts-xen-install into a generic
> > helper (which requires using ::EO and ::EI for namespacing) for use
> > with target_editfile and use it to edit /etc/sysctl.conf to set
> > kernel.core_pattern on boot.
> > 
> > Tested in standalone mode by installing and running a C program
> > containing "*(int *)0 = 1;" which, after running "ulimit -c unlimited"
> > produces the expected core file. ts-logs-capture when run in
> > standalone mode then picks them up.
> > 
> > I've not yet figured out how to make the desired rlimit take affect
> > for all processes (including e.g. daemons spawned on boot). Likely
> > this will involve some combination of pam_limits.so PAM module and
> > adding explicit ulimit calls to the initscripts which we care about
> > (primarily xencommons and libvirt initscripts).
> 
> I am not sure about debian, but for fedora the places are:

Thanks, these look broadly similar to Debian.

> /etc/security/limits.conf:
> *  soft  core  unlimited

FYI "*" explicitly excludes root, at least on Debian, so a separate line
would be needed for root logins.

> /etc/profile:
> ulimit -S -c unlimited > /dev/null 2>&1
> 
> /etc/sysctl.conf
> fs.suid_dumpable = 1

Not 100% sure if we would need this in the context of osstest.

> /etc/sysconfig/init:
> DAEMON_COREFILE_LIMIT='unlimited'
> 
> Note: The last depends on:
>   /etc/init.d/functions:
>   ulimit -S -c ${DAEMON_COREFILE_LIMIT:-0} >/dev/null 2>&1

I think this one is a Red Hat ism.

> Hope this helps.

It did, thanks.

>    -Don Slutz
> 
> > 
> > I did debate making the presence of cores in /var/core fail the test
> > (somehow), but I decided that would be annoying for standalone mode or
> > shared host scenarios where the core files might be stale or related
> > to another job.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> >  Osstest/TestSupport.pm | 22 ++++++++++++++++++++++
> >  ts-host-install        |  9 +++++++++
> >  ts-logs-capture        |  2 ++
> >  ts-xen-install         | 19 ++-----------------
> >  4 files changed, 35 insertions(+), 17 deletions(-)
> > 
> > diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
> > index 8754e22..ece2282 100644
> > --- a/Osstest/TestSupport.pm
> > +++ b/Osstest/TestSupport.pm
> > @@ -57,6 +57,7 @@ BEGIN {
> >                        target_put_guest_image target_editfile
> >                        target_editfile_cancel
> >                        target_editfile_root target_file_exists
> > +                      target_editfile_kvp_replace
> >                        target_run_apt
> >                        target_install_packages target_install_packages_norec
> >                        target_jobdir target_extract_jobdistpath_subdir
> > @@ -542,6 +543,27 @@ sub teditfileex {
> >  	if $install;
> >  }
> >  
> > +# Replace a Key=Value style line in a config file.
> > +#
> > +# To be used as 3rd argument to target_editfile(_root) as:
> > +#    target_editfile_root($ho, "/path/to/a/file",
> > +#			 sub { target_editfile_kvp_replace($key, $value) });
> > +sub target_editfile_kvp_replace ($$)
> > +{
> > +    my ($key,$value) = @_;
> > +    my $prnow;
> > +    $prnow= sub {
> > +	print ::EO "$key=$value\n" or die $!;
> > +	$prnow= sub { };
> > +    };
> > +    while (<::EI>) {
> > +	print ::EO or die $! unless m/^$key\b/;
> > +	$prnow->() if m/^#$key/;
> > +    }
> > +    print ::EO "\n" or die $!;
> > +    $prnow->();
> > +};
> > +
> >  sub target_editfile_root ($$$;$$) { teditfileex('root',@_); }
> >  sub target_editfile      ($$$;$$) { teditfileex('osstest',@_); }
> >      # my $code= pop @_;
> > diff --git a/ts-host-install b/ts-host-install
> > index 9656079..b60abae 100755
> > --- a/ts-host-install
> > +++ b/ts-host-install
> > @@ -139,6 +139,15 @@ END
> >  	});
> >      }
> >  
> > +    target_cmd_root($ho, 'mkdir -p /var/core');
> > +
> > +    target_editfile_root($ho, '/etc/sysctl.conf',
> > +	sub { target_editfile_kvp_replace(
> > +		  "kernel.core_pattern",
> > +		  # %p==pid,%e==executable name,%t==timestamp
> > +		  "/var/core/%t.%p.%e.core") });
> > +    target_cmd_root($ho, "sysctl --load /etc/sysctl.conf");
> > +
> >      target_cmd_root($ho, "update-rc.d osstest-confirm-booted start 99 2 .");
> >  
> >      logm('OK: install completed');
> > diff --git a/ts-logs-capture b/ts-logs-capture
> > index 453b03d..45b0a38 100755
> > --- a/ts-logs-capture
> > +++ b/ts-logs-capture
> > @@ -136,6 +136,8 @@ sub fetch_logs_host_guests () {
> >  
> >                    /home/osstest/osstest-confirm-booted.log
> >  
> > +                  /var/core/*.core
> > +
> >                    )];
> >      if (!try_fetch_logs($ho, $logs)) {
> >          logm("log fetching failed, trying hard host reboot...");
> > diff --git a/ts-xen-install b/ts-xen-install
> > index 5282f0a..da64a90 100755
> > --- a/ts-xen-install
> > +++ b/ts-xen-install
> > @@ -114,26 +114,11 @@ sub adjustconfig () {
> >      }
> >      die unless defined $trace_config_file;
> >  
> > -    my $kvp_replace = sub($$) {
> > -	my ($key,$value) = @_;
> > -        my $prnow;
> > -        $prnow= sub {
> > -            print EO "$key=$value\n" or die $!;
> > -            $prnow= sub { };
> > -        };
> > -        while (<EI>) {
> > -            print EO or die $! unless m/^$key\b/;
> > -            $prnow->() if m/^#$key/;
> > -        }
> > -        print EO "\n" or die $!;
> > -        $prnow->();
> > -    };
> > -
> >      target_editfile_root($ho, $trace_config_file,
> > -			 sub { $kvp_replace->("XENCONSOLED_TRACE", "guest") });
> > +	sub { target_editfile_kvp_replace("XENCONSOLED_TRACE", "guest") });
> >  
> >      target_editfile_root($ho, '/etc/libvirt/libvirtd.conf',
> > -			 sub { $kvp_replace->("log_level", "1") })
> > +		sub { target_editfile_kvp_replace("log_level", "1") })
> >  	if toolstack($ho)->{Name} eq "libvirt";
> >  
> >      target_cmd_root($ho, 'mkdir -p /var/log/xen/console');
> > 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them"):
> Refactor the $kvp_replace helper in ts-xen-install into a generic
> helper (which requires using ::EO and ::EI for namespacing) for use
> with target_editfile [etc.]

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

> I did debate making the presence of cores in /var/core fail the test
> (somehow), but I decided that would be annoying for standalone mode or
> shared host scenarios where the core files might be stale or related
> to another job.

I think you should add /var/core to the leak check step.

Ian.

Re: [PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them

[Ian Campbell]

On Tue, 2015-03-24 at 12:40 +0000, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them"):
> > Refactor the $kvp_replace helper in ts-xen-install into a generic
> > helper (which requires using ::EO and ::EI for namespacing) for use
> > with target_editfile [etc.]
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> > I did debate making the presence of cores in /var/core fail the test
> > (somehow), but I decided that would be annoying for standalone mode or
> > shared host scenarios where the core files might be stale or related
> > to another job.
> 
> I think you should add /var/core to the leak check step.

Which in practice just means listing it in the arugment to
inventory_files?

Re: [PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them"):
> On Tue, 2015-03-24 at 12:40 +0000, Ian Jackson wrote:
> > I think you should add /var/core to the leak check step.
> 
> Which in practice just means listing it in the arugment to
> inventory_files?

Yes.

Ian.

Re: [PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them

[Ian Campbell]

On Tue, 2015-03-24 at 18:35 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them"):
> > On Tue, 2015-03-24 at 12:40 +0000, Ian Jackson wrote:
> > > I think you should add /var/core to the leak check step.
> > 
> > Which in practice just means listing it in the arugment to
> > inventory_files?
> 
> Yes.

Good, v2 in <1427207038-28090-1-git-send-email-ian.campbell@citrix.com>
does just that.

Do you have any thoughts on how to best universally arrange for the
rlimits to be increased? Probably inserting a ulimit call into the
libvirt initscript would be an easy first step, and since that's the
thing we most often see crashing it seems. I'll send such a patch
shortly.

Adding it to xencommons would be OK too, which just leaves catching e.g.
daemonised xl processes launched via ssh. Hacking /etc/profile or
something in TestSupport::cmdex perhaps?

Ian.

Re: [PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them

[Ian Campbell]

On Wed, 2015-03-25 at 09:40 +0000, Ian Campbell wrote:
> Do you have any thoughts on how to best universally arrange for the
> rlimits to be increased? Probably inserting a ulimit call into the
> libvirt initscript would be an easy first step, and since that's the
> thing we most often see crashing it seems. I'll send such a patch
> shortly.
> 
> Adding it to xencommons would be OK too, which just leaves catching e.g.
> daemonised xl processes launched via ssh. Hacking /etc/profile or
> something in TestSupport::cmdex perhaps?

I handled this by writing an /etc/security/limits.d/ file during host
install, which seems to have done the trick. I just sent a patch which
along with the libvirt one I sent earlier today covers everything I
think we care about except our daemons which are launched from
xencommons.

The initscript in xen.git doesn't offer a useful hook for this, we could
add something to the end of /etc/default/xencommons, which is sourced by
the script, but that seems like a bit of an abuse.

Ian.

Re: [PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them"):
> Do you have any thoughts on how to best universally arrange for the
> rlimits to be increased? Probably inserting a ulimit call into the
> libvirt initscript would be an easy first step, and since that's the
> thing we most often see crashing it seems. I'll send such a patch
> shortly.

Maybe pam ?

Ian.

Re: [PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them

[Ian Campbell]

On Wed, 2015-03-25 at 16:18 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them"):
> > Do you have any thoughts on how to best universally arrange for the
> > rlimits to be increased? Probably inserting a ulimit call into the
> > libvirt initscript would be an easy first step, and since that's the
> > thing we most often see crashing it seems. I'll send such a patch
> > shortly.
> 
> Maybe pam ?

Yes, I think that's what my subsequent patch to configure
via /etc/security/limits.d has achived.

Ian.