[PATCH v2 qemu 0/3] Allow dump-guest-memory to output standard kdump format

[PATCH v2 qemu 0/3] Allow dump-guest-memory to output standard kdump format

[Stephen Brennan]

Hello all,

This is the second version of my patch series about the kdump format,
you can see the first version here [1].

The current output format for dump-guest-memory's kdump compressed
format is actually the "makedumpfile flattened" format. It was done
intentionally to allow the flexibility to write to non-seekable file
descriptors, like pipes [2], without using temporary files. Currently
libvirt uses this ability when VIR_DUMP_BYPASS_CACHE flag is set, to
avoid the dump process using page cache. The standard kdump output
format needs the page cache so that it can seek back and forth as part
of writing the dump file.

So the default kdump dump format cannot be changed to the standard
format. This patch series adds the ability to use the standard format,
and adds a QMP / HMP argument to enable it.

An open question for Daniel et al.:

Would it be possible to make flattened the default only for libvirt? I
totally agree that this would be a bad backward incompatible change
there. But for QMP / HMP commands, I think using the standard, broadly
compatible format as the default is important for user friendliness. If
a user needs to know the difference between flavors of kdump formats
like the flattened format, in order to set the correct option, then
we've already lost.

Changes from v1 to v2:
- Keep the default as the flattened format
- Add QMP / HMP arguments for "reassembled"

[1]: https://lore.kernel.org/qemu-devel/20230717163855.7383-1-stephen.s.brennan@oracle.com/
[2]: https://lore.kernel.org/qemu-devel/1390890126-17377-1-git-send-email-qiaonuohan@cn.fujitsu.com/
-> see "changes from v4 to v5" in this patch

Stephen Brennan (3):
  dump: Pass DumpState to write_ functions
  dump: Allow directly outputting reassembled kdumps
  dump: Add qmp argument "reassembled"

 dump/dump-hmp-cmds.c  |  8 +++-
 dump/dump.c           | 86 ++++++++++++++++++++++++++++++-------------
 hmp-commands.hx       |  7 +++-
 include/sysemu/dump.h |  3 +-
 qapi/dump.json        | 14 ++++++-
 5 files changed, 87 insertions(+), 31 deletions(-)

-- 
2.39.3

[PATCH v2 qemu 1/3] dump: Pass DumpState to write_ functions

[Stephen Brennan]

For the next patch, we need a reference to DumpState when writing data.

Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
---
 dump/dump.c           | 40 ++++++++++++++++++++--------------------
 include/sysemu/dump.h |  2 +-
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/dump/dump.c b/dump/dump.c
index d4ef713cd0..74071a1565 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -809,7 +809,7 @@ static void create_vmcore(DumpState *s, Error **errp)
     dump_end(s, errp);
 }
 
-static int write_start_flat_header(int fd)
+static int write_start_flat_header(DumpState *s)
 {
     MakedumpfileHeader *mh;
     int ret = 0;
@@ -824,7 +824,7 @@ static int write_start_flat_header(int fd)
     mh->version = cpu_to_be64(VERSION_FLAT_HEADER);
 
     size_t written_size;
-    written_size = qemu_write_full(fd, mh, MAX_SIZE_MDF_HEADER);
+    written_size = qemu_write_full(s->fd, mh, MAX_SIZE_MDF_HEADER);
     if (written_size != MAX_SIZE_MDF_HEADER) {
         ret = -1;
     }
@@ -833,7 +833,7 @@ static int write_start_flat_header(int fd)
     return ret;
 }
 
-static int write_end_flat_header(int fd)
+static int write_end_flat_header(DumpState *s)
 {
     MakedumpfileDataHeader mdh;
 
@@ -841,7 +841,7 @@ static int write_end_flat_header(int fd)
     mdh.buf_size = END_FLAG_FLAT_HEADER;
 
     size_t written_size;
-    written_size = qemu_write_full(fd, &mdh, sizeof(mdh));
+    written_size = qemu_write_full(s->fd, &mdh, sizeof(mdh));
     if (written_size != sizeof(mdh)) {
         return -1;
     }
@@ -849,7 +849,7 @@ static int write_end_flat_header(int fd)
     return 0;
 }
 
