From: Stephen Brennan <stephen.s.brennan@oracle.com>
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 <stephen.s.brennan@oracle.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
[ Marc-André: replace loff_t with off_t, indent fixes ]
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20230918233233.1431858-4-stephen.s.brennan@oracle.com>
---
qapi/dump.json | 24 ++++++++++++++++++++----
dump/dump-hmp-cmds.c | 21 +++++++++++++++++----
dump/dump.c | 33 ++++++++++++++++++++++++++++++++-
hmp-commands.hx | 9 +++++++--
4 files changed, 76 insertions(+), 11 deletions(-)
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:
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
--
2.41.0
On Fri, 3 Nov 2023 at 07:02, <marcandre.lureau@redhat.com> wrote:
>
> From: Stephen Brennan <stephen.s.brennan@oracle.com>
>
> 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 <stephen.s.brennan@oracle.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> [ Marc-André: replace loff_t with off_t, indent fixes ]
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Message-Id: <20230918233233.1431858-4-stephen.s.brennan@oracle.com>
Hi; Coverity points out some issues in this commit:
> 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;
This is dead code, beacuse the outer conditional "(zlib && raw)"
ensures that raw can't be false here. What was the intention here?
(CID 1523841)
> + }
> }
>
> 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) {
> @@ -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;
> + }
This error-exit return path forgets to close(fd), so we leak
the file descriptor. (CID 1523842)
>
> if (!dump_migration_blocker) {
> error_setg(&dump_migration_blocker,
thanks
-- PMM
Peter Maydell <peter.maydell@linaro.org> writes:
> On Fri, 3 Nov 2023 at 07:02, <marcandre.lureau@redhat.com> wrote:
>>
>> From: Stephen Brennan <stephen.s.brennan@oracle.com>
>>
>> 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 <stephen.s.brennan@oracle.com>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> [ Marc-André: replace loff_t with off_t, indent fixes ]
>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Message-Id: <20230918233233.1431858-4-stephen.s.brennan@oracle.com>
>
> Hi; Coverity points out some issues in this commit:
Proposed fixes:
Subject: [PATCH 0/2] dump: Fix issues flagged by Coverity
Message-ID: <20231108053731.112043-1-armbru@redhat.com>
Markus Armbruster <armbru@redhat.com> writes: > Peter Maydell <peter.maydell@linaro.org> writes: > >> On Fri, 3 Nov 2023 at 07:02, <marcandre.lureau@redhat.com> wrote: >>> >>> From: Stephen Brennan <stephen.s.brennan@oracle.com> >>> >>> 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 <stephen.s.brennan@oracle.com> >>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> >>> [ Marc-André: replace loff_t with off_t, indent fixes ] >>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> >>> Message-Id: <20230918233233.1431858-4-stephen.s.brennan@oracle.com> >> >> Hi; Coverity points out some issues in this commit: > > Proposed fixes: > > Subject: [PATCH 0/2] dump: Fix issues flagged by Coverity > Message-ID: <20231108053731.112043-1-armbru@redhat.com> Closing the loop on this, these fixes were exactly right. Sorry for the extra work here folks. Stephen
© 2016 - 2024 Red Hat, Inc.