Re: [Qemu-devel] [PATCH 5/7] kdump: add vmcoreinfo ELF note

[Marc-André Lureau]

Hi

On Wed, Jul 5, 2017 at 2:07 AM, Laszlo Ersek  wrote:
> On 06/29/17 15:23, Marc-André Lureau wrote:
>> kdump header provides offset and size of the vmcoreinfo ELF note,
>> append it if available.
>>
>> Signed-off-by: Marc-André Lureau 
>> ---
>>  dump.c | 48 
>>  1 file changed, 44 insertions(+), 4 deletions(-)
>>
>> diff --git a/dump.c b/dump.c
>> index 8fda5cc1ed..b78bc1fda7 100644
>> --- a/dump.c
>> +++ b/dump.c
>> @@ -788,8 +788,9 @@ static void create_header32(DumpState *s, Error **errp)
>>  uint32_t sub_hdr_size;
>>  uint32_t bitmap_blocks;
>>  uint32_t status = 0;
>> -uint64_t offset_note;
>> +uint64_t offset_note, offset_vmcoreinfo, size_vmcoreinfo = 0;
>>  Error *local_err = NULL;
>> +uint8_t *vmcoreinfo = NULL;
>>
>>  /* write common header, the version of kdump-compressed format is 6th */
>>  size = sizeof(DiskDumpHeader32);
>> @@ -838,7 +839,18 @@ static void create_header32(DumpState *s, Error **errp)
>>  kh->phys_base = cpu_to_dump32(s, s->dump_info.phys_base);
>>  kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
>>
>> -offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
>> +offset_vmcoreinfo = DISKDUMP_HEADER_BLOCKS * block_size + size;
>> +if (s->vmcoreinfo) {
>> +uint64_t hsize, name_size;
>> +
>> +get_note_sizes(s, s->vmcoreinfo, , _size, 
>> _vmcoreinfo);
>
> Should we round up "size_vmcoreinfo" as well? (Without the rounding,
> offset_note might become unaligned, plus I simply don't know what
> alignment is expected from kh->size_vmcoreinfo.)

In the last iteration, it only sets the kh->offset/size_vmcoreinfo. I
don't see any alignment requirement in crash.
Dave, any comment?

>
>> +vmcoreinfo =
>> +s->vmcoreinfo + (DIV_ROUND_UP(hsize, 4) + 
>> DIV_ROUND_UP(name_size, 4)) * 4;
>> +kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
>> +kh->size_vmcoreinfo = cpu_to_dump32(s, size_vmcoreinfo);
>> +}
>> +
>> +offset_note = offset_vmcoreinfo + size_vmcoreinfo;
>>  kh->offset_note = cpu_to_dump64(s, offset_note);
>>  kh->note_size = cpu_to_dump32(s, s->note_size);
>>
>> @@ -848,6 +860,14 @@ static void create_header32(DumpState *s, Error **errp)
>>  goto out;
>>  }
>>
>> +if (vmcoreinfo) {
>> +if (write_buffer(s->fd, offset_vmcoreinfo, vmcoreinfo,
>> + size_vmcoreinfo) < 0) {
>> +error_setg(errp, "dump: failed to vmcoreinfo");
>
> The verb "write" is missing from the message.
>
> Same comments for create_header64() below.
>

gone

thanks

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 5/7] kdump: add vmcoreinfo ELF note

[Laszlo Ersek]

On 06/29/17 15:23, Marc-André Lureau wrote:
> kdump header provides offset and size of the vmcoreinfo ELF note,
> append it if available.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  dump.c | 48 
>  1 file changed, 44 insertions(+), 4 deletions(-)
> 
> diff --git a/dump.c b/dump.c
> index 8fda5cc1ed..b78bc1fda7 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -788,8 +788,9 @@ static void create_header32(DumpState *s, Error **errp)
>  uint32_t sub_hdr_size;
>  uint32_t bitmap_blocks;
>  uint32_t status = 0;
> -uint64_t offset_note;
> +uint64_t offset_note, offset_vmcoreinfo, size_vmcoreinfo = 0;
>  Error *local_err = NULL;
> +uint8_t *vmcoreinfo = NULL;
>  
>  /* write common header, the version of kdump-compressed format is 6th */
>  size = sizeof(DiskDumpHeader32);
> @@ -838,7 +839,18 @@ static void create_header32(DumpState *s, Error **errp)
>  kh->phys_base = cpu_to_dump32(s, s->dump_info.phys_base);
>  kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
>  
> -offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
> +offset_vmcoreinfo = DISKDUMP_HEADER_BLOCKS * block_size + size;
> +if (s->vmcoreinfo) {
> +uint64_t hsize, name_size;
> +
> +get_note_sizes(s, s->vmcoreinfo, , _size, 
> _vmcoreinfo);

Should we round up "size_vmcoreinfo" as well? (Without the rounding,
offset_note might become unaligned, plus I simply don't know what
alignment is expected from kh->size_vmcoreinfo.)

> +vmcoreinfo =
> +s->vmcoreinfo + (DIV_ROUND_UP(hsize, 4) + 
> DIV_ROUND_UP(name_size, 4)) * 4;
> +kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
> +kh->size_vmcoreinfo = cpu_to_dump32(s, size_vmcoreinfo);
> +}
> +
> +offset_note = offset_vmcoreinfo + size_vmcoreinfo;
>  kh->offset_note = cpu_to_dump64(s, offset_note);
>  kh->note_size = cpu_to_dump32(s, s->note_size);
>  
> @@ -848,6 +860,14 @@ static void create_header32(DumpState *s, Error **errp)
>  goto out;
>  }
>  
> +if (vmcoreinfo) {
> +if (write_buffer(s->fd, offset_vmcoreinfo, vmcoreinfo,
> + size_vmcoreinfo) < 0) {
> +error_setg(errp, "dump: failed to vmcoreinfo");

The verb "write" is missing from the message.

Same comments for create_header64() below.

Thanks
Laszlo

> +goto out;
> +}
> +}
> +
>  /* write note */
>  s->note_buf = g_malloc0(s->note_size);
>  s->note_buf_offset = 0;
> @@ -888,8 +908,9 @@ static void create_header64(DumpState *s, Error **errp)
>  uint32_t sub_hdr_size;
>  uint32_t bitmap_blocks;
>  uint32_t status = 0;
> -uint64_t offset_note;
> +uint64_t offset_note, offset_vmcoreinfo, size_vmcoreinfo = 0;
>  Error *local_err = NULL;
> +uint8_t *vmcoreinfo = NULL;
>  
>  /* write common header, the version of kdump-compressed format is 6th */
>  size = sizeof(DiskDumpHeader64);
> @@ -938,7 +959,18 @@ static void create_header64(DumpState *s, Error **errp)
>  kh->phys_base = cpu_to_dump64(s, s->dump_info.phys_base);
>  kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
>  
> -offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
> +offset_vmcoreinfo = DISKDUMP_HEADER_BLOCKS * block_size + size;
> +if (s->vmcoreinfo) {
> +uint64_t hsize, name_size;
> +
> +get_note_sizes(s, s->vmcoreinfo, , _size, 
> _vmcoreinfo);
> +vmcoreinfo =
> +s->vmcoreinfo + (DIV_ROUND_UP(hsize, 4) + 
> DIV_ROUND_UP(name_size, 4)) * 4;
> +kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
> +kh->size_vmcoreinfo = cpu_to_dump64(s, size_vmcoreinfo);
> +}
> +
> +offset_note = offset_vmcoreinfo + size_vmcoreinfo;
>  kh->offset_note = cpu_to_dump64(s, offset_note);
>  kh->note_size = cpu_to_dump64(s, s->note_size);
>  
> @@ -948,6 +980,14 @@ static void create_header64(DumpState *s, Error **errp)
>  goto out;
>  }
>  
> +if (vmcoreinfo) {
> +if (write_buffer(s->fd, offset_vmcoreinfo, vmcoreinfo,
> + size_vmcoreinfo) < 0) {
> +error_setg(errp, "dump: failed to vmcoreinfo");
> +goto out;
> +}
> +}
> +
>  /* write note */
>  s->note_buf = g_malloc0(s->note_size);
>  s->note_buf_offset = 0;
> 