-static int write_buffer(int fd, off_t offset, const void *buf, size_t size)
+static int write_buffer(DumpState *s, off_t offset, const void *buf, size_t size)
 {
     size_t written_size;
     MakedumpfileDataHeader mdh;
@@ -857,12 +857,12 @@ static int write_buffer(int fd, off_t offset, const void *buf, size_t size)
     mdh.offset = cpu_to_be64(offset);
     mdh.buf_size = cpu_to_be64(size);
 
-    written_size = qemu_write_full(fd, &mdh, sizeof(mdh));
+    written_size = qemu_write_full(s->fd, &mdh, sizeof(mdh));
     if (written_size != sizeof(mdh)) {
         return -1;
     }
 
-    written_size = qemu_write_full(fd, buf, size);
+    written_size = qemu_write_full(s->fd, buf, size);
     if (written_size != size) {
         return -1;
     }
@@ -982,7 +982,7 @@ static void create_header32(DumpState *s, Error **errp)
 #endif
     dh->status = cpu_to_dump32(s, status);
 
-    if (write_buffer(s->fd, 0, dh, size) < 0) {
+    if (write_buffer(s, 0, dh, size) < 0) {
         error_setg(errp, "dump: failed to write disk dump header");
         goto out;
     }
@@ -1012,7 +1012,7 @@ static void create_header32(DumpState *s, Error **errp)
     kh->offset_note = cpu_to_dump64(s, offset_note);
     kh->note_size = cpu_to_dump32(s, s->note_size);
 
-    if (write_buffer(s->fd, DISKDUMP_HEADER_BLOCKS *
+    if (write_buffer(s, DISKDUMP_HEADER_BLOCKS *
                      block_size, kh, size) < 0) {
         error_setg(errp, "dump: failed to write kdump sub header");
         goto out;
@@ -1027,7 +1027,7 @@ static void create_header32(DumpState *s, Error **errp)
     if (*errp) {
         goto out;
     }
-    if (write_buffer(s->fd, offset_note, s->note_buf,
+    if (write_buffer(s, offset_note, s->note_buf,
                      s->note_size) < 0) {
         error_setg(errp, "dump: failed to write notes");
         goto out;
@@ -1093,7 +1093,7 @@ static void create_header64(DumpState *s, Error **errp)
 #endif
     dh->status = cpu_to_dump32(s, status);
 
-    if (write_buffer(s->fd, 0, dh, size) < 0) {
+    if (write_buffer(s, 0, dh, size) < 0) {
         error_setg(errp, "dump: failed to write disk dump header");
         goto out;
     }
@@ -1123,7 +1123,7 @@ static void create_header64(DumpState *s, Error **errp)
     kh->offset_note = cpu_to_dump64(s, offset_note);
     kh->note_size = cpu_to_dump64(s, s->note_size);
 
-    if (write_buffer(s->fd, DISKDUMP_HEADER_BLOCKS *
+    if (write_buffer(s, DISKDUMP_HEADER_BLOCKS *
                      block_size, kh, size) < 0) {
         error_setg(errp, "dump: failed to write kdump sub header");
         goto out;
@@ -1139,7 +1139,7 @@ static void create_header64(DumpState *s, Error **errp)
         goto out;
     }
 
-    if (write_buffer(s->fd, offset_note, s->note_buf,
+    if (write_buffer(s, offset_note, s->note_buf,
                      s->note_size) < 0) {
         error_setg(errp, "dump: failed to write notes");
         goto out;
@@ -1204,7 +1204,7 @@ static int set_dump_bitmap(uint64_t last_pfn, uint64_t pfn, bool value,
     while (old_offset < new_offset) {
         /* calculate the offset and write dump_bitmap */
         offset_bitmap1 = s->offset_dump_bitmap + old_offset;
-        if (write_buffer(s->fd, offset_bitmap1, buf,
+        if (write_buffer(s, offset_bitmap1, buf,
                          bitmap_bufsize) < 0) {
             return -1;
         }
@@ -1212,7 +1212,7 @@ static int set_dump_bitmap(uint64_t last_pfn, uint64_t pfn, bool value,
         /* dump level 1 is chosen, so 1st and 2nd bitmap are same */
         offset_bitmap2 = s->offset_dump_bitmap + s->len_dump_bitmap +
                          old_offset;
-        if (write_buffer(s->fd, offset_bitmap2, buf,
+        if (write_buffer(s, offset_bitmap2, buf,
                          bitmap_bufsize) < 0) {
             return -1;
         }
@@ -1380,7 +1380,7 @@ out:
 static void prepare_data_cache(DataCache *data_cache, DumpState *s,
                                off_t offset)
 {
-    data_cache->fd = s->fd;
+    data_cache->state = s;
     data_cache->data_size = 0;
     data_cache->buf_size = 4 * dump_bitmap_get_bufsize(s);
     data_cache->buf = g_malloc0(data_cache->buf_size);
@@ -1399,11 +1399,11 @@ static int write_cache(DataCache *dc, const void *buf, size_t size,
     /*
      * if flag_sync is set, synchronize data in dc->buf into vmcore.
      * otherwise check if the space is enough for caching data in buf, if not,
-     * write the data in dc->buf to dc->fd and reset dc->buf
+     * write the data in dc->buf to dc->state->fd and reset dc->buf
      */
     if ((!flag_sync && dc->data_size + size > dc->buf_size) ||
         (flag_sync && dc->data_size > 0)) {
-        if (write_buffer(dc->fd, dc->offset, dc->buf, dc->data_size) < 0) {
+        if (write_buffer(dc->state, dc->offset, dc->buf, dc->data_size) < 0) {
             return -1;
         }
 
@@ -1644,7 +1644,7 @@ static void create_kdump_vmcore(DumpState *s, Error **errp)
      *  +------------------------------------------+
      */
 
-    ret = write_start_flat_header(s->fd);
+    ret = write_start_flat_header(s);
     if (ret < 0) {
         error_setg(errp, "dump: failed to write start flat header");
         return;
@@ -1665,7 +1665,7 @@ static void create_kdump_vmcore(DumpState *s, Error **errp)
         return;
     }
 
-    ret = write_end_flat_header(s->fd);
+    ret = write_end_flat_header(s);
     if (ret < 0) {
         error_setg(errp, "dump: failed to write end flat header");
         return;
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 7008d43d04..e27af8fb34 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -137,7 +137,7 @@ typedef struct QEMU_PACKED KdumpSubHeader64 {
 } KdumpSubHeader64;
 
 typedef struct DataCache {
-    int fd;             /* fd of the file where to write the cached data */
+    DumpState *state;   /* dump state related to this data */
     uint8_t *buf;       /* buffer for cached data */
     size_t buf_size;    /* size of the buf */
     size_t data_size;   /* size of cached data in buf */
-- 
2.39.3

[PATCH v2 qemu 2/3] dump: Allow directly outputting reassembled kdumps

[Stephen Brennan]

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. reassembled) format. This flag will
be added to the QMP commands in the next change.

[1]: https://github.com/makedumpfile/makedumpfile/blob/f23bb943568188a2746dbf9b6692668f5a2ac3b6/makedumpfile.c#L5008-L5040

Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
---
 dump/dump.c           | 38 +++++++++++++++++++++++++++++++-------
 include/sysemu/dump.h |  1 +
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/dump/dump.c b/dump/dump.c
index 74071a1565..fb9040cfbc 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -814,6 +814,16 @@ static int write_start_flat_header(DumpState *s)
     MakedumpfileHeader *mh;
     int ret = 0;
 
+    /* The user can request we not use the flattened format, but
+     * if the file is not seekable, we need to fall back to flattened. */
+    if (s->kdump_reassembled) {
+        if (lseek(s->fd, 0, SEEK_CUR) == (loff_t) -1) {
+            s->kdump_reassembled = false;
+        } else {
+            return 0;
+        }
+    }
+
     QEMU_BUILD_BUG_ON(sizeof *mh > MAX_SIZE_MDF_HEADER);
     mh = g_malloc0(MAX_SIZE_MDF_HEADER);
 
@@ -837,6 +847,10 @@ static int write_end_flat_header(DumpState *s)
 {
     MakedumpfileDataHeader mdh;
 
+    if (s->kdump_reassembled) {
+        return 0;
+    }
+
     mdh.offset = END_FLAG_FLAT_HEADER;
     mdh.buf_size = END_FLAG_FLAT_HEADER;
 
@@ -853,13 +867,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->kdump_reassembled) {
+        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 +1797,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_reassembled,
+                      Error **errp)
 {
     ERRP_GUARD();
     VMCoreInfoState *vmci = vmcoreinfo_find();
@@ -1786,6 +1809,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_reassembled = kdump_reassembled;
 
     /* kdump-compressed is conflict with paging and filter */
     if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
@@ -2168,7 +2192,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..71ec492fce 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_reassembled;
     hwaddr memory_offset;
     int fd;
 
-- 
2.39.3

[PATCH v2 qemu 3/3] dump: Add qmp argument "reassembled"

[Stephen Brennan]

This can be used from QMP command line as "-R" to mirror the
corresponding flag for makedumpfile. This enables the kdump_reassembled
flag introduced in the previous patch.

Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
---
 dump/dump-hmp-cmds.c |  8 +++++++-
 dump/dump.c          | 12 +++++++++++-
 hmp-commands.hx      |  7 +++++--
 qapi/dump.json       | 14 +++++++++++++-
 4 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/dump/dump-hmp-cmds.c b/dump/dump-hmp-cmds.c
index b038785fee..1d882e4bd8 100644
--- a/dump/dump-hmp-cmds.c
+++ b/dump/dump-hmp-cmds.c
@@ -24,9 +24,11 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
     bool has_begin = qdict_haskey(qdict, "begin");
     bool has_length = qdict_haskey(qdict, "length");
     bool has_detach = qdict_haskey(qdict, "detach");
+    bool has_reassembled = qdict_haskey(qdict, "reassembled");
     int64_t begin = 0;
     int64_t length = 0;
     bool detach = false;
+    bool reassembled = false;
     enum DumpGuestMemoryFormat dump_format = DUMP_GUEST_MEMORY_FORMAT_ELF;
     char *prot;
 
@@ -61,11 +63,15 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
     if (has_detach) {
         detach = qdict_get_bool(qdict, "detach");
     }
+    if (has_reassembled) {
+        reassembled = qdict_get_bool(qdict, "reassembled");
+    }
 
     prot = g_strconcat("file:", file, NULL);
 
     qmp_dump_guest_memory(paging, prot, true, detach, has_begin, begin,
-                          has_length, length, true, dump_format, &err);
+                          has_length, length, true, has_reassembled,
+                          reassembled, dump_format, &err);
     hmp_handle_error(mon, err);
     g_free(prot);
 }
diff --git a/dump/dump.c b/dump/dump.c
index fb9040cfbc..42d4015fb3 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -2089,6 +2089,7 @@ void qmp_dump_guest_memory(bool paging, const char *file,
                            bool has_detach, bool detach,
                            bool has_begin, int64_t begin, bool has_length,
                            int64_t length, bool has_format,
+                           bool has_reassembled, bool reassembled,
                            DumpGuestMemoryFormat format, Error **errp)
 {
     ERRP_GUARD();
@@ -2119,6 +2120,12 @@ void qmp_dump_guest_memory(bool paging, const char *file,
                          "filter");
         return;
     }
+    if (has_reassembled && format != DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB
+                        && format != DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO
+                        && format != DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY) {
+        error_setg(errp, "'reassembled' only applies to kdump format");
+        return;
+    }
     if (has_begin && !has_length) {
         error_setg(errp, QERR_MISSING_PARAMETER, "length");
         return;
@@ -2130,6 +2137,9 @@ void qmp_dump_guest_memory(bool paging, const char *file,
     if (has_detach) {
         detach_p = detach;
     }
+    if (!has_reassembled) {
+        reassembled = false;
+    }
 
     /* check whether lzo/snappy is supported */
 #ifndef CONFIG_LZO
@@ -2192,7 +2202,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, false, errp);
+              begin, length, reassembled, errp);
     if (*errp) {
         qatomic_set(&s->status, DUMP_STATUS_FAILED);
         return;
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 2cbd0f77a0..c3062da470 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1085,14 +1085,15 @@ ERST
 
     {
         .name       = "dump-guest-memory",
-        .args_type  = "paging:-p,detach:-d,windmp:-w,zlib:-z,lzo:-l,snappy:-s,filename:F,begin:l?,length:l?",
-        .params     = "[-p] [-d] [-z|-l|-s|-w] filename [begin length]",
+        .args_type  = "paging:-p,detach:-d,windmp:-w,zlib:-z,lzo:-l,snappy:-s,reassembled:-R,filename:F,begin:l?,length:l?",
+        .params     = "[-p] [-d] [-z|-l|-s|-w] [-R] filename [begin length]",
         .help       = "dump guest memory into file 'filename'.\n\t\t\t"
                       "-p: do paging to get guest's memory mapping.\n\t\t\t"
                       "-d: return immediately (do not wait for completion).\n\t\t\t"
                       "-z: dump in kdump-compressed format, with zlib compression.\n\t\t\t"
                       "-l: dump in kdump-compressed format, with lzo compression.\n\t\t\t"
                       "-s: dump in kdump-compressed format, with snappy compression.\n\t\t\t"
+                      "-R: when using kdump (-z, -l, -s), try to avoid the flattened format.\n\t\t\t"
                       "-w: dump in Windows crashdump format (can be used instead of ELF-dump converting),\n\t\t\t"
                       "    for Windows x86 and x64 guests with vmcoreinfo driver only.\n\t\t\t"
                       "begin: the starting physical address.\n\t\t\t"
@@ -1115,6 +1116,8 @@ SRST
     dump in kdump-compressed format, with lzo compression.
   ``-s``
     dump in kdump-compressed format, with snappy compression.
+  ``-R``
+    when using kdump (-z, -l, -s), try to avoid the flattened format.
   ``-w``
     dump in Windows crashdump format (can be used instead of ELF-dump converting),
     for Windows x64 guests with vmcoreinfo driver only
diff --git a/qapi/dump.json b/qapi/dump.json
index 4ae1f722a9..9cc7c3ea93 100644
--- a/qapi/dump.json
+++ b/qapi/dump.json
@@ -69,6 +69,18 @@
 #     to dump all guest's memory, please specify the start @begin and
 #     @length
 #
+# @reassembled: if false (the default), the kdump output formats will use the
+#     "makedumpfile flattened" variant of the format, which is less broadly
+#     compatible with analysis tools. The flattened dump can be reassembled
+#     after the fact using the command "makedumpfile -R". If true, Qemu
+#     attempts to generate the standard kdump format. This requires a
+#     seekable file as output -- if the output file is not seekable, then
+#     the flattened format is still generated. The standard format is more
+#     broadly compatible with debugging tools, but generating it requires a
+#     seekable output file descriptor, and could use more system memory due
+#     to page cache utilization. This should be left unspecified for non-kdump
+#     output formats.
+#
 # @format: if specified, the format of guest memory dump.  But non-elf
 #     format is conflict with paging and filter, ie.  @paging, @begin
 #     and @length is not allowed to be specified with non-elf @format
@@ -89,7 +101,7 @@
 { 'command': 'dump-guest-memory',
   'data': { 'paging': 'bool', 'protocol': 'str', '*detach': 'bool',
             '*begin': 'int', '*length': 'int',
-            '*format': 'DumpGuestMemoryFormat'} }
+            '*reassembled': 'bool', '*format': 'DumpGuestMemoryFormat'} }
 
 ##
 # @DumpStatus:
-- 
2.39.3

Re: [PATCH v2 qemu 2/3] dump: Allow directly outputting reassembled kdumps

[Marc-André Lureau]

Hi

On Thu, Sep 14, 2023 at 5:04 AM Stephen Brennan
<stephen.s.brennan@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. reassembled) format. This flag will
> be added to the QMP commands in the next change.
>
> [1]: https://github.com/makedumpfile/makedumpfile/blob/f23bb943568188a2746dbf9b6692668f5a2ac3b6/makedumpfile.c#L5008-L5040
>
> Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
> ---
>  dump/dump.c           | 38 +++++++++++++++++++++++++++++++-------
>  include/sysemu/dump.h |  1 +
>  2 files changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/dump/dump.c b/dump/dump.c
> index 74071a1565..fb9040cfbc 100644
> --- a/dump/dump.c
> +++ b/dump/dump.c
> @@ -814,6 +814,16 @@ static int write_start_flat_header(DumpState *s)
>      MakedumpfileHeader *mh;
>      int ret = 0;
>
> +    /* The user can request we not use the flattened format, but
> +     * if the file is not seekable, we need to fall back to flattened. */
> +    if (s->kdump_reassembled) {
> +        if (lseek(s->fd, 0, SEEK_CUR) == (loff_t) -1) {
> +            s->kdump_reassembled = false;

Since it was explicitly asked by the user, we better return an error
instead of falling back silently on flattened.


> +        } else {
> +            return 0;
> +        }
> +    }
> +
>      QEMU_BUILD_BUG_ON(sizeof *mh > MAX_SIZE_MDF_HEADER);
>      mh = g_malloc0(MAX_SIZE_MDF_HEADER);
>
> @@ -837,6 +847,10 @@ static int write_end_flat_header(DumpState *s)
>  {
>      MakedumpfileDataHeader mdh;
>
> +    if (s->kdump_reassembled) {
> +        return 0;
> +    }
> +
>      mdh.offset = END_FLAG_FLAT_HEADER;
>      mdh.buf_size = END_FLAG_FLAT_HEADER;
>
> @@ -853,13 +867,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->kdump_reassembled) {
> +        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 +1797,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_reassembled,
> +                      Error **errp)
>  {
>      ERRP_GUARD();
>      VMCoreInfoState *vmci = vmcoreinfo_find();
> @@ -1786,6 +1809,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_reassembled = kdump_reassembled;
>
>      /* kdump-compressed is conflict with paging and filter */
>      if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
> @@ -2168,7 +2192,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..71ec492fce 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_reassembled;
>      hwaddr memory_offset;
>      int fd;
>
> --
> 2.39.3
>


--
Marc-André Lureau

Re: [PATCH v2 qemu 3/3] dump: Add qmp argument "reassembled"

[Marc-André Lureau]

Hi

On Thu, Sep 14, 2023 at 5:03 AM Stephen Brennan
<stephen.s.brennan@oracle.com> wrote:
>
> This can be used from QMP command line as "-R" to mirror the
> corresponding flag for makedumpfile. This enables the kdump_reassembled
> flag introduced in the previous patch.
>
> Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
> ---
>  dump/dump-hmp-cmds.c |  8 +++++++-
>  dump/dump.c          | 12 +++++++++++-
>  hmp-commands.hx      |  7 +++++--
>  qapi/dump.json       | 14 +++++++++++++-
>  4 files changed, 36 insertions(+), 5 deletions(-)
>
> diff --git a/dump/dump-hmp-cmds.c b/dump/dump-hmp-cmds.c
> index b038785fee..1d882e4bd8 100644
> --- a/dump/dump-hmp-cmds.c
> +++ b/dump/dump-hmp-cmds.c
> @@ -24,9 +24,11 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
>      bool has_begin = qdict_haskey(qdict, "begin");
>      bool has_length = qdict_haskey(qdict, "length");
>      bool has_detach = qdict_haskey(qdict, "detach");
> +    bool has_reassembled = qdict_haskey(qdict, "reassembled");
>      int64_t begin = 0;
>      int64_t length = 0;
>      bool detach = false;
> +    bool reassembled = false;
>      enum DumpGuestMemoryFormat dump_format = DUMP_GUEST_MEMORY_FORMAT_ELF;
>      char *prot;
>
> @@ -61,11 +63,15 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
>      if (has_detach) {
>          detach = qdict_get_bool(qdict, "detach");
>      }
> +    if (has_reassembled) {
> +        reassembled = qdict_get_bool(qdict, "reassembled");
> +    }
>
>      prot = g_strconcat("file:", file, NULL);
>
>      qmp_dump_guest_memory(paging, prot, true, detach, has_begin, begin,
> -                          has_length, length, true, dump_format, &err);
> +                          has_length, length, true, has_reassembled,
> +                          reassembled, dump_format, &err);
>      hmp_handle_error(mon, err);
>      g_free(prot);
>  }
> diff --git a/dump/dump.c b/dump/dump.c
> index fb9040cfbc..42d4015fb3 100644
> --- a/dump/dump.c
> +++ b/dump/dump.c
> @@ -2089,6 +2089,7 @@ void qmp_dump_guest_memory(bool paging, const char *file,
>                             bool has_detach, bool detach,
>                             bool has_begin, int64_t begin, bool has_length,
>                             int64_t length, bool has_format,
> +                           bool has_reassembled, bool reassembled,
>                             DumpGuestMemoryFormat format, Error **errp)
>  {
>      ERRP_GUARD();
> @@ -2119,6 +2120,12 @@ void qmp_dump_guest_memory(bool paging, const char *file,
>                           "filter");
>          return;
>      }
> +    if (has_reassembled && format != DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB
> +                        && format != DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO
> +                        && format != DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY) {
> +        error_setg(errp, "'reassembled' only applies to kdump format");
> +        return;
> +    }
>      if (has_begin && !has_length) {
>          error_setg(errp, QERR_MISSING_PARAMETER, "length");
>          return;
> @@ -2130,6 +2137,9 @@ void qmp_dump_guest_memory(bool paging, const char *file,
>      if (has_detach) {
>          detach_p = detach;
>      }
> +    if (!has_reassembled) {
> +        reassembled = false;
> +    }
>
>      /* check whether lzo/snappy is supported */
>  #ifndef CONFIG_LZO
> @@ -2192,7 +2202,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, false, errp);
> +              begin, length, reassembled, errp);
>      if (*errp) {
>          qatomic_set(&s->status, DUMP_STATUS_FAILED);
>          return;
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 2cbd0f77a0..c3062da470 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1085,14 +1085,15 @@ ERST
>
>      {
>          .name       = "dump-guest-memory",
> -        .args_type  = "paging:-p,detach:-d,windmp:-w,zlib:-z,lzo:-l,snappy:-s,filename:F,begin:l?,length:l?",
> -        .params     = "[-p] [-d] [-z|-l|-s|-w] filename [begin length]",
> +        .args_type  = "paging:-p,detach:-d,windmp:-w,zlib:-z,lzo:-l,snappy:-s,reassembled:-R,filename:F,begin:l?,length:l?",
> +        .params     = "[-p] [-d] [-z|-l|-s|-w] [-R] filename [begin length]",
>          .help       = "dump guest memory into file 'filename'.\n\t\t\t"
>                        "-p: do paging to get guest's memory mapping.\n\t\t\t"
>                        "-d: return immediately (do not wait for completion).\n\t\t\t"
>                        "-z: dump in kdump-compressed format, with zlib compression.\n\t\t\t"
>                        "-l: dump in kdump-compressed format, with lzo compression.\n\t\t\t"
>                        "-s: dump in kdump-compressed format, with snappy compression.\n\t\t\t"
> +                      "-R: when using kdump (-z, -l, -s), try to avoid the flattened format.\n\t\t\t"
>                        "-w: dump in Windows crashdump format (can be used instead of ELF-dump converting),\n\t\t\t"
>                        "    for Windows x86 and x64 guests with vmcoreinfo driver only.\n\t\t\t"
>                        "begin: the starting physical address.\n\t\t\t"
> @@ -1115,6 +1116,8 @@ SRST
>      dump in kdump-compressed format, with lzo compression.
>    ``-s``
>      dump in kdump-compressed format, with snappy compression.
> +  ``-R``
> +    when using kdump (-z, -l, -s), try to avoid the flattened format.
>    ``-w``
>      dump in Windows crashdump format (can be used instead of ELF-dump converting),
>      for Windows x64 guests with vmcoreinfo driver only
> diff --git a/qapi/dump.json b/qapi/dump.json
> index 4ae1f722a9..9cc7c3ea93 100644
> --- a/qapi/dump.json
> +++ b/qapi/dump.json
> @@ -69,6 +69,18 @@
>  #     to dump all guest's memory, please specify the start @begin and
>  #     @length
>  #
> +# @reassembled: if false (the default), the kdump output formats will use the
> +#     "makedumpfile flattened" variant of the format, which is less broadly
> +#     compatible with analysis tools. The flattened dump can be reassembled
> +#     after the fact using the command "makedumpfile -R". If true, Qemu

QEMU

> +#     attempts to generate the standard kdump format. This requires a
> +#     seekable file as output -- if the output file is not seekable, then
> +#     the flattened format is still generated. The standard format is more

Will have to be adjusted to return an error if we drop the fallback behaviour.

> +#     broadly compatible with debugging tools, but generating it requires a
> +#     seekable output file descriptor, and could use more system memory due
> +#     to page cache utilization. This should be left unspecified for non-kdump
> +#     output formats.
> +#
>  # @format: if specified, the format of guest memory dump.  But non-elf
>  #     format is conflict with paging and filter, ie.  @paging, @begin
>  #     and @length is not allowed to be specified with non-elf @format
> @@ -89,7 +101,7 @@
>  { 'command': 'dump-guest-memory',
>    'data': { 'paging': 'bool', 'protocol': 'str', '*detach': 'bool',
>              '*begin': 'int', '*length': 'int',
> -            '*format': 'DumpGuestMemoryFormat'} }
> +            '*reassembled': 'bool', '*format': 'DumpGuestMemoryFormat'} }
>
>  ##
>  # @DumpStatus:
> --
> 2.39.3
>


-- 
Marc-André Lureau

Re: [PATCH v2 qemu 3/3] dump: Add qmp argument "reassembled"

[Daniel P. Berrangé]

On Wed, Sep 13, 2023 at 06:03:15PM -0700, Stephen Brennan wrote:
> This can be used from QMP command line as "-R" to mirror the
> corresponding flag for makedumpfile. This enables the kdump_reassembled
> flag introduced in the previous patch.
> 
> Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
> ---
>  dump/dump-hmp-cmds.c |  8 +++++++-
>  dump/dump.c          | 12 +++++++++++-
>  hmp-commands.hx      |  7 +++++--
>  qapi/dump.json       | 14 +++++++++++++-
>  4 files changed, 36 insertions(+), 5 deletions(-)

> diff --git a/qapi/dump.json b/qapi/dump.json
> index 4ae1f722a9..9cc7c3ea93 100644
> --- a/qapi/dump.json
> +++ b/qapi/dump.json
> @@ -69,6 +69,18 @@
>  #     to dump all guest's memory, please specify the start @begin and
>  #     @length
>  #
> +# @reassembled: if false (the default), the kdump output formats will use the
> +#     "makedumpfile flattened" variant of the format, which is less broadly
> +#     compatible with analysis tools. The flattened dump can be reassembled
> +#     after the fact using the command "makedumpfile -R". If true, Qemu
> +#     attempts to generate the standard kdump format. This requires a
> +#     seekable file as output -- if the output file is not seekable, then
> +#     the flattened format is still generated. The standard format is more
> +#     broadly compatible with debugging tools, but generating it requires a
> +#     seekable output file descriptor, and could use more system memory due
> +#     to page cache utilization. This should be left unspecified for non-kdump
> +#     output formats.
> +#
>  # @format: if specified, the format of guest memory dump.  But non-elf
>  #     format is conflict with paging and filter, ie.  @paging, @begin
>  #     and @length is not allowed to be specified with non-elf @format
> @@ -89,7 +101,7 @@
>  { 'command': 'dump-guest-memory',
>    'data': { 'paging': 'bool', 'protocol': 'str', '*detach': 'bool',
>              '*begin': 'int', '*length': 'int',
> -            '*format': 'DumpGuestMemoryFormat'} }
> +            '*reassembled': 'bool', '*format': 'DumpGuestMemoryFormat'} }

The 'reassembled' flag is changing the meaning of 3 out of the 5
'format' enum values. IMHO, we should just be adding new formats
instead of changing the meaning of existing formats. It is a shame
we have the current 'kdump' naming prefix, but we're stuck with
that for backwards compat, we need a new prefix. I'd suggest
'kdump-raw'. eg

#
# @DumpGuestMemoryFormat:
#
# An enumeration of guest-memory-dump's format.
#
# @elf: elf format
#
# @kdump-zlib: makedumpfile flattened, kdump-compressed format with zlib-compressed
#
# @kdump-lzo: makedumpfile flattened, kdump-compressed format with lzo-compressed
#
# @kdump-snappy: makedumpfile flattened, kdump-compressed format with snappy-compressed
#
# @kdump-raw-zlib: raw assembled kdump-compressed format with zlib-compressed (since 8.2)
#
# @kdump-raw-lzo: raw assembled kdump-compressed format with lzo-compressed (since 8.2)
#
# @kdump-raw-snappy: raw assembled kdump-compressed format with snappy-compressed (since 8.2)
#
# @win-dmp: Windows full crashdump format, can be used instead of ELF
#     converting (since 2.13)
#
# Since: 2.0
##
{ 'enum': 'DumpGuestMemoryFormat',
  'data': [ 'elf',
            'kdump-zlib', 'kdump-lzo', 'kdump-snappy',
            'kdump-raw-zlib', 'kdump-raw-lzo', 'kdump-raw-snappy',
            'win-dmp' ] }


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 qemu 0/3] Allow dump-guest-memory to output standard kdump format

[Daniel P. Berrangé]

On Wed, Sep 13, 2023 at 06:03:12PM -0700, Stephen Brennan wrote:
> Hello all,
> 
> This is the second version of my patch series about the kdump format,
> you can see the first version here [1].
> 
> The current output format for dump-guest-memory's kdump compressed
> format is actually the "makedumpfile flattened" format. It was done
> intentionally to allow the flexibility to write to non-seekable file
> descriptors, like pipes [2], without using temporary files. Currently
> libvirt uses this ability when VIR_DUMP_BYPASS_CACHE flag is set, to
> avoid the dump process using page cache. The standard kdump output
> format needs the page cache so that it can seek back and forth as part
> of writing the dump file.
> 
> So the default kdump dump format cannot be changed to the standard
> format. This patch series adds the ability to use the standard format,
> and adds a QMP / HMP argument to enable it.
> 
> An open question for Daniel et al.:
> 
> Would it be possible to make flattened the default only for libvirt? I
> totally agree that this would be a bad backward incompatible change
> there. But for QMP / HMP commands, I think using the standard, broadly
> compatible format as the default is important for user friendliness. If
> a user needs to know the difference between flavors of kdump formats
> like the flattened format, in order to set the correct option, then
> we've already lost.

The default is 'elf' - any use of kdump formats is already an opt-in,
and with the new kdump variants represented as enums, the user can
just specify which they want explicitly.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 qemu 3/3] dump: Add qmp argument "reassembled"

[Stephen Brennan]

Daniel P. Berrangé <berrange@redhat.com> writes:
> On Wed, Sep 13, 2023 at 06:03:15PM -0700, Stephen Brennan wrote:
>> This can be used from QMP command line as "-R" to mirror the
>> corresponding flag for makedumpfile. This enables the kdump_reassembled
>> flag introduced in the previous patch.
>> 
>> Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
>> ---
>>  dump/dump-hmp-cmds.c |  8 +++++++-
>>  dump/dump.c          | 12 +++++++++++-
>>  hmp-commands.hx      |  7 +++++--
>>  qapi/dump.json       | 14 +++++++++++++-
>>  4 files changed, 36 insertions(+), 5 deletions(-)
>
>> diff --git a/qapi/dump.json b/qapi/dump.json
>> index 4ae1f722a9..9cc7c3ea93 100644
>> --- a/qapi/dump.json
>> +++ b/qapi/dump.json
>> @@ -69,6 +69,18 @@
>>  #     to dump all guest's memory, please specify the start @begin and
>>  #     @length
>>  #
>> +# @reassembled: if false (the default), the kdump output formats will use the
>> +#     "makedumpfile flattened" variant of the format, which is less broadly
>> +#     compatible with analysis tools. The flattened dump can be reassembled
>> +#     after the fact using the command "makedumpfile -R". If true, Qemu
>> +#     attempts to generate the standard kdump format. This requires a
>> +#     seekable file as output -- if the output file is not seekable, then
>> +#     the flattened format is still generated. The standard format is more
>> +#     broadly compatible with debugging tools, but generating it requires a
>> +#     seekable output file descriptor, and could use more system memory due
>> +#     to page cache utilization. This should be left unspecified for non-kdump
>> +#     output formats.
>> +#
>>  # @format: if specified, the format of guest memory dump.  But non-elf
>>  #     format is conflict with paging and filter, ie.  @paging, @begin
>>  #     and @length is not allowed to be specified with non-elf @format
>> @@ -89,7 +101,7 @@
>>  { 'command': 'dump-guest-memory',
>>    'data': { 'paging': 'bool', 'protocol': 'str', '*detach': 'bool',
>>              '*begin': 'int', '*length': 'int',
>> -            '*format': 'DumpGuestMemoryFormat'} }
>> +            '*reassembled': 'bool', '*format': 'DumpGuestMemoryFormat'} }
>
> The 'reassembled' flag is changing the meaning of 3 out of the 5
> 'format' enum values. IMHO, we should just be adding new formats
> instead of changing the meaning of existing formats. It is a shame
> we have the current 'kdump' naming prefix, but we're stuck with
> that for backwards compat, we need a new prefix. I'd suggest
> 'kdump-raw'. eg
>
> #
> # @DumpGuestMemoryFormat:
> #
> # An enumeration of guest-memory-dump's format.
> #
> # @elf: elf format
> #
> # @kdump-zlib: makedumpfile flattened, kdump-compressed format with zlib-compressed
> #
> # @kdump-lzo: makedumpfile flattened, kdump-compressed format with lzo-compressed
> #
> # @kdump-snappy: makedumpfile flattened, kdump-compressed format with snappy-compressed
> #
> # @kdump-raw-zlib: raw assembled kdump-compressed format with zlib-compressed (since 8.2)
> #
> # @kdump-raw-lzo: raw assembled kdump-compressed format with lzo-compressed (since 8.2)
> #
> # @kdump-raw-snappy: raw assembled kdump-compressed format with snappy-compressed (since 8.2)
> #
> # @win-dmp: Windows full crashdump format, can be used instead of ELF
> #     converting (since 2.13)
> #
> # Since: 2.0
> ##
> { 'enum': 'DumpGuestMemoryFormat',
>   'data': [ 'elf',
>             'kdump-zlib', 'kdump-lzo', 'kdump-snappy',
>             'kdump-raw-zlib', 'kdump-raw-lzo', 'kdump-raw-snappy',
>             'win-dmp' ] }

Hi Daniel,

Sure, I'll go ahead and use this approach instead. One question: I see
that this generates the enumeration DumpGuestMemoryFormat in
qapi-types-dump.h. I just wanted to double-check if there's any ABI
considerations for the numbering of this enum? Inserting kdump-raw-* at
this point would result in 'win-dmp' getting a different numbering, and
it seems possible that the API/ABI which libvirt uses might depend on
the enumeration values not changing. E.G. if libvirt is built against
one version of Qemu and then used with a different one.

Thanks,
Stephen

> With regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 qemu 3/3] dump: Add qmp argument "reassembled"

[Daniel P. Berrangé]

On Mon, Sep 18, 2023 at 10:34:30AM -0700, Stephen Brennan wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> > #
> > # @DumpGuestMemoryFormat:
> > #
> > # An enumeration of guest-memory-dump's format.
> > #
> > # @elf: elf format
> > #
> > # @kdump-zlib: makedumpfile flattened, kdump-compressed format with zlib-compressed
> > #
> > # @kdump-lzo: makedumpfile flattened, kdump-compressed format with lzo-compressed
> > #
> > # @kdump-snappy: makedumpfile flattened, kdump-compressed format with snappy-compressed
> > #
> > # @kdump-raw-zlib: raw assembled kdump-compressed format with zlib-compressed (since 8.2)
> > #
> > # @kdump-raw-lzo: raw assembled kdump-compressed format with lzo-compressed (since 8.2)
> > #
> > # @kdump-raw-snappy: raw assembled kdump-compressed format with snappy-compressed (since 8.2)
> > #
> > # @win-dmp: Windows full crashdump format, can be used instead of ELF
> > #     converting (since 2.13)
> > #
> > # Since: 2.0
> > ##
> > { 'enum': 'DumpGuestMemoryFormat',
> >   'data': [ 'elf',
> >             'kdump-zlib', 'kdump-lzo', 'kdump-snappy',
> >             'kdump-raw-zlib', 'kdump-raw-lzo', 'kdump-raw-snappy',
> >             'win-dmp' ] }
> 
> Hi Daniel,
> 
> Sure, I'll go ahead and use this approach instead. One question: I see
> that this generates the enumeration DumpGuestMemoryFormat in
> qapi-types-dump.h. I just wanted to double-check if there's any ABI
> considerations for the numbering of this enum? Inserting kdump-raw-* at
> this point would result in 'win-dmp' getting a different numbering, and
> it seems possible that the API/ABI which libvirt uses might depend on
> the enumeration values not changing. E.G. if libvirt is built against
> one version of Qemu and then used with a different one.

The QAPI integer representation of enums is a private internal impl
detail known only to QEMU.

In terms of QMP, the on the wire representation is exclusively string
format, so safe wrt re-ordering for new/old QEMU and new/old libvirt.


In livirt's own public API, if we chose to expose these new formats,
then we have to strictly append after the existing enums constants
in libvirt's header file.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|