Hi Stephen
On Tue, Sep 19, 2023 at 3:32 AM Stephen Brennan
<stephen.s.bren...@oracle.com> wrote:
>
> The flattened format (currently output by QEMU) 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 because 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]. However, QEMU has always used the
> flattened format. For compatibility it is best not to change the default
> output format without warning. So, add a flag to DumpState which changes
> the output to use the normal (i.e. raw) format. This flag will be added
> to the QMP and HMP commands in the next change.
>
> [1]:
> https://github.com/makedumpfile/makedumpfile/blob/f23bb943568188a2746dbf9b6692668f5a2ac3b6/makedumpfile.c#L5008-L5040
>
> Signed-off-by: Stephen Brennan <stephen.s.bren...@oracle.com>
> ---
> dump/dump.c | 32 +++++++++++++++++++++++++-------
> include/sysemu/dump.h | 1 +
> 2 files changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/dump/dump.c b/dump/dump.c
> index 74071a1565..10aa2c79e0 100644
> --- a/dump/dump.c
> +++ b/dump/dump.c
> @@ -814,6 +814,10 @@ static int write_start_flat_header(DumpState *s)
> MakedumpfileHeader *mh;
> int ret = 0;
>
> + if (s->kdump_raw) {
> + return 0;
> + }
> +
> QEMU_BUILD_BUG_ON(sizeof *mh > MAX_SIZE_MDF_HEADER);
> mh = g_malloc0(MAX_SIZE_MDF_HEADER);
>
> @@ -837,6 +841,10 @@ static int write_end_flat_header(DumpState *s)
> {
> MakedumpfileDataHeader mdh;
>
> + if (s->kdump_raw) {
> + return 0;
> + }
> +
> mdh.offset = END_FLAG_FLAT_HEADER;
> mdh.buf_size = END_FLAG_FLAT_HEADER;
>
> @@ -853,13 +861,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;
Any reason to use loff_t over off_t here? It fails to compile on win32
for ex. I can touch on PR commit otherwise.
>
> - mdh.offset = cpu_to_be64(offset);
> - mdh.buf_size = cpu_to_be64(size);
> + if (s->kdump_raw) {
> + 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);
> @@ -1775,7 +1791,8 @@ static void vmcoreinfo_update_phys_base(DumpState *s)
>
> static void dump_init(DumpState *s, int fd, bool has_format,
> DumpGuestMemoryFormat format, bool paging, bool
> has_filter,
> - int64_t begin, int64_t length, Error **errp)
> + int64_t begin, int64_t length, bool kdump_raw,
> + Error **errp)
> {
> ERRP_GUARD();
> VMCoreInfoState *vmci = vmcoreinfo_find();
> @@ -1786,6 +1803,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->kdump_raw = kdump_raw;
>
> /* kdump-compressed is conflict with paging and filter */
> if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
> @@ -2168,7 +2186,7 @@ void qmp_dump_guest_memory(bool paging, const char
> *file,
> dump_state_prepare(s);
>
> dump_init(s, fd, has_format, format, paging, has_begin,
> - begin, length, errp);
> + begin, length, false, errp);
> if (*errp) {
> qatomic_set(&s->status, DUMP_STATUS_FAILED);
> return;
> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
> index e27af8fb34..d702854853 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 kdump_raw;
> hwaddr memory_offset;
> int fd;
>
> --
> 2.39.3
>
--
Marc-André Lureau