[Qemu-devel] [PATCH 5/7] kdump: add vmcoreinfo ELF note

[Marc-André Lureau]

kdump header provides offset and size of the vmcoreinfo ELF note,
append it if available.

Signed-off-by: Marc-André Lureau 
---
 dump.c | 48 
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/dump.c b/dump.c
index 8fda5cc1ed..b78bc1fda7 100644
--- a/dump.c
+++ b/dump.c
@@ -788,8 +788,9 @@ static void create_header32(DumpState *s, Error **errp)
 uint32_t sub_hdr_size;
 uint32_t bitmap_blocks;
 uint32_t status = 0;
-uint64_t offset_note;
+uint64_t offset_note, offset_vmcoreinfo, size_vmcoreinfo = 0;
 Error *local_err = NULL;
+uint8_t *vmcoreinfo = NULL;
 
 /* write common header, the version of kdump-compressed format is 6th */
 size = sizeof(DiskDumpHeader32);
@@ -838,7 +839,18 @@ static void create_header32(DumpState *s, Error **errp)
 kh->phys_base = cpu_to_dump32(s, s->dump_info.phys_base);
 kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
 
-offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
+offset_vmcoreinfo = DISKDUMP_HEADER_BLOCKS * block_size + size;
+if (s->vmcoreinfo) {
+uint64_t hsize, name_size;
+
+get_note_sizes(s, s->vmcoreinfo, , _size, _vmcoreinfo);
+vmcoreinfo =
+s->vmcoreinfo + (DIV_ROUND_UP(hsize, 4) + DIV_ROUND_UP(name_size, 
4)) * 4;
+kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
+kh->size_vmcoreinfo = cpu_to_dump32(s, size_vmcoreinfo);
+}
+
+offset_note = offset_vmcoreinfo + size_vmcoreinfo;
 kh->offset_note = cpu_to_dump64(s, offset_note);
 kh->note_size = cpu_to_dump32(s, s->note_size);
 
@@ -848,6 +860,14 @@ static void create_header32(DumpState *s, Error **errp)
 goto out;
 }
 
+if (vmcoreinfo) {
+if (write_buffer(s->fd, offset_vmcoreinfo, vmcoreinfo,
+ size_vmcoreinfo) < 0) {
+error_setg(errp, "dump: failed to vmcoreinfo");
+goto out;
+}
+}
+
 /* write note */
 s->note_buf = g_malloc0(s->note_size);
 s->note_buf_offset = 0;
@@ -888,8 +908,9 @@ static void create_header64(DumpState *s, Error **errp)
 uint32_t sub_hdr_size;
 uint32_t bitmap_blocks;
 uint32_t status = 0;
-uint64_t offset_note;
+uint64_t offset_note, offset_vmcoreinfo, size_vmcoreinfo = 0;
 Error *local_err = NULL;
+uint8_t *vmcoreinfo = NULL;
 
 /* write common header, the version of kdump-compressed format is 6th */
 size = sizeof(DiskDumpHeader64);
@@ -938,7 +959,18 @@ static void create_header64(DumpState *s, Error **errp)
 kh->phys_base = cpu_to_dump64(s, s->dump_info.phys_base);
 kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
 
-offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
+offset_vmcoreinfo = DISKDUMP_HEADER_BLOCKS * block_size + size;
+if (s->vmcoreinfo) {
+uint64_t hsize, name_size;
+
+get_note_sizes(s, s->vmcoreinfo, , _size, _vmcoreinfo);
+vmcoreinfo =
+s->vmcoreinfo + (DIV_ROUND_UP(hsize, 4) + DIV_ROUND_UP(name_size, 
4)) * 4;
+kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
+kh->size_vmcoreinfo = cpu_to_dump64(s, size_vmcoreinfo);
+}
+
+offset_note = offset_vmcoreinfo + size_vmcoreinfo;
 kh->offset_note = cpu_to_dump64(s, offset_note);
 kh->note_size = cpu_to_dump64(s, s->note_size);
 
@@ -948,6 +980,14 @@ static void create_header64(DumpState *s, Error **errp)
 goto out;
 }
 
+if (vmcoreinfo) {
+if (write_buffer(s->fd, offset_vmcoreinfo, vmcoreinfo,
+ size_vmcoreinfo) < 0) {
+error_setg(errp, "dump: failed to vmcoreinfo");
+goto out;
+}
+}
+
 /* write note */
 s->note_buf = g_malloc0(s->note_size);
 s->note_buf_offset = 0;
-- 
2.13.1.395.gf7b71de06