* [Qemu-devel] [PATCH 01/13 v7] dump: const-qualify the buf of WriteCoreDumpFunction
2014-01-17 7:46 [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
@ 2014-01-17 7:46 ` qiaonuohan
2014-01-22 15:48 ` Laszlo Ersek
2014-01-17 7:46 ` [Qemu-devel] [PATCH 02/13 v7] dump: add argument to write_elfxx_notes qiaonuohan
` (13 subsequent siblings)
14 siblings, 1 reply; 37+ messages in thread
From: qiaonuohan @ 2014-01-17 7:46 UTC (permalink / raw)
To: lersek, stefanha, lcapitulino, afaerber, eblake
Cc: kumagai-atsushi, qiaonuohan, anderson, qemu-devel
WriteCoreDumpFunction is a function pointer that points to the function used to
write content in "buf" into core file, so "buf" should be const-qualify.
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
---
dump.c | 2 +-
include/qom/cpu.h | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/dump.c b/dump.c
index 80a9116..42622de 100644
--- a/dump.c
+++ b/dump.c
@@ -99,7 +99,7 @@ static void dump_error(DumpState *s, const char *reason)
dump_cleanup(s);
}
-static int fd_write_vmcore(void *buf, size_t size, void *opaque)
+static int fd_write_vmcore(const void *buf, size_t size, void *opaque)
{
DumpState *s = opaque;
size_t written_size;
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 7739e00..57b4164 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -28,7 +28,8 @@
#include "qemu/tls.h"
#include "qemu/typedefs.h"
-typedef int (*WriteCoreDumpFunction)(void *buf, size_t size, void *opaque);
+typedef int (*WriteCoreDumpFunction)(const void *buf, size_t size,
+ void *opaque);
/**
* vaddr:
--
1.7.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 01/13 v7] dump: const-qualify the buf of WriteCoreDumpFunction
2014-01-17 7:46 ` [Qemu-devel] [PATCH 01/13 v7] dump: const-qualify the buf of WriteCoreDumpFunction qiaonuohan
@ 2014-01-22 15:48 ` Laszlo Ersek
0 siblings, 0 replies; 37+ messages in thread
From: Laszlo Ersek @ 2014-01-22 15:48 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi,
afaerber
On 01/17/14 08:46, qiaonuohan wrote:
> WriteCoreDumpFunction is a function pointer that points to the function used to
> write content in "buf" into core file, so "buf" should be const-qualify.
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> ---
> dump.c | 2 +-
> include/qom/cpu.h | 3 ++-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/dump.c b/dump.c
> index 80a9116..42622de 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -99,7 +99,7 @@ static void dump_error(DumpState *s, const char *reason)
> dump_cleanup(s);
> }
>
> -static int fd_write_vmcore(void *buf, size_t size, void *opaque)
> +static int fd_write_vmcore(const void *buf, size_t size, void *opaque)
> {
> DumpState *s = opaque;
> size_t written_size;
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 7739e00..57b4164 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -28,7 +28,8 @@
> #include "qemu/tls.h"
> #include "qemu/typedefs.h"
>
> -typedef int (*WriteCoreDumpFunction)(void *buf, size_t size, void *opaque);
> +typedef int (*WriteCoreDumpFunction)(const void *buf, size_t size,
> + void *opaque);
>
> /**
> * vaddr:
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH 02/13 v7] dump: add argument to write_elfxx_notes
2014-01-17 7:46 [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
2014-01-17 7:46 ` [Qemu-devel] [PATCH 01/13 v7] dump: const-qualify the buf of WriteCoreDumpFunction qiaonuohan
@ 2014-01-17 7:46 ` qiaonuohan
2014-01-22 15:56 ` Laszlo Ersek
2014-01-17 7:46 ` [Qemu-devel] [PATCH 03/13 v7] dump: add API to write header of flatten format qiaonuohan
` (12 subsequent siblings)
14 siblings, 1 reply; 37+ messages in thread
From: qiaonuohan @ 2014-01-17 7:46 UTC (permalink / raw)
To: lersek, stefanha, lcapitulino, afaerber, eblake
Cc: kumagai-atsushi, qiaonuohan, anderson, qemu-devel
write_elf32_notes/wirte_elf64_notes use fd_write_vmcore to write elf notes to
vmcore. Adding parameter "WriteCoreDumpFunction f" makes it available to choose
the method of writing elf notes
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
dump.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/dump.c b/dump.c
index 42622de..c9d3492 100644
--- a/dump.c
+++ b/dump.c
@@ -271,7 +271,7 @@ static inline int cpu_index(CPUState *cpu)
return cpu->cpu_index + 1;
}
-static int write_elf64_notes(DumpState *s)
+static int write_elf64_notes(WriteCoreDumpFunction f, DumpState *s)
{
CPUState *cpu;
int ret;
@@ -279,7 +279,7 @@ static int write_elf64_notes(DumpState *s)
CPU_FOREACH(cpu) {
id = cpu_index(cpu);
- ret = cpu_write_elf64_note(fd_write_vmcore, cpu, id, s);
+ ret = cpu_write_elf64_note(f, cpu, id, s);
if (ret < 0) {
dump_error(s, "dump: failed to write elf notes.\n");
return -1;
@@ -287,7 +287,7 @@ static int write_elf64_notes(DumpState *s)
}
CPU_FOREACH(cpu) {
- ret = cpu_write_elf64_qemunote(fd_write_vmcore, cpu, s);
+ ret = cpu_write_elf64_qemunote(f, cpu, s);
if (ret < 0) {
dump_error(s, "dump: failed to write CPU status.\n");
return -1;
@@ -321,7 +321,7 @@ static int write_elf32_note(DumpState *s)
return 0;
}
-static int write_elf32_notes(DumpState *s)
+static int write_elf32_notes(WriteCoreDumpFunction f, DumpState *s)
{
CPUState *cpu;
int ret;
@@ -329,7 +329,7 @@ static int write_elf32_notes(DumpState *s)
CPU_FOREACH(cpu) {
id = cpu_index(cpu);
- ret = cpu_write_elf32_note(fd_write_vmcore, cpu, id, s);
+ ret = cpu_write_elf32_note(f, cpu, id, s);
if (ret < 0) {
dump_error(s, "dump: failed to write elf notes.\n");
return -1;
@@ -337,7 +337,7 @@ static int write_elf32_notes(DumpState *s)
}
CPU_FOREACH(cpu) {
- ret = cpu_write_elf32_qemunote(fd_write_vmcore, cpu, s);
+ ret = cpu_write_elf32_qemunote(f, cpu, s);
if (ret < 0) {
dump_error(s, "dump: failed to write CPU status.\n");
return -1;
@@ -574,7 +574,7 @@ static int dump_begin(DumpState *s)
}
/* write notes to vmcore */
- if (write_elf64_notes(s) < 0) {
+ if (write_elf64_notes(fd_write_vmcore, s) < 0) {
return -1;
}
@@ -597,7 +597,7 @@ static int dump_begin(DumpState *s)
}
/* write notes to vmcore */
- if (write_elf32_notes(s) < 0) {
+ if (write_elf32_notes(fd_write_vmcore, s) < 0) {
return -1;
}
}
--
1.7.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 02/13 v7] dump: add argument to write_elfxx_notes
2014-01-17 7:46 ` [Qemu-devel] [PATCH 02/13 v7] dump: add argument to write_elfxx_notes qiaonuohan
@ 2014-01-22 15:56 ` Laszlo Ersek
0 siblings, 0 replies; 37+ messages in thread
From: Laszlo Ersek @ 2014-01-22 15:56 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi,
afaerber
On 01/17/14 08:46, qiaonuohan wrote:
> write_elf32_notes/wirte_elf64_notes use fd_write_vmcore to write elf notes to
> vmcore. Adding parameter "WriteCoreDumpFunction f" makes it available to choose
> the method of writing elf notes
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
> dump.c | 16 ++++++++--------
> 1 files changed, 8 insertions(+), 8 deletions(-)
OK this seems to be unchanged, so my R-b stands.
Laszlo
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH 03/13 v7] dump: add API to write header of flatten format
2014-01-17 7:46 [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
2014-01-17 7:46 ` [Qemu-devel] [PATCH 01/13 v7] dump: const-qualify the buf of WriteCoreDumpFunction qiaonuohan
2014-01-17 7:46 ` [Qemu-devel] [PATCH 02/13 v7] dump: add argument to write_elfxx_notes qiaonuohan
@ 2014-01-17 7:46 ` qiaonuohan
2014-01-22 16:03 ` Laszlo Ersek
2014-01-17 7:46 ` [Qemu-devel] [PATCH 04/13 v7] dump: add API to write vmcore qiaonuohan
` (11 subsequent siblings)
14 siblings, 1 reply; 37+ messages in thread
From: qiaonuohan @ 2014-01-17 7:46 UTC (permalink / raw)
To: lersek, stefanha, lcapitulino, afaerber, eblake
Cc: kumagai-atsushi, qiaonuohan, anderson, qemu-devel
flatten format will be used when writing kdump-compressed format. The format is
also used by makedumpfile, you can refer to the following URL to get more
detailed information about flatten format of kdump-compressed format:
http://sourceforge.net/projects/makedumpfile/
The two functions here are used to write start flat header and end flat header
to vmcore, and they will be called later when flatten format is used.
struct MakedumpfileHeader stored at the head of vmcore is used to indicate the
vmcore is in flatten format.
struct MakedumpfileHeader {
char signature[16]; /* = "makedumpfile" */
int64_t type; /* = 1 */
int64_t version; /* = 1 */
};
And struct MakedumpfileDataHeader, with offset and buf_size set to -1, is used
to indicate the end of vmcore in flatten format.
struct MakedumpfileDataHeader {
int64_t offset; /* = -1 */
int64_t buf_size; /* = -1 */
};
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
dump.c | 42 ++++++++++++++++++++++++++++++++++++++++++
include/sysemu/dump.h | 17 +++++++++++++++++
2 files changed, 59 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index c9d3492..f233b3e 100644
--- a/dump.c
+++ b/dump.c
@@ -686,6 +686,48 @@ static int create_vmcore(DumpState *s)
return 0;
}
+static int write_start_flat_header(int fd)
+{
+ uint8_t *buf;
+ MakedumpfileHeader mh;
+ int ret = 0;
+
+ memset(&mh, 0, sizeof(mh));
+ strncpy(mh.signature, MAKEDUMPFILE_SIGNATURE,
+ strlen(MAKEDUMPFILE_SIGNATURE));
+
+ mh.type = cpu_to_be64(TYPE_FLAT_HEADER);
+ mh.version = cpu_to_be64(VERSION_FLAT_HEADER);
+
+ buf = g_malloc0(MAX_SIZE_MDF_HEADER);
+ memcpy(buf, &mh, sizeof(mh));
+
+ size_t written_size;
+ written_size = qemu_write_full(fd, buf, MAX_SIZE_MDF_HEADER);
+ if (written_size != MAX_SIZE_MDF_HEADER) {
+ ret = -1;
+ }
+
+ g_free(buf);
+ return ret;
+}
+
+static int write_end_flat_header(int fd)
+{
+ MakedumpfileDataHeader mdh;
+
+ mdh.offset = END_FLAG_FLAT_HEADER;
+ mdh.buf_size = END_FLAG_FLAT_HEADER;
+
+ size_t written_size;
+ written_size = qemu_write_full(fd, &mdh, sizeof(mdh));
+ if (written_size != sizeof(mdh)) {
+ return -1;
+ }
+
+ return 0;
+}
+
static ram_addr_t get_start_block(DumpState *s)
{
GuestPhysBlock *block;
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 19fafb2..b32b390 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -14,12 +14,29 @@
#ifndef DUMP_H
#define DUMP_H
+#define MAKEDUMPFILE_SIGNATURE "makedumpfile"
+#define MAX_SIZE_MDF_HEADER (4096) /* max size of makedumpfile_header */
+#define TYPE_FLAT_HEADER (1) /* type of flattened format */
+#define VERSION_FLAT_HEADER (1) /* version of flattened format */
+#define END_FLAG_FLAT_HEADER (-1)
+
typedef struct ArchDumpInfo {
int d_machine; /* Architecture */
int d_endian; /* ELFDATA2LSB or ELFDATA2MSB */
int d_class; /* ELFCLASS32 or ELFCLASS64 */
} ArchDumpInfo;
+typedef struct QEMU_PACKED MakedumpfileHeader {
+ char signature[16]; /* = "makedumpfile" */
+ int64_t type;
+ int64_t version;
+} MakedumpfileHeader;
+
+typedef struct QEMU_PACKED MakedumpfileDataHeader {
+ int64_t offset;
+ int64_t buf_size;
+} MakedumpfileDataHeader;
+
struct GuestPhysBlockList; /* memory_mapping.h */
int cpu_get_dump_info(ArchDumpInfo *info,
const struct GuestPhysBlockList *guest_phys_blocks);
--
1.7.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 03/13 v7] dump: add API to write header of flatten format
2014-01-17 7:46 ` [Qemu-devel] [PATCH 03/13 v7] dump: add API to write header of flatten format qiaonuohan
@ 2014-01-22 16:03 ` Laszlo Ersek
0 siblings, 0 replies; 37+ messages in thread
From: Laszlo Ersek @ 2014-01-22 16:03 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi,
afaerber
On 01/17/14 08:46, qiaonuohan wrote:
> flatten format will be used when writing kdump-compressed format. The format is
> also used by makedumpfile, you can refer to the following URL to get more
> detailed information about flatten format of kdump-compressed format:
> http://sourceforge.net/projects/makedumpfile/
>
> The two functions here are used to write start flat header and end flat header
> to vmcore, and they will be called later when flatten format is used.
>
> struct MakedumpfileHeader stored at the head of vmcore is used to indicate the
> vmcore is in flatten format.
>
> struct MakedumpfileHeader {
> char signature[16]; /* = "makedumpfile" */
> int64_t type; /* = 1 */
> int64_t version; /* = 1 */
> };
>
> And struct MakedumpfileDataHeader, with offset and buf_size set to -1, is used
> to indicate the end of vmcore in flatten format.
>
> struct MakedumpfileDataHeader {
> int64_t offset; /* = -1 */
> int64_t buf_size; /* = -1 */
> };
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
> dump.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> include/sysemu/dump.h | 17 +++++++++++++++++
> 2 files changed, 59 insertions(+), 0 deletions(-)
OK, comparing this with v6 02/11, I can see that you changed
write_start_flat_header():
- element type of "buf" from char to uint8_t,
- the array is now dynamically allocated,
- it is not leaked even in case of error.
My R-b stands.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH 04/13 v7] dump: add API to write vmcore
2014-01-17 7:46 [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (2 preceding siblings ...)
2014-01-17 7:46 ` [Qemu-devel] [PATCH 03/13 v7] dump: add API to write header of flatten format qiaonuohan
@ 2014-01-17 7:46 ` qiaonuohan
2014-01-22 16:06 ` Laszlo Ersek
2014-01-17 7:46 ` [Qemu-devel] [PATCH 05/13 v7] dump: add API to write elf notes to buffer qiaonuohan
` (10 subsequent siblings)
14 siblings, 1 reply; 37+ messages in thread
From: qiaonuohan @ 2014-01-17 7:46 UTC (permalink / raw)
To: lersek, stefanha, lcapitulino, afaerber, eblake
Cc: kumagai-atsushi, qiaonuohan, anderson, qemu-devel
Function is used to write vmcore in flatten format. In flatten format, data is
written block by block, and in front of each block, a struct
MakedumpfileDataHeader is stored there to indicate the offset and size of the
data block.
struct MakedumpfileDataHeader {
int64_t offset;
int64_t buf_size;
};
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
dump.c | 21 +++++++++++++++++++++
1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index f233b3e..238ffa5 100644
--- a/dump.c
+++ b/dump.c
@@ -728,6 +728,27 @@ 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)
+{
+ size_t written_size;
+ MakedumpfileDataHeader mdh;
+
+ mdh.offset = cpu_to_be64(offset);
+ mdh.buf_size = cpu_to_be64(size);
+
+ written_size = qemu_write_full(fd, &mdh, sizeof(mdh));
+ if (written_size != sizeof(mdh)) {
+ return -1;
+ }
+
+ written_size = qemu_write_full(fd, buf, size);
+ if (written_size != size) {
+ return -1;
+ }
+
+ return 0;
+}
+
static ram_addr_t get_start_block(DumpState *s)
{
GuestPhysBlock *block;
--
1.7.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 04/13 v7] dump: add API to write vmcore
2014-01-17 7:46 ` [Qemu-devel] [PATCH 04/13 v7] dump: add API to write vmcore qiaonuohan
@ 2014-01-22 16:06 ` Laszlo Ersek
0 siblings, 0 replies; 37+ messages in thread
From: Laszlo Ersek @ 2014-01-22 16:06 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi,
afaerber
On 01/17/14 08:46, qiaonuohan wrote:
> Function is used to write vmcore in flatten format. In flatten format, data is
> written block by block, and in front of each block, a struct
> MakedumpfileDataHeader is stored there to indicate the offset and size of the
> data block.
>
> struct MakedumpfileDataHeader {
> int64_t offset;
> int64_t buf_size;
> };
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
> dump.c | 21 +++++++++++++++++++++
> 1 files changed, 21 insertions(+), 0 deletions(-)
Compared with v6 03/11, this patch hardwires the flat format (and adapts
the commit message too). My R-b stands.
Laszlo
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH 05/13 v7] dump: add API to write elf notes to buffer
2014-01-17 7:46 [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (3 preceding siblings ...)
2014-01-17 7:46 ` [Qemu-devel] [PATCH 04/13 v7] dump: add API to write vmcore qiaonuohan
@ 2014-01-17 7:46 ` qiaonuohan
2014-01-22 16:09 ` Laszlo Ersek
2014-01-17 7:46 ` [Qemu-devel] [PATCH 06/13 v7] dump: add support for lzo/snappy qiaonuohan
` (9 subsequent siblings)
14 siblings, 1 reply; 37+ messages in thread
From: qiaonuohan @ 2014-01-17 7:46 UTC (permalink / raw)
To: lersek, stefanha, lcapitulino, afaerber, eblake
Cc: kumagai-atsushi, qiaonuohan, anderson, qemu-devel
the function can be used by write_elf32_notes/write_elf64_notes to write notes
to a buffer. If fd_write_vmcore is used, write_elf32_notes/write_elf64_notes
will write elf notes to vmcore directly. Instead, if buf_write_note is used,
elf notes will be written to opaque->note_buf at first.
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
dump.c | 19 +++++++++++++++++++
1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index 238ffa5..2b940bd 100644
--- a/dump.c
+++ b/dump.c
@@ -76,6 +76,9 @@ typedef struct DumpState {
int64_t begin;
int64_t length;
Error **errp;
+
+ uint8_t *note_buf; /* buffer for notes */
+ size_t note_buf_offset; /* the writing place in note_buf */
} DumpState;
static int dump_cleanup(DumpState *s)
@@ -749,6 +752,22 @@ static int write_buffer(int fd, off_t offset, const void *buf, size_t size)
return 0;
}
+static int buf_write_note(const void *buf, size_t size, void *opaque)
+{
+ DumpState *s = opaque;
+
+ /* note_buf is not enough */
+ if (s->note_buf_offset + size > s->note_size) {
+ return -1;
+ }
+
+ memcpy(s->note_buf + s->note_buf_offset, buf, size);
+
+ s->note_buf_offset += size;
+
+ return 0;
+}
+
static ram_addr_t get_start_block(DumpState *s)
{
GuestPhysBlock *block;
--
1.7.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 05/13 v7] dump: add API to write elf notes to buffer
2014-01-17 7:46 ` [Qemu-devel] [PATCH 05/13 v7] dump: add API to write elf notes to buffer qiaonuohan
@ 2014-01-22 16:09 ` Laszlo Ersek
0 siblings, 0 replies; 37+ messages in thread
From: Laszlo Ersek @ 2014-01-22 16:09 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi,
afaerber
On 01/17/14 08:46, qiaonuohan wrote:
> the function can be used by write_elf32_notes/write_elf64_notes to write notes
> to a buffer. If fd_write_vmcore is used, write_elf32_notes/write_elf64_notes
> will write elf notes to vmcore directly. Instead, if buf_write_note is used,
> elf notes will be written to opaque->note_buf at first.
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
> dump.c | 19 +++++++++++++++++++
> 1 files changed, 19 insertions(+), 0 deletions(-)
Thanks for addressing my tangential comments for v6 04/11. My R-b stands.
Laszlo
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH 06/13 v7] dump: add support for lzo/snappy
2014-01-17 7:46 [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (4 preceding siblings ...)
2014-01-17 7:46 ` [Qemu-devel] [PATCH 05/13 v7] dump: add API to write elf notes to buffer qiaonuohan
@ 2014-01-17 7:46 ` qiaonuohan
2014-01-22 16:12 ` Laszlo Ersek
2014-01-17 7:46 ` [Qemu-devel] [PATCH 07/13 v7] dump: add members to DumpState and init some of them qiaonuohan
` (8 subsequent siblings)
14 siblings, 1 reply; 37+ messages in thread
From: qiaonuohan @ 2014-01-17 7:46 UTC (permalink / raw)
To: lersek, stefanha, lcapitulino, afaerber, eblake
Cc: kumagai-atsushi, qiaonuohan, anderson, qemu-devel
kdump-compressed format supports three compression format, zlib/lzo/snappy.
Currently, only zlib is available. This patch is used to support lzo/snappy.
'--enable-lzo/--enable-snappy' is needed to be specified with configure to make
lzo/snappy available for qemu
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
configure | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 54 insertions(+), 0 deletions(-)
diff --git a/configure b/configure
index 3782a6a..0bd0321 100755
--- a/configure
+++ b/configure
@@ -245,6 +245,8 @@ libusb=""
usb_redir=""
glx=""
zlib="yes"
+lzo="no"
+snappy="no"
guest_agent=""
guest_agent_with_vss="no"
vss_win32_sdk=""
@@ -947,6 +949,10 @@ for opt do
;;
--disable-zlib-test) zlib="no"
;;
+ --enable-lzo) lzo="yes"
+ ;;
+ --enable-snappy) snappy="yes"
+ ;;
--enable-guest-agent) guest_agent="yes"
;;
--disable-guest-agent) guest_agent="no"
@@ -1234,6 +1240,8 @@ Advanced options (experts only):
--enable-libusb enable libusb (for usb passthrough)
--disable-usb-redir disable usb network redirection support
--enable-usb-redir enable usb network redirection support
+ --enable-lzo enable the support of lzo compression library
+ --enable-snappy enable the support of snappy compression library
--disable-guest-agent disable building of the QEMU Guest Agent
--enable-guest-agent enable building of the QEMU Guest Agent
--with-vss-sdk=SDK-path enable Windows VSS support in QEMU Guest Agent
@@ -1538,6 +1546,42 @@ fi
libs_softmmu="$libs_softmmu -lz"
##########################################
+# lzo check
+
+if test "$lzo" != "no" ; then
+ cat > $TMPC << EOF
+#include <lzo/lzo1x.h>
+int main(void) { lzo_version(); return 0; }
+EOF
+ if compile_prog "" "-llzo2" ; then
+ :
+ else
+ error_exit "lzo check failed" \
+ "Make sure to have the lzo libs and headers installed."
+ fi
+
+ libs_softmmu="$libs_softmmu -llzo2"
+fi
+
+##########################################
+# snappy check
+
+if test "$snappy" != "no" ; then
+ cat > $TMPC << EOF
+#include <snappy-c.h>
+int main(void) { snappy_max_compressed_length(4096); return 0; }
+EOF
+ if compile_prog "" "-lsnappy" ; then
+ :
+ else
+ error_exit "snappy check failed" \
+ "Make sure to have the snappy libs and headers installed."
+ fi
+
+ libs_softmmu="$libs_softmmu -lsnappy"
+fi
+
+##########################################
# libseccomp check
if test "$seccomp" != "no" ; then
@@ -3839,6 +3883,8 @@ echo "libssh2 support $libssh2"
echo "TPM passthrough $tpm_passthrough"
echo "QOM debugging $qom_cast_debug"
echo "vhdx $vhdx"
+echo "lzo support $lzo"
+echo "snappy support $snappy"
if test "$sdl_too_old" = "yes"; then
echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -4154,6 +4200,14 @@ if test "$glx" = "yes" ; then
echo "GLX_LIBS=$glx_libs" >> $config_host_mak
fi
+if test "$lzo" = "yes" ; then
+ echo "CONFIG_LZO=y" >> $config_host_mak
+fi
+
+if test "$snappy" = "yes" ; then
+ echo "CONFIG_SNAPPY=y" >> $config_host_mak
+fi
+
if test "$libiscsi" = "yes" ; then
echo "CONFIG_LIBISCSI=y" >> $config_host_mak
if test "$libiscsi_version" = "1.4.0"; then
--
1.7.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 06/13 v7] dump: add support for lzo/snappy
2014-01-17 7:46 ` [Qemu-devel] [PATCH 06/13 v7] dump: add support for lzo/snappy qiaonuohan
@ 2014-01-22 16:12 ` Laszlo Ersek
0 siblings, 0 replies; 37+ messages in thread
From: Laszlo Ersek @ 2014-01-22 16:12 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi,
afaerber
On 01/17/14 08:46, qiaonuohan wrote:
> kdump-compressed format supports three compression format, zlib/lzo/snappy.
> Currently, only zlib is available. This patch is used to support lzo/snappy.
> '--enable-lzo/--enable-snappy' is needed to be specified with configure to make
> lzo/snappy available for qemu
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
> configure | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 54 insertions(+), 0 deletions(-)
In comparison with v6 05/11:
- "--enable-lzo" and "--enable-snappy" advertised in the experts'
options block,
- same settings confirmed in the settings report,
- rebased on top of unrelated libiscsi changes in the configure script.
My R-b stands.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH 07/13 v7] dump: add members to DumpState and init some of them
2014-01-17 7:46 [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (5 preceding siblings ...)
2014-01-17 7:46 ` [Qemu-devel] [PATCH 06/13 v7] dump: add support for lzo/snappy qiaonuohan
@ 2014-01-17 7:46 ` qiaonuohan
2014-01-22 17:04 ` Laszlo Ersek
2014-01-17 7:46 ` [Qemu-devel] [PATCH 08/13 v7] dump: add API to write dump header qiaonuohan
` (7 subsequent siblings)
14 siblings, 1 reply; 37+ messages in thread
From: qiaonuohan @ 2014-01-17 7:46 UTC (permalink / raw)
To: lersek, stefanha, lcapitulino, afaerber, eblake
Cc: kumagai-atsushi, qiaonuohan, anderson, qemu-devel
add some members to DumpState that will be used in writing vmcore in
kdump-compressed format. some of them, like page_size, will be initialized
in the patch.
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
---
dump.c | 30 ++++++++++++++++++++++++++++++
include/sysemu/dump.h | 7 +++++++
2 files changed, 37 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index 2b940bd..bf7d31d 100644
--- a/dump.c
+++ b/dump.c
@@ -79,6 +79,16 @@ typedef struct DumpState {
uint8_t *note_buf; /* buffer for notes */
size_t note_buf_offset; /* the writing place in note_buf */
+ uint32_t nr_cpus; /* number of guest's cpu */
+ size_t page_size; /* guest's page size */
+ uint32_t page_shift; /* guest's page shift */
+ uint64_t max_mapnr; /* the biggest guest's phys-mem's number */
+ size_t len_dump_bitmap; /* the size of the place used to store
+ dump_bitmap in vmcore */
+ off_t offset_dump_bitmap; /* offset of dump_bitmap part in vmcore */
+ off_t offset_page; /* offset of page part in vmcore */
+ size_t num_dumpable; /* number of page that can be dumped */
+ uint32_t flag_compress; /* indicate the compression format */
} DumpState;
static int dump_cleanup(DumpState *s)
@@ -796,6 +806,16 @@ static ram_addr_t get_start_block(DumpState *s)
return -1;
}
+static void get_max_mapnr(DumpState *s)
+{
+ MemoryMapping *memory_mapping;
+
+ QTAILQ_FOREACH(memory_mapping, &s->list.head, next) {
+ s->max_mapnr = paddr_to_pfn(memory_mapping->phys_addr +
+ memory_mapping->length, s->page_shift);
+ }
+}
+
static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
int64_t begin, int64_t length, Error **errp)
{
@@ -864,6 +884,16 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
qemu_get_guest_simple_memory_mapping(&s->list, &s->guest_phys_blocks);
}
+ s->nr_cpus = nr_cpus;
+ s->page_size = TARGET_PAGE_SIZE;
+ s->page_shift = ffs(s->page_size) - 1;
+
+ get_max_mapnr(s);
+
+ uint64_t tmp;
+ tmp = DIV_ROUND_UP(DIV_ROUND_UP(s->max_mapnr, CHAR_BIT), s->page_size);
+ s->len_dump_bitmap = tmp * s->page_size;
+
if (s->has_filter) {
memory_mapping_filter(&s->list, s->begin, s->length);
}
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index b32b390..995bf47 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -20,6 +20,13 @@
#define VERSION_FLAT_HEADER (1) /* version of flattened format */
#define END_FLAG_FLAT_HEADER (-1)
+#define ARCH_PFN_OFFSET (0)
+
+#define paddr_to_pfn(X, page_shift) \
+ (((unsigned long long)(X) >> (page_shift)) - ARCH_PFN_OFFSET)
+#define pfn_to_paddr(X, page_shift) \
+ (((unsigned long long)(X) + ARCH_PFN_OFFSET) << (page_shift))
+
typedef struct ArchDumpInfo {
int d_machine; /* Architecture */
int d_endian; /* ELFDATA2LSB or ELFDATA2MSB */
--
1.7.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 07/13 v7] dump: add members to DumpState and init some of them
2014-01-17 7:46 ` [Qemu-devel] [PATCH 07/13 v7] dump: add members to DumpState and init some of them qiaonuohan
@ 2014-01-22 17:04 ` Laszlo Ersek
2014-01-24 1:52 ` Qiao Nuohan
0 siblings, 1 reply; 37+ messages in thread
From: Laszlo Ersek @ 2014-01-22 17:04 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi,
afaerber
comments below
On 01/17/14 08:46, qiaonuohan wrote:
> add some members to DumpState that will be used in writing vmcore in
> kdump-compressed format. some of them, like page_size, will be initialized
> in the patch.
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> ---
> dump.c | 30 ++++++++++++++++++++++++++++++
> include/sysemu/dump.h | 7 +++++++
> 2 files changed, 37 insertions(+), 0 deletions(-)
>
> diff --git a/dump.c b/dump.c
> index 2b940bd..bf7d31d 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -79,6 +79,16 @@ typedef struct DumpState {
>
> uint8_t *note_buf; /* buffer for notes */
> size_t note_buf_offset; /* the writing place in note_buf */
> + uint32_t nr_cpus; /* number of guest's cpu */
> + size_t page_size; /* guest's page size */
> + uint32_t page_shift; /* guest's page shift */
> + uint64_t max_mapnr; /* the biggest guest's phys-mem's number */
> + size_t len_dump_bitmap; /* the size of the place used to store
> + dump_bitmap in vmcore */
> + off_t offset_dump_bitmap; /* offset of dump_bitmap part in vmcore */
> + off_t offset_page; /* offset of page part in vmcore */
> + size_t num_dumpable; /* number of page that can be dumped */
> + uint32_t flag_compress; /* indicate the compression format */
> } DumpState;
v6 06/11 addded these, but we have the following changes here:
- flag_flatten is gone, OK,
- bunch of comments, good,
- page_shift and num_dumpable are now added at once (originally in v6
07/11).
>
> static int dump_cleanup(DumpState *s)
> @@ -796,6 +806,16 @@ static ram_addr_t get_start_block(DumpState *s)
> return -1;
> }
>
> +static void get_max_mapnr(DumpState *s)
> +{
> + MemoryMapping *memory_mapping;
> +
> + QTAILQ_FOREACH(memory_mapping, &s->list.head, next) {
> + s->max_mapnr = paddr_to_pfn(memory_mapping->phys_addr +
> + memory_mapping->length, s->page_shift);
> + }
> +}
> +
> static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
> int64_t begin, int64_t length, Error **errp)
> {
This is from v6 10/11, OK.
> @@ -864,6 +884,16 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
> qemu_get_guest_simple_memory_mapping(&s->list, &s->guest_phys_blocks);
> }
>
> + s->nr_cpus = nr_cpus;
> + s->page_size = TARGET_PAGE_SIZE;
> + s->page_shift = ffs(s->page_size) - 1;
> +
> + get_max_mapnr(s);
Again from v6 10/11, good. The flag_flatten assignment has been dropped.
Initialization seems to happen in a good spot this time too.
> +
> + uint64_t tmp;
> + tmp = DIV_ROUND_UP(DIV_ROUND_UP(s->max_mapnr, CHAR_BIT), s->page_size);
> + s->len_dump_bitmap = tmp * s->page_size;
> +
> if (s->has_filter) {
> memory_mapping_filter(&s->list, s->begin, s->length);
> }
Again from v6 10/11.
These assignments now all occur without depending on a user request for
a compressed dump (kept this way in v7 12/13 too), but they are not
costly. The loop in get_max_mapnr() iterates over less than 10 mappings
in the non-paging dump case, and in the paging dump case it also
shouldn't be more than a hundred or so (as I recall from earlier
testing). This might be worth some regression-testing (perf-wise), but
it looks OK to me.
> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
> index b32b390..995bf47 100644
> --- a/include/sysemu/dump.h
> +++ b/include/sysemu/dump.h
> @@ -20,6 +20,13 @@
> #define VERSION_FLAT_HEADER (1) /* version of flattened format */
> #define END_FLAG_FLAT_HEADER (-1)
>
> +#define ARCH_PFN_OFFSET (0)
> +
> +#define paddr_to_pfn(X, page_shift) \
> + (((unsigned long long)(X) >> (page_shift)) - ARCH_PFN_OFFSET)
>From v6 07/11, needed by get_max_mapnr().
> +#define pfn_to_paddr(X, page_shift) \
> + (((unsigned long long)(X) + ARCH_PFN_OFFSET) << (page_shift))
> +
> typedef struct ArchDumpInfo {
> int d_machine; /* Architecture */
> int d_endian; /* ELFDATA2LSB or ELFDATA2MSB */
>
>From v6 09/11. Not strictly needed right now, but it does make sense for
consistency.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 07/13 v7] dump: add members to DumpState and init some of them
2014-01-22 17:04 ` Laszlo Ersek
@ 2014-01-24 1:52 ` Qiao Nuohan
2014-01-24 10:00 ` Laszlo Ersek
0 siblings, 1 reply; 37+ messages in thread
From: Qiao Nuohan @ 2014-01-24 1:52 UTC (permalink / raw)
To: Laszlo Ersek
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi,
afaerber
On 01/23/2014 01:04 AM, Laszlo Ersek wrote:
>> @@ -864,6 +884,16 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
>> > qemu_get_guest_simple_memory_mapping(&s->list,&s->guest_phys_blocks);
>> > }
>> >
>> > + s->nr_cpus = nr_cpus;
>> > + s->page_size = TARGET_PAGE_SIZE;
>> > + s->page_shift = ffs(s->page_size) - 1;
>> > +
>> > + get_max_mapnr(s);
> Again from v6 10/11, good. The flag_flatten assignment has been dropped.
> Initialization seems to happen in a good spot this time too.
>
>> > +
>> > + uint64_t tmp;
>> > + tmp = DIV_ROUND_UP(DIV_ROUND_UP(s->max_mapnr, CHAR_BIT), s->page_size);
>> > + s->len_dump_bitmap = tmp * s->page_size;
>> > +
>> > if (s->has_filter) {
>> > memory_mapping_filter(&s->list, s->begin, s->length);
>> > }
> Again from v6 10/11.
>
> These assignments now all occur without depending on a user request for
> a compressed dump (kept this way in v7 12/13 too), but they are not
> costly. The loop in get_max_mapnr() iterates over less than 10 mappings
> in the non-paging dump case, and in the paging dump case it also
> shouldn't be more than a hundred or so (as I recall from earlier
> testing). This might be worth some regression-testing (perf-wise), but
> it looks OK to me.
>
I see, moving them into "if(format...) {...}" block would be better. But, I
have no idea of "regression-testing (perf-wise)", would you mind give some hint?
--
Regards
Qiao Nuohan
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 07/13 v7] dump: add members to DumpState and init some of them
2014-01-24 1:52 ` Qiao Nuohan
@ 2014-01-24 10:00 ` Laszlo Ersek
0 siblings, 0 replies; 37+ messages in thread
From: Laszlo Ersek @ 2014-01-24 10:00 UTC (permalink / raw)
To: Qiao Nuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi,
afaerber
On 01/24/14 02:52, Qiao Nuohan wrote:
> On 01/23/2014 01:04 AM, Laszlo Ersek wrote:
>>> @@ -864,6 +884,16 @@ static int dump_init(DumpState *s, int fd, bool
>>> paging, bool has_filter,
>>> >
>>> qemu_get_guest_simple_memory_mapping(&s->list,&s->guest_phys_blocks);
>>> > }
>>> >
>>> > + s->nr_cpus = nr_cpus;
>>> > + s->page_size = TARGET_PAGE_SIZE;
>>> > + s->page_shift = ffs(s->page_size) - 1;
>>> > +
>>> > + get_max_mapnr(s);
>> Again from v6 10/11, good. The flag_flatten assignment has been dropped.
>> Initialization seems to happen in a good spot this time too.
>>
>>> > +
>>> > + uint64_t tmp;
>>> > + tmp = DIV_ROUND_UP(DIV_ROUND_UP(s->max_mapnr, CHAR_BIT),
>>> s->page_size);
>>> > + s->len_dump_bitmap = tmp * s->page_size;
>>> > +
>>> > if (s->has_filter) {
>>> > memory_mapping_filter(&s->list, s->begin, s->length);
>>> > }
>> Again from v6 10/11.
>>
>> These assignments now all occur without depending on a user request for
>> a compressed dump (kept this way in v7 12/13 too), but they are not
>> costly. The loop in get_max_mapnr() iterates over less than 10 mappings
>> in the non-paging dump case, and in the paging dump case it also
>> shouldn't be more than a hundred or so (as I recall from earlier
>> testing). This might be worth some regression-testing (perf-wise), but
>> it looks OK to me.
>>
>
> I see, moving them into "if(format...) {...}" block would be better. But, I
> have no idea of "regression-testing (perf-wise)", would you mind give
> some hint?
I meant comparing how long it would take to dump in paging mode before
this patchset, vs. after this patchset. In order to see the difference
that is introduced by get_max_mapnr() when paging is enabled.
However, please ignore this point. First, the loop is most probably
negligible even for a paging dump. Second, you could make it conditional
on compressed dumps (which force non-paging + non-filtering), where the
number of mappings is very low. And third, as I wrote in later, the loop
should be replaced anyway with an O(1) QTAILQ_LAST() access.
So please just ignore this "performance" remark.
Ultimately, what I suggest for get_max_mapnr() is:
- rebase it to guest_phys_blocks, just like the other two places (which
are now calling get_next_page()),
- use QTAILQ_LAST() in it,
- don't bother making it conditional (ie. its current call site is
fine), because:
- It'll be fast with QTAILQ_LAST(),
- guest_phys_blocks is available in any case, so you can access it
always
Thanks
Laszlo
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH 08/13 v7] dump: add API to write dump header
2014-01-17 7:46 [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (6 preceding siblings ...)
2014-01-17 7:46 ` [Qemu-devel] [PATCH 07/13 v7] dump: add members to DumpState and init some of them qiaonuohan
@ 2014-01-17 7:46 ` qiaonuohan
2014-01-22 18:07 ` Laszlo Ersek
2014-01-23 14:29 ` Ekaterina Tumanova
2014-01-17 7:46 ` [Qemu-devel] [PATCH 09/13 v7] dump: add API to write dump_bitmap qiaonuohan
` (6 subsequent siblings)
14 siblings, 2 replies; 37+ messages in thread
From: qiaonuohan @ 2014-01-17 7:46 UTC (permalink / raw)
To: lersek, stefanha, lcapitulino, afaerber, eblake
Cc: kumagai-atsushi, qiaonuohan, anderson, qemu-devel
the functions are used to write header of kdump-compressed format to vmcore.
Header of kdump-compressed format includes:
1. common header: DiskDumpHeader32 / DiskDumpHeader64
2. sub header: KdumpSubHeader32 / KdumpSubHeader64
3. extra information: only elf notes here
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
dump.c | 223 +++++++++++++++++++++++++++++++++++++++++++++++++
include/sysemu/dump.h | 96 +++++++++++++++++++++
2 files changed, 319 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index bf7d31d..a7fdc3f 100644
--- a/dump.c
+++ b/dump.c
@@ -778,6 +778,229 @@ static int buf_write_note(const void *buf, size_t size, void *opaque)
return 0;
}
+/* write common header, sub header and elf note to vmcore */
+static int create_header32(DumpState *s)
+{
+ int ret = 0;
+ DiskDumpHeader32 *dh = NULL;
+ KdumpSubHeader32 *kh = NULL;
+ size_t size;
+ int endian = s->dump_info.d_endian;
+ uint32_t block_size;
+ uint32_t sub_hdr_size;
+ uint32_t bitmap_blocks;
+ uint32_t status = 0;
+ uint64_t offset_note;
+
+ /* write common header, the version of kdump-compressed format is 6th */
+ size = sizeof(DiskDumpHeader32);
+ dh = g_malloc0(size);
+
+ strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
+ dh->header_version = cpu_convert_to_target32(6, endian);
+ block_size = s->page_size;
+ dh->block_size = cpu_convert_to_target32(block_size, endian);
+ sub_hdr_size = sizeof(struct KdumpSubHeader32) + s->note_size;
+ sub_hdr_size = DIV_ROUND_UP(sub_hdr_size, block_size);
+ dh->sub_hdr_size = cpu_convert_to_target32(sub_hdr_size, endian);
+ /* dh->max_mapnr may be truncated, full 64bit is in kh.max_mapnr_64 */
+ dh->max_mapnr = cpu_convert_to_target32(MIN(s->max_mapnr, UINT_MAX),
+ endian);
+ dh->nr_cpus = cpu_convert_to_target32(s->nr_cpus, endian);
+ bitmap_blocks = DIV_ROUND_UP(s->len_dump_bitmap, block_size) * 2;
+ dh->bitmap_blocks = cpu_convert_to_target32(bitmap_blocks, endian);
+ memcpy(&(dh->utsname.machine), "i686", 4);
+
+ if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
+ status |= DUMP_DH_COMPRESSED_ZLIB;
+ }
+#ifdef CONFIG_LZO
+ if (s->flag_compress & DUMP_DH_COMPRESSED_LZO) {
+ status |= DUMP_DH_COMPRESSED_LZO;
+ }
+#endif
+#ifdef CONFIG_SNAPPY
+ if (s->flag_compress & DUMP_DH_COMPRESSED_SNAPPY) {
+ status |= DUMP_DH_COMPRESSED_SNAPPY;
+ }
+#endif
+ dh->status = cpu_convert_to_target32(status, endian);
+
+ if (write_buffer(s->fd, 0, dh, size) < 0) {
+ dump_error(s, "dump: failed to write disk dump header.\n");
+ ret = -1;
+ goto out;
+ }
+
+ /* write sub header */
+ size = sizeof(KdumpSubHeader32);
+ kh = g_malloc0(size);
+
+ /* 64bit max_mapnr_64 */
+ kh->max_mapnr_64 = cpu_convert_to_target64(s->max_mapnr, endian);
+ kh->phys_base = cpu_convert_to_target32(PHYS_BASE, endian);
+ kh->dump_level = cpu_convert_to_target32(DUMP_LEVEL, endian);
+
+ offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
+ kh->offset_note = cpu_convert_to_target64(offset_note, endian);
+ kh->note_size = cpu_convert_to_target32(s->note_size, endian);
+
+ if (write_buffer(s->fd, DISKDUMP_HEADER_BLOCKS *
+ block_size, kh, size) < 0) {
+ dump_error(s, "dump: failed to write kdump sub header.\n");
+ ret = -1;
+ goto out;
+ }
+
+ /* write note */
+ s->note_buf = g_malloc0(s->note_size);
+ s->note_buf_offset = 0;
+
+ /* use s->note_buf to store notes temporarily */
+ if (write_elf32_notes(buf_write_note, s) < 0) {
+ ret = -1;
+ goto out;
+ }
+
+ if (write_buffer(s->fd, offset_note, s->note_buf,
+ s->note_size) < 0) {
+ dump_error(s, "dump: failed to write notes");
+ ret = -1;
+ goto out;
+ }
+
+ /* get offset of dump_bitmap */
+ s->offset_dump_bitmap = (DISKDUMP_HEADER_BLOCKS + sub_hdr_size) *
+ block_size;
+
+ /* get offset of page */
+ s->offset_page = (DISKDUMP_HEADER_BLOCKS + sub_hdr_size + bitmap_blocks) *
+ block_size;
+
+out:
+ g_free(dh);
+ g_free(kh);
+ g_free(s->note_buf);
+
+ return ret;
+}
+
+/* write common header, sub header and elf note to vmcore */
+static int create_header64(DumpState *s)
+{
+ int ret = 0;
+ DiskDumpHeader64 *dh = NULL;
+ KdumpSubHeader64 *kh = NULL;
+ size_t size;
+ int endian = s->dump_info.d_endian;
+ uint32_t block_size;
+ uint32_t sub_hdr_size;
+ uint32_t bitmap_blocks;
+ uint32_t status = 0;
+ uint64_t offset_note;
+
+ /* write common header, the version of kdump-compressed format is 6th */
+ size = sizeof(DiskDumpHeader64);
+ dh = g_malloc0(size);
+
+ strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
+ dh->header_version = cpu_convert_to_target32(6, endian);
+ block_size = s->page_size;
+ dh->block_size = cpu_convert_to_target32(block_size, endian);
+ sub_hdr_size = sizeof(struct KdumpSubHeader64) + s->note_size;
+ sub_hdr_size = DIV_ROUND_UP(sub_hdr_size, dh->block_size);
+ dh->sub_hdr_size = cpu_convert_to_target32(sub_hdr_size, endian);
+ /* dh->max_mapnr may be truncated, full 64bit is in kh.max_mapnr_64 */
+ dh->max_mapnr = cpu_convert_to_target32(MIN(s->max_mapnr, UINT_MAX),
+ endian);
+ dh->nr_cpus = cpu_convert_to_target32(s->nr_cpus, endian);
+ bitmap_blocks = DIV_ROUND_UP(s->len_dump_bitmap, dh->block_size) * 2;
+ dh->bitmap_blocks = cpu_convert_to_target32(bitmap_blocks, endian);
+ memcpy(&(dh->utsname.machine), "x86_64", 6);
+
+ if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
+ status |= DUMP_DH_COMPRESSED_ZLIB;
+ }
+#ifdef CONFIG_LZO
+ if (s->flag_compress & DUMP_DH_COMPRESSED_LZO) {
+ status |= DUMP_DH_COMPRESSED_LZO;
+ }
+#endif
+#ifdef CONFIG_SNAPPY
+ if (s->flag_compress & DUMP_DH_COMPRESSED_SNAPPY) {
+ status |= DUMP_DH_COMPRESSED_SNAPPY;
+ }
+#endif
+ dh->status = cpu_convert_to_target32(status, endian);
+
+ if (write_buffer(s->fd, 0, dh, size) < 0) {
+ dump_error(s, "dump: failed to write disk dump header.\n");
+ ret = -1;
+ goto out;
+ }
+
+ /* write sub header */
+ size = sizeof(KdumpSubHeader64);
+ kh = g_malloc0(size);
+
+ /* 64bit max_mapnr_64 */
+ kh->max_mapnr_64 = cpu_convert_to_target64(s->max_mapnr, endian);
+ kh->phys_base = cpu_convert_to_target64(PHYS_BASE, endian);
+ kh->dump_level = cpu_convert_to_target32(DUMP_LEVEL, endian);
+
+ offset_note = DISKDUMP_HEADER_BLOCKS * dh->block_size + size;
+ kh->offset_note = cpu_convert_to_target64(offset_note, endian);
+ kh->note_size = cpu_convert_to_target64(s->note_size, endian);
+
+ if (write_buffer(s->fd, DISKDUMP_HEADER_BLOCKS *
+ dh->block_size, kh, size) < 0) {
+ dump_error(s, "dump: failed to write kdump sub header.\n");
+ ret = -1;
+ goto out;
+ }
+
+ /* write note */
+ s->note_buf = g_malloc0(s->note_size);
+ s->note_buf_offset = 0;
+
+ /* use s->note_buf to store notes temporarily */
+ if (write_elf64_notes(buf_write_note, s) < 0) {
+ ret = -1;
+ goto out;
+ }
+
+ if (write_buffer(s->fd, offset_note, s->note_buf,
+ s->note_size) < 0) {
+ dump_error(s, "dump: failed to write notes");
+ ret = -1;
+ goto out;
+ }
+
+ /* get offset of dump_bitmap */
+ s->offset_dump_bitmap = (DISKDUMP_HEADER_BLOCKS + sub_hdr_size) *
+ block_size;
+
+ /* get offset of page */
+ s->offset_page = (DISKDUMP_HEADER_BLOCKS + sub_hdr_size + bitmap_blocks) *
+ block_size;
+
+out:
+ g_free(dh);
+ g_free(kh);
+ g_free(s->note_buf);
+
+ return ret;
+}
+
+static int write_dump_header(DumpState *s)
+{
+ if (s->dump_info.d_machine == EM_386) {
+ return create_header32(s);
+ } else {
+ return create_header64(s);
+ }
+}
+
static ram_addr_t get_start_block(DumpState *s)
{
GuestPhysBlock *block;
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 995bf47..dfee238 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -27,6 +27,19 @@
#define pfn_to_paddr(X, page_shift) \
(((unsigned long long)(X) + ARCH_PFN_OFFSET) << (page_shift))
+/*
+ * flag for compressed format
+ */
+#define DUMP_DH_COMPRESSED_ZLIB (0x1)
+#define DUMP_DH_COMPRESSED_LZO (0x2)
+#define DUMP_DH_COMPRESSED_SNAPPY (0x4)
+
+#define KDUMP_SIGNATURE "KDUMP "
+#define SIG_LEN (sizeof(KDUMP_SIGNATURE) - 1)
+#define PHYS_BASE (0)
+#define DUMP_LEVEL (1)
+#define DISKDUMP_HEADER_BLOCKS (1)
+
typedef struct ArchDumpInfo {
int d_machine; /* Architecture */
int d_endian; /* ELFDATA2LSB or ELFDATA2MSB */
@@ -44,6 +57,89 @@ typedef struct QEMU_PACKED MakedumpfileDataHeader {
int64_t buf_size;
} MakedumpfileDataHeader;
+typedef struct QEMU_PACKED NewUtsname {
+ char sysname[65];
+ char nodename[65];
+ char release[65];
+ char version[65];
+ char machine[65];
+ char domainname[65];
+} NewUtsname;
+
+typedef struct QEMU_PACKED DiskDumpHeader32 {
+ char signature[SIG_LEN]; /* = "KDUMP " */
+ uint32_t header_version; /* Dump header version */
+ NewUtsname utsname; /* copy of system_utsname */
+ char timestamp[10]; /* Time stamp */
+ uint32_t status; /* Above flags */
+ uint32_t block_size; /* Size of a block in byte */
+ uint32_t sub_hdr_size; /* Size of arch dependent header in block */
+ uint32_t bitmap_blocks; /* Size of Memory bitmap in block */
+ uint32_t max_mapnr; /* = max_mapnr ,
+ obsoleted in header_version 6 */
+ uint32_t total_ram_blocks; /* Number of blocks should be written */
+ uint32_t device_blocks; /* Number of total blocks in dump device */
+ uint32_t written_blocks; /* Number of written blocks */
+ uint32_t current_cpu; /* CPU# which handles dump */
+ uint32_t nr_cpus; /* Number of CPUs */
+} DiskDumpHeader32;
+
+typedef struct QEMU_PACKED DiskDumpHeader64 {
+ char signature[SIG_LEN]; /* = "KDUMP " */
+ uint32_t header_version; /* Dump header version */
+ NewUtsname utsname; /* copy of system_utsname */
+ char timestamp[22]; /* Time stamp */
+ uint32_t status; /* Above flags */
+ uint32_t block_size; /* Size of a block in byte */
+ uint32_t sub_hdr_size; /* Size of arch dependent header in block */
+ uint32_t bitmap_blocks; /* Size of Memory bitmap in block */
+ uint32_t max_mapnr; /* = max_mapnr,
+ obsoleted in header_version 6 */
+ uint32_t total_ram_blocks; /* Number of blocks should be written */
+ uint32_t device_blocks; /* Number of total blocks in dump device */
+ uint32_t written_blocks; /* Number of written blocks */
+ uint32_t current_cpu; /* CPU# which handles dump */
+ uint32_t nr_cpus; /* Number of CPUs */
+} DiskDumpHeader64;
+
+typedef struct QEMU_PACKED KdumpSubHeader32 {
+ uint32_t phys_base;
+ uint32_t dump_level; /* header_version 1 and later */
+ uint32_t split; /* header_version 2 and later */
+ uint32_t start_pfn; /* header_version 2 and later,
+ obsoleted in header_version 6 */
+ uint32_t end_pfn; /* header_version 2 and later,
+ obsoleted in header_version 6 */
+ uint64_t offset_vmcoreinfo; /* header_version 3 and later */
+ uint32_t size_vmcoreinfo; /* header_version 3 and later */
+ uint64_t offset_note; /* header_version 4 and later */
+ uint32_t note_size; /* header_version 4 and later */
+ uint64_t offset_eraseinfo; /* header_version 5 and later */
+ uint32_t size_eraseinfo; /* header_version 5 and later */
+ uint64_t start_pfn_64; /* header_version 6 and later */
+ uint64_t end_pfn_64; /* header_version 6 and later */
+ uint64_t max_mapnr_64; /* header_version 6 and later */
+} KdumpSubHeader32;
+
+typedef struct QEMU_PACKED KdumpSubHeader64 {
+ uint64_t phys_base;
+ uint32_t dump_level; /* header_version 1 and later */
+ uint32_t split; /* header_version 2 and later */
+ uint64_t start_pfn; /* header_version 2 and later,
+ obsoleted in header_version 6 */
+ uint64_t end_pfn; /* header_version 2 and later,
+ obsoleted in header_version 6 */
+ uint64_t offset_vmcoreinfo; /* header_version 3 and later */
+ uint64_t size_vmcoreinfo; /* header_version 3 and later */
+ uint64_t offset_note; /* header_version 4 and later */
+ uint64_t note_size; /* header_version 4 and later */
+ uint64_t offset_eraseinfo; /* header_version 5 and later */
+ uint64_t size_eraseinfo; /* header_version 5 and later */
+ uint64_t start_pfn_64; /* header_version 6 and later */
+ uint64_t end_pfn_64; /* header_version 6 and later */
+ uint64_t max_mapnr_64; /* header_version 6 and later */
+} KdumpSubHeader64;
+
struct GuestPhysBlockList; /* memory_mapping.h */
int cpu_get_dump_info(ArchDumpInfo *info,
const struct GuestPhysBlockList *guest_phys_blocks);
--
1.7.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 08/13 v7] dump: add API to write dump header
2014-01-17 7:46 ` [Qemu-devel] [PATCH 08/13 v7] dump: add API to write dump header qiaonuohan
@ 2014-01-22 18:07 ` Laszlo Ersek
2014-01-23 14:29 ` Ekaterina Tumanova
1 sibling, 0 replies; 37+ messages in thread
From: Laszlo Ersek @ 2014-01-22 18:07 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi,
afaerber
remarks below
On 01/17/14 08:46, qiaonuohan wrote:
> the functions are used to write header of kdump-compressed format to vmcore.
> Header of kdump-compressed format includes:
> 1. common header: DiskDumpHeader32 / DiskDumpHeader64
> 2. sub header: KdumpSubHeader32 / KdumpSubHeader64
> 3. extra information: only elf notes here
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
> dump.c | 223 +++++++++++++++++++++++++++++++++++++++++++++++++
> include/sysemu/dump.h | 96 +++++++++++++++++++++
> 2 files changed, 319 insertions(+), 0 deletions(-)
>
> diff --git a/dump.c b/dump.c
> index bf7d31d..a7fdc3f 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -778,6 +778,229 @@ static int buf_write_note(const void *buf, size_t size, void *opaque)
> return 0;
> }
>
> +/* write common header, sub header and elf note to vmcore */
> +static int create_header32(DumpState *s)
> +{
> + int ret = 0;
> + DiskDumpHeader32 *dh = NULL;
> + KdumpSubHeader32 *kh = NULL;
> + size_t size;
> + int endian = s->dump_info.d_endian;
> + uint32_t block_size;
> + uint32_t sub_hdr_size;
> + uint32_t bitmap_blocks;
> + uint32_t status = 0;
> + uint64_t offset_note;
> +
> + /* write common header, the version of kdump-compressed format is 6th */
thanks for fixing the comment
> + size = sizeof(DiskDumpHeader32);
> + dh = g_malloc0(size);
> +
> + strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
> + dh->header_version = cpu_convert_to_target32(6, endian);
> + block_size = s->page_size;
> + dh->block_size = cpu_convert_to_target32(block_size, endian);
> + sub_hdr_size = sizeof(struct KdumpSubHeader32) + s->note_size;
> + sub_hdr_size = DIV_ROUND_UP(sub_hdr_size, block_size);
> + dh->sub_hdr_size = cpu_convert_to_target32(sub_hdr_size, endian);
> + /* dh->max_mapnr may be truncated, full 64bit is in kh.max_mapnr_64 */
> + dh->max_mapnr = cpu_convert_to_target32(MIN(s->max_mapnr, UINT_MAX),
> + endian);
> + dh->nr_cpus = cpu_convert_to_target32(s->nr_cpus, endian);
> + bitmap_blocks = DIV_ROUND_UP(s->len_dump_bitmap, block_size) * 2;
> + dh->bitmap_blocks = cpu_convert_to_target32(bitmap_blocks, endian);
> + memcpy(&(dh->utsname.machine), "i686", 4);
> +
> + if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
> + status |= DUMP_DH_COMPRESSED_ZLIB;
> + }
> +#ifdef CONFIG_LZO
> + if (s->flag_compress & DUMP_DH_COMPRESSED_LZO) {
> + status |= DUMP_DH_COMPRESSED_LZO;
> + }
> +#endif
> +#ifdef CONFIG_SNAPPY
> + if (s->flag_compress & DUMP_DH_COMPRESSED_SNAPPY) {
> + status |= DUMP_DH_COMPRESSED_SNAPPY;
> + }
> +#endif
> + dh->status = cpu_convert_to_target32(status, endian);
taking care about endianness looks reasonable to me thus far
> +
> + if (write_buffer(s->fd, 0, dh, size) < 0) {
> + dump_error(s, "dump: failed to write disk dump header.\n");
> + ret = -1;
> + goto out;
> + }
calling dump_error() for cleanup, looks good
(of course I'm not checking completeness, either for the endianness
question or cleanup-on-error coverage, but they do seem sound)
> +
> + /* write sub header */
> + size = sizeof(KdumpSubHeader32);
> + kh = g_malloc0(size);
> +
> + /* 64bit max_mapnr_64 */
> + kh->max_mapnr_64 = cpu_convert_to_target64(s->max_mapnr, endian);
> + kh->phys_base = cpu_convert_to_target32(PHYS_BASE, endian);
> + kh->dump_level = cpu_convert_to_target32(DUMP_LEVEL, endian);
> +
> + offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
> + kh->offset_note = cpu_convert_to_target64(offset_note, endian);
> + kh->note_size = cpu_convert_to_target32(s->note_size, endian);
looks good
> +
> + if (write_buffer(s->fd, DISKDUMP_HEADER_BLOCKS *
> + block_size, kh, size) < 0) {
OK, flag_flatten is dropped. Plus the multiplication that I asked for is
being done; thanks.
> + dump_error(s, "dump: failed to write kdump sub header.\n");
> + ret = -1;
> + goto out;
> + }
dump_error() looks good
> +
> + /* write note */
> + s->note_buf = g_malloc0(s->note_size);
zeroing it out is now consistent with the 64-bit version
> + s->note_buf_offset = 0;
> +
> + /* use s->note_buf to store notes temporarily */
> + if (write_elf32_notes(buf_write_note, s) < 0) {
> + ret = -1;
> + goto out;
> + }
> +
> + if (write_buffer(s->fd, offset_note, s->note_buf,
> + s->note_size) < 0) {
> + dump_error(s, "dump: failed to write notes");
> + ret = -1;
> + goto out;
> + }
OK, you opted for the third option here, 'allocating "s->note_buf" with
g_malloc0()', thanks.
> +
> + /* get offset of dump_bitmap */
> + s->offset_dump_bitmap = (DISKDUMP_HEADER_BLOCKS + sub_hdr_size) *
> + block_size;
> +
> + /* get offset of page */
> + s->offset_page = (DISKDUMP_HEADER_BLOCKS + sub_hdr_size + bitmap_blocks) *
> + block_size;
> +
> +out:
> + g_free(dh);
> + g_free(kh);
> + g_free(s->note_buf);
> +
> + return ret;
> +}
The 32-bit version looks OK to me, the error handling too. Basically the
promise is that this function will have called dump_error() on error
exit. write_elf32_notes() does that internally (and we take it into
account correctly), for any other (ie. write_buffer()) errors we make
the call ourselves.
I also sought to verify that dh->foo and kh->bar fields are only
assigned-to, never read back and used for any calculation.
> +
> +/* write common header, sub header and elf note to vmcore */
> +static int create_header64(DumpState *s)
> +{
> + int ret = 0;
> + DiskDumpHeader64 *dh = NULL;
> + KdumpSubHeader64 *kh = NULL;
> + size_t size;
> + int endian = s->dump_info.d_endian;
> + uint32_t block_size;
> + uint32_t sub_hdr_size;
> + uint32_t bitmap_blocks;
> + uint32_t status = 0;
> + uint64_t offset_note;
> +
> + /* write common header, the version of kdump-compressed format is 6th */
thanks for fixing the comment
> + size = sizeof(DiskDumpHeader64);
> + dh = g_malloc0(size);
> +
> + strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
> + dh->header_version = cpu_convert_to_target32(6, endian);
> + block_size = s->page_size;
> + dh->block_size = cpu_convert_to_target32(block_size, endian);
> + sub_hdr_size = sizeof(struct KdumpSubHeader64) + s->note_size;
> + sub_hdr_size = DIV_ROUND_UP(sub_hdr_size, dh->block_size);
Unfortunately, this is a bug, introduced by the new endian-awareness.
You should divide here by "block_size", not by "dh->block_size", because
that's already converted to target endianness.
I found this problem by extracting the 32-bit and 64-bit versions of the
function (after applying this patch) to separate text files, and diffing
them against each other.
> + dh->sub_hdr_size = cpu_convert_to_target32(sub_hdr_size, endian);
> + /* dh->max_mapnr may be truncated, full 64bit is in kh.max_mapnr_64 */
> + dh->max_mapnr = cpu_convert_to_target32(MIN(s->max_mapnr, UINT_MAX),
> + endian);
> + dh->nr_cpus = cpu_convert_to_target32(s->nr_cpus, endian);
> + bitmap_blocks = DIV_ROUND_UP(s->len_dump_bitmap, dh->block_size) * 2;
Same problem as above: divisor should be host-endian.
> + dh->bitmap_blocks = cpu_convert_to_target32(bitmap_blocks, endian);
> + memcpy(&(dh->utsname.machine), "x86_64", 6);
> +
> + if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
> + status |= DUMP_DH_COMPRESSED_ZLIB;
> + }
> +#ifdef CONFIG_LZO
> + if (s->flag_compress & DUMP_DH_COMPRESSED_LZO) {
> + status |= DUMP_DH_COMPRESSED_LZO;
> + }
> +#endif
> +#ifdef CONFIG_SNAPPY
> + if (s->flag_compress & DUMP_DH_COMPRESSED_SNAPPY) {
> + status |= DUMP_DH_COMPRESSED_SNAPPY;
> + }
> +#endif
> + dh->status = cpu_convert_to_target32(status, endian);
> +
> + if (write_buffer(s->fd, 0, dh, size) < 0) {
> + dump_error(s, "dump: failed to write disk dump header.\n");
> + ret = -1;
> + goto out;
> + }
> +
> + /* write sub header */
> + size = sizeof(KdumpSubHeader64);
> + kh = g_malloc0(size);
> +
> + /* 64bit max_mapnr_64 */
> + kh->max_mapnr_64 = cpu_convert_to_target64(s->max_mapnr, endian);
> + kh->phys_base = cpu_convert_to_target64(PHYS_BASE, endian);
> + kh->dump_level = cpu_convert_to_target32(DUMP_LEVEL, endian);
> +
> + offset_note = DISKDUMP_HEADER_BLOCKS * dh->block_size + size;
Again, multiplier should be host-endian.
> + kh->offset_note = cpu_convert_to_target64(offset_note, endian);
> + kh->note_size = cpu_convert_to_target64(s->note_size, endian);
> +
> + if (write_buffer(s->fd, DISKDUMP_HEADER_BLOCKS *
> + dh->block_size, kh, size) < 0) {
Ditto.
(These bugs are unnoticeable in testing if your host and target
endiannesses match.)
The rest of the patch seems fine. Please replace these four instances of
"dh->block_size" with the host-endian local variable.
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 08/13 v7] dump: add API to write dump header
2014-01-17 7:46 ` [Qemu-devel] [PATCH 08/13 v7] dump: add API to write dump header qiaonuohan
2014-01-22 18:07 ` Laszlo Ersek
@ 2014-01-23 14:29 ` Ekaterina Tumanova
1 sibling, 0 replies; 37+ messages in thread
From: Ekaterina Tumanova @ 2014-01-23 14:29 UTC (permalink / raw)
To: qemu-devel
On 01/17/2014 11:46 AM, qiaonuohan wrote:
> the functions are used to write header of kdump-compressed format to vmcore.
> Header of kdump-compressed format includes:
> 1. common header: DiskDumpHeader32 / DiskDumpHeader64
> 2. sub header: KdumpSubHeader32 / KdumpSubHeader64
> 3. extra information: only elf notes here
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
> dump.c | 223 +++++++++++++++++++++++++++++++++++++++++++++++++
> include/sysemu/dump.h | 96 +++++++++++++++++++++
> 2 files changed, 319 insertions(+), 0 deletions(-)
>
> diff --git a/dump.c b/dump.c
> index bf7d31d..a7fdc3f 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -778,6 +778,229 @@ static int buf_write_note(const void *buf, size_t size, void *opaque)
> return 0;
> }
>
> +/* write common header, sub header and elf note to vmcore */
> +static int create_header32(DumpState *s)
> +{
> + int ret = 0;
> + DiskDumpHeader32 *dh = NULL;
> + KdumpSubHeader32 *kh = NULL;
> + size_t size;
> + int endian = s->dump_info.d_endian;
> + uint32_t block_size;
> + uint32_t sub_hdr_size;
> + uint32_t bitmap_blocks;
> + uint32_t status = 0;
> + uint64_t offset_note;
> +
> + /* write common header, the version of kdump-compressed format is 6th */
> + size = sizeof(DiskDumpHeader32);
> + dh = g_malloc0(size);
> +
> + strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
> + dh->header_version = cpu_convert_to_target32(6, endian);
> + block_size = s->page_size;
> + dh->block_size = cpu_convert_to_target32(block_size, endian);
> + sub_hdr_size = sizeof(struct KdumpSubHeader32) + s->note_size;
> + sub_hdr_size = DIV_ROUND_UP(sub_hdr_size, block_size);
> + dh->sub_hdr_size = cpu_convert_to_target32(sub_hdr_size, endian);
> + /* dh->max_mapnr may be truncated, full 64bit is in kh.max_mapnr_64 */
> + dh->max_mapnr = cpu_convert_to_target32(MIN(s->max_mapnr, UINT_MAX),
> + endian);
> + dh->nr_cpus = cpu_convert_to_target32(s->nr_cpus, endian);
> + bitmap_blocks = DIV_ROUND_UP(s->len_dump_bitmap, block_size) * 2;
> + dh->bitmap_blocks = cpu_convert_to_target32(bitmap_blocks, endian);
> + memcpy(&(dh->utsname.machine), "i686", 4);
I actually tried to use your patch on s390x, and the only problem I
found is that you hardcode the architecture into the header in
arch-independent dump.c file here. Then it becomes unreadable for crash
utility on different architecture. If would be great, if you could
somehow get rid of hardcoding the arch in
create_header32/create_header64 and instead use the target arch.
> +
> + if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
> + status |= DUMP_DH_COMPRESSED_ZLIB;
> + }
> +#ifdef CONFIG_LZO
> + if (s->flag_compress & DUMP_DH_COMPRESSED_LZO) {
> + status |= DUMP_DH_COMPRESSED_LZO;
> + }
> +#endif
> +#ifdef CONFIG_SNAPPY
> + if (s->flag_compress & DUMP_DH_COMPRESSED_SNAPPY) {
> + status |= DUMP_DH_COMPRESSED_SNAPPY;
> + }
> +#endif
> + dh->status = cpu_convert_to_target32(status, endian);
> +
> + if (write_buffer(s->fd, 0, dh, size) < 0) {
> + dump_error(s, "dump: failed to write disk dump header.\n");
> + ret = -1;
> + goto out;
> + }
> +
> + /* write sub header */
> + size = sizeof(KdumpSubHeader32);
> + kh = g_malloc0(size);
> +
> + /* 64bit max_mapnr_64 */
> + kh->max_mapnr_64 = cpu_convert_to_target64(s->max_mapnr, endian);
> + kh->phys_base = cpu_convert_to_target32(PHYS_BASE, endian);
> + kh->dump_level = cpu_convert_to_target32(DUMP_LEVEL, endian);
> +
> + offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
> + kh->offset_note = cpu_convert_to_target64(offset_note, endian);
> + kh->note_size = cpu_convert_to_target32(s->note_size, endian);
> +
> + if (write_buffer(s->fd, DISKDUMP_HEADER_BLOCKS *
> + block_size, kh, size) < 0) {
> + dump_error(s, "dump: failed to write kdump sub header.\n");
> + ret = -1;
> + goto out;
> + }
> +
> + /* write note */
> + s->note_buf = g_malloc0(s->note_size);
> + s->note_buf_offset = 0;
> +
> + /* use s->note_buf to store notes temporarily */
> + if (write_elf32_notes(buf_write_note, s) < 0) {
> + ret = -1;
> + goto out;
> + }
> +
> + if (write_buffer(s->fd, offset_note, s->note_buf,
> + s->note_size) < 0) {
> + dump_error(s, "dump: failed to write notes");
> + ret = -1;
> + goto out;
> + }
> +
> + /* get offset of dump_bitmap */
> + s->offset_dump_bitmap = (DISKDUMP_HEADER_BLOCKS + sub_hdr_size) *
> + block_size;
> +
> + /* get offset of page */
> + s->offset_page = (DISKDUMP_HEADER_BLOCKS + sub_hdr_size + bitmap_blocks) *
> + block_size;
> +
> +out:
> + g_free(dh);
> + g_free(kh);
> + g_free(s->note_buf);
> +
> + return ret;
> +}
> +
> +/* write common header, sub header and elf note to vmcore */
> +static int create_header64(DumpState *s)
> +{
> + int ret = 0;
> + DiskDumpHeader64 *dh = NULL;
> + KdumpSubHeader64 *kh = NULL;
> + size_t size;
> + int endian = s->dump_info.d_endian;
> + uint32_t block_size;
> + uint32_t sub_hdr_size;
> + uint32_t bitmap_blocks;
> + uint32_t status = 0;
> + uint64_t offset_note;
> +
> + /* write common header, the version of kdump-compressed format is 6th */
> + size = sizeof(DiskDumpHeader64);
> + dh = g_malloc0(size);
> +
> + strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
> + dh->header_version = cpu_convert_to_target32(6, endian);
> + block_size = s->page_size;
> + dh->block_size = cpu_convert_to_target32(block_size, endian);
> + sub_hdr_size = sizeof(struct KdumpSubHeader64) + s->note_size;
> + sub_hdr_size = DIV_ROUND_UP(sub_hdr_size, dh->block_size);
> + dh->sub_hdr_size = cpu_convert_to_target32(sub_hdr_size, endian);
> + /* dh->max_mapnr may be truncated, full 64bit is in kh.max_mapnr_64 */
> + dh->max_mapnr = cpu_convert_to_target32(MIN(s->max_mapnr, UINT_MAX),
> + endian);
> + dh->nr_cpus = cpu_convert_to_target32(s->nr_cpus, endian);
> + bitmap_blocks = DIV_ROUND_UP(s->len_dump_bitmap, dh->block_size) * 2;
> + dh->bitmap_blocks = cpu_convert_to_target32(bitmap_blocks, endian);
> + memcpy(&(dh->utsname.machine), "x86_64", 6);
Same problem here
> +
> + if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
> + status |= DUMP_DH_COMPRESSED_ZLIB;
> + }
> +#ifdef CONFIG_LZO
> + if (s->flag_compress & DUMP_DH_COMPRESSED_LZO) {
> + status |= DUMP_DH_COMPRESSED_LZO;
> + }
> +#endif
> +#ifdef CONFIG_SNAPPY
> + if (s->flag_compress & DUMP_DH_COMPRESSED_SNAPPY) {
> + status |= DUMP_DH_COMPRESSED_SNAPPY;
> + }
> +#endif
> + dh->status = cpu_convert_to_target32(status, endian);
> +
> + if (write_buffer(s->fd, 0, dh, size) < 0) {
> + dump_error(s, "dump: failed to write disk dump header.\n");
> + ret = -1;
> + goto out;
> + }
> +
> + /* write sub header */
> + size = sizeof(KdumpSubHeader64);
> + kh = g_malloc0(size);
> +
> + /* 64bit max_mapnr_64 */
> + kh->max_mapnr_64 = cpu_convert_to_target64(s->max_mapnr, endian);
> + kh->phys_base = cpu_convert_to_target64(PHYS_BASE, endian);
> + kh->dump_level = cpu_convert_to_target32(DUMP_LEVEL, endian);
> +
> + offset_note = DISKDUMP_HEADER_BLOCKS * dh->block_size + size;
> + kh->offset_note = cpu_convert_to_target64(offset_note, endian);
> + kh->note_size = cpu_convert_to_target64(s->note_size, endian);
> +
> + if (write_buffer(s->fd, DISKDUMP_HEADER_BLOCKS *
> + dh->block_size, kh, size) < 0) {
> + dump_error(s, "dump: failed to write kdump sub header.\n");
> + ret = -1;
> + goto out;
> + }
> +
> + /* write note */
> + s->note_buf = g_malloc0(s->note_size);
> + s->note_buf_offset = 0;
> +
> + /* use s->note_buf to store notes temporarily */
> + if (write_elf64_notes(buf_write_note, s) < 0) {
> + ret = -1;
> + goto out;
> + }
> +
> + if (write_buffer(s->fd, offset_note, s->note_buf,
> + s->note_size) < 0) {
> + dump_error(s, "dump: failed to write notes");
> + ret = -1;
> + goto out;
> + }
> +
> + /* get offset of dump_bitmap */
> + s->offset_dump_bitmap = (DISKDUMP_HEADER_BLOCKS + sub_hdr_size) *
> + block_size;
> +
> + /* get offset of page */
> + s->offset_page = (DISKDUMP_HEADER_BLOCKS + sub_hdr_size + bitmap_blocks) *
> + block_size;
> +
> +out:
> + g_free(dh);
> + g_free(kh);
> + g_free(s->note_buf);
> +
> + return ret;
> +}
> +
> +static int write_dump_header(DumpState *s)
> +{
> + if (s->dump_info.d_machine == EM_386) {
> + return create_header32(s);
> + } else {
> + return create_header64(s);
> + }
> +}
> +
> static ram_addr_t get_start_block(DumpState *s)
> {
> GuestPhysBlock *block;
> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
> index 995bf47..dfee238 100644
> --- a/include/sysemu/dump.h
> +++ b/include/sysemu/dump.h
> @@ -27,6 +27,19 @@
> #define pfn_to_paddr(X, page_shift) \
> (((unsigned long long)(X) + ARCH_PFN_OFFSET) << (page_shift))
>
> +/*
> + * flag for compressed format
> + */
> +#define DUMP_DH_COMPRESSED_ZLIB (0x1)
> +#define DUMP_DH_COMPRESSED_LZO (0x2)
> +#define DUMP_DH_COMPRESSED_SNAPPY (0x4)
> +
> +#define KDUMP_SIGNATURE "KDUMP "
> +#define SIG_LEN (sizeof(KDUMP_SIGNATURE) - 1)
> +#define PHYS_BASE (0)
> +#define DUMP_LEVEL (1)
> +#define DISKDUMP_HEADER_BLOCKS (1)
> +
> typedef struct ArchDumpInfo {
> int d_machine; /* Architecture */
> int d_endian; /* ELFDATA2LSB or ELFDATA2MSB */
> @@ -44,6 +57,89 @@ typedef struct QEMU_PACKED MakedumpfileDataHeader {
> int64_t buf_size;
> } MakedumpfileDataHeader;
>
> +typedef struct QEMU_PACKED NewUtsname {
> + char sysname[65];
> + char nodename[65];
> + char release[65];
> + char version[65];
> + char machine[65];
> + char domainname[65];
> +} NewUtsname;
> +
> +typedef struct QEMU_PACKED DiskDumpHeader32 {
> + char signature[SIG_LEN]; /* = "KDUMP " */
> + uint32_t header_version; /* Dump header version */
> + NewUtsname utsname; /* copy of system_utsname */
> + char timestamp[10]; /* Time stamp */
> + uint32_t status; /* Above flags */
> + uint32_t block_size; /* Size of a block in byte */
> + uint32_t sub_hdr_size; /* Size of arch dependent header in block */
> + uint32_t bitmap_blocks; /* Size of Memory bitmap in block */
> + uint32_t max_mapnr; /* = max_mapnr ,
> + obsoleted in header_version 6 */
> + uint32_t total_ram_blocks; /* Number of blocks should be written */
> + uint32_t device_blocks; /* Number of total blocks in dump device */
> + uint32_t written_blocks; /* Number of written blocks */
> + uint32_t current_cpu; /* CPU# which handles dump */
> + uint32_t nr_cpus; /* Number of CPUs */
> +} DiskDumpHeader32;
> +
> +typedef struct QEMU_PACKED DiskDumpHeader64 {
> + char signature[SIG_LEN]; /* = "KDUMP " */
> + uint32_t header_version; /* Dump header version */
> + NewUtsname utsname; /* copy of system_utsname */
> + char timestamp[22]; /* Time stamp */
> + uint32_t status; /* Above flags */
> + uint32_t block_size; /* Size of a block in byte */
> + uint32_t sub_hdr_size; /* Size of arch dependent header in block */
> + uint32_t bitmap_blocks; /* Size of Memory bitmap in block */
> + uint32_t max_mapnr; /* = max_mapnr,
> + obsoleted in header_version 6 */
> + uint32_t total_ram_blocks; /* Number of blocks should be written */
> + uint32_t device_blocks; /* Number of total blocks in dump device */
> + uint32_t written_blocks; /* Number of written blocks */
> + uint32_t current_cpu; /* CPU# which handles dump */
> + uint32_t nr_cpus; /* Number of CPUs */
> +} DiskDumpHeader64;
> +
> +typedef struct QEMU_PACKED KdumpSubHeader32 {
> + uint32_t phys_base;
> + uint32_t dump_level; /* header_version 1 and later */
> + uint32_t split; /* header_version 2 and later */
> + uint32_t start_pfn; /* header_version 2 and later,
> + obsoleted in header_version 6 */
> + uint32_t end_pfn; /* header_version 2 and later,
> + obsoleted in header_version 6 */
> + uint64_t offset_vmcoreinfo; /* header_version 3 and later */
> + uint32_t size_vmcoreinfo; /* header_version 3 and later */
> + uint64_t offset_note; /* header_version 4 and later */
> + uint32_t note_size; /* header_version 4 and later */
> + uint64_t offset_eraseinfo; /* header_version 5 and later */
> + uint32_t size_eraseinfo; /* header_version 5 and later */
> + uint64_t start_pfn_64; /* header_version 6 and later */
> + uint64_t end_pfn_64; /* header_version 6 and later */
> + uint64_t max_mapnr_64; /* header_version 6 and later */
> +} KdumpSubHeader32;
> +
> +typedef struct QEMU_PACKED KdumpSubHeader64 {
> + uint64_t phys_base;
> + uint32_t dump_level; /* header_version 1 and later */
> + uint32_t split; /* header_version 2 and later */
> + uint64_t start_pfn; /* header_version 2 and later,
> + obsoleted in header_version 6 */
> + uint64_t end_pfn; /* header_version 2 and later,
> + obsoleted in header_version 6 */
> + uint64_t offset_vmcoreinfo; /* header_version 3 and later */
> + uint64_t size_vmcoreinfo; /* header_version 3 and later */
> + uint64_t offset_note; /* header_version 4 and later */
> + uint64_t note_size; /* header_version 4 and later */
> + uint64_t offset_eraseinfo; /* header_version 5 and later */
> + uint64_t size_eraseinfo; /* header_version 5 and later */
> + uint64_t start_pfn_64; /* header_version 6 and later */
> + uint64_t end_pfn_64; /* header_version 6 and later */
> + uint64_t max_mapnr_64; /* header_version 6 and later */
> +} KdumpSubHeader64;
> +
> struct GuestPhysBlockList; /* memory_mapping.h */
> int cpu_get_dump_info(ArchDumpInfo *info,
> const struct GuestPhysBlockList *guest_phys_blocks);
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH 09/13 v7] dump: add API to write dump_bitmap
2014-01-17 7:46 [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (7 preceding siblings ...)
2014-01-17 7:46 ` [Qemu-devel] [PATCH 08/13 v7] dump: add API to write dump header qiaonuohan
@ 2014-01-17 7:46 ` qiaonuohan
2014-01-23 13:56 ` Laszlo Ersek
2014-01-17 7:46 ` [Qemu-devel] [PATCH 10/13 v7] dump: add APIs to operate DataCache qiaonuohan
` (5 subsequent siblings)
14 siblings, 1 reply; 37+ messages in thread
From: qiaonuohan @ 2014-01-17 7:46 UTC (permalink / raw)
To: lersek, stefanha, lcapitulino, afaerber, eblake
Cc: kumagai-atsushi, qiaonuohan, anderson, qemu-devel
functions are used to write 1st and 2nd dump_bitmap of kdump-compressed format,
which is used to indicate whether the corresponded page is existed in vmcore.
1st and 2nd dump_bitmap are same, because dump level is specified to 1 here.
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
dump.c | 172 +++++++++++++++++++++++++++++++++++++++++++++++++
include/sysemu/dump.h | 2 +
2 files changed, 174 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index a7fdc3f..26a1756 100644
--- a/dump.c
+++ b/dump.c
@@ -1001,6 +1001,178 @@ static int write_dump_header(DumpState *s)
}
}
+/*
+ * set dump_bitmap sequencely. the bit before last_pfn is not allowed to be
+ * rewritten, so if need to set the first bit, set last_pfn and pfn to 0.
+ * set_dump_bitmap will always leave the recently set bit un-sync. And setting
+ * (last bit + sizeof(buf) * 8) to 0 will do flushing the content in buf into
+ * vmcore, ie. synchronizing un-sync bit into vmcore.
+ */
+static int set_dump_bitmap(uint64_t last_pfn, uint64_t pfn, bool value,
+ uint8_t *buf, DumpState *s)
+{
+ off_t old_offset, new_offset;
+ off_t offset_bitmap1, offset_bitmap2;
+ uint32_t byte, bit;
+
+ /* should not set the previous place */
+ assert(last_pfn <= pfn);
+
+ /*
+ * if the bit needed to be set is not cached in buf, flush the data in buf
+ * to vmcore firstly.
+ * making new_offset be bigger than old_offset can also sync remained data
+ * into vmcore.
+ */
+ old_offset = BUFSIZE_BITMAP * (last_pfn / PFN_BUFBITMAP);
+ new_offset = BUFSIZE_BITMAP * (pfn / PFN_BUFBITMAP);
+
+ 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,
+ BUFSIZE_BITMAP) < 0) {
+ return -1;
+ }
+
+ /* 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,
+ BUFSIZE_BITMAP) < 0) {
+ return -1;
+ }
+
+ memset(buf, 0, BUFSIZE_BITMAP);
+ old_offset += BUFSIZE_BITMAP;
+ }
+
+ /* get the exact place of the bit in the buf, and set it */
+ byte = (pfn % PFN_BUFBITMAP) / CHAR_BIT;
+ bit = (pfn % PFN_BUFBITMAP) % CHAR_BIT;
+ if (value) {
+ buf[byte] |= 1u << bit;
+ } else {
+ buf[byte] &= ~(1u << bit);
+ }
+
+ return 0;
+}
+
+/*
+ * exam every page and return the page from number and the address of the page.
+ * pfnptr and bufptr can be NULL. note: the blocks here is supposed to reflect
+ * guest-phys blocks, so block->target_start and block->target_end should be
+ * interal multiples of the target page size.
+ */
+static int get_next_page(uint64_t *pfnptr, uint8_t **bufptr, DumpState *s)
+{
+ static GuestPhysBlock *block;
+ static uint64_t pfn;
+ hwaddr addr;
+ uint8_t *buf;
+
+ /* block == NULL means the start of the iteration */
+ if (!block) {
+ block = QTAILQ_FIRST(&s->guest_phys_blocks.head);
+ assert(block->target_start % s->page_size == 0);
+ assert(block->target_end % s->page_size == 0);
+ pfn = paddr_to_pfn(block->target_start, s->page_shift);
+ if (pfnptr) {
+ *pfnptr = pfn;
+ }
+ if (bufptr) {
+ *bufptr = block->host_addr;
+ }
+ return 1;
+ }
+
+ pfn++;
+ addr = pfn_to_paddr(pfn, s->page_shift);
+
+ if ((addr >= block->target_start) &&
+ (addr + s->page_size <= block->target_end)) {
+ buf = block->host_addr + (addr - block->target_start);
+ } else {
+ /* the next page is in the next block */
+ block = QTAILQ_NEXT(block, next);
+ /*
+ * iteration comes to end and block is set to NULL, next time when
+ * get_next_page is called, the iteration will start from the first
+ * block
+ */
+ if (!block) {
+ return 0;
+ }
+ assert(block->target_start % s->page_size == 0);
+ assert(block->target_end % s->page_size == 0);
+ pfn = paddr_to_pfn(block->target_start, s->page_shift);
+ buf = block->host_addr;
+ }
+
+ if (pfnptr) {
+ *pfnptr = pfn;
+ }
+ if (bufptr) {
+ *bufptr = buf;
+ }
+
+ return 1;
+}
+
+static int write_dump_bitmap(DumpState *s)
+{
+ int ret = 0;
+ uint64_t last_pfn, pfn;
+ void *dump_bitmap_buf;
+ size_t num_dumpable;
+
+ /* dump_bitmap_buf is used to store dump_bitmap temporarily */
+ dump_bitmap_buf = g_malloc0(BUFSIZE_BITMAP);
+
+ num_dumpable = 0;
+ last_pfn = 0;
+
+ /*
+ * exam memory page by page, and set the bit in dump_bitmap corresponded
+ * to the existing page.
+ */
+ while (get_next_page(&pfn, NULL, s)) {
+ ret = set_dump_bitmap(last_pfn, pfn, true, dump_bitmap_buf, s);
+ if (ret < 0) {
+ dump_error(s, "dump: failed to set dump_bitmap.\n");
+ ret = -1;
+ goto out;
+ }
+
+ last_pfn = pfn;
+ num_dumpable++;
+ }
+
+ /*
+ * set_dump_bitmap will always leave the recently set bit un-sync. Here we
+ * set last_pfn + PFN_BUFBITMAP to 0 and those set but un-sync bit will be
+ * synchronized into vmcore.
+ */
+ if (num_dumpable > 0) {
+ ret = set_dump_bitmap(last_pfn, last_pfn + PFN_BUFBITMAP, false,
+ dump_bitmap_buf, s);
+ if (ret < 0) {
+ dump_error(s, "dump: failed to sync dump_bitmap.\n");
+ ret = -1;
+ goto out;
+ }
+ }
+
+ /* number of dumpable pages that will be dumped later */
+ s->num_dumpable = num_dumpable;
+
+out:
+ g_free(dump_bitmap_buf);
+
+ return ret;
+}
+
static ram_addr_t get_start_block(DumpState *s)
{
GuestPhysBlock *block;
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index dfee238..6d4d0bc 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -39,6 +39,8 @@
#define PHYS_BASE (0)
#define DUMP_LEVEL (1)
#define DISKDUMP_HEADER_BLOCKS (1)
+#define BUFSIZE_BITMAP (TARGET_PAGE_SIZE)
+#define PFN_BUFBITMAP (CHAR_BIT * BUFSIZE_BITMAP)
typedef struct ArchDumpInfo {
int d_machine; /* Architecture */
--
1.7.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 09/13 v7] dump: add API to write dump_bitmap
2014-01-17 7:46 ` [Qemu-devel] [PATCH 09/13 v7] dump: add API to write dump_bitmap qiaonuohan
@ 2014-01-23 13:56 ` Laszlo Ersek
0 siblings, 0 replies; 37+ messages in thread
From: Laszlo Ersek @ 2014-01-23 13:56 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi,
afaerber
comments below
On 01/17/14 08:46, qiaonuohan wrote:
> functions are used to write 1st and 2nd dump_bitmap of kdump-compressed format,
> which is used to indicate whether the corresponded page is existed in vmcore.
> 1st and 2nd dump_bitmap are same, because dump level is specified to 1 here.
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Please don't keep my R-b when you implement intrusive changes in a new
version.
> ---
> dump.c | 172 +++++++++++++++++++++++++++++++++++++++++++++++++
> include/sysemu/dump.h | 2 +
> 2 files changed, 174 insertions(+), 0 deletions(-)
>
> diff --git a/dump.c b/dump.c
> index a7fdc3f..26a1756 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -1001,6 +1001,178 @@ static int write_dump_header(DumpState *s)
> }
> }
>
> +/*
> + * set dump_bitmap sequencely. the bit before last_pfn is not allowed to be
> + * rewritten, so if need to set the first bit, set last_pfn and pfn to 0.
> + * set_dump_bitmap will always leave the recently set bit un-sync. And setting
> + * (last bit + sizeof(buf) * 8) to 0 will do flushing the content in buf into
> + * vmcore, ie. synchronizing un-sync bit into vmcore.
> + */
> +static int set_dump_bitmap(uint64_t last_pfn, uint64_t pfn, bool value,
> + uint8_t *buf, DumpState *s)
> +{
> + off_t old_offset, new_offset;
> + off_t offset_bitmap1, offset_bitmap2;
> + uint32_t byte, bit;
> +
> + /* should not set the previous place */
> + assert(last_pfn <= pfn);
> +
> + /*
> + * if the bit needed to be set is not cached in buf, flush the data in buf
> + * to vmcore firstly.
> + * making new_offset be bigger than old_offset can also sync remained data
> + * into vmcore.
> + */
> + old_offset = BUFSIZE_BITMAP * (last_pfn / PFN_BUFBITMAP);
> + new_offset = BUFSIZE_BITMAP * (pfn / PFN_BUFBITMAP);
> +
> + 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,
> + BUFSIZE_BITMAP) < 0) {
> + return -1;
> + }
> +
> + /* 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,
> + BUFSIZE_BITMAP) < 0) {
> + return -1;
> + }
> +
> + memset(buf, 0, BUFSIZE_BITMAP);
> + old_offset += BUFSIZE_BITMAP;
> + }
> +
> + /* get the exact place of the bit in the buf, and set it */
> + byte = (pfn % PFN_BUFBITMAP) / CHAR_BIT;
> + bit = (pfn % PFN_BUFBITMAP) % CHAR_BIT;
> + if (value) {
> + buf[byte] |= 1u << bit;
> + } else {
> + buf[byte] &= ~(1u << bit);
> + }
> +
> + return 0;
> +}
Seems OK, thanks for addressing my earlier comments.
> +
> +/*
> + * exam every page and return the page from number and the address of the page.
> + * pfnptr and bufptr can be NULL. note: the blocks here is supposed to reflect
> + * guest-phys blocks, so block->target_start and block->target_end should be
> + * interal multiples of the target page size.
> + */
> +static int get_next_page(uint64_t *pfnptr, uint8_t **bufptr, DumpState *s)
> +{
> + static GuestPhysBlock *block;
> + static uint64_t pfn;
Using static variables is incorrect. Suppose that dump attempt #1 fails
because the target disk becomes full mid-dump. Then dump attempt #2 will
be incorrect because the "block" and "pfn" variables here are not
reinitialized.
I think you should
- move the "block" variable to the caller, ie. write_dump_bitmap(), and
take only its address here,
- eliminate the "pfn" variable, and make "pfnptr" mandatory (ie. require
that it be non-NULL).
This way the loop state will be maintained by the caller.
Also, "bool" would be a nicer return type.
> + hwaddr addr;
> + uint8_t *buf;
> +
> + /* block == NULL means the start of the iteration */
> + if (!block) {
> + block = QTAILQ_FIRST(&s->guest_phys_blocks.head);
I guess we can rely on the guest having at least one page. OK. (I think
this is even enforced somewhere near the startup code; memory size is
clamped to some minimum.)
> + assert(block->target_start % s->page_size == 0);
> + assert(block->target_end % s->page_size == 0);
> + pfn = paddr_to_pfn(block->target_start, s->page_shift);
> + if (pfnptr) {
> + *pfnptr = pfn;
> + }
> + if (bufptr) {
> + *bufptr = block->host_addr;
> + }
> + return 1;
> + }
The logic seems otherwise sane.
(1) I can see you rebased this loop from the MemoryMapping list to the
guest_phys_blocks list. Considering the v6 discussion ("for a compressed
dump both paging and filtering are rejected"), this is correct.
(2) However, in this case I wonder whether get_max_mapnr(), now moved to
v7 07/13, should also be rebased to guest_phys_blocks (that function
still uses the MemoryMapping list).
Admittedly, get_max_mapnr() is called also in the non-compressed dump
case, when paging/filtering are possibly enabled. However, the limit
determined in get_max_mapnr(), ie. "s->max_mapnr", is not even used then.
You might want to make the call to get_max_mapnr() conditinal on the
compressed dump case, and then rebase get_max_mapnr() to the
guest_phys_blocks list. After all, "s->max_mapnr" is even documented now
in the struct def as "the biggest guest's phys-mem's number".
(3) I should have noticed this earlier, sorry:
With regard to the loop in get_max_mapnr(), you already rely on that
list being increasing. You determine the maximum by finding the last
element. This is correct.
However we're talking a QTAILQ here, which is double-ended. You don't
need to iterate, just call QTAILQ_LAST() to grab the last element.
> +
> + pfn++;
> + addr = pfn_to_paddr(pfn, s->page_shift);
> +
> + if ((addr >= block->target_start) &&
I can't see how this sub-condition could ever be false. But it shouldn't
hurt.
> + (addr + s->page_size <= block->target_end)) {
> + buf = block->host_addr + (addr - block->target_start);
OK
> + } else {
> + /* the next page is in the next block */
> + block = QTAILQ_NEXT(block, next);
> + /*
> + * iteration comes to end and block is set to NULL, next time when
> + * get_next_page is called, the iteration will start from the first
> + * block
> + */
> + if (!block) {
> + return 0;
> + }
Yes, but this misses the possibility of aborting the dump mid-way.
> + assert(block->target_start % s->page_size == 0);
> + assert(block->target_end % s->page_size == 0);
> + pfn = paddr_to_pfn(block->target_start, s->page_shift);
> + buf = block->host_addr;
> + }
> +
> + if (pfnptr) {
> + *pfnptr = pfn;
> + }
> + if (bufptr) {
> + *bufptr = buf;
> + }
> +
> + return 1;
> +}
In general the logic seems fine.
> +
> +static int write_dump_bitmap(DumpState *s)
> +{
> + int ret = 0;
> + uint64_t last_pfn, pfn;
> + void *dump_bitmap_buf;
> + size_t num_dumpable;
> +
> + /* dump_bitmap_buf is used to store dump_bitmap temporarily */
> + dump_bitmap_buf = g_malloc0(BUFSIZE_BITMAP);
> +
> + num_dumpable = 0;
> + last_pfn = 0;
> +
> + /*
> + * exam memory page by page, and set the bit in dump_bitmap corresponded
> + * to the existing page.
> + */
> + while (get_next_page(&pfn, NULL, s)) {
> + ret = set_dump_bitmap(last_pfn, pfn, true, dump_bitmap_buf, s);
> + if (ret < 0) {
> + dump_error(s, "dump: failed to set dump_bitmap.\n");
> + ret = -1;
> + goto out;
> + }
> +
> + last_pfn = pfn;
> + num_dumpable++;
> + }
> +
> + /*
> + * set_dump_bitmap will always leave the recently set bit un-sync. Here we
> + * set last_pfn + PFN_BUFBITMAP to 0 and those set but un-sync bit will be
> + * synchronized into vmcore.
> + */
> + if (num_dumpable > 0) {
> + ret = set_dump_bitmap(last_pfn, last_pfn + PFN_BUFBITMAP, false,
> + dump_bitmap_buf, s);
> + if (ret < 0) {
> + dump_error(s, "dump: failed to sync dump_bitmap.\n");
> + ret = -1;
> + goto out;
> + }
> + }
> +
> + /* number of dumpable pages that will be dumped later */
> + s->num_dumpable = num_dumpable;
> +
> +out:
> + g_free(dump_bitmap_buf);
> +
> + return ret;
> +}
Seems OK, thanks.
> +
> static ram_addr_t get_start_block(DumpState *s)
> {
> GuestPhysBlock *block;
> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
> index dfee238..6d4d0bc 100644
> --- a/include/sysemu/dump.h
> +++ b/include/sysemu/dump.h
> @@ -39,6 +39,8 @@
> #define PHYS_BASE (0)
> #define DUMP_LEVEL (1)
> #define DISKDUMP_HEADER_BLOCKS (1)
> +#define BUFSIZE_BITMAP (TARGET_PAGE_SIZE)
> +#define PFN_BUFBITMAP (CHAR_BIT * BUFSIZE_BITMAP)
>
> typedef struct ArchDumpInfo {
> int d_machine; /* Architecture */
>
OK.
To summarize:
- the static variables in get_next_page() are a blocker, they should be
fixed;
- addressing the rest would improve code quality in my opinion, but I
don't insist.
Thank you,
Laszlo
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH 10/13 v7] dump: add APIs to operate DataCache
2014-01-17 7:46 [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (8 preceding siblings ...)
2014-01-17 7:46 ` [Qemu-devel] [PATCH 09/13 v7] dump: add API to write dump_bitmap qiaonuohan
@ 2014-01-17 7:46 ` qiaonuohan
2014-01-23 14:50 ` Laszlo Ersek
2014-01-17 7:46 ` [Qemu-devel] [PATCH 11/13 v7] dump: add API to write dump pages qiaonuohan
` (4 subsequent siblings)
14 siblings, 1 reply; 37+ messages in thread
From: qiaonuohan @ 2014-01-17 7:46 UTC (permalink / raw)
To: lersek, stefanha, lcapitulino, afaerber, eblake
Cc: kumagai-atsushi, qiaonuohan, anderson, qemu-devel
DataCache is used to store data temporarily, then the data will be written to
vmcore. These functions will be called later when writing data of page to
vmcore.
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
dump.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
include/sysemu/dump.h | 9 +++++++++
2 files changed, 56 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index 26a1756..fa183cb 100644
--- a/dump.c
+++ b/dump.c
@@ -1173,6 +1173,53 @@ out:
return ret;
}
+static void prepare_data_cache(DataCache *data_cache, DumpState *s,
+ off_t offset)
+{
+ data_cache->fd = s->fd;
+ data_cache->data_size = 0;
+ data_cache->buf_size = BUFSIZE_DATA_CACHE;
+ data_cache->buf = g_malloc0(BUFSIZE_DATA_CACHE);
+ data_cache->offset = offset;
+}
+
+static int write_cache(DataCache *dc, const void *buf, size_t size,
+ bool flag_sync)
+{
+ /*
+ * dc->buf_size should not be less than size, otherwise dc will never be
+ * enough
+ */
+ assert(size <= dc->buf_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
+ */
+ 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) {
+ return -1;
+ }
+
+ dc->offset += dc->data_size;
+ dc->data_size = 0;
+ }
+
+ if (!flag_sync) {
+ memcpy(dc->buf + dc->data_size, buf, size);
+ dc->data_size += size;
+ }
+
+ return 0;
+}
+
+static void free_data_cache(DataCache *data_cache)
+{
+ g_free(data_cache->buf);
+}
+
static ram_addr_t get_start_block(DumpState *s)
{
GuestPhysBlock *block;
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 6d4d0bc..92a95e4 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -41,6 +41,7 @@
#define DISKDUMP_HEADER_BLOCKS (1)
#define BUFSIZE_BITMAP (TARGET_PAGE_SIZE)
#define PFN_BUFBITMAP (CHAR_BIT * BUFSIZE_BITMAP)
+#define BUFSIZE_DATA_CACHE (TARGET_PAGE_SIZE * 4)
typedef struct ArchDumpInfo {
int d_machine; /* Architecture */
@@ -142,6 +143,14 @@ typedef struct QEMU_PACKED KdumpSubHeader64 {
uint64_t max_mapnr_64; /* header_version 6 and later */
} KdumpSubHeader64;
+typedef struct DataCache {
+ int fd; /* fd of the file where to write the cached 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 */
+ off_t offset; /* offset of the file */
+} DataCache;
+
struct GuestPhysBlockList; /* memory_mapping.h */
int cpu_get_dump_info(ArchDumpInfo *info,
const struct GuestPhysBlockList *guest_phys_blocks);
--
1.7.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 10/13 v7] dump: add APIs to operate DataCache
2014-01-17 7:46 ` [Qemu-devel] [PATCH 10/13 v7] dump: add APIs to operate DataCache qiaonuohan
@ 2014-01-23 14:50 ` Laszlo Ersek
0 siblings, 0 replies; 37+ messages in thread
From: Laszlo Ersek @ 2014-01-23 14:50 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi,
afaerber
one comment below
On 01/17/14 08:46, qiaonuohan wrote:
> DataCache is used to store data temporarily, then the data will be written to
> vmcore. These functions will be called later when writing data of page to
> vmcore.
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
> dump.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> include/sysemu/dump.h | 9 +++++++++
> 2 files changed, 56 insertions(+), 0 deletions(-)
>
> diff --git a/dump.c b/dump.c
> index 26a1756..fa183cb 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -1173,6 +1173,53 @@ out:
> return ret;
> }
>
> +static void prepare_data_cache(DataCache *data_cache, DumpState *s,
> + off_t offset)
> +{
> + data_cache->fd = s->fd;
> + data_cache->data_size = 0;
> + data_cache->buf_size = BUFSIZE_DATA_CACHE;
> + data_cache->buf = g_malloc0(BUFSIZE_DATA_CACHE);
> + data_cache->offset = offset;
> +}
> +
> +static int write_cache(DataCache *dc, const void *buf, size_t size,
> + bool flag_sync)
> +{
> + /*
> + * dc->buf_size should not be less than size, otherwise dc will never be
> + * enough
> + */
> + assert(size <= dc->buf_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
> + */
> + 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) {
> + return -1;
> + }
> +
> + dc->offset += dc->data_size;
> + dc->data_size = 0;
> + }
> +
> + if (!flag_sync) {
> + memcpy(dc->buf + dc->data_size, buf, size);
> + dc->data_size += size;
> + }
You don't need to make this block conditional on !flag_sync. By removing
that restriction, the function would become more general.
But I assume you don't need that -- I believe you feed all data to this
function with flag_sync==false, then call it one last time with
(flag_sync==false && size==0).
Thanks for adding the assert() and for the type change on "buf".
My R-b stands.
Laszlo
> +
> + return 0;
> +}
> +
> +static void free_data_cache(DataCache *data_cache)
> +{
> + g_free(data_cache->buf);
> +}
> +
> static ram_addr_t get_start_block(DumpState *s)
> {
> GuestPhysBlock *block;
> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
> index 6d4d0bc..92a95e4 100644
> --- a/include/sysemu/dump.h
> +++ b/include/sysemu/dump.h
> @@ -41,6 +41,7 @@
> #define DISKDUMP_HEADER_BLOCKS (1)
> #define BUFSIZE_BITMAP (TARGET_PAGE_SIZE)
> #define PFN_BUFBITMAP (CHAR_BIT * BUFSIZE_BITMAP)
> +#define BUFSIZE_DATA_CACHE (TARGET_PAGE_SIZE * 4)
>
> typedef struct ArchDumpInfo {
> int d_machine; /* Architecture */
> @@ -142,6 +143,14 @@ typedef struct QEMU_PACKED KdumpSubHeader64 {
> uint64_t max_mapnr_64; /* header_version 6 and later */
> } KdumpSubHeader64;
>
> +typedef struct DataCache {
> + int fd; /* fd of the file where to write the cached 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 */
> + off_t offset; /* offset of the file */
> +} DataCache;
> +
> struct GuestPhysBlockList; /* memory_mapping.h */
> int cpu_get_dump_info(ArchDumpInfo *info,
> const struct GuestPhysBlockList *guest_phys_blocks);
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH 11/13 v7] dump: add API to write dump pages
2014-01-17 7:46 [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (9 preceding siblings ...)
2014-01-17 7:46 ` [Qemu-devel] [PATCH 10/13 v7] dump: add APIs to operate DataCache qiaonuohan
@ 2014-01-17 7:46 ` qiaonuohan
2014-01-23 15:32 ` Laszlo Ersek
2014-01-17 7:46 ` [Qemu-devel] [PATCH 12/13 v7] dump: make kdump-compressed format available for 'dump-guest-memory' qiaonuohan
` (3 subsequent siblings)
14 siblings, 1 reply; 37+ messages in thread
From: qiaonuohan @ 2014-01-17 7:46 UTC (permalink / raw)
To: lersek, stefanha, lcapitulino, afaerber, eblake
Cc: kumagai-atsushi, qiaonuohan, anderson, qemu-devel
functions are used to write page to vmcore. vmcore is written page by page.
page desc is used to store the information of a page, including a page's size,
offset, compression format, etc.
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
---
dump.c | 229 +++++++++++++++++++++++++++++++++++++++++++++++++
include/sysemu/dump.h | 7 ++
2 files changed, 236 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index fa183cb..bb03ef7 100644
--- a/dump.c
+++ b/dump.c
@@ -25,6 +25,14 @@
#include "qapi/error.h"
#include "qmp-commands.h"
+#include <zlib.h>
+#ifdef CONFIG_LZO
+#include <lzo/lzo1x.h>
+#endif
+#ifdef CONFIG_SNAPPY
+#include <snappy-c.h>
+#endif
+
static uint16_t cpu_convert_to_target16(uint16_t val, int endian)
{
if (endian == ELFDATA2LSB) {
@@ -1220,6 +1228,227 @@ static void free_data_cache(DataCache *data_cache)
g_free(data_cache->buf);
}
+static size_t get_len_buf_out(size_t page_size, uint32_t flag_compress)
+{
+ size_t len_buf_out_zlib, len_buf_out_lzo, len_buf_out_snappy;
+ size_t len_buf_out;
+
+ /* init buf_out */
+ len_buf_out_zlib = len_buf_out_lzo = len_buf_out_snappy = 0;
+
+ /* buf size for zlib */
+ len_buf_out_zlib = compressBound(page_size);
+
+ /* buf size for lzo */
+#ifdef CONFIG_LZO
+ if (flag_compress & DUMP_DH_COMPRESSED_LZO) {
+ if (lzo_init() != LZO_E_OK) {
+ /* return 0 to indicate lzo is unavailable */
+ return 0;
+ }
+ }
+
+ /*
+ * LZO will expand incompressible data by a little amount. please check the
+ * following URL to see the expansion calculation:
+ * http://www.oberhumer.com/opensource/lzo/lzofaq.php
+ */
+ len_buf_out_lzo = page_size + page_size / 16 + 64 + 3;
+#endif
+
+#ifdef CONFIG_SNAPPY
+ /* buf size for snappy */
+ len_buf_out_snappy = snappy_max_compressed_length(page_size);
+#endif
+
+ /* get the biggest that can store all kinds of compressed page */
+ len_buf_out = MAX(len_buf_out_zlib,
+ MAX(len_buf_out_lzo, len_buf_out_snappy));
+
+ return len_buf_out;
+}
+
+/*
+ * check if the page is all 0
+ */
+static inline bool is_zero_page(const uint8_t *buf, size_t page_size)
+{
+ return buffer_is_zero(buf, page_size);
+}
+
+static int write_dump_pages(DumpState *s)
+{
+ int ret = 0;
+ DataCache page_desc, page_data;
+ size_t len_buf_out, size_out;
+#ifdef CONFIG_LZO
+ lzo_bytep wrkmem = NULL;
+#endif
+ uint8_t *buf_out = NULL;
+ off_t offset_desc, offset_data;
+ PageDesc pd, pd_zero;
+ uint8_t *buf;
+ int endian = s->dump_info.d_endian;
+
+ /* get offset of page_desc and page_data in dump file */
+ offset_desc = s->offset_page;
+ offset_data = offset_desc + sizeof(PageDesc) * s->num_dumpable;
+
+ prepare_data_cache(&page_desc, s, offset_desc);
+ prepare_data_cache(&page_data, s, offset_data);
+
+ /* prepare buffer to store compressed data */
+ len_buf_out = get_len_buf_out(s->page_size, s->flag_compress);
+ if (len_buf_out == 0) {
+ dump_error(s, "dump: failed to get length of output buffer.\n");
+ goto out;
+ }
+
+#ifdef CONFIG_LZO
+ wrkmem = g_malloc(LZO1X_1_MEM_COMPRESS);
+#endif
+
+ buf_out = g_malloc(len_buf_out);
+
+ /*
+ * init zero page's page_desc and page_data, because every zero page
+ * uses the same page_data
+ */
+ pd_zero.size = cpu_convert_to_target32(s->page_size, endian);
+ pd_zero.flags = cpu_convert_to_target32(0, endian);
+ pd_zero.offset = cpu_convert_to_target64(offset_data, endian);
+ pd_zero.page_flags = cpu_convert_to_target64(0, endian);
+ buf = g_malloc0(s->page_size);
+ ret = write_cache(&page_data, buf, s->page_size, false);
+ g_free(buf);
+ if (ret < 0) {
+ dump_error(s, "dump: failed to write page data(zero page).\n");
+ goto out;
+ }
+
+ offset_data += s->page_size;
+
+ /*
+ * dump memory to vmcore page by page. zero page will all be resided in the
+ * first page of page section
+ */
+ while (get_next_page(NULL, &buf, s)) {
+ /* check zero page */
+ if (is_zero_page(buf, s->page_size)) {
+ ret = write_cache(&page_desc, &pd_zero, sizeof(PageDesc),
+ false);
+ if (ret < 0) {
+ dump_error(s, "dump: failed to write page desc.\n");
+ goto out;
+ }
+ } else {
+ /*
+ * not zero page, then:
+ * 1. compress the page
+ * 2. write the compressed page into the cache of page_data
+ * 3. get page desc of the compressed page and write it into the
+ * cache of page_desc
+ *
+ * only one compression format will be used here, for
+ * s->flag_compress is set. But when compression fails to work,
+ * we fall back to save in plaintext.
+ */
+ size_out = len_buf_out;
+ if ((s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) &&
+ (compress2(buf_out, &size_out, buf, s->page_size,
+ Z_BEST_SPEED) == Z_OK) && (size_out < s->page_size)) {
+ pd.flags = cpu_convert_to_target32(DUMP_DH_COMPRESSED_ZLIB,
+ endian);
+ pd.size = cpu_convert_to_target32(size_out, endian);
+
+ ret = write_cache(&page_data, buf_out, size_out, false);
+ if (ret < 0) {
+ dump_error(s, "dump: failed to write page data.\n");
+ goto out;
+ }
+#ifdef CONFIG_LZO
+ } else if ((s->flag_compress & DUMP_DH_COMPRESSED_LZO) &&
+ (lzo1x_1_compress(buf, s->page_size, buf_out,
+ &size_out, wrkmem) == LZO_E_OK) &&
+ (size_out < s->page_size)) {
+ pd.flags = cpu_convert_to_target32(DUMP_DH_COMPRESSED_LZO,
+ endian);
+ pd.size = cpu_convert_to_target32(size_out, endian);
+
+ ret = write_cache(&page_data, buf_out, size_out, false);
+ if (ret < 0) {
+ dump_error(s, "dump: failed to write page data.\n");
+ goto out;
+ }
+#endif
+#ifdef CONFIG_SNAPPY
+ } else if ((s->flag_compress & DUMP_DH_COMPRESSED_SNAPPY) &&
+ (snappy_compress((char *)buf, s->page_size,
+ (char *)buf_out, &size_out) == SNAPPY_OK) &&
+ (size_out < s->page_size)) {
+ pd.flags = cpu_convert_to_target32(
+ DUMP_DH_COMPRESSED_SNAPPY, endian);
+ pd.size = cpu_convert_to_target32(size_out, endian);
+
+ ret = write_cache(&page_data, buf_out, size_out, false);
+ if (ret < 0) {
+ dump_error(s, "dump: failed to write page data.\n");
+ goto out;
+ }
+#endif
+ } else {
+ /*
+ * fall back to save in plaintext, size_out should be
+ * assigned to s->page_size
+ */
+ pd.flags = cpu_convert_to_target32(0, endian);
+ size_out = s->page_size;
+ pd.size = cpu_convert_to_target32(size_out, endian);
+
+ ret = write_cache(&page_data, buf, s->page_size, false);
+ if (ret < 0) {
+ dump_error(s, "dump: failed to write page data.\n");
+ goto out;
+ }
+ }
+
+ /* get and write page desc here */
+ pd.page_flags = cpu_convert_to_target64(0, endian);
+ pd.offset = cpu_convert_to_target64(offset_data, endian);
+ offset_data += size_out;
+
+ ret = write_cache(&page_desc, &pd, sizeof(PageDesc), false);
+ if (ret < 0) {
+ dump_error(s, "dump: failed to write page desc.\n");
+ goto out;
+ }
+ }
+ }
+
+ ret = write_cache(&page_desc, NULL, 0, true);
+ if (ret < 0) {
+ dump_error(s, "dump: failed to sync cache for page_desc.\n");
+ goto out;
+ }
+ ret = write_cache(&page_data, NULL, 0, true);
+ if (ret < 0) {
+ dump_error(s, "dump: failed to sync cache for page_data.\n");
+ goto out;
+ }
+
+out:
+ free_data_cache(&page_desc);
+ free_data_cache(&page_data);
+
+#ifdef CONFIG_LZO
+ g_free(wrkmem);
+#endif
+
+ g_free(buf_out);
+
+ return ret;
+}
+
static ram_addr_t get_start_block(DumpState *s)
{
GuestPhysBlock *block;
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 92a95e4..72cc258 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -151,6 +151,13 @@ typedef struct DataCache {
off_t offset; /* offset of the file */
} DataCache;
+typedef struct PageDesc {
+ uint64_t offset; /* the offset of the page data*/
+ uint32_t size; /* the size of this dump page */
+ uint32_t flags; /* flags */
+ uint64_t page_flags; /* page flags */
+} PageDesc;
+
struct GuestPhysBlockList; /* memory_mapping.h */
int cpu_get_dump_info(ArchDumpInfo *info,
const struct GuestPhysBlockList *guest_phys_blocks);
--
1.7.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 11/13 v7] dump: add API to write dump pages
2014-01-17 7:46 ` [Qemu-devel] [PATCH 11/13 v7] dump: add API to write dump pages qiaonuohan
@ 2014-01-23 15:32 ` Laszlo Ersek
0 siblings, 0 replies; 37+ messages in thread
From: Laszlo Ersek @ 2014-01-23 15:32 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi,
afaerber
On 01/17/14 08:46, qiaonuohan wrote:
> functions are used to write page to vmcore. vmcore is written page by page.
> page desc is used to store the information of a page, including a page's size,
> offset, compression format, etc.
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> ---
> dump.c | 229 +++++++++++++++++++++++++++++++++++++++++++++++++
> include/sysemu/dump.h | 7 ++
> 2 files changed, 236 insertions(+), 0 deletions(-)
You seem to have addressed everything I said for v6 09/11, except the
QEMU_PACKED for PageDesc, but that's fine.
(BTW I've just noticed that "PageDesc" is already used in
"translate-all.c". No conflict here, but you might want to rename your
struct, if you spin a v8 anyway.)
All in all, great job!
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH 12/13 v7] dump: make kdump-compressed format available for 'dump-guest-memory'
2014-01-17 7:46 [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (10 preceding siblings ...)
2014-01-17 7:46 ` [Qemu-devel] [PATCH 11/13 v7] dump: add API to write dump pages qiaonuohan
@ 2014-01-17 7:46 ` qiaonuohan
2014-01-23 15:17 ` Ekaterina Tumanova
2014-01-24 12:06 ` Laszlo Ersek
2014-01-17 7:46 ` [Qemu-devel] [PATCH 13/13 v7] dump: add 'query-dump-guest-memory-capability' command qiaonuohan
` (2 subsequent siblings)
14 siblings, 2 replies; 37+ messages in thread
From: qiaonuohan @ 2014-01-17 7:46 UTC (permalink / raw)
To: lersek, stefanha, lcapitulino, afaerber, eblake
Cc: kumagai-atsushi, qiaonuohan, anderson, qemu-devel
Make monitor command 'dump-guest-memory' be able to dump in kdump-compressed
format. The command's usage:
dump [-p] protocol [begin] [length] [format]
'format' is used to specified the format of vmcore and can be:
1. 'elf': ELF format, without compression
2. 'kdump-zlib': kdump-compressed format, with zlib-compressed
3. 'kdump-lzo': kdump-compressed format, with lzo-compressed
4. 'kdump-snappy': kdump-compressed format, with snappy-compressed
Without 'format' being set, it is same as 'elf'. And if non-elf format is
specified, paging and filter is not allowed.
Note:
1. The kdump-compressed format is readable only with the crash utility and
makedumpfile, and it can be smaller than the ELF format because of the
compression support.
2. The kdump-compressed format is the 6th edition.
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
---
dump.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++++++---
hmp.c | 5 ++-
qapi-schema.json | 25 ++++++++++-
qmp-commands.hx | 7 ++-
4 files changed, 156 insertions(+), 10 deletions(-)
diff --git a/dump.c b/dump.c
index bb03ef7..dbf4bb6 100644
--- a/dump.c
+++ b/dump.c
@@ -1449,6 +1449,64 @@ out:
return ret;
}
+static int create_kdump_vmcore(DumpState *s)
+{
+ int ret;
+
+ /*
+ * the kdump-compressed format is:
+ * File offset
+ * +------------------------------------------+ 0x0
+ * | main header (struct disk_dump_header) |
+ * |------------------------------------------+ block 1
+ * | sub header (struct kdump_sub_header) |
+ * |------------------------------------------+ block 2
+ * | 1st-dump_bitmap |
+ * |------------------------------------------+ block 2 + X blocks
+ * | 2nd-dump_bitmap | (aligned by block)
+ * |------------------------------------------+ block 2 + 2 * X blocks
+ * | page desc for pfn 0 (struct page_desc) | (aligned by block)
+ * | page desc for pfn 1 (struct page_desc) |
+ * | : |
+ * |------------------------------------------| (not aligned by block)
+ * | page data (pfn 0) |
+ * | page data (pfn 1) |
+ * | : |
+ * +------------------------------------------+
+ */
+
+ ret = write_start_flat_header(s->fd);
+ if (ret < 0) {
+ dump_error(s, "dump: failed to write start flat header.\n");
+ return -1;
+ }
+
+ ret = write_dump_header(s);
+ if (ret < 0) {
+ return -1;
+ }
+
+ ret = write_dump_bitmap(s);
+ if (ret < 0) {
+ return -1;
+ }
+
+ ret = write_dump_pages(s);
+ if (ret < 0) {
+ return -1;
+ }
+
+ ret = write_end_flat_header(s->fd);
+ if (ret < 0) {
+ dump_error(s, "dump: failed to write end flat header.\n");
+ return -1;
+ }
+
+ dump_completed(s);
+
+ return 0;
+}
+
static ram_addr_t get_start_block(DumpState *s)
{
GuestPhysBlock *block;
@@ -1487,7 +1545,8 @@ static void get_max_mapnr(DumpState *s)
}
}
-static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
+static int dump_init(DumpState *s, int fd, bool has_format,
+ DumpGuestMemoryFormat format, bool paging, bool has_filter,
int64_t begin, int64_t length, Error **errp)
{
CPUState *cpu;
@@ -1495,6 +1554,11 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
Error *err = NULL;
int ret;
+ /* kdump-compressed is conflict with paging and filter */
+ if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
+ assert(!paging && !has_filter);
+ }
+
if (runstate_is_running()) {
vm_stop(RUN_STATE_SAVE_VM);
s->resume = true;
@@ -1565,6 +1629,28 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
tmp = DIV_ROUND_UP(DIV_ROUND_UP(s->max_mapnr, CHAR_BIT), s->page_size);
s->len_dump_bitmap = tmp * s->page_size;
+ /* init for kdump-compressed format */
+ if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
+ switch (format) {
+ case DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB:
+ s->flag_compress = DUMP_DH_COMPRESSED_ZLIB;
+ break;
+
+ case DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO:
+ s->flag_compress = DUMP_DH_COMPRESSED_LZO;
+ break;
+
+ case DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY:
+ s->flag_compress = DUMP_DH_COMPRESSED_SNAPPY;
+ break;
+
+ default:
+ s->flag_compress = 0;
+ }
+
+ return 0;
+ }
+
if (s->has_filter) {
memory_mapping_filter(&s->list, s->begin, s->length);
}
@@ -1624,14 +1710,25 @@ cleanup:
}
void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
- int64_t begin, bool has_length, int64_t length,
- Error **errp)
+ int64_t begin, bool has_length,
+ int64_t length, bool has_format,
+ DumpGuestMemoryFormat format, Error **errp)
{
const char *p;
int fd = -1;
DumpState *s;
int ret;
+ /*
+ * kdump-compressed format need the whole memory dumped, so paging or
+ * filter is not supported here.
+ */
+ if ((has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) &&
+ (paging || has_begin || has_length)) {
+ error_setg(errp, "kdump-compressed format doesn't support paging or "
+ "filter");
+ return;
+ }
if (has_begin && !has_length) {
error_set(errp, QERR_MISSING_PARAMETER, "length");
return;
@@ -1641,6 +1738,19 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
return;
}
+ /* check whether lzo/snappy is supported */
+#ifndef CONFIG_LZO
+ if (has_format && format == DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO) {
+ error_setg(errp, "kdump-lzo is not available now");
+ }
+#endif
+
+#ifndef CONFIG_SNAPPY
+ if (has_format && format == DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY) {
+ error_setg(errp, "kdump-snappy is not available now");
+ }
+#endif
+
#if !defined(WIN32)
if (strstart(file, "fd:", &p)) {
fd = monitor_get_fd(cur_mon, p, errp);
@@ -1665,14 +1775,21 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
s = g_malloc0(sizeof(DumpState));
- ret = dump_init(s, fd, paging, has_begin, begin, length, errp);
+ ret = dump_init(s, fd, has_format, format, paging, has_begin,
+ begin, length, errp);
if (ret < 0) {
g_free(s);
return;
}
- if (create_vmcore(s) < 0 && !error_is_set(s->errp)) {
- error_set(errp, QERR_IO_ERROR);
+ if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
+ if (create_kdump_vmcore(s) < 0 && !error_is_set(s->errp)) {
+ error_set(errp, QERR_IO_ERROR);
+ }
+ } else {
+ if (create_vmcore(s) < 0 && !error_is_set(s->errp)) {
+ error_set(errp, QERR_IO_ERROR);
+ }
}
g_free(s);
diff --git a/hmp.c b/hmp.c
index 79f9c7d..5245e62 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1308,8 +1308,11 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
const char *file = qdict_get_str(qdict, "filename");
bool has_begin = qdict_haskey(qdict, "begin");
bool has_length = qdict_haskey(qdict, "length");
+ /* kdump-compressed format is not supported for HMP */
+ bool has_format = false;
int64_t begin = 0;
int64_t length = 0;
+ enum DumpGuestMemoryFormat dump_format = DUMP_GUEST_MEMORY_FORMAT_ELF;
char *prot;
if (has_begin) {
@@ -1322,7 +1325,7 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
prot = g_strconcat("file:", file, NULL);
qmp_dump_guest_memory(paging, prot, has_begin, begin, has_length, length,
- &errp);
+ has_format, dump_format, &errp);
hmp_handle_error(mon, &errp);
g_free(prot);
}
diff --git a/qapi-schema.json b/qapi-schema.json
index f27c48a..52df894 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2677,6 +2677,24 @@
{ 'command': 'device_del', 'data': {'id': 'str'} }
##
+# @DumpGuestMemoryFormat:
+#
+# An enumeration of guest-memory-dump's format.
+#
+# @elf: elf format
+#
+# @kdump-zlib: kdump-compressed format with zlib-compressed
+#
+# @kdump-lzo: kdump-compressed format with lzo-compressed
+#
+# @kdump-snappy: kdump-compressed format with snappy-compressed
+#
+# Since: 2.0
+##
+{ 'enum': 'DumpGuestMemoryFormat',
+ 'data': [ 'elf', 'kdump-zlib', 'kdump-lzo', 'kdump-snappy' ] }
+
+##
# @dump-guest-memory
#
# Dump guest's memory to vmcore. It is a synchronous operation that can take
@@ -2712,13 +2730,18 @@
# want to dump all guest's memory, please specify the start @begin
# and @length
#
+# @format: #optional 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 @format at the same time
+# (since 2.0)
+#
# Returns: nothing on success
#
# Since: 1.2
##
{ 'command': 'dump-guest-memory',
'data': { 'paging': 'bool', 'protocol': 'str', '*begin': 'int',
- '*length': 'int' } }
+ '*length': 'int', '*format': 'DumpGuestMemoryFormat' } }
##
# @netdev_add:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 02cc815..9158f22 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -791,8 +791,8 @@ EQMP
{
.name = "dump-guest-memory",
- .args_type = "paging:b,protocol:s,begin:i?,end:i?",
- .params = "-p protocol [begin] [length]",
+ .args_type = "paging:b,protocol:s,begin:i?,end:i?,format:s?",
+ .params = "-p protocol [begin] [length] [format]",
.help = "dump guest memory to file",
.user_print = monitor_user_noop,
.mhandler.cmd_new = qmp_marshal_input_dump_guest_memory,
@@ -813,6 +813,9 @@ Arguments:
with length together (json-int)
- "length": the memory size, in bytes. It's optional, and should be specified
with begin together (json-int)
+- "format": the format of guest memory dump. It's optional, and can be
+ elf|kdump-zlib|kdump-lzo|kdump-snappy, but non-elf formats will
+ conflict with paging and filter, ie. begin and length(json-string)
Example:
--
1.7.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 12/13 v7] dump: make kdump-compressed format available for 'dump-guest-memory'
2014-01-17 7:46 ` [Qemu-devel] [PATCH 12/13 v7] dump: make kdump-compressed format available for 'dump-guest-memory' qiaonuohan
@ 2014-01-23 15:17 ` Ekaterina Tumanova
2014-01-24 11:27 ` Laszlo Ersek
2014-01-24 12:06 ` Laszlo Ersek
1 sibling, 1 reply; 37+ messages in thread
From: Ekaterina Tumanova @ 2014-01-23 15:17 UTC (permalink / raw)
To: qemu-devel
On 01/17/2014 11:46 AM, qiaonuohan wrote:
> Make monitor command 'dump-guest-memory' be able to dump in kdump-compressed
> format. The command's usage:
>
> dump [-p] protocol [begin] [length] [format]
>
> 'format' is used to specified the format of vmcore and can be:
> 1. 'elf': ELF format, without compression
> 2. 'kdump-zlib': kdump-compressed format, with zlib-compressed
> 3. 'kdump-lzo': kdump-compressed format, with lzo-compressed
> 4. 'kdump-snappy': kdump-compressed format, with snappy-compressed
> Without 'format' being set, it is same as 'elf'. And if non-elf format is
> specified, paging and filter is not allowed.
>
> Note:
> 1. The kdump-compressed format is readable only with the crash utility and
> makedumpfile, and it can be smaller than the ELF format because of the
> compression support.
> 2. The kdump-compressed format is the 6th edition.
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> ---
> dump.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++++++---
> hmp.c | 5 ++-
> qapi-schema.json | 25 ++++++++++-
> qmp-commands.hx | 7 ++-
> 4 files changed, 156 insertions(+), 10 deletions(-)
>
> diff --git a/dump.c b/dump.c
> index bb03ef7..dbf4bb6 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -1449,6 +1449,64 @@ out:
> return ret;
> }
>
> +static int create_kdump_vmcore(DumpState *s)
> +{
> + int ret;
> +
> + /*
> + * the kdump-compressed format is:
> + * File offset
> + * +------------------------------------------+ 0x0
> + * | main header (struct disk_dump_header) |
> + * |------------------------------------------+ block 1
> + * | sub header (struct kdump_sub_header) |
> + * |------------------------------------------+ block 2
> + * | 1st-dump_bitmap |
> + * |------------------------------------------+ block 2 + X blocks
> + * | 2nd-dump_bitmap | (aligned by block)
> + * |------------------------------------------+ block 2 + 2 * X blocks
> + * | page desc for pfn 0 (struct page_desc) | (aligned by block)
> + * | page desc for pfn 1 (struct page_desc) |
> + * | : |
> + * |------------------------------------------| (not aligned by block)
> + * | page data (pfn 0) |
> + * | page data (pfn 1) |
> + * | : |
> + * +------------------------------------------+
> + */
> +
> + ret = write_start_flat_header(s->fd);
> + if (ret < 0) {
> + dump_error(s, "dump: failed to write start flat header.\n");
> + return -1;
> + }
> +
> + ret = write_dump_header(s);
> + if (ret < 0) {
> + return -1;
> + }
> +
> + ret = write_dump_bitmap(s);
> + if (ret < 0) {
> + return -1;
> + }
> +
> + ret = write_dump_pages(s);
> + if (ret < 0) {
> + return -1;
> + }
> +
> + ret = write_end_flat_header(s->fd);
> + if (ret < 0) {
> + dump_error(s, "dump: failed to write end flat header.\n");
> + return -1;
> + }
> +
> + dump_completed(s);
> +
> + return 0;
> +}
> +
> static ram_addr_t get_start_block(DumpState *s)
> {
> GuestPhysBlock *block;
> @@ -1487,7 +1545,8 @@ static void get_max_mapnr(DumpState *s)
> }
> }
>
> -static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
> +static int dump_init(DumpState *s, int fd, bool has_format,
> + DumpGuestMemoryFormat format, bool paging, bool has_filter,
> int64_t begin, int64_t length, Error **errp)
> {
> CPUState *cpu;
> @@ -1495,6 +1554,11 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
> Error *err = NULL;
> int ret;
>
> + /* kdump-compressed is conflict with paging and filter */
> + if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
> + assert(!paging && !has_filter);
> + }
> +
> if (runstate_is_running()) {
> vm_stop(RUN_STATE_SAVE_VM);
> s->resume = true;
> @@ -1565,6 +1629,28 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
> tmp = DIV_ROUND_UP(DIV_ROUND_UP(s->max_mapnr, CHAR_BIT), s->page_size);
> s->len_dump_bitmap = tmp * s->page_size;
>
> + /* init for kdump-compressed format */
> + if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
> + switch (format) {
> + case DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB:
> + s->flag_compress = DUMP_DH_COMPRESSED_ZLIB;
> + break;
> +
> + case DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO:
> + s->flag_compress = DUMP_DH_COMPRESSED_LZO;
> + break;
> +
> + case DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY:
> + s->flag_compress = DUMP_DH_COMPRESSED_SNAPPY;
> + break;
> +
> + default:
> + s->flag_compress = 0;
> + }
> +
> + return 0;
> + }
> +
> if (s->has_filter) {
> memory_mapping_filter(&s->list, s->begin, s->length);
> }
> @@ -1624,14 +1710,25 @@ cleanup:
> }
>
> void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
> - int64_t begin, bool has_length, int64_t length,
> - Error **errp)
> + int64_t begin, bool has_length,
> + int64_t length, bool has_format,
> + DumpGuestMemoryFormat format, Error **errp)
> {
> const char *p;
> int fd = -1;
> DumpState *s;
> int ret;
>
> + /*
> + * kdump-compressed format need the whole memory dumped, so paging or
> + * filter is not supported here.
> + */
> + if ((has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) &&
> + (paging || has_begin || has_length)) {
> + error_setg(errp, "kdump-compressed format doesn't support paging or "
> + "filter");
> + return;
> + }
> if (has_begin && !has_length) {
> error_set(errp, QERR_MISSING_PARAMETER, "length");
> return;
> @@ -1641,6 +1738,19 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
> return;
> }
>
> + /* check whether lzo/snappy is supported */
> +#ifndef CONFIG_LZO
> + if (has_format && format == DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO) {
> + error_setg(errp, "kdump-lzo is not available now");
> + }
> +#endif
When kdump-lzo is not available, command still writes a dump,
which is half smaller then uncompressed one (and is read as partial
dump), but is much bigger then the compressed one. Is it supposed to
behave like that?
Kate.
> +
> +#ifndef CONFIG_SNAPPY
> + if (has_format && format == DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY) {
> + error_setg(errp, "kdump-snappy is not available now");
> + }
> +#endif
> +
> #if !defined(WIN32)
> if (strstart(file, "fd:", &p)) {
> fd = monitor_get_fd(cur_mon, p, errp);
> @@ -1665,14 +1775,21 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
>
> s = g_malloc0(sizeof(DumpState));
>
> - ret = dump_init(s, fd, paging, has_begin, begin, length, errp);
> + ret = dump_init(s, fd, has_format, format, paging, has_begin,
> + begin, length, errp);
> if (ret < 0) {
> g_free(s);
> return;
> }
>
> - if (create_vmcore(s) < 0 && !error_is_set(s->errp)) {
> - error_set(errp, QERR_IO_ERROR);
> + if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
> + if (create_kdump_vmcore(s) < 0 && !error_is_set(s->errp)) {
> + error_set(errp, QERR_IO_ERROR);
> + }
> + } else {
> + if (create_vmcore(s) < 0 && !error_is_set(s->errp)) {
> + error_set(errp, QERR_IO_ERROR);
> + }
> }
>
> g_free(s);
> diff --git a/hmp.c b/hmp.c
> index 79f9c7d..5245e62 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1308,8 +1308,11 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
> const char *file = qdict_get_str(qdict, "filename");
> bool has_begin = qdict_haskey(qdict, "begin");
> bool has_length = qdict_haskey(qdict, "length");
> + /* kdump-compressed format is not supported for HMP */
> + bool has_format = false;
> int64_t begin = 0;
> int64_t length = 0;
> + enum DumpGuestMemoryFormat dump_format = DUMP_GUEST_MEMORY_FORMAT_ELF;
> char *prot;
>
> if (has_begin) {
> @@ -1322,7 +1325,7 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
> prot = g_strconcat("file:", file, NULL);
>
> qmp_dump_guest_memory(paging, prot, has_begin, begin, has_length, length,
> - &errp);
> + has_format, dump_format, &errp);
> hmp_handle_error(mon, &errp);
> g_free(prot);
> }
> diff --git a/qapi-schema.json b/qapi-schema.json
> index f27c48a..52df894 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2677,6 +2677,24 @@
> { 'command': 'device_del', 'data': {'id': 'str'} }
>
> ##
> +# @DumpGuestMemoryFormat:
> +#
> +# An enumeration of guest-memory-dump's format.
> +#
> +# @elf: elf format
> +#
> +# @kdump-zlib: kdump-compressed format with zlib-compressed
> +#
> +# @kdump-lzo: kdump-compressed format with lzo-compressed
> +#
> +# @kdump-snappy: kdump-compressed format with snappy-compressed
> +#
> +# Since: 2.0
> +##
> +{ 'enum': 'DumpGuestMemoryFormat',
> + 'data': [ 'elf', 'kdump-zlib', 'kdump-lzo', 'kdump-snappy' ] }
> +
> +##
> # @dump-guest-memory
> #
> # Dump guest's memory to vmcore. It is a synchronous operation that can take
> @@ -2712,13 +2730,18 @@
> # want to dump all guest's memory, please specify the start @begin
> # and @length
> #
> +# @format: #optional 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 @format at the same time
> +# (since 2.0)
> +#
> # Returns: nothing on success
> #
> # Since: 1.2
> ##
> { 'command': 'dump-guest-memory',
> 'data': { 'paging': 'bool', 'protocol': 'str', '*begin': 'int',
> - '*length': 'int' } }
> + '*length': 'int', '*format': 'DumpGuestMemoryFormat' } }
>
> ##
> # @netdev_add:
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 02cc815..9158f22 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -791,8 +791,8 @@ EQMP
>
> {
> .name = "dump-guest-memory",
> - .args_type = "paging:b,protocol:s,begin:i?,end:i?",
> - .params = "-p protocol [begin] [length]",
> + .args_type = "paging:b,protocol:s,begin:i?,end:i?,format:s?",
> + .params = "-p protocol [begin] [length] [format]",
> .help = "dump guest memory to file",
> .user_print = monitor_user_noop,
> .mhandler.cmd_new = qmp_marshal_input_dump_guest_memory,
> @@ -813,6 +813,9 @@ Arguments:
> with length together (json-int)
> - "length": the memory size, in bytes. It's optional, and should be specified
> with begin together (json-int)
> +- "format": the format of guest memory dump. It's optional, and can be
> + elf|kdump-zlib|kdump-lzo|kdump-snappy, but non-elf formats will
> + conflict with paging and filter, ie. begin and length(json-string)
>
> Example:
>
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 12/13 v7] dump: make kdump-compressed format available for 'dump-guest-memory'
2014-01-23 15:17 ` Ekaterina Tumanova
@ 2014-01-24 11:27 ` Laszlo Ersek
0 siblings, 0 replies; 37+ messages in thread
From: Laszlo Ersek @ 2014-01-24 11:27 UTC (permalink / raw)
To: Ekaterina Tumanova; +Cc: qemu-devel
On 01/23/14 16:17, Ekaterina Tumanova wrote:
> On 01/17/2014 11:46 AM, qiaonuohan wrote:
>> Make monitor command 'dump-guest-memory' be able to dump in
>> kdump-compressed
>> format. The command's usage:
>>
>> dump [-p] protocol [begin] [length] [format]
>>
>> 'format' is used to specified the format of vmcore and can be:
>> 1. 'elf': ELF format, without compression
>> 2. 'kdump-zlib': kdump-compressed format, with zlib-compressed
>> 3. 'kdump-lzo': kdump-compressed format, with lzo-compressed
>> 4. 'kdump-snappy': kdump-compressed format, with snappy-compressed
>> Without 'format' being set, it is same as 'elf'. And if non-elf format is
>> specified, paging and filter is not allowed.
>>
>> Note:
>> 1. The kdump-compressed format is readable only with the crash
>> utility and
>> makedumpfile, and it can be smaller than the ELF format because
>> of the
>> compression support.
>> 2. The kdump-compressed format is the 6th edition.
>>
>> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
>> ---
>> dump.c | 129
>> +++++++++++++++++++++++++++++++++++++++++++++++++++---
>> hmp.c | 5 ++-
>> qapi-schema.json | 25 ++++++++++-
>> qmp-commands.hx | 7 ++-
>> 4 files changed, 156 insertions(+), 10 deletions(-)
>>
>> diff --git a/dump.c b/dump.c
>> index bb03ef7..dbf4bb6 100644
>> --- a/dump.c
>> +++ b/dump.c
>> @@ -1449,6 +1449,64 @@ out:
>> return ret;
>> }
>>
>> +static int create_kdump_vmcore(DumpState *s)
>> +{
>> + int ret;
>> +
>> + /*
>> + * the kdump-compressed format is:
>> + * File offset
>> + * +------------------------------------------+ 0x0
>> + * | main header (struct disk_dump_header) |
>> + * |------------------------------------------+ block 1
>> + * | sub header (struct kdump_sub_header) |
>> + * |------------------------------------------+ block 2
>> + * | 1st-dump_bitmap |
>> + * |------------------------------------------+ block 2 + X blocks
>> + * | 2nd-dump_bitmap | (aligned by block)
>> + * |------------------------------------------+ block 2 + 2 * X
>> blocks
>> + * | page desc for pfn 0 (struct page_desc) | (aligned by block)
>> + * | page desc for pfn 1 (struct page_desc) |
>> + * | : |
>> + * |------------------------------------------| (not aligned by
>> block)
>> + * | page data (pfn 0) |
>> + * | page data (pfn 1) |
>> + * | : |
>> + * +------------------------------------------+
>> + */
>> +
>> + ret = write_start_flat_header(s->fd);
>> + if (ret < 0) {
>> + dump_error(s, "dump: failed to write start flat header.\n");
>> + return -1;
>> + }
>> +
>> + ret = write_dump_header(s);
>> + if (ret < 0) {
>> + return -1;
>> + }
>> +
>> + ret = write_dump_bitmap(s);
>> + if (ret < 0) {
>> + return -1;
>> + }
>> +
>> + ret = write_dump_pages(s);
>> + if (ret < 0) {
>> + return -1;
>> + }
>> +
>> + ret = write_end_flat_header(s->fd);
>> + if (ret < 0) {
>> + dump_error(s, "dump: failed to write end flat header.\n");
>> + return -1;
>> + }
>> +
>> + dump_completed(s);
>> +
>> + return 0;
>> +}
>> +
>> static ram_addr_t get_start_block(DumpState *s)
>> {
>> GuestPhysBlock *block;
>> @@ -1487,7 +1545,8 @@ static void get_max_mapnr(DumpState *s)
>> }
>> }
>>
>> -static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
>> +static int dump_init(DumpState *s, int fd, bool has_format,
>> + DumpGuestMemoryFormat format, bool paging, bool
>> has_filter,
>> int64_t begin, int64_t length, Error **errp)
>> {
>> CPUState *cpu;
>> @@ -1495,6 +1554,11 @@ static int dump_init(DumpState *s, int fd, bool
>> paging, bool has_filter,
>> Error *err = NULL;
>> int ret;
>>
>> + /* kdump-compressed is conflict with paging and filter */
>> + if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
>> + assert(!paging && !has_filter);
>> + }
>> +
>> if (runstate_is_running()) {
>> vm_stop(RUN_STATE_SAVE_VM);
>> s->resume = true;
>> @@ -1565,6 +1629,28 @@ static int dump_init(DumpState *s, int fd, bool
>> paging, bool has_filter,
>> tmp = DIV_ROUND_UP(DIV_ROUND_UP(s->max_mapnr, CHAR_BIT),
>> s->page_size);
>> s->len_dump_bitmap = tmp * s->page_size;
>>
>> + /* init for kdump-compressed format */
>> + if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
>> + switch (format) {
>> + case DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB:
>> + s->flag_compress = DUMP_DH_COMPRESSED_ZLIB;
>> + break;
>> +
>> + case DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO:
>> + s->flag_compress = DUMP_DH_COMPRESSED_LZO;
>> + break;
>> +
>> + case DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY:
>> + s->flag_compress = DUMP_DH_COMPRESSED_SNAPPY;
>> + break;
>> +
>> + default:
>> + s->flag_compress = 0;
>> + }
>> +
>> + return 0;
>> + }
>> +
>> if (s->has_filter) {
>> memory_mapping_filter(&s->list, s->begin, s->length);
>> }
>> @@ -1624,14 +1710,25 @@ cleanup:
>> }
>>
>> void qmp_dump_guest_memory(bool paging, const char *file, bool
>> has_begin,
>> - int64_t begin, bool has_length, int64_t
>> length,
>> - Error **errp)
>> + int64_t begin, bool has_length,
>> + int64_t length, bool has_format,
>> + DumpGuestMemoryFormat format, Error **errp)
>> {
>> const char *p;
>> int fd = -1;
>> DumpState *s;
>> int ret;
>>
>> + /*
>> + * kdump-compressed format need the whole memory dumped, so
>> paging or
>> + * filter is not supported here.
>> + */
>> + if ((has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) &&
>> + (paging || has_begin || has_length)) {
>> + error_setg(errp, "kdump-compressed format doesn't support
>> paging or "
>> + "filter");
>> + return;
>> + }
>> if (has_begin && !has_length) {
>> error_set(errp, QERR_MISSING_PARAMETER, "length");
>> return;
>> @@ -1641,6 +1738,19 @@ void qmp_dump_guest_memory(bool paging, const
>> char *file, bool has_begin,
>> return;
>> }
>>
>> + /* check whether lzo/snappy is supported */
>> +#ifndef CONFIG_LZO
>> + if (has_format && format == DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO) {
>> + error_setg(errp, "kdump-lzo is not available now");
>> + }
>> +#endif
>
> When kdump-lzo is not available, command still writes a dump,
> which is half smaller then uncompressed one (and is read as partial
> dump), but is much bigger then the compressed one. Is it supposed to
> behave like that?
No, it's not.
The format availability checks set the error like they should, but the
return statements are missing. Compare these lzo and snappy fmt avail
tests to the paging/filtering test just above -- after setting the
error, qmp_dump_guest_memory() has no business continuing, it must bail out.
Good catch. I missed it in the v6 review (perhaps because I already
found other warts to clean up in these parts -- at least that's what I'm
telling myself :)).
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 12/13 v7] dump: make kdump-compressed format available for 'dump-guest-memory'
2014-01-17 7:46 ` [Qemu-devel] [PATCH 12/13 v7] dump: make kdump-compressed format available for 'dump-guest-memory' qiaonuohan
2014-01-23 15:17 ` Ekaterina Tumanova
@ 2014-01-24 12:06 ` Laszlo Ersek
1 sibling, 0 replies; 37+ messages in thread
From: Laszlo Ersek @ 2014-01-24 12:06 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi,
afaerber
Thanks for addressing my earlier comments. Some new ones below:
On 01/17/14 08:46, qiaonuohan wrote:
> + /* check whether lzo/snappy is supported */
> +#ifndef CONFIG_LZO
> + if (has_format && format == DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO) {
> + error_setg(errp, "kdump-lzo is not available now");
> + }
> +#endif
> +
> +#ifndef CONFIG_SNAPPY
> + if (has_format && format == DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY) {
> + error_setg(errp, "kdump-snappy is not available now");
> + }
> +#endif
Ekaterina caught in testing that these two blocks must be complemented
with a return statement each -- when you detect these errors,
qmp_dump_guest_memory() must not proceed. Apologies for not noticing
them in v6.
> +##
> # @dump-guest-memory
> #
> # Dump guest's memory to vmcore. It is a synchronous operation that can take
> @@ -2712,13 +2730,18 @@
> # want to dump all guest's memory, please specify the start @begin
> # and @length
> #
> +# @format: #optional 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 @format at the same time
Please make it more precise with "non-elf", as in:
... are not allowed to be specified with *non-elf* @format at the same
time
Not really relevant but since you'll respin anyway... :)
> +# (since 2.0)
> +#
> # Returns: nothing on success
> #
> # Since: 1.2
> ##
> { 'command': 'dump-guest-memory',
> 'data': { 'paging': 'bool', 'protocol': 'str', '*begin': 'int',
> - '*length': 'int' } }
> + '*length': 'int', '*format': 'DumpGuestMemoryFormat' } }
>
> ##
> # @netdev_add:
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 02cc815..9158f22 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -791,8 +791,8 @@ EQMP
>
> {
> .name = "dump-guest-memory",
> - .args_type = "paging:b,protocol:s,begin:i?,end:i?",
> - .params = "-p protocol [begin] [length]",
> + .args_type = "paging:b,protocol:s,begin:i?,end:i?,format:s?",
> + .params = "-p protocol [begin] [length] [format]",
> .help = "dump guest memory to file",
> .user_print = monitor_user_noop,
> .mhandler.cmd_new = qmp_marshal_input_dump_guest_memory,
> @@ -813,6 +813,9 @@ Arguments:
> with length together (json-int)
> - "length": the memory size, in bytes. It's optional, and should be specified
> with begin together (json-int)
> +- "format": the format of guest memory dump. It's optional, and can be
> + elf|kdump-zlib|kdump-lzo|kdump-snappy, but non-elf formats will
> + conflict with paging and filter, ie. begin and length(json-string)
Please add a space between "length" and "(json-string)".
Thank you!
Laszlo
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH 13/13 v7] dump: add 'query-dump-guest-memory-capability' command
2014-01-17 7:46 [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (11 preceding siblings ...)
2014-01-17 7:46 ` [Qemu-devel] [PATCH 12/13 v7] dump: make kdump-compressed format available for 'dump-guest-memory' qiaonuohan
@ 2014-01-17 7:46 ` qiaonuohan
2014-01-24 12:31 ` Laszlo Ersek
2014-01-17 8:50 ` [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format Christian Borntraeger
2014-01-21 9:56 ` Qiao Nuohan
14 siblings, 1 reply; 37+ messages in thread
From: qiaonuohan @ 2014-01-17 7:46 UTC (permalink / raw)
To: lersek, stefanha, lcapitulino, afaerber, eblake
Cc: kumagai-atsushi, qiaonuohan, anderson, qemu-devel
'query-dump-guest-memory-capability' is used to query whether option 'format'
is available for 'dump-guest-memory' and the available format. The output
of the command will be like:
-> { "execute": "query-dump-guest-memory-capability" }
<- { "return": {
"format-option": "optional",
"capabilities": [
{"available": true, "format": "elf"},
{"available": true, "format": "kdump-zlib"},
{"available": true, "format": "kdump-lzo"},
{"available": true, "format": "kdump-snappy"}
]
}
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
---
dump.c | 42 ++++++++++++++++++++++++++++++++++++++++++
qapi-schema.json | 13 +++++++++++++
qmp-commands.hx | 31 +++++++++++++++++++++++++++++++
3 files changed, 86 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index dbf4bb6..c288cd3 100644
--- a/dump.c
+++ b/dump.c
@@ -1794,3 +1794,45 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
g_free(s);
}
+
+DumpGuestMemoryCapability *qmp_query_dump_guest_memory_capability(Error **errp)
+{
+ int i;
+ DumpGuestMemoryCapabilityStatusList *item;
+ DumpGuestMemoryCapability *cap =
+ g_malloc0(sizeof(DumpGuestMemoryCapability));
+
+ cap->format_option = g_strdup_printf("optional");
+
+ for (i = 0; i < DUMP_GUEST_MEMORY_FORMAT_MAX; i++) {
+ if (cap->capabilities == NULL) {
+ item = g_malloc0(sizeof(DumpGuestMemoryCapabilityStatusList));
+ cap->capabilities = item;
+ } else {
+ item->next = g_malloc0(sizeof(DumpGuestMemoryCapabilityStatusList));
+ item = item->next;
+ }
+
+ item->value = g_malloc0(sizeof(struct DumpGuestMemoryCapabilityStatus));
+ item->value->format = i;
+
+ if (i == DUMP_GUEST_MEMORY_FORMAT_ELF || i ==
+ DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB) {
+ item->value->available = true;
+ }
+
+#ifdef CONFIG_LZO
+ if (i == DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO) {
+ item->value->available = true;
+ }
+#endif
+
+#ifdef CONFIG_SNAPPY
+ if (i == DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY) {
+ item->value->available = true;
+ }
+#endif
+ }
+
+ return cap;
+}
diff --git a/qapi-schema.json b/qapi-schema.json
index 52df894..3085294 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2744,6 +2744,19 @@
'*length': 'int', '*format': 'DumpGuestMemoryFormat' } }
##
+# Since: 2.0
+##
+{ 'type': 'DumpGuestMemoryCapabilityStatus',
+ 'data': { 'format': 'DumpGuestMemoryFormat', 'available': 'bool' } }
+
+{ 'type': 'DumpGuestMemoryCapability',
+ 'data': {
+ 'format-option': 'str',
+ 'capabilities': ['DumpGuestMemoryCapabilityStatus'] } }
+
+{ 'command': 'query-dump-guest-memory-capability', 'returns': 'DumpGuestMemoryCapability' }
+
+##
# @netdev_add:
#
# Add a network backend.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 9158f22..ad51de0 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -829,6 +829,37 @@ Notes:
EQMP
{
+ .name = "query-dump-guest-memory-capability",
+ .args_type = "",
+ .mhandler.cmd_new = qmp_marshal_input_query_dump_guest_memory_capability,
+ },
+
+SQMP
+query-dump-guest-memory-capability
+----------
+
+Show whether option 'format' is available for 'dump-guest-memory' and the
+available format.
+
+Example:
+
+-> { "execute": "query-dump-guest-memory-capability" }
+<- { "return": {
+ "format-option": "optional",
+ "capabilities": [
+ {"available": true, "format": "elf"},
+ {"available": true, "format": "kdump-zlib"},
+ {"available": true, "format": "kdump-lzo"},
+ {"available": true, "format": "kdump-snappy"}
+ ]
+ }
+
+Note: This is a light-weight introspection to let management know whether format
+ option is available and the supported compression format.
+
+EQMP
+
+ {
.name = "netdev_add",
.args_type = "netdev:O",
.mhandler.cmd_new = qmp_netdev_add,
--
1.7.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 13/13 v7] dump: add 'query-dump-guest-memory-capability' command
2014-01-17 7:46 ` [Qemu-devel] [PATCH 13/13 v7] dump: add 'query-dump-guest-memory-capability' command qiaonuohan
@ 2014-01-24 12:31 ` Laszlo Ersek
0 siblings, 0 replies; 37+ messages in thread
From: Laszlo Ersek @ 2014-01-24 12:31 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi,
afaerber
two comments below
On 01/17/14 08:46, qiaonuohan wrote:
> 'query-dump-guest-memory-capability' is used to query whether option 'format'
> is available for 'dump-guest-memory' and the available format. The output
> of the command will be like:
>
> -> { "execute": "query-dump-guest-memory-capability" }
> <- { "return": {
> "format-option": "optional",
> "capabilities": [
> {"available": true, "format": "elf"},
> {"available": true, "format": "kdump-zlib"},
> {"available": true, "format": "kdump-lzo"},
> {"available": true, "format": "kdump-snappy"}
> ]
> }
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> ---
> dump.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> qapi-schema.json | 13 +++++++++++++
> qmp-commands.hx | 31 +++++++++++++++++++++++++++++++
> 3 files changed, 86 insertions(+), 0 deletions(-)
>
> diff --git a/dump.c b/dump.c
> index dbf4bb6..c288cd3 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -1794,3 +1794,45 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
>
> g_free(s);
> }
> +
> +DumpGuestMemoryCapability *qmp_query_dump_guest_memory_capability(Error **errp)
> +{
> + int i;
> + DumpGuestMemoryCapabilityStatusList *item;
> + DumpGuestMemoryCapability *cap =
> + g_malloc0(sizeof(DumpGuestMemoryCapability));
> +
> + cap->format_option = g_strdup_printf("optional");
> +
> + for (i = 0; i < DUMP_GUEST_MEMORY_FORMAT_MAX; i++) {
> + if (cap->capabilities == NULL) {
> + item = g_malloc0(sizeof(DumpGuestMemoryCapabilityStatusList));
> + cap->capabilities = item;
> + } else {
> + item->next = g_malloc0(sizeof(DumpGuestMemoryCapabilityStatusList));
> + item = item->next;
> + }
> +
> + item->value = g_malloc0(sizeof(struct DumpGuestMemoryCapabilityStatus));
> + item->value->format = i;
> +
> + if (i == DUMP_GUEST_MEMORY_FORMAT_ELF || i ==
> + DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB) {
Line wrapping hiccup.
> + item->value->available = true;
> + }
> +
> +#ifdef CONFIG_LZO
> + if (i == DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO) {
> + item->value->available = true;
> + }
> +#endif
> +
> +#ifdef CONFIG_SNAPPY
> + if (i == DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY) {
> + item->value->available = true;
> + }
> +#endif
> + }
> +
> + return cap;
> +}
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 52df894..3085294 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2744,6 +2744,19 @@
> '*length': 'int', '*format': 'DumpGuestMemoryFormat' } }
>
> ##
> +# Since: 2.0
> +##
> +{ 'type': 'DumpGuestMemoryCapabilityStatus',
> + 'data': { 'format': 'DumpGuestMemoryFormat', 'available': 'bool' } }
> +
> +{ 'type': 'DumpGuestMemoryCapability',
> + 'data': {
> + 'format-option': 'str',
> + 'capabilities': ['DumpGuestMemoryCapabilityStatus'] } }
> +
> +{ 'command': 'query-dump-guest-memory-capability', 'returns': 'DumpGuestMemoryCapability' }
> +
> +##
> # @netdev_add:
> #
> # Add a network backend.
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 9158f22..ad51de0 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -829,6 +829,37 @@ Notes:
> EQMP
>
> {
> + .name = "query-dump-guest-memory-capability",
> + .args_type = "",
> + .mhandler.cmd_new = qmp_marshal_input_query_dump_guest_memory_capability,
> + },
> +
> +SQMP
> +query-dump-guest-memory-capability
> +----------
> +
> +Show whether option 'format' is available for 'dump-guest-memory' and the
> +available format.
> +
> +Example:
> +
> +-> { "execute": "query-dump-guest-memory-capability" }
> +<- { "return": {
> + "format-option": "optional",
> + "capabilities": [
> + {"available": true, "format": "elf"},
> + {"available": true, "format": "kdump-zlib"},
> + {"available": true, "format": "kdump-lzo"},
> + {"available": true, "format": "kdump-snappy"}
> + ]
> + }
> +
> +Note: This is a light-weight introspection to let management know whether format
> + option is available and the supported compression format.
> +
> +EQMP
> +
> + {
> .name = "netdev_add",
> .args_type = "netdev:O",
> .mhandler.cmd_new = qmp_netdev_add,
>
Although in general I don't like to obsess about English grammar, this
should say available / supported "formats", plural (two instances in the
above text).
I'm only proposing these because you'll respin anyway, but I don't
insist. If you change nothing else in this patch, then you can add my
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format
2014-01-17 7:46 [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (12 preceding siblings ...)
2014-01-17 7:46 ` [Qemu-devel] [PATCH 13/13 v7] dump: add 'query-dump-guest-memory-capability' command qiaonuohan
@ 2014-01-17 8:50 ` Christian Borntraeger
2014-01-17 9:04 ` Qiao Nuohan
2014-01-21 9:56 ` Qiao Nuohan
14 siblings, 1 reply; 37+ messages in thread
From: Christian Borntraeger @ 2014-01-17 8:50 UTC (permalink / raw)
To: qiaonuohan, lersek, stefanha, lcapitulino, afaerber, eblake
Cc: anderson, Ekaterina Tumanova, kumagai-atsushi, qemu-devel
On 17/01/14 08:46, qiaonuohan wrote:
> Hi, all
>
> The last version is here:
> http://lists.nongnu.org/archive/html/qemu-devel/2014-01/msg00209.html
>
> Command 'dump-guest-memory' was introduced to dump guest's memory. But the
> vmcore's format is only elf32 or elf64. The message is here:
> http://lists.gnu.org/archive/html/qemu-devel/2012-04/msg03379.html
>
> Compared with migration, the missing of compression feature means regression
> to 'dump-guest-memory'. So we post these patches to make 'dump-guest-memory' be
> able to dump guest's in kdump-compressed format. Then vmcore can be much
> smaller, and easily to be delivered.
Nice, we certainly want that.
>
> The kdump-compressed format is *linux specific* *linux standard* crash dump
> format used in kdump framework. The kdump-compressed format is readable only
> with the crash utility, and it can be smaller than the ELF format because of
> the compression support. To get more detailed information about
> kdump-compressed format, please refer to the following URL:
> http://sourceforge.net/projects/makedumpfile/
>
> Note, similar to 'dump-guest-memory':
> 1. The guest should be x86 or x86_64. The other arch is not supported now.
What is the reason for that? Looking over the patch set, there seem to
be no architecture folder involved and others like s390x also have the
dump-guest-memory support.
[...]
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format
2014-01-17 8:50 ` [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format Christian Borntraeger
@ 2014-01-17 9:04 ` Qiao Nuohan
0 siblings, 0 replies; 37+ messages in thread
From: Qiao Nuohan @ 2014-01-17 9:04 UTC (permalink / raw)
To: Christian Borntraeger
Cc: stefanha, Ekaterina Tumanova, qemu-devel, lcapitulino,
kumagai-atsushi, anderson, lersek, afaerber
On 01/17/2014 04:50 PM, Christian Borntraeger wrote:
> What is the reason for that? Looking over the patch set, there seem to
> be no architecture folder involved and others like s390x also have the
> dump-guest-memory support.
> [...]
I found dump-guest-memory have support s390/ppc. I do not restrict the guest
should only be x86/x86_64, actually, I get no knowledge about other arch, so
what I can do now is just make x86/x86_64 works and be tested.
--
Regards
Qiao Nuohan
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format
2014-01-17 7:46 [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (13 preceding siblings ...)
2014-01-17 8:50 ` [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format Christian Borntraeger
@ 2014-01-21 9:56 ` Qiao Nuohan
2014-01-21 10:14 ` Laszlo Ersek
14 siblings, 1 reply; 37+ messages in thread
From: Qiao Nuohan @ 2014-01-21 9:56 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi,
lersek, afaerber
Hello,
Do you have some comments on the version?
On 01/17/2014 03:46 PM, qiaonuohan wrote:
> Hi, all
>
> The last version is here:
> http://lists.nongnu.org/archive/html/qemu-devel/2014-01/msg00209.html
>
> Command 'dump-guest-memory' was introduced to dump guest's memory. But the
> vmcore's format is only elf32 or elf64. The message is here:
> http://lists.gnu.org/archive/html/qemu-devel/2012-04/msg03379.html
>
> Compared with migration, the missing of compression feature means regression
> to 'dump-guest-memory'. So we post these patches to make 'dump-guest-memory' be
> able to dump guest's in kdump-compressed format. Then vmcore can be much
> smaller, and easily to be delivered.
>
> The kdump-compressed format is *linux specific* *linux standard* crash dump
> format used in kdump framework. The kdump-compressed format is readable only
> with the crash utility, and it can be smaller than the ELF format because of
> the compression support. To get more detailed information about
> kdump-compressed format, please refer to the following URL:
> http://sourceforge.net/projects/makedumpfile/
>
> Note, similar to 'dump-guest-memory':
> 1. The guest should be x86 or x86_64. The other arch is not supported now.
> 2. If the OS is in the second kernel, gdb may not work well, and crash can
> work by specifying '--machdep phys_addr=xxx' in the command line. The
> reason is that the second kernel will update the page table, and we can
> not get the page table for the first kernel.
> 3. The cpu's state is stored in QEMU note.
> 4. The vmcore are able to be compressed with zlib, lzo or snappy. zlib is
> available by default, and option '--enable-lzo' or '--enable-snappy'
> should be specified with 'configure' to make lzo or snappy available.
>
> Changelog:
> Changes from v6 to v7:
> 1. support BE host
> 2. abandon non-flatten format to avoid using seek on vmcore
> 3. abandon using of very large array
> 4. use get_next_page to replace the iteration of guest's pages
> 5. abandon the support of HMP
>
> Changes from v5 to v6:
> 1. add run-time check for compression format(lzo/snappy)
> 2. address Stefan's comments about reusing code and coding style
> 3. update the version of kdump-compressed format to 6th
> 4. resplit the patches
> 5. Add 'query-dump-guest-memory-capability' command
>
> Changes from v4 to v5:
> 1. using flatten format to avoid using temporary files according to Stefan's
> comments
> 2. Address Andreas's comments about coding style
>
> Changes from v3 to v4:
> 1. change to avoid conflict with Andreas's patches
> 2. rebase
>
> Changes from v2 to v3:
> 1. Address Eric's comment
>
> Changes from v1 to v2:
> 1. Address Eric& Daniel's comment: fix manner of string copy.
> 2. Address Eric's comment: replace reinventing new constants by using the
> ready-made ones accoring.
> 3. Address Andreas's comment: remove useless include.
>
> qiaonuohan (13):
> dump: const-qualify the buf of WriteCoreDumpFunction
> dump: add argument to write_elfxx_notes
> dump: add API to write header of flatten format
> dump: add API to write vmcore
> dump: add API to write elf notes to buffer
> dump: add support for lzo/snappy
> dump: add members to DumpState and init some of them
> dump: add API to write dump header
> dump: add API to write dump_bitmap
> dump: add APIs to operate DataCache
> dump: add API to write dump pages
> dump: make kdump-compressed format available for 'dump-guest-memory'
> dump: add 'query-dump-guest-memory-capability' command
>
> configure | 54 +++
> dump.c | 972 ++++++++++++++++++++++++++++++++++++++++++++++++-
> hmp.c | 5 +-
> include/qom/cpu.h | 3 +-
> include/sysemu/dump.h | 138 +++++++
> qapi-schema.json | 38 ++-
> qmp-commands.hx | 38 ++-
> 7 files changed, 1228 insertions(+), 20 deletions(-)
>
>
--
Regards
Qiao Nuohan
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format
2014-01-21 9:56 ` Qiao Nuohan
@ 2014-01-21 10:14 ` Laszlo Ersek
2014-01-22 1:27 ` Qiao Nuohan
0 siblings, 1 reply; 37+ messages in thread
From: Laszlo Ersek @ 2014-01-21 10:14 UTC (permalink / raw)
To: Qiao Nuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi,
afaerber
Hi,
On 01/21/14 10:56, Qiao Nuohan wrote:
> Do you have some comments on the version?
it's in my review queue. The last version took a lot of energy on my
part to review (it's long and complex) so I'm still "gearing up".
I very much hope I can review this version by diffing it with the last
version, and checking the differences against my earlier comments. If
that approach doesn't work then it'll take days again *after* I start.
Apologies :(
Thanks
Laszlo
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 00/13 v7] Make 'dump-guest-memory' dump in kdump-compressed format
2014-01-21 10:14 ` Laszlo Ersek
@ 2014-01-22 1:27 ` Qiao Nuohan
0 siblings, 0 replies; 37+ messages in thread
From: Qiao Nuohan @ 2014-01-22 1:27 UTC (permalink / raw)
To: Laszlo Ersek
Cc: stefanha, qemu-devel, lcapitulino, kumagai-atsushi, anderson,
afaerber
On 01/21/2014 06:14 PM, Laszlo Ersek wrote:
> it's in my review queue. The last version took a lot of energy on my
> part to review (it's long and complex) so I'm still "gearing up".
>
> I very much hope I can review this version by diffing it with the last
> version, and checking the differences against my earlier comments. If
> that approach doesn't work then it'll take days again*after* I start.
> Apologies:(
I see. Thanks for your help. According to your comments, I did a lot of changes
to my last series. Patch 01 are newly added, and patch 07 just move some
initialization from other patches. You can check the differences between the
reset patches with the last version. And thanks again.
--
Regards
Qiao Nuohan
^ permalink raw reply [flat|nested] 37+ messages in thread