Hi

On Mon, Jul 17, 2023 at 8:45 PM Stephen Brennan <
stephen.s.bren...@oracle.com> wrote:

> The flattened format is used by makedumpfile only when it is outputting
> a vmcore to a file which is not seekable. The flattened format functions
> essentially as a set of instructions of the form "seek to the given
> offset, then write the given bytes out".
>
> The flattened format can be reconstructed using makedumpfile -R, or
> makedumpfile-R.pl, but it is a slow process beacuse it requires copying
> the entire vmcore. The flattened format can also be directly read by
> crash, but still, it requires a lengthy reassembly phase.
>
> To sum up, the flattened format is not an ideal one: it should only be
> used on files which are actually not seekable. This is the exact
> strategy which makedumpfile uses, as seen in the implementation of
> "write_buffer()" in makedumpfile [1].
>
> So, update the "dump-guest-memory" monitor command implementation so
> that it will directly do the seeks and writes, in the same strategy as
> makedumpfile. However, if the file is not seekable, then we can fall
> back to the flattened format.
>
> [1]:
> https://github.com/makedumpfile/makedumpfile/blob/f23bb943568188a2746dbf9b6692668f5a2ac3b6/makedumpfile.c#L5008-L5040
>
> Signed-off-by: Stephen Brennan <stephen.s.bren...@oracle.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
Tested-by: Marc-André Lureau <marcandre.lur...@redhat.com>

I am a bit reluctant to change the dump format by default. But since the
flatten format is more "complicated" than the "normal" format, perhaps we
can assume all users will handle it.

The change is probably late for 8.1 though..

---
>  dump/dump.c           | 30 +++++++++++++++++++++++++-----
>  include/sysemu/dump.h |  1 +
>  2 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/dump/dump.c b/dump/dump.c
> index 2708ddc135..384d275e39 100644
> --- a/dump/dump.c
> +++ b/dump/dump.c
> @@ -813,6 +813,13 @@ static int write_start_flat_header(DumpState *s)
>  {
>      MakedumpfileHeader *mh;
>      int ret = 0;
> +    loff_t offset = lseek(s->fd, 0, SEEK_CUR);
> +
> +    /* If the file is seekable, don't output flattened header */
> +    if (offset == 0) {
> +        s->seekable = true;
> +        return 0;
> +    }
>
>      QEMU_BUILD_BUG_ON(sizeof *mh > MAX_SIZE_MDF_HEADER);
>      mh = g_malloc0(MAX_SIZE_MDF_HEADER);
> @@ -837,6 +844,10 @@ static int write_end_flat_header(DumpState *s)
>  {
>      MakedumpfileDataHeader mdh;
>
> +    if (s->seekable) {
> +        return 0;
> +    }
> +
>      mdh.offset = END_FLAG_FLAT_HEADER;
>      mdh.buf_size = END_FLAG_FLAT_HEADER;
>
> @@ -853,13 +864,21 @@ static int write_buffer(DumpState *s, off_t offset,
> const void *buf, size_t size
>  {
>      size_t written_size;
>      MakedumpfileDataHeader mdh;
> +    loff_t seek_loc;
>
> -    mdh.offset = cpu_to_be64(offset);
> -    mdh.buf_size = cpu_to_be64(size);
> +    if (s->seekable) {
> +        seek_loc = lseek(s->fd, offset, SEEK_SET);
> +        if (seek_loc == (off_t) -1) {
> +            return -1;
> +        }
> +    } else {
> +        mdh.offset = cpu_to_be64(offset);
> +        mdh.buf_size = cpu_to_be64(size);
>
> -    written_size = qemu_write_full(s->fd, &mdh, sizeof(mdh));
> -    if (written_size != sizeof(mdh)) {
> -        return -1;
> +        written_size = qemu_write_full(s->fd, &mdh, sizeof(mdh));
> +        if (written_size != sizeof(mdh)) {
> +            return -1;
> +        }
>      }
>
>      written_size = qemu_write_full(s->fd, buf, size);
> @@ -1786,6 +1805,7 @@ static void dump_init(DumpState *s, int fd, bool
> has_format,
>      s->has_format = has_format;
>      s->format = format;
>      s->written_size = 0;
> +    s->seekable = false;
>
>      /* kdump-compressed is conflict with paging and filter */
>      if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
> index e27af8fb34..ab703c3a5e 100644
> --- a/include/sysemu/dump.h
> +++ b/include/sysemu/dump.h
> @@ -157,6 +157,7 @@ typedef struct DumpState {
>      MemoryMappingList list;
>      bool resume;
>      bool detached;
> +    bool seekable;
>      hwaddr memory_offset;
>      int fd;
>
> --
> 2.39.2
>
>
>

-- 
Marc-André Lureau

Reply via email to