diff mbox series

[v3,05/14] dump: Split write of section headers and data and add a prepare step

Message ID 20220721132256.2171-6-frankja@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series dump: Add arch section and s390x PV dump | expand

Commit Message

Janosch Frank July 21, 2022, 1:22 p.m. UTC
By splitting the writing of the section headers and (future) section
data we prepare for the addition of a string table section and
architecture sections.

At the same time we move the writing of the section to the end of the
dump process. This allows the upcoming architecture section code to
add data after all of the common dump data has been written.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 dump/dump.c           | 114 ++++++++++++++++++++++++++++++++----------
 include/sysemu/dump.h |   4 ++
 2 files changed, 92 insertions(+), 26 deletions(-)

Comments

Marc-André Lureau July 21, 2022, 2:41 p.m. UTC | #1
On Thu, Jul 21, 2022 at 5:23 PM Janosch Frank <frankja@linux.ibm.com> wrote:
>
> By splitting the writing of the section headers and (future) section
> data we prepare for the addition of a string table section and
> architecture sections.
>
> At the same time we move the writing of the section to the end of the
> dump process. This allows the upcoming architecture section code to
> add data after all of the common dump data has been written.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  dump/dump.c           | 114 ++++++++++++++++++++++++++++++++----------
>  include/sysemu/dump.h |   4 ++
>  2 files changed, 92 insertions(+), 26 deletions(-)
>
> diff --git a/dump/dump.c b/dump/dump.c
> index 2d04e06815..980702d476 100644
> --- a/dump/dump.c
> +++ b/dump/dump.c
> @@ -342,30 +342,71 @@ static void write_elf_phdr_note(DumpState *s, Error **errp)
>      }
>  }
>
> -static void write_elf_section(DumpState *s, int type, Error **errp)
> +static size_t prepare_elf_section_hdr_zero(DumpState *s, void *buff)
>  {
> -    Elf32_Shdr shdr32;
> -    Elf64_Shdr shdr64;
> -    int shdr_size;
> -    void *shdr;
> -    int ret;
> +    if (dump_is_64bit(s)) {
> +        Elf64_Shdr *shdr64 = buff;
>
> -    if (type == 0) {
> -        shdr_size = sizeof(Elf32_Shdr);
> -        memset(&shdr32, 0, shdr_size);
> -        shdr32.sh_info = cpu_to_dump32(s, s->phdr_num);
> -        shdr = &shdr32;
> +        shdr64->sh_info = cpu_to_dump32(s, s->phdr_num);
>      } else {
> -        shdr_size = sizeof(Elf64_Shdr);
> -        memset(&shdr64, 0, shdr_size);
> -        shdr64.sh_info = cpu_to_dump32(s, s->phdr_num);
> -        shdr = &shdr64;
> +        Elf32_Shdr *shdr32 = buff;
> +
> +        shdr32->sh_info = cpu_to_dump32(s, s->phdr_num);
>      }
>
> -    ret = fd_write_vmcore(shdr, shdr_size, s);
> +    return dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr);
> +}
> +
> +static void prepare_elf_section_hdrs(DumpState *s)
> +{
> +    uint8_t *buff_hdr;
> +    size_t len, sizeof_shdr;
> +
> +    /*
> +     * Section ordering:
> +     * - HDR zero (if needed)
> +     */
> +    sizeof_shdr = dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr);
> +    len = sizeof_shdr * s->shdr_num;
> +    s->elf_section_hdrs = g_malloc0(len);
> +    buff_hdr = s->elf_section_hdrs;
> +
> +    /* Write special section first */
> +    if (s->phdr_num == PN_XNUM) {
> +            prepare_elf_section_hdr_zero(s, buff_hdr);

From v2 review:

Eventually, drop buff_hdr, and pass only "s" as argument

+ Indentation is off



> +    }
> +}
> +
> +static void prepare_elf_sections(DumpState *s, Error **errp)
> +{
> +    if (!s->shdr_num) {
> +        return;
> +    }
> +
> +    prepare_elf_section_hdrs(s);
> +}
> +
> +static void write_elf_section_headers(DumpState *s, Error **errp)
> +{
> +    size_t sizeof_shdr;
> +    int ret;
> +
> +    sizeof_shdr = dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr);
> +
> +    ret = fd_write_vmcore(s->elf_section_hdrs, s->shdr_num * sizeof_shdr, s);
>      if (ret < 0) {
> -        error_setg_errno(errp, -ret,
> -                         "dump: failed to write section header table");
> +        error_setg_errno(errp, -ret, "dump: failed to write section headers");
> +    }
> +}
> +
> +static void write_elf_sections(DumpState *s, Error **errp)
> +{
> +    int ret;
> +
> +    /* Write section zero */
> +    ret = fd_write_vmcore(s->elf_section_data, s->elf_section_data_size, s);
> +    if (ret < 0) {
> +        error_setg_errno(errp, -ret, "dump: failed to write section data");
>      }
>  }
>
> @@ -557,12 +598,22 @@ static void dump_begin(DumpState *s, Error **errp)
>      /* Write elf header to buffer */
>      prepare_elf_header(s);
>
> +    prepare_elf_sections(s, errp);
> +    if (*errp) {
> +        return;
> +    }
> +
>      /* Start to write stuff into file descriptor */
>      write_elf_header(s, errp);
>      if (*errp) {
>          return;
>      }
>
> +    write_elf_section_headers(s, errp);
> +    if (*errp) {
> +        return;
> +    }
> +
>      /* write PT_NOTE to vmcore */
>      write_elf_phdr_note(s, errp);
>      if (*errp) {
> @@ -575,14 +626,6 @@ static void dump_begin(DumpState *s, Error **errp)
>          return;
>      }
>
> -    /* write section to vmcore */
> -    if (s->shdr_num) {
> -        write_elf_section(s, 1, errp);
> -        if (*errp) {
> -            return;
> -        }
> -    }
> -
>      /* write notes to vmcore */
>      write_elf_notes(s, errp);
>  }
> @@ -610,6 +653,19 @@ static void dump_iterate(DumpState *s, Error **errp)
>      }
>  }
>
> +static void dump_end(DumpState *s, Error **errp)
> +{
> +    ERRP_GUARD();
> +
> +    if (!s->elf_section_data_size) {
> +        return;
> +    }
> +    s->elf_section_data = g_malloc0(s->elf_section_data_size);
> +
> +    /* write sections to vmcore */
> +    write_elf_sections(s, errp);
> +}
> +
>  static void create_vmcore(DumpState *s, Error **errp)
>  {
>      ERRP_GUARD();
> @@ -620,6 +676,12 @@ static void create_vmcore(DumpState *s, Error **errp)
>      }
>
>      dump_iterate(s, errp);
> +    if (*errp) {
> +        return;
> +    }
> +
> +    /* Write section data after memory has been dumped */
> +    dump_end(s, errp);
>  }
>
>  static int write_start_flat_header(int fd)
> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
> index 58f41bbf45..dad10dee0b 100644
> --- a/include/sysemu/dump.h
> +++ b/include/sysemu/dump.h
> @@ -172,6 +172,10 @@ typedef struct DumpState {
>      int64_t length;            /* Length of the dump we want to dump */
>
>      void *elf_header;
> +    void *elf_section_hdrs;
> +    uint64_t elf_section_data_size;
> +    void *elf_section_data;
> +
>      uint8_t *note_buf;          /* buffer for notes */
>      size_t note_buf_offset;     /* the writing place in note_buf */
>      uint32_t nr_cpus;           /* number of guest's cpu */
> --
> 2.34.1
>
Janosch Frank July 21, 2022, 2:59 p.m. UTC | #2
On 7/21/22 16:41, Marc-André Lureau wrote:
> On Thu, Jul 21, 2022 at 5:23 PM Janosch Frank <frankja@linux.ibm.com> wrote:
>>
>> By splitting the writing of the section headers and (future) section
>> data we prepare for the addition of a string table section and
>> architecture sections.
>>
>> At the same time we move the writing of the section to the end of the
>> dump process. This allows the upcoming architecture section code to
>> add data after all of the common dump data has been written.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>   dump/dump.c           | 114 ++++++++++++++++++++++++++++++++----------
>>   include/sysemu/dump.h |   4 ++
>>   2 files changed, 92 insertions(+), 26 deletions(-)
>>
>> diff --git a/dump/dump.c b/dump/dump.c
>> index 2d04e06815..980702d476 100644
>> --- a/dump/dump.c
>> +++ b/dump/dump.c
>> @@ -342,30 +342,71 @@ static void write_elf_phdr_note(DumpState *s, Error **errp)
>>       }
>>   }
>>
>> -static void write_elf_section(DumpState *s, int type, Error **errp)
>> +static size_t prepare_elf_section_hdr_zero(DumpState *s, void *buff)
>>   {
>> -    Elf32_Shdr shdr32;
>> -    Elf64_Shdr shdr64;
>> -    int shdr_size;
>> -    void *shdr;
>> -    int ret;
>> +    if (dump_is_64bit(s)) {
>> +        Elf64_Shdr *shdr64 = buff;
>>
>> -    if (type == 0) {
>> -        shdr_size = sizeof(Elf32_Shdr);
>> -        memset(&shdr32, 0, shdr_size);
>> -        shdr32.sh_info = cpu_to_dump32(s, s->phdr_num);
>> -        shdr = &shdr32;
>> +        shdr64->sh_info = cpu_to_dump32(s, s->phdr_num);
>>       } else {
>> -        shdr_size = sizeof(Elf64_Shdr);
>> -        memset(&shdr64, 0, shdr_size);
>> -        shdr64.sh_info = cpu_to_dump32(s, s->phdr_num);
>> -        shdr = &shdr64;
>> +        Elf32_Shdr *shdr32 = buff;
>> +
>> +        shdr32->sh_info = cpu_to_dump32(s, s->phdr_num);
>>       }
>>
>> -    ret = fd_write_vmcore(shdr, shdr_size, s);
>> +    return dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr);
>> +}
>> +
>> +static void prepare_elf_section_hdrs(DumpState *s)
>> +{
>> +    uint8_t *buff_hdr;
>> +    size_t len, sizeof_shdr;
>> +
>> +    /*
>> +     * Section ordering:
>> +     * - HDR zero (if needed)
>> +     */
>> +    sizeof_shdr = dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr);
>> +    len = sizeof_shdr * s->shdr_num;
>> +    s->elf_section_hdrs = g_malloc0(len);
>> +    buff_hdr = s->elf_section_hdrs;
>> +
>> +    /* Write special section first */
>> +    if (s->phdr_num == PN_XNUM) {
>> +            prepare_elf_section_hdr_zero(s, buff_hdr);
> 
>  From v2 review:
> 
> Eventually, drop buff_hdr, and pass only "s" as argument

You need to specify what you mean by "eventually" here.

> 
> + Indentation is off

I fixed that Tuesday but somehow it didn't end up in this mail...
I'll have another go at it.

> 
> 
> 
>> +    }
>> +}
>> +
>> +static void prepare_elf_sections(DumpState *s, Error **errp)
>> +{
>> +    if (!s->shdr_num) {
>> +        return;
>> +    }
>> +
>> +    prepare_elf_section_hdrs(s);
>> +}
>> +
>> +static void write_elf_section_headers(DumpState *s, Error **errp)
>> +{
>> +    size_t sizeof_shdr;
>> +    int ret;
>> +
>> +    sizeof_shdr = dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr);
>> +
>> +    ret = fd_write_vmcore(s->elf_section_hdrs, s->shdr_num * sizeof_shdr, s);
>>       if (ret < 0) {
>> -        error_setg_errno(errp, -ret,
>> -                         "dump: failed to write section header table");
>> +        error_setg_errno(errp, -ret, "dump: failed to write section headers");
>> +    }
>> +}
>> +
>> +static void write_elf_sections(DumpState *s, Error **errp)
>> +{
>> +    int ret;
>> +
>> +    /* Write section zero */
>> +    ret = fd_write_vmcore(s->elf_section_data, s->elf_section_data_size, s);
>> +    if (ret < 0) {
>> +        error_setg_errno(errp, -ret, "dump: failed to write section data");
>>       }
>>   }
>>
>> @@ -557,12 +598,22 @@ static void dump_begin(DumpState *s, Error **errp)
>>       /* Write elf header to buffer */
>>       prepare_elf_header(s);
>>
>> +    prepare_elf_sections(s, errp);
>> +    if (*errp) {
>> +        return;
>> +    }
>> +
>>       /* Start to write stuff into file descriptor */
>>       write_elf_header(s, errp);
>>       if (*errp) {
>>           return;
>>       }
>>
>> +    write_elf_section_headers(s, errp);
>> +    if (*errp) {
>> +        return;
>> +    }
>> +
>>       /* write PT_NOTE to vmcore */
>>       write_elf_phdr_note(s, errp);
>>       if (*errp) {
>> @@ -575,14 +626,6 @@ static void dump_begin(DumpState *s, Error **errp)
>>           return;
>>       }
>>
>> -    /* write section to vmcore */
>> -    if (s->shdr_num) {
>> -        write_elf_section(s, 1, errp);
>> -        if (*errp) {
>> -            return;
>> -        }
>> -    }
>> -
>>       /* write notes to vmcore */
>>       write_elf_notes(s, errp);
>>   }
>> @@ -610,6 +653,19 @@ static void dump_iterate(DumpState *s, Error **errp)
>>       }
>>   }
>>
>> +static void dump_end(DumpState *s, Error **errp)
>> +{
>> +    ERRP_GUARD();
>> +
>> +    if (!s->elf_section_data_size) {
>> +        return;
>> +    }
>> +    s->elf_section_data = g_malloc0(s->elf_section_data_size);
>> +
>> +    /* write sections to vmcore */
>> +    write_elf_sections(s, errp);
>> +}
>> +
>>   static void create_vmcore(DumpState *s, Error **errp)
>>   {
>>       ERRP_GUARD();
>> @@ -620,6 +676,12 @@ static void create_vmcore(DumpState *s, Error **errp)
>>       }
>>
>>       dump_iterate(s, errp);
>> +    if (*errp) {
>> +        return;
>> +    }
>> +
>> +    /* Write section data after memory has been dumped */
>> +    dump_end(s, errp);
>>   }
>>
>>   static int write_start_flat_header(int fd)
>> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
>> index 58f41bbf45..dad10dee0b 100644
>> --- a/include/sysemu/dump.h
>> +++ b/include/sysemu/dump.h
>> @@ -172,6 +172,10 @@ typedef struct DumpState {
>>       int64_t length;            /* Length of the dump we want to dump */
>>
>>       void *elf_header;
>> +    void *elf_section_hdrs;
>> +    uint64_t elf_section_data_size;
>> +    void *elf_section_data;
>> +
>>       uint8_t *note_buf;          /* buffer for notes */
>>       size_t note_buf_offset;     /* the writing place in note_buf */
>>       uint32_t nr_cpus;           /* number of guest's cpu */
>> --
>> 2.34.1
>>
>
diff mbox series

Patch

diff --git a/dump/dump.c b/dump/dump.c
index 2d04e06815..980702d476 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -342,30 +342,71 @@  static void write_elf_phdr_note(DumpState *s, Error **errp)
     }
 }
 
-static void write_elf_section(DumpState *s, int type, Error **errp)
+static size_t prepare_elf_section_hdr_zero(DumpState *s, void *buff)
 {
-    Elf32_Shdr shdr32;
-    Elf64_Shdr shdr64;
-    int shdr_size;
-    void *shdr;
-    int ret;
+    if (dump_is_64bit(s)) {
+        Elf64_Shdr *shdr64 = buff;
 
-    if (type == 0) {
-        shdr_size = sizeof(Elf32_Shdr);
-        memset(&shdr32, 0, shdr_size);
-        shdr32.sh_info = cpu_to_dump32(s, s->phdr_num);
-        shdr = &shdr32;
+        shdr64->sh_info = cpu_to_dump32(s, s->phdr_num);
     } else {
-        shdr_size = sizeof(Elf64_Shdr);
-        memset(&shdr64, 0, shdr_size);
-        shdr64.sh_info = cpu_to_dump32(s, s->phdr_num);
-        shdr = &shdr64;
+        Elf32_Shdr *shdr32 = buff;
+
+        shdr32->sh_info = cpu_to_dump32(s, s->phdr_num);
     }
 
-    ret = fd_write_vmcore(shdr, shdr_size, s);
+    return dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr);
+}
+
+static void prepare_elf_section_hdrs(DumpState *s)
+{
+    uint8_t *buff_hdr;
+    size_t len, sizeof_shdr;
+
+    /*
+     * Section ordering:
+     * - HDR zero (if needed)
+     */
+    sizeof_shdr = dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr);
+    len = sizeof_shdr * s->shdr_num;
+    s->elf_section_hdrs = g_malloc0(len);
+    buff_hdr = s->elf_section_hdrs;
+
+    /* Write special section first */
+    if (s->phdr_num == PN_XNUM) {
+            prepare_elf_section_hdr_zero(s, buff_hdr);
+    }
+}
+
+static void prepare_elf_sections(DumpState *s, Error **errp)
+{
+    if (!s->shdr_num) {
+        return;
+    }
+
+    prepare_elf_section_hdrs(s);
+}
+
+static void write_elf_section_headers(DumpState *s, Error **errp)
+{
+    size_t sizeof_shdr;
+    int ret;
+
+    sizeof_shdr = dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr);
+
+    ret = fd_write_vmcore(s->elf_section_hdrs, s->shdr_num * sizeof_shdr, s);
     if (ret < 0) {
-        error_setg_errno(errp, -ret,
-                         "dump: failed to write section header table");
+        error_setg_errno(errp, -ret, "dump: failed to write section headers");
+    }
+}
+
+static void write_elf_sections(DumpState *s, Error **errp)
+{
+    int ret;
+
+    /* Write section zero */
+    ret = fd_write_vmcore(s->elf_section_data, s->elf_section_data_size, s);
+    if (ret < 0) {
+        error_setg_errno(errp, -ret, "dump: failed to write section data");
     }
 }
 
@@ -557,12 +598,22 @@  static void dump_begin(DumpState *s, Error **errp)
     /* Write elf header to buffer */
     prepare_elf_header(s);
 
+    prepare_elf_sections(s, errp);
+    if (*errp) {
+        return;
+    }
+
     /* Start to write stuff into file descriptor */
     write_elf_header(s, errp);
     if (*errp) {
         return;
     }
 
+    write_elf_section_headers(s, errp);
+    if (*errp) {
+        return;
+    }
+
     /* write PT_NOTE to vmcore */
     write_elf_phdr_note(s, errp);
     if (*errp) {
@@ -575,14 +626,6 @@  static void dump_begin(DumpState *s, Error **errp)
         return;
     }
 
-    /* write section to vmcore */
-    if (s->shdr_num) {
-        write_elf_section(s, 1, errp);
-        if (*errp) {
-            return;
-        }
-    }
-
     /* write notes to vmcore */
     write_elf_notes(s, errp);
 }
@@ -610,6 +653,19 @@  static void dump_iterate(DumpState *s, Error **errp)
     }
 }
 
+static void dump_end(DumpState *s, Error **errp)
+{
+    ERRP_GUARD();
+
+    if (!s->elf_section_data_size) {
+        return;
+    }
+    s->elf_section_data = g_malloc0(s->elf_section_data_size);
+
+    /* write sections to vmcore */
+    write_elf_sections(s, errp);
+}
+
 static void create_vmcore(DumpState *s, Error **errp)
 {
     ERRP_GUARD();
@@ -620,6 +676,12 @@  static void create_vmcore(DumpState *s, Error **errp)
     }
 
     dump_iterate(s, errp);
+    if (*errp) {
+        return;
+    }
+
+    /* Write section data after memory has been dumped */
+    dump_end(s, errp);
 }
 
 static int write_start_flat_header(int fd)
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 58f41bbf45..dad10dee0b 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -172,6 +172,10 @@  typedef struct DumpState {
     int64_t length;            /* Length of the dump we want to dump */
 
     void *elf_header;
+    void *elf_section_hdrs;
+    uint64_t elf_section_data_size;
+    void *elf_section_data;
+
     uint8_t *note_buf;          /* buffer for notes */
     size_t note_buf_offset;     /* the writing place in note_buf */
     uint32_t nr_cpus;           /* number of guest's cpu */