----- Original Message ----- > Hi Dave, > > On 06/13/2014 12:55 PM, Dave Anderson wrote: > > > > > > ----- Original Message ----- > >> > >> > >> ----- Original Message ----- > >>> When the crash command line includes two filenames where one is a > >>> compressed kernel and the other is a debuginfo file and they appear in > >>> that order then if the uncompressed temporary version of the kernel is > >>> actually larger than the debuginfo then crash will end with an error but > >>> will also unlink the debuginfo file and will not clean up the (intended > >>> temporary) uncompressed copy of the kernel. > >> > >> Hi Dave, > >> > >> It's taken me awhile to wrap my head around this, but my first question is: > >> how is it possible that the uncompressed stripped kernel can possibly be > >> larger than the debuginfo file? > >> > >> I only have old RHEL3 kernels as examples, but the .debug versions > >> of the kernel are 5 to 6 times larger than the stripped kernel. > >> > >>> This patch at least fixes the unintended unlink and leaving the > >>> temporary present. It doesn't fix the failure to start but that's > >>> because the wrong files are assumed the debuginfo and kernel. The size > >>> case that led to this discovery is probably rare. > >> > >> Is the unintended unlink() done in the first display_sys_stats() or > >> in clean_exit()? > >> > >> And then with your fix applied, why does the crash session still fail? > >> > >> Dave > > > > Hi Dave, > > > > Although I'm still interested in answers to the questions above, this > > patch also fixes a different scenario. > > > > In testing the 8 possible combinations using RHEL3 vmlinux/vmlinux.gz > > and vmlinux.debug/vmlinux.debug.gz files, I found that I could reproduce > > the problem with this particular combination/order: > > > > $ crash vmlinux.debug.gz vmlinux ... > > > > Similar to your setup, the pc->namelist_orig field continues to > > be non-NULL, and therefore pc->namelist (vmlinux) gets removed in > > clean_exit(), and since pc->namelist_debug_orig doesn't get set, > > the uncompressed temporary debug file remains! > > Yes, vmlinux.gz passes is_compressed_kernel() and there is a tmpname > (vmlinux.debug_XXXXXX with unique name replacement of XXXXXX). > select_namelist() gets called with tmpname (as the argument new) and all > the namelist values in pc are NULL. select_namelist() uses new to set > pc->namelist (to vmlinux.debug_XXXXXX). On return to main(), > pc->namelist_orig gets set to argv[optind] (vmlinux.debug.gz). Right -- once I had my own reproducer, the problem became self-evident. But I was still interested in how your particular case was possible. Queued for crash-7.0.8: https://github.com/crash-utility/crash/commit/13c0faff75cc12a322f4d16d3c4712f30399cdde Thanks again, Dave > argv[optind] = vmlinux.debug.gz > > // State before select_namelist() > pc->namelist is NULL > pc->namelist_orig is NULL > pc->namelist_debug is NULL > pc->namelist_debug_orig is NULL > > select_namelist(vmlinux.debug_XXXXXX) > > // State shortly after select_namelist() > pc->namelist is vmlinux.debug_XXXXXX > pc->namelist_orig is vmlinux.debug.gz > pc->namelist_debug is NULL > pc->namelist_debug_orig is NULL > > The next argument, vmlinux, passes is_kernel() and select_namelist() > gets called with argv[optind] (as the argument new). Since there is a > namelist value already select_namelist() gets the struct stat for each > file (vmlinux.debug_XXXXXX and vmlinux, the first is a larger file). > select_namelist() copies pc->namelist (vmlinux.debug_XXXXXX) to > pc->namelist_debug and sets pc->namelist from new (vmlinux) leaving > pc->namelist_orig set to vmlinux.debug.gz and pc->namelist_debug_orig as > NULL. > > argv[optind] = vmlinux > > // State before select_namelist() > pc->namelist is vmlinux.debug_XXXXXX > pc->namelist_orig is vmlinux.debug.gz > pc->namelist_debug is NULL > pc->namelist_debug_orig is NULL > > select_namelist(vmlinux) > > // State shortly after select_namelist() > pc->namelist is vmlinux > pc->namelist_orig is vmlinux.debug.gz > pc->namelist_debug is vmlinux.debug_XXXXXX > pc->namelist_debug_orig is NULL > > That leaves namelist_debug with no indication it is an uncompressed, > temporary file (the equivalent _orig member of pc is NULL) and indicates > vmlinux is an uncompressed temporary file (the equivalent _orig member > of pc is non-NULL) when clean_exit() is used so there is an unintended > unlink of vmlinux and an uncleaned vmlinux.debug_XXXXXX temporary. > > > Nice fix! > > Thanks, I'm glad to help. > > -- > David. > > > > >> > >> > >>> The cause is that evidence of a temporary file to unlink is that there > >>> is a value in pc->namelist and pc->namelist_orig (or pc->namelist_debug > >>> and pc->namelist_orig_debug) but when the file size test in > >>> select_namelist() results in the pc->namelist copy to pc->namelist_debug > >>> the _orig is not copied as well so the implied file to unlink is the one > >>> set in pc->namelist (the debuginfo filename now). > >>> > >>> The patch causes a populated namelist_orig value to be swapped with > >>> namelist_debug_orig if the namelist value is copied to namelist_debug. > >>> > >>> Signed-off-by: David Mair <dmair@xxxxxxxx> > >>> --- > >>> symbols.c | 7 +++++++ > >>> 1 file changed, 7 insertions(+) > >>> > >>> diff --git a/symbols.c b/symbols.c > >>> index 4c6fbf4..e1ed719 100644 > >>> --- a/symbols.c > >>> +++ b/symbols.c > >>> @@ -3520,6 +3520,7 @@ int > >>> select_namelist(char *new) > >>> { > >>> struct stat stat1, stat2; > >>> + char *namecp; > >>> > >>> if (pc->server_namelist) { > >>> pc->namelist_debug = new; > >>> @@ -3533,6 +3534,12 @@ select_namelist(char *new) > >>> > >>> if (stat1.st_size > stat2.st_size) { > >>> pc->namelist_debug = pc->namelist; > >>> + if (pc->namelist_orig) > >>> + { > >>> + namecp = pc->namelist_debug_orig; > >>> + pc->namelist_debug_orig = pc->namelist_orig; > >>> + pc->namelist_orig = namecp; > >>> + } > >>> pc->namelist = new; > >>> } else if (stat2.st_size > stat1.st_size) > >>> pc->namelist_debug = new; > >>> > >>> -- > >>> Crash-utility mailing list > >>> Crash-utility@xxxxxxxxxx > >>> https://www.redhat.com/mailman/listinfo/crash-utility > >>> > >> > >> -- > >> Crash-utility mailing list > >> Crash-utility@xxxxxxxxxx > >> https://www.redhat.com/mailman/listinfo/crash-utility > >> > > > > -- > > Crash-utility mailing list > > Crash-utility@xxxxxxxxxx > > https://www.redhat.com/mailman/listinfo/crash-utility > > > > -- > Crash-utility mailing list > Crash-utility@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/crash-utility > -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility