From 4d7dd4ed4f42e659a214e4f81c1a15ee991352df Mon Sep 17 00:00:00 2001 From: Stephen Brennan Date: Mon, 18 Sep 2023 16:32:31 -0700 Subject: [PATCH 1/8] dump: Pass DumpState to write_ functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For the next patch, we need a reference to DumpState when writing data. Signed-off-by: Stephen Brennan Reviewed-by: Daniel P. Berrangé Reviewed-by: Marc-André Lureau Message-Id: <20230918233233.1431858-2-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 d355ada62e..eec34c4738 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 */ From d43a01db285fd10f9c429476eb9c63fa5e00f3cc Mon Sep 17 00:00:00 2001 From: Stephen Brennan Date: Mon, 18 Sep 2023 16:32:32 -0700 Subject: [PATCH 2/8] dump: Allow directly outputting raw kdump format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Daniel P. Berrangé [ Marc-André: replace loff_t with off_t ] Reviewed-by: Marc-André Lureau Message-Id: <20230918233233.1431858-3-stephen.s.brennan@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 eec34c4738..0f913e1f5c 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; + off_t seek_loc; - 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; From e6549197f7edf2c590894f51aaed2fa81991becc Mon Sep 17 00:00:00 2001 From: Stephen Brennan Date: Mon, 18 Sep 2023 16:32:33 -0700 Subject: [PATCH 3/8] dump: Add command interface for kdump-raw formats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The QMP dump API represents the dump format as an enumeration. Add three new enumerators, one for each supported kdump compression, each named "kdump-raw-*". For the HMP command line, rather than adding a new flag corresponding to each format, it seems more human-friendly to add a single flag "-R" to switch the kdump formats to "raw" mode. The choice of "-R" also correlates nicely to the "makedumpfile -R" option, which would serve to reassemble a flattened vmcore. Signed-off-by: Stephen Brennan Reviewed-by: Daniel P. Berrangé [ Marc-André: replace loff_t with off_t, indent fixes ] Reviewed-by: Marc-André Lureau Message-Id: <20230918233233.1431858-4-stephen.s.brennan@oracle.com> --- dump/dump-hmp-cmds.c | 21 +++++++++++++++++---- dump/dump.c | 33 ++++++++++++++++++++++++++++++++- hmp-commands.hx | 9 +++++++-- qapi/dump.json | 24 ++++++++++++++++++++---- 4 files changed, 76 insertions(+), 11 deletions(-) diff --git a/dump/dump-hmp-cmds.c b/dump/dump-hmp-cmds.c index b038785fee..b428ec33df 100644 --- a/dump/dump-hmp-cmds.c +++ b/dump/dump-hmp-cmds.c @@ -19,6 +19,7 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict) bool paging = qdict_get_try_bool(qdict, "paging", false); bool zlib = qdict_get_try_bool(qdict, "zlib", false); bool lzo = qdict_get_try_bool(qdict, "lzo", false); + bool raw = qdict_get_try_bool(qdict, "raw", false); bool snappy = qdict_get_try_bool(qdict, "snappy", false); const char *file = qdict_get_str(qdict, "filename"); bool has_begin = qdict_haskey(qdict, "begin"); @@ -40,16 +41,28 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict) dump_format = DUMP_GUEST_MEMORY_FORMAT_WIN_DMP; } - if (zlib) { - dump_format = DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB; + if (zlib && raw) { + if (raw) { + dump_format = DUMP_GUEST_MEMORY_FORMAT_KDUMP_RAW_ZLIB; + } else { + dump_format = DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB; + } } if (lzo) { - dump_format = DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO; + if (raw) { + dump_format = DUMP_GUEST_MEMORY_FORMAT_KDUMP_RAW_LZO; + } else { + dump_format = DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO; + } } if (snappy) { - dump_format = DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY; + if (raw) { + dump_format = DUMP_GUEST_MEMORY_FORMAT_KDUMP_RAW_SNAPPY; + } else { + dump_format = DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY; + } } if (has_begin) { diff --git a/dump/dump.c b/dump/dump.c index 0f913e1f5c..44b27fef45 100644 --- a/dump/dump.c +++ b/dump/dump.c @@ -2090,6 +2090,7 @@ void qmp_dump_guest_memory(bool paging, const char *file, int fd = -1; DumpState *s; bool detach_p = false; + bool kdump_raw = false; if (runstate_check(RUN_STATE_INMIGRATE)) { error_setg(errp, "Dump not allowed during incoming migration."); @@ -2103,6 +2104,29 @@ void qmp_dump_guest_memory(bool paging, const char *file, return; } + /* + * externally, we represent kdump-raw-* as separate formats, but internally + * they are handled the same, except for the "raw" flag + */ + if (has_format) { + switch (format) { + case DUMP_GUEST_MEMORY_FORMAT_KDUMP_RAW_ZLIB: + format = DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB; + kdump_raw = true; + break; + case DUMP_GUEST_MEMORY_FORMAT_KDUMP_RAW_LZO: + format = DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO; + kdump_raw = true; + break; + case DUMP_GUEST_MEMORY_FORMAT_KDUMP_RAW_SNAPPY: + format = DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY; + kdump_raw = true; + break; + default: + break; + } + } + /* * kdump-compressed format need the whole memory dumped, so paging or * filter is not supported here. @@ -2166,6 +2190,10 @@ void qmp_dump_guest_memory(bool paging, const char *file, error_setg(errp, QERR_INVALID_PARAMETER, "protocol"); return; } + if (kdump_raw && lseek(fd, 0, SEEK_CUR) == (off_t) -1) { + error_setg(errp, "kdump-raw formats require a seekable file"); + return; + } if (!dump_migration_blocker) { error_setg(&dump_migration_blocker, @@ -2186,7 +2214,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, kdump_raw, errp); if (*errp) { qatomic_set(&s->status, DUMP_STATUS_FAILED); return; @@ -2214,15 +2242,18 @@ DumpGuestMemoryCapability *qmp_query_dump_guest_memory_capability(Error **errp) /* kdump-zlib is always available */ QAPI_LIST_APPEND(tail, DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB); + QAPI_LIST_APPEND(tail, DUMP_GUEST_MEMORY_FORMAT_KDUMP_RAW_ZLIB); /* add new item if kdump-lzo is available */ #ifdef CONFIG_LZO QAPI_LIST_APPEND(tail, DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO); + QAPI_LIST_APPEND(tail, DUMP_GUEST_MEMORY_FORMAT_KDUMP_RAW_LZO); #endif /* add new item if kdump-snappy is available */ #ifdef CONFIG_SNAPPY QAPI_LIST_APPEND(tail, DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY); + QAPI_LIST_APPEND(tail, DUMP_GUEST_MEMORY_FORMAT_KDUMP_RAW_SNAPPY); #endif if (win_dump_available(NULL)) { diff --git a/hmp-commands.hx b/hmp-commands.hx index 63eac22734..c0a27688b6 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1085,14 +1085,16 @@ 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,raw:-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), use raw rather than makedumpfile-flattened\n\t\t\t" + " 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 +1117,9 @@ 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), use raw rather than makedumpfile-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..5cbc237ad9 100644 --- a/qapi/dump.json +++ b/qapi/dump.json @@ -15,11 +15,23 @@ # # @elf: elf format # -# @kdump-zlib: kdump-compressed format with zlib-compressed +# @kdump-zlib: makedumpfile flattened, kdump-compressed format with zlib +# compression # -# @kdump-lzo: kdump-compressed format with lzo-compressed +# @kdump-lzo: makedumpfile flattened, kdump-compressed format with lzo +# compression # -# @kdump-snappy: kdump-compressed format with snappy-compressed +# @kdump-snappy: makedumpfile flattened, kdump-compressed format with snappy +# compression +# +# @kdump-raw-zlib: raw assembled kdump-compressed format with zlib compression +# (since 8.2) +# +# @kdump-raw-lzo: raw assembled kdump-compressed format with lzo compression +# (since 8.2) +# +# @kdump-raw-snappy: raw assembled kdump-compressed format with snappy +# compression (since 8.2) # # @win-dmp: Windows full crashdump format, can be used instead of ELF # converting (since 2.13) @@ -27,7 +39,11 @@ # Since: 2.0 ## { 'enum': 'DumpGuestMemoryFormat', - 'data': [ 'elf', 'kdump-zlib', 'kdump-lzo', 'kdump-snappy', 'win-dmp' ] } + 'data': [ + 'elf', + 'kdump-zlib', 'kdump-lzo', 'kdump-snappy', + 'kdump-raw-zlib', 'kdump-raw-lzo', 'kdump-raw-snappy', + 'win-dmp' ] } ## # @dump-guest-memory: From 8beaeed73496395f75a3e65191d45e0e299fb25b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 31 Oct 2023 11:45:27 +0100 Subject: [PATCH 4/8] dump: Rename qmp_dump_guest_memory() parameter to match QAPI schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The name of the second parameter differs between QAPI schema and C implementation: it's @protocol in the former and @file in the latter. Potentially confusing. Change the C implementation to match the QAPI schema. Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Message-Id: <20231031104531.3169721-2-armbru@redhat.com> --- dump/dump.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/dump/dump.c b/dump/dump.c index 44b27fef45..9cdb4a2bf8 100644 --- a/dump/dump.c +++ b/dump/dump.c @@ -2079,11 +2079,12 @@ DumpQueryResult *qmp_query_dump(Error **errp) return result; } -void qmp_dump_guest_memory(bool paging, const char *file, +void qmp_dump_guest_memory(bool paging, const char *protocol, bool has_detach, bool detach, - bool has_begin, int64_t begin, bool has_length, - int64_t length, bool has_format, - DumpGuestMemoryFormat format, Error **errp) + bool has_begin, int64_t begin, + bool has_length, int64_t length, + bool has_format, DumpGuestMemoryFormat format, + Error **errp) { ERRP_GUARD(); const char *p; @@ -2170,7 +2171,7 @@ void qmp_dump_guest_memory(bool paging, const char *file, } #if !defined(WIN32) - if (strstart(file, "fd:", &p)) { + if (strstart(protocol, "fd:", &p)) { fd = monitor_get_fd(monitor_cur(), p, errp); if (fd == -1) { return; @@ -2178,7 +2179,7 @@ void qmp_dump_guest_memory(bool paging, const char *file, } #endif - if (strstart(file, "file:", &p)) { + if (strstart(protocol, "file:", &p)) { fd = qemu_open_old(p, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR); if (fd < 0) { error_setg_file_open(errp, errno, p); From 96afbc571c91b115ba51d9740352a0e45111edc9 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 31 Oct 2023 11:45:28 +0100 Subject: [PATCH 5/8] dump: Fix g_array_unref(NULL) in dump-guest-memory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When dump_init()'s check for non-zero @length fails, dump_cleanup() passes null s->string_table_buf to g_array_unref(), which spews "GLib: g_array_unref: assertion 'array' failed" to stderr. Guard the g_array_unref(). Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Message-Id: <20231031104531.3169721-3-armbru@redhat.com> --- dump/dump.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dump/dump.c b/dump/dump.c index 9cdb4a2bf8..24c829e705 100644 --- a/dump/dump.c +++ b/dump/dump.c @@ -100,7 +100,7 @@ static int dump_cleanup(DumpState *s) memory_mapping_list_free(&s->list); close(s->fd); g_free(s->guest_note); - g_array_unref(s->string_table_buf); + g_clear_pointer(&s->string_table_buf, g_array_unref); s->guest_note = NULL; if (s->resume) { if (s->detached) { From f8c49724cbe13fa30b5893eff33f9ccee7e4466a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 31 Oct 2023 11:45:29 +0100 Subject: [PATCH 6/8] dump: Recognize "fd:" protocols on Windows hosts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A few QMP command can work with named file descriptors. The only way to create a named file descriptor used to be QMP command getfd, which only works on POSIX hosts. Thus, named file descriptors were actually usable only there. They became usable on Windows hosts when we added QMP command get-win32-socket (commit 4cda177c601 "qmp: add 'get-win32-socket'"). Except in dump-guest-memory, because qmp_dump_guest_memory() compiles its named file descriptor code only #if !defined(WIN32). Compile it unconditionally, like we do for the other commands supporting them. Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Message-Id: <20231031104531.3169721-4-armbru@redhat.com> --- dump/dump.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/dump/dump.c b/dump/dump.c index 24c829e705..2d0b5bd22b 100644 --- a/dump/dump.c +++ b/dump/dump.c @@ -2170,14 +2170,12 @@ void qmp_dump_guest_memory(bool paging, const char *protocol, return; } -#if !defined(WIN32) if (strstart(protocol, "fd:", &p)) { fd = monitor_get_fd(monitor_cur(), p, errp); if (fd == -1) { return; } } -#endif if (strstart(protocol, "file:", &p)) { fd = qemu_open_old(p, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR); From 28035bed1c565eace7db18971a7e960a8d1f7c44 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 31 Oct 2023 11:45:30 +0100 Subject: [PATCH 7/8] dump: Improve some dump-guest-memory error messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zero @length is rejected with "Invalid parameter 'length'". Improve to "parameter 'length' expects a non-zero length". qemu_open_old() is a wrapper around qemu_open_internal() that throws away error information. Switch to the wrapper that doesn't: qemu_create(). Example improvement: (qemu) dump-guest-memory /dev/fdset/x 0 1 Error: Could not open '/dev/fdset/x': Invalid argument becomes Error: Could not parse fdset /dev/fdset/x @protocol values not starting with "fd:" or "file:" are rejected with "Invalid parameter 'protocol'". Improve to "parameter 'protocol' must start with 'file:' or 'fd:'". While there, make the conditional checking @protocol a little more obvious. Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Message-Id: <20231031104531.3169721-5-armbru@redhat.com> --- dump/dump.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/dump/dump.c b/dump/dump.c index 2d0b5bd22b..ec2cfcf9f7 100644 --- a/dump/dump.c +++ b/dump/dump.c @@ -1828,7 +1828,7 @@ static void dump_init(DumpState *s, int fd, bool has_format, s->fd = fd; if (has_filter && !length) { - error_setg(errp, QERR_INVALID_PARAMETER, "length"); + error_setg(errp, "parameter 'length' expects a non-zero size"); goto cleanup; } s->filter_area_begin = begin; @@ -2088,7 +2088,7 @@ void qmp_dump_guest_memory(bool paging, const char *protocol, { ERRP_GUARD(); const char *p; - int fd = -1; + int fd; DumpState *s; bool detach_p = false; bool kdump_raw = false; @@ -2175,18 +2175,14 @@ void qmp_dump_guest_memory(bool paging, const char *protocol, if (fd == -1) { return; } - } - - if (strstart(protocol, "file:", &p)) { - fd = qemu_open_old(p, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR); + } else if (strstart(protocol, "file:", &p)) { + fd = qemu_create(p, O_WRONLY | O_TRUNC | O_BINARY, S_IRUSR, errp); if (fd < 0) { - error_setg_file_open(errp, errno, p); return; } - } - - if (fd == -1) { - error_setg(errp, QERR_INVALID_PARAMETER, "protocol"); + } else { + error_setg(errp, + "parameter 'protocol' must start with 'file:' or 'fd:'"); return; } if (kdump_raw && lseek(fd, 0, SEEK_CUR) == (off_t) -1) { From 4023839757b2c01e0350ce92902c6f74dce7d011 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 31 Oct 2023 11:45:31 +0100 Subject: [PATCH 8/8] dump: Drop redundant check for empty dump MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dump_init() first computes the size of the dump, taking the filter area into account, and fails if its zero. It then looks for memory in the filter area, and fails if there is none. This is redundant: if the size of the dump is zero, there is no memory, and vice versa. Delete this check. Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Message-Id: <20231031104531.3169721-6-armbru@redhat.com> --- dump/dump.c | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/dump/dump.c b/dump/dump.c index ec2cfcf9f7..1c304cadfd 100644 --- a/dump/dump.c +++ b/dump/dump.c @@ -1688,26 +1688,6 @@ static void create_kdump_vmcore(DumpState *s, Error **errp) } } -static int validate_start_block(DumpState *s) -{ - GuestPhysBlock *block; - - if (!dump_has_filter(s)) { - return 0; - } - - QTAILQ_FOREACH(block, &s->guest_phys_blocks.head, next) { - /* This block is out of the range */ - if (block->target_start >= s->filter_area_begin + s->filter_area_length || - block->target_end <= s->filter_area_begin) { - continue; - } - return 0; - } - - return -1; -} - static void get_max_mapnr(DumpState *s) { GuestPhysBlock *last_block; @@ -1857,12 +1837,6 @@ static void dump_init(DumpState *s, int fd, bool has_format, goto cleanup; } - /* Is the filter filtering everything? */ - if (validate_start_block(s) == -1) { - error_setg(errp, QERR_INVALID_PARAMETER, "begin"); - goto cleanup; - } - /* get dump info: endian, class and architecture. * If the target architecture is not supported, cpu_get_dump_info() will * return -1.