[PATCH] --dump-dmesg fix for post 3.5 kernels

[kumagai-atsushi@xxxxxxxxxxxxxxxxx (Atsushi Kumagai)]

Hello Louis,

On Thu, 31 Jan 2013 12:48:08 +0100
Louis Bouchard <louis.bouchard at canonical.com> wrote:

>        This patch enable the --dump-dmesg functionality that
>        was broken with post 3.5 kernel which use the variable-length
>        record format for the kernel log buffer.

I appreciate your hard work.
Your patch seems to work correctly, but I have some comments.
(You don't need to resend the patch.)

> 
> Signed-off-by: Louis Bouchard <louis.bouchard at canonical.com>
> ---
>  makedumpfile.c |  242 +++++++++++++++++++++++++++++++++++++++++++++-----------
>  makedumpfile.h |   16 ++++
>  2 files changed, 213 insertions(+), 45 deletions(-)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index 715ca6e..a5180f6 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -20,6 +20,7 @@
>  #include "erase_info.h"
>  #include "sadump_info.h"
>  #include <stddef.h>
> +#include <ctype.h>
>  #include <sys/time.h>
>  
>  struct symbol_table	symbol_table;
> @@ -848,6 +849,8 @@ get_symbol_info(void)
>  	SYMBOL_INIT(log_buf, "log_buf");
>  	SYMBOL_INIT(log_buf_len, "log_buf_len");
>  	SYMBOL_INIT(log_end, "log_end");
> +	SYMBOL_INIT(log_first_idx, "log_first_idx");
> +	SYMBOL_INIT(log_next_idx, "log_next_idx");
>  	SYMBOL_INIT(max_pfn, "max_pfn");
>  	SYMBOL_INIT(modules, "modules");
>  	SYMBOL_INIT(high_memory, "high_memory");
> @@ -1176,6 +1179,13 @@ get_structure_info(void)
>  	OFFSET_INIT(elf64_phdr.p_paddr, "elf64_phdr", "p_paddr");
>  	OFFSET_INIT(elf64_phdr.p_memsz, "elf64_phdr", "p_memsz");
>  
> +	SIZE_INIT(log, "log");
> +	OFFSET_INIT(log.ts_nsec, "log", "ts_nsec");
> +	OFFSET_INIT(log.len, "log", "len");
> +	OFFSET_INIT(log.text_len, "log", "text_len");
> +	OFFSET_INIT(log.dict_len, "log", "dict_len");
> +	OFFSET_INIT(log.dict_len, "log", "level");
> +

Why you overwrite log.dict_len with log.level ?
And is log.dict_len necessary for this feature ?

>  	return TRUE;
>  }
>  
> @@ -1354,6 +1364,8 @@ write_vmcoreinfo_data(void)
>  	WRITE_SYMBOL("log_buf", log_buf);
>  	WRITE_SYMBOL("log_buf_len", log_buf_len);
>  	WRITE_SYMBOL("log_end", log_end);
> +	WRITE_SYMBOL("log_first_idx", log_first_idx);
> +	WRITE_SYMBOL("log_next_idx", log_next_idx);
>  	WRITE_SYMBOL("max_pfn", max_pfn);
>  	WRITE_SYMBOL("high_memory", high_memory);
>  	WRITE_SYMBOL("node_remap_start_vaddr", node_remap_start_vaddr);
> @@ -1404,6 +1416,10 @@ write_vmcoreinfo_data(void)
>  	WRITE_MEMBER_OFFSET("node_memblk_s.size", node_memblk_s.size);
>  	WRITE_MEMBER_OFFSET("node_memblk_s.nid", node_memblk_s.nid);
>  	WRITE_MEMBER_OFFSET("vm_struct.addr", vm_struct.addr);
> +	WRITE_MEMBER_OFFSET("log.ts_nsec",log.ts_nsec);
> +	WRITE_MEMBER_OFFSET("log.len",log.len);
> +	WRITE_MEMBER_OFFSET("log.text_len",log.text_len);
> +	WRITE_MEMBER_OFFSET("log.dict_len",log.dict_len);
>  
>  	if (SYMBOL(node_data) != NOT_FOUND_SYMBOL)
>  		WRITE_ARRAY_LENGTH("node_data", node_data);
> @@ -1664,6 +1680,8 @@ read_vmcoreinfo(void)
>  	READ_SYMBOL("log_buf", log_buf);
>  	READ_SYMBOL("log_buf_len", log_buf_len);
>  	READ_SYMBOL("log_end", log_end);
> +	READ_SYMBOL("log_first_idx",log_first_idx);
> +	READ_SYMBOL("log_next_idx",log_next_idx);
>  	READ_SYMBOL("max_pfn", max_pfn);
>  	READ_SYMBOL("high_memory", high_memory);
>  	READ_SYMBOL("node_remap_start_vaddr", node_remap_start_vaddr);
> @@ -1679,6 +1697,7 @@ read_vmcoreinfo(void)
>  	READ_STRUCTURE_SIZE("node_memblk_s", node_memblk_s);
>  	READ_STRUCTURE_SIZE("nodemask_t", nodemask_t);
>  	READ_STRUCTURE_SIZE("pageflags", pageflags);
> +	READ_STRUCTURE_SIZE("log", log);

You forgot to add "WRITE_STRUCTURE_SIZE("log", log)", I'll add it.

Lastly, your patch causes the following warning:

  makedumpfile.c: In function 'dump_dmesg':
  makedumpfile.c:3562: warning: 'log_end' may be used uninitialized in this function

So, I'll fix this patch with the change below:


diff --git a/makedumpfile.c b/makedumpfile.c
index 2a2fcbd..ebedd07 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -1384,6 +1384,7 @@ write_vmcoreinfo_data(void)
	WRITE_STRUCTURE_SIZE("node_memblk_s", node_memblk_s);
	WRITE_STRUCTURE_SIZE("nodemask_t", nodemask_t);
	WRITE_STRUCTURE_SIZE("pageflags", pageflags);
+	WRITE_STRUCTURE_SIZE("log", log);

	/*
	 * write the member offset of 1st kernel
@@ -3626,7 +3627,9 @@ dump_dmesg()
			}
			log_end = log_end_2_6_24;
		}
-	}
+	} else
+		log_end = 0;
+
	if (!readmem(VADDR, SYMBOL(log_buf_len), &log_buf_len,
	    sizeof(log_buf_len))) {
		ERRMSG("Can't get log_buf_len.\n");


Thanks
Atsushi Kumagai