* [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format
@ 2014-01-28 6:21 qiaonuohan
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 01/13] dump: const-qualify the buf of WriteCoreDumpFunction qiaonuohan
` (16 more replies)
0 siblings, 17 replies; 71+ messages in thread
From: qiaonuohan @ 2014-01-28 6:21 UTC (permalink / raw)
To: lersek, stefanha, lcapitulino, afaerber, eblake
Cc: kumagai-atsushi, qiaonuohan, anderson, qemu-devel
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 v7 to v8:
1. rebase get_max_mapnr()
2. fix bug of using dh->block_size directly in create_header64()
3. abandon using static variables in get_next_page()
4. redefine 'struct PageDesc' to 'struct QEMU_PACKED PageDescriptor'
5. add return when format checking fails
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 | 966 ++++++++++++++++++++++++++++++++++++++++++++++++-
hmp.c | 5 +-
include/qom/cpu.h | 3 +-
include/sysemu/dump.h | 138 +++++++
qapi-schema.json | 38 ++-
qmp-commands.hx | 38 ++-
7 files changed, 1222 insertions(+), 20 deletions(-)
^ permalink raw reply [flat|nested] 71+ messages in thread
* [Qemu-devel] [PATCH v8 01/13] dump: const-qualify the buf of WriteCoreDumpFunction
2014-01-28 6:21 [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
@ 2014-01-28 6:21 ` qiaonuohan
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 02/13] dump: add argument to write_elfxx_notes qiaonuohan
` (15 subsequent siblings)
16 siblings, 0 replies; 71+ messages in thread
From: qiaonuohan @ 2014-01-28 6:21 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>
Reviewed-by: Laszlo Ersek <lersek@redhat.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] 71+ messages in thread
* [Qemu-devel] [PATCH v8 02/13] dump: add argument to write_elfxx_notes
2014-01-28 6:21 [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 01/13] dump: const-qualify the buf of WriteCoreDumpFunction qiaonuohan
@ 2014-01-28 6:21 ` qiaonuohan
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 03/13] dump: add API to write header of flatten format qiaonuohan
` (14 subsequent siblings)
16 siblings, 0 replies; 71+ messages in thread
From: qiaonuohan @ 2014-01-28 6:21 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] 71+ messages in thread
* [Qemu-devel] [PATCH v8 03/13] dump: add API to write header of flatten format
2014-01-28 6:21 [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 01/13] dump: const-qualify the buf of WriteCoreDumpFunction qiaonuohan
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 02/13] dump: add argument to write_elfxx_notes qiaonuohan
@ 2014-01-28 6:21 ` qiaonuohan
2014-02-10 19:35 ` Luiz Capitulino
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 04/13] dump: add API to write vmcore qiaonuohan
` (13 subsequent siblings)
16 siblings, 1 reply; 71+ messages in thread
From: qiaonuohan @ 2014-01-28 6:21 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] 71+ messages in thread
* [Qemu-devel] [PATCH v8 04/13] dump: add API to write vmcore
2014-01-28 6:21 [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (2 preceding siblings ...)
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 03/13] dump: add API to write header of flatten format qiaonuohan
@ 2014-01-28 6:21 ` qiaonuohan
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 05/13] dump: add API to write elf notes to buffer qiaonuohan
` (12 subsequent siblings)
16 siblings, 0 replies; 71+ messages in thread
From: qiaonuohan @ 2014-01-28 6:21 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] 71+ messages in thread
* [Qemu-devel] [PATCH v8 05/13] dump: add API to write elf notes to buffer
2014-01-28 6:21 [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (3 preceding siblings ...)
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 04/13] dump: add API to write vmcore qiaonuohan
@ 2014-01-28 6:21 ` qiaonuohan
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 06/13] dump: add support for lzo/snappy qiaonuohan
` (11 subsequent siblings)
16 siblings, 0 replies; 71+ messages in thread
From: qiaonuohan @ 2014-01-28 6:21 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] 71+ messages in thread
* [Qemu-devel] [PATCH v8 06/13] dump: add support for lzo/snappy
2014-01-28 6:21 [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (4 preceding siblings ...)
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 05/13] dump: add API to write elf notes to buffer qiaonuohan
@ 2014-01-28 6:21 ` qiaonuohan
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 07/13] dump: add members to DumpState and init some of them qiaonuohan
` (10 subsequent siblings)
16 siblings, 0 replies; 71+ messages in thread
From: qiaonuohan @ 2014-01-28 6:21 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 b472694..d1b1005 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=""
@@ -948,6 +950,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"
@@ -1235,6 +1241,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
@@ -1539,6 +1547,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
@@ -3843,6 +3887,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"
@@ -4158,6 +4204,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] 71+ messages in thread
* [Qemu-devel] [PATCH v8 07/13] dump: add members to DumpState and init some of them
2014-01-28 6:21 [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (5 preceding siblings ...)
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 06/13] dump: add support for lzo/snappy qiaonuohan
@ 2014-01-28 6:22 ` qiaonuohan
2014-01-29 14:11 ` Laszlo Ersek
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 08/13] dump: add API to write dump header qiaonuohan
` (9 subsequent siblings)
16 siblings, 1 reply; 71+ messages in thread
From: qiaonuohan @ 2014-01-28 6:22 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 | 28 ++++++++++++++++++++++++++++
include/sysemu/dump.h | 7 +++++++
2 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index 2b940bd..3a1944e 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,14 @@ static ram_addr_t get_start_block(DumpState *s)
return -1;
}
+static void get_max_mapnr(DumpState *s)
+{
+ GuestPhysBlock *last_block;
+
+ last_block = QTAILQ_LAST(&s->guest_phys_blocks.head, GuestPhysBlockHead);
+ s->max_mapnr = paddr_to_pfn(last_block->target_end, 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 +882,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] 71+ messages in thread
* [Qemu-devel] [PATCH v8 08/13] dump: add API to write dump header
2014-01-28 6:21 [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (6 preceding siblings ...)
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 07/13] dump: add members to DumpState and init some of them qiaonuohan
@ 2014-01-28 6:22 ` qiaonuohan
2014-01-28 11:51 ` Ekaterina Tumanova
` (2 more replies)
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 09/13] dump: add API to write dump_bitmap qiaonuohan
` (8 subsequent siblings)
16 siblings, 3 replies; 71+ messages in thread
From: qiaonuohan @ 2014-01-28 6:22 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 3a1944e..4b2799f 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, 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), "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 * 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 *
+ 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] 71+ messages in thread
* [Qemu-devel] [PATCH v8 09/13] dump: add API to write dump_bitmap
2014-01-28 6:21 [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (7 preceding siblings ...)
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 08/13] dump: add API to write dump header qiaonuohan
@ 2014-01-28 6:22 ` qiaonuohan
2014-01-29 14:32 ` Laszlo Ersek
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 10/13] dump: add APIs to operate DataCache qiaonuohan
` (7 subsequent siblings)
16 siblings, 1 reply; 71+ messages in thread
From: qiaonuohan @ 2014-01-28 6:22 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>
---
dump.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++++
include/sysemu/dump.h | 2 +
2 files changed, 166 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index 4b2799f..5755534 100644
--- a/dump.c
+++ b/dump.c
@@ -1001,6 +1001,170 @@ 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 frame number and the address of the page.
+ * 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 bool get_next_page(GuestPhysBlock **blockptr, uint64_t *pfnptr,
+ uint8_t **bufptr, DumpState *s)
+{
+ GuestPhysBlock *block = *blockptr;
+ hwaddr addr;
+ uint8_t *buf;
+
+ /* block == NULL means the start of the iteration */
+ if (!block) {
+ block = QTAILQ_FIRST(&s->guest_phys_blocks.head);
+ *blockptr = block;
+ assert(block->target_start % s->page_size == 0);
+ assert(block->target_end % s->page_size == 0);
+ *pfnptr = paddr_to_pfn(block->target_start, s->page_shift);
+ if (bufptr) {
+ *bufptr = block->host_addr;
+ }
+ return true;
+ }
+
+ *pfnptr = *pfnptr + 1;
+ addr = pfn_to_paddr(*pfnptr, 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);
+ *blockptr = block;
+ if (!block) {
+ return false;
+ }
+ assert(block->target_start % s->page_size == 0);
+ assert(block->target_end % s->page_size == 0);
+ *pfnptr = paddr_to_pfn(block->target_start, s->page_shift);
+ buf = block->host_addr;
+ }
+
+ if (bufptr) {
+ *bufptr = buf;
+ }
+
+ return true;
+}
+
+static int write_dump_bitmap(DumpState *s)
+{
+ int ret = 0;
+ uint64_t last_pfn, pfn;
+ void *dump_bitmap_buf;
+ size_t num_dumpable;
+ GuestPhysBlock *block_iter = NULL;
+
+ /* 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(&block_iter, &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] 71+ messages in thread
* [Qemu-devel] [PATCH v8 10/13] dump: add APIs to operate DataCache
2014-01-28 6:21 [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (8 preceding siblings ...)
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 09/13] dump: add API to write dump_bitmap qiaonuohan
@ 2014-01-28 6:22 ` qiaonuohan
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 11/13] dump: add API to write dump pages qiaonuohan
` (6 subsequent siblings)
16 siblings, 0 replies; 71+ messages in thread
From: qiaonuohan @ 2014-01-28 6:22 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 5755534..a7a85d3 100644
--- a/dump.c
+++ b/dump.c
@@ -1165,6 +1165,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] 71+ messages in thread
* [Qemu-devel] [PATCH v8 11/13] dump: add API to write dump pages
2014-01-28 6:21 [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (9 preceding siblings ...)
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 10/13] dump: add APIs to operate DataCache qiaonuohan
@ 2014-01-28 6:22 ` qiaonuohan
2014-01-29 14:39 ` Laszlo Ersek
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 12/13] dump: make kdump-compressed format available for 'dump-guest-memory' qiaonuohan
` (5 subsequent siblings)
16 siblings, 1 reply; 71+ messages in thread
From: qiaonuohan @ 2014-01-28 6:22 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>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
dump.c | 231 +++++++++++++++++++++++++++++++++++++++++++++++++
include/sysemu/dump.h | 7 ++
2 files changed, 238 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index a7a85d3..31be686 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) {
@@ -1212,6 +1220,229 @@ 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;
+ PageDescriptor pd, pd_zero;
+ uint8_t *buf;
+ int endian = s->dump_info.d_endian;
+ GuestPhysBlock *block_iter = NULL;
+ uint64_t pfn_iter;
+
+ /* get offset of page_desc and page_data in dump file */
+ offset_desc = s->offset_page;
+ offset_data = offset_desc + sizeof(PageDescriptor) * 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(&block_iter, &pfn_iter, &buf, s)) {
+ /* check zero page */
+ if (is_zero_page(buf, s->page_size)) {
+ ret = write_cache(&page_desc, &pd_zero, sizeof(PageDescriptor),
+ 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(PageDescriptor), 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..efab7a3 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 QEMU_PACKED PageDescriptor {
+ 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 */
+} PageDescriptor;
+
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] 71+ messages in thread
* [Qemu-devel] [PATCH v8 12/13] dump: make kdump-compressed format available for 'dump-guest-memory'
2014-01-28 6:21 [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (10 preceding siblings ...)
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 11/13] dump: add API to write dump pages qiaonuohan
@ 2014-01-28 6:22 ` qiaonuohan
2014-01-29 14:45 ` Laszlo Ersek
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 13/13] dump: add 'query-dump-guest-memory-capability' command qiaonuohan
` (4 subsequent siblings)
16 siblings, 1 reply; 71+ messages in thread
From: qiaonuohan @ 2014-01-28 6:22 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 | 131 +++++++++++++++++++++++++++++++++++++++++++++++++++---
hmp.c | 5 ++-
qapi-schema.json | 25 ++++++++++-
qmp-commands.hx | 7 ++-
4 files changed, 158 insertions(+), 10 deletions(-)
diff --git a/dump.c b/dump.c
index 31be686..2ebbb23 100644
--- a/dump.c
+++ b/dump.c
@@ -1443,6 +1443,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;
@@ -1479,7 +1537,8 @@ static void get_max_mapnr(DumpState *s)
s->max_mapnr = paddr_to_pfn(last_block->target_end, s->page_shift);
}
-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;
@@ -1487,6 +1546,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;
@@ -1557,6 +1621,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);
}
@@ -1616,14 +1702,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;
@@ -1633,6 +1730,21 @@ 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");
+ return;
+ }
+#endif
+
+#ifndef CONFIG_SNAPPY
+ if (has_format && format == DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY) {
+ error_setg(errp, "kdump-snappy is not available now");
+ return;
+ }
+#endif
+
#if !defined(WIN32)
if (strstart(file, "fd:", &p)) {
fd = monitor_get_fd(cur_mon, p, errp);
@@ -1657,14 +1769,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 1af0809..4f756ce 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1310,8 +1310,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) {
@@ -1324,7 +1327,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 05ced9d..7f62007 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2716,6 +2716,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
@@ -2751,13 +2769,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 non-elf @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 cce6b81..019dde6 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] 71+ messages in thread
* [Qemu-devel] [PATCH v8 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-01-28 6:21 [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (11 preceding siblings ...)
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 12/13] dump: make kdump-compressed format available for 'dump-guest-memory' qiaonuohan
@ 2014-01-28 6:22 ` qiaonuohan
2014-02-10 19:10 ` Luiz Capitulino
2014-01-28 6:37 ` [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format Qiao Nuohan
` (3 subsequent siblings)
16 siblings, 1 reply; 71+ messages in thread
From: qiaonuohan @ 2014-01-28 6:22 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>
Reviewed-by: Laszlo Ersek <lersek@redhat.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 2ebbb23..8f64aab 100644
--- a/dump.c
+++ b/dump.c
@@ -1788,3 +1788,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 7f62007..012c70c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2783,6 +2783,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 019dde6..e1b311a 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 formats.
+
+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 formats.
+
+EQMP
+
+ {
.name = "netdev_add",
.args_type = "netdev:O",
.mhandler.cmd_new = qmp_netdev_add,
--
1.7.1
^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format
2014-01-28 6:21 [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (12 preceding siblings ...)
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 13/13] dump: add 'query-dump-guest-memory-capability' command qiaonuohan
@ 2014-01-28 6:37 ` Qiao Nuohan
2014-01-29 14:50 ` Laszlo Ersek
` (2 subsequent siblings)
16 siblings, 0 replies; 71+ messages in thread
From: Qiao Nuohan @ 2014-01-28 6:37 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, kumagai-atsushi, anderson,
lersek, afaerber
On 01/28/2014 02:21 PM, qiaonuohan wrote:
> 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
Forget to change the URL, sorry. The last version is:
http://lists.gnu.org/archive/html/qemu-devel/2014-01/msg02139.html
--
Regards
Qiao Nuohan
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 08/13] dump: add API to write dump header
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 08/13] dump: add API to write dump header qiaonuohan
@ 2014-01-28 11:51 ` Ekaterina Tumanova
2014-01-28 13:37 ` Laszlo Ersek
2014-01-29 14:19 ` Laszlo Ersek
2014-01-30 17:14 ` Ekaterina Tumanova
2 siblings, 1 reply; 71+ messages in thread
From: Ekaterina Tumanova @ 2014-01-28 11:51 UTC (permalink / raw)
To: qemu-devel, Christian Borntraeger
On 01/28/2014 10:22 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 3a1944e..4b2799f 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);
> +
In my opinion, it's not a right thing to hardcode the architecture in
arch-independent module dump.c
you hardcode it here in create_header32 routine by
memcpy(&(dh->utsname.machine), "i686", 4);
and in create_header64 routine by
memcpy(&(dh->utsname.machine), "x86_64", 6);
Besides that you code actually can work on s390x arch. IMHO, target
arch should be coded here.
Maybe you could make use of this function: cpu_to_uname_machine.
> + 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, 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), "x86_64", 6);
dito
hardcoding the architecture should be fixed, IMHO (see above):
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 * 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 *
> + 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] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 08/13] dump: add API to write dump header
2014-01-28 11:51 ` Ekaterina Tumanova
@ 2014-01-28 13:37 ` Laszlo Ersek
2014-01-28 14:42 ` Ekaterina Tumanova
2014-01-29 1:39 ` Qiao Nuohan
0 siblings, 2 replies; 71+ messages in thread
From: Laszlo Ersek @ 2014-01-28 13:37 UTC (permalink / raw)
To: Ekaterina Tumanova, Qiao Nuohan; +Cc: Christian Borntraeger, qemu-devel
Hello Ekaterina,
On 01/28/14 12:51, Ekaterina Tumanova wrote:
> On 01/28/2014 10:22 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 3a1944e..4b2799f 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);
>> +
>
> In my opinion, it's not a right thing to hardcode the architecture in
> arch-independent module dump.c
>
> you hardcode it here in create_header32 routine by
>
> memcpy(&(dh->utsname.machine), "i686", 4);
>
> and in create_header64 routine by
>
> memcpy(&(dh->utsname.machine), "x86_64", 6);
>
> Besides that you code actually can work on s390x arch. IMHO, target
> arch should be coded here.
>
> Maybe you could make use of this function: cpu_to_uname_machine.
I seem to recall that Qiao Nuohan stated that he (*) couldn't test this
feature on any other architecture than i686/x86_64.
So my proposal is, let's not block the series based on just this one
point. Let's review it and allow it to be merged (if there are no other
problems).
Then you and/or Christian could post a small patch that allows the
feature to work on s390x too, checking also that your patch doesn't
regress it on x86_64. Perhaps Qiao Nuohan could re-test at that time for
regressions (on x86_64), and follow up with his (*) Tested-by.
Does it sound acceptable?
(*) Qiao Nuohan, could you please state if you'd like to be referred to
as "he" or "she" in third person; also, as "Qiao" or "Nuohan" when using
just one name? I apoligize but I can't figure it out :(
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 08/13] dump: add API to write dump header
2014-01-28 13:37 ` Laszlo Ersek
@ 2014-01-28 14:42 ` Ekaterina Tumanova
2014-01-28 14:55 ` Laszlo Ersek
2014-01-29 1:39 ` Qiao Nuohan
1 sibling, 1 reply; 71+ messages in thread
From: Ekaterina Tumanova @ 2014-01-28 14:42 UTC (permalink / raw)
To: qemu-devel, Christian Borntraeger
On 01/28/2014 05:37 PM, Laszlo Ersek wrote:
> Hello Ekaterina,
>
> On 01/28/14 12:51, Ekaterina Tumanova wrote:
>> On 01/28/2014 10:22 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 3a1944e..4b2799f 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);
>>> +
>>
>> In my opinion, it's not a right thing to hardcode the architecture in
>> arch-independent module dump.c
>>
>> you hardcode it here in create_header32 routine by
>>
>> memcpy(&(dh->utsname.machine), "i686", 4);
>>
>> and in create_header64 routine by
>>
>> memcpy(&(dh->utsname.machine), "x86_64", 6);
>>
>> Besides that you code actually can work on s390x arch. IMHO, target
>> arch should be coded here.
>>
>> Maybe you could make use of this function: cpu_to_uname_machine.
>
> I seem to recall that Qiao Nuohan stated that he (*) couldn't test this
> feature on any other architecture than i686/x86_64.
>
> So my proposal is, let's not block the series based on just this one
> point. Let's review it and allow it to be merged (if there are no other
> problems).
>
> Then you and/or Christian could post a small patch that allows the
> feature to work on s390x too, checking also that your patch doesn't
> regress it on x86_64. Perhaps Qiao Nuohan could re-test at that time for
> regressions (on x86_64), and follow up with his (*) Tested-by.
>
> Does it sound acceptable?
>
> (*) Qiao Nuohan, could you please state if you'd like to be referred to
> as "he" or "she" in third person; also, as "Qiao" or "Nuohan" when using
> just one name? I apoligize but I can't figure it out :(
>
> Thanks,
> Laszlo
>
Thanks you for your reply, Laszlo.
Just couple of thoughts:
Firstly, I already mentioned this in previous review cycle, but there
was no answer/changes :(
Secondly, it feels really wrong to hardcode the specific arch in
the arch-independent file, especially because patch doesn't
prevent this function to be compiled and used on other architectures
(only mentions this in commit message).
Thirdly, the fix seems pretty simple, so why do not incorporate it
in the first place...
Regards,
Kate.
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 08/13] dump: add API to write dump header
2014-01-28 14:42 ` Ekaterina Tumanova
@ 2014-01-28 14:55 ` Laszlo Ersek
0 siblings, 0 replies; 71+ messages in thread
From: Laszlo Ersek @ 2014-01-28 14:55 UTC (permalink / raw)
To: Ekaterina Tumanova; +Cc: Christian Borntraeger, Qiao Nuohan, qemu-devel
On 01/28/14 15:42, Ekaterina Tumanova wrote:
> On 01/28/2014 05:37 PM, Laszlo Ersek wrote:
>> Hello Ekaterina,
>>
>> On 01/28/14 12:51, Ekaterina Tumanova wrote:
>>> On 01/28/2014 10:22 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 3a1944e..4b2799f 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);
>>>> +
>>>
>>> In my opinion, it's not a right thing to hardcode the architecture in
>>> arch-independent module dump.c
>>>
>>> you hardcode it here in create_header32 routine by
>>>
>>> memcpy(&(dh->utsname.machine), "i686", 4);
>>>
>>> and in create_header64 routine by
>>>
>>> memcpy(&(dh->utsname.machine), "x86_64", 6);
>>>
>>> Besides that you code actually can work on s390x arch. IMHO, target
>>> arch should be coded here.
>>>
>>> Maybe you could make use of this function: cpu_to_uname_machine.
>>
>> I seem to recall that Qiao Nuohan stated that he (*) couldn't test this
>> feature on any other architecture than i686/x86_64.
>>
>> So my proposal is, let's not block the series based on just this one
>> point. Let's review it and allow it to be merged (if there are no other
>> problems).
>>
>> Then you and/or Christian could post a small patch that allows the
>> feature to work on s390x too, checking also that your patch doesn't
>> regress it on x86_64. Perhaps Qiao Nuohan could re-test at that time for
>> regressions (on x86_64), and follow up with his (*) Tested-by.
>>
>> Does it sound acceptable?
>>
>> (*) Qiao Nuohan, could you please state if you'd like to be referred to
>> as "he" or "she" in third person; also, as "Qiao" or "Nuohan" when using
>> just one name? I apoligize but I can't figure it out :(
>>
>> Thanks,
>> Laszlo
>>
>
> Thanks you for your reply, Laszlo.
>
> Just couple of thoughts:
>
> Firstly, I already mentioned this in previous review cycle, but there
> was no answer/changes :(
I thought that Qiao had addressed this before (replying to Christian):
http://thread.gmane.org/gmane.comp.emulators.qemu/251215/focus=251227
But I agree that no consensus was reached.
> Secondly, it feels really wrong to hardcode the specific arch in
> the arch-independent file, especially because patch doesn't
> prevent this function to be compiled and used on other architectures
> (only mentions this in commit message).
I do agree with that, but it doesn't regress existing functionality (ie.
it's pure improvement on x86 and x86_64, and doesn't break existing code
on other arches).
> Thirdly, the fix seems pretty simple, so why do not incorporate it
> in the first place...
If you're implying (as before) cpu_to_uname_machine(), that per se would
break the x86_64 case (which is the primary target arch for the
feature). Namely:
- the dump header needs: x86_64
- the proposed function returns: x86-64
Note the difference between underscore (_) and hyphen (-).
Thanks
Laszlo
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 08/13] dump: add API to write dump header
2014-01-28 13:37 ` Laszlo Ersek
2014-01-28 14:42 ` Ekaterina Tumanova
@ 2014-01-29 1:39 ` Qiao Nuohan
1 sibling, 0 replies; 71+ messages in thread
From: Qiao Nuohan @ 2014-01-29 1:39 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: Christian Borntraeger, Ekaterina Tumanova, qemu-devel
On 01/28/2014 09:37 PM, Laszlo Ersek wrote:
> I seem to recall that Qiao Nuohan stated that he (*) couldn't test this
> feature on any other architecture than i686/x86_64.
>
> So my proposal is, let's not block the series based on just this one
> point. Let's review it and allow it to be merged (if there are no other
> problems).
>
> Then you and/or Christian could post a small patch that allows the
> feature to work on s390x too, checking also that your patch doesn't
> regress it on x86_64. Perhaps Qiao Nuohan could re-test at that time for
> regressions (on x86_64), and follow up with his (*) Tested-by.
>
> Does it sound acceptable?
I will retest it at that time. Thanks for your help on other architecture.
And unfortunately, Ekaterina, I have just noticed that I failed to receive
your mails. :(
BTW, I am male.
--
Regards
Qiao Nuohan
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 07/13] dump: add members to DumpState and init some of them
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 07/13] dump: add members to DumpState and init some of them qiaonuohan
@ 2014-01-29 14:11 ` Laszlo Ersek
0 siblings, 0 replies; 71+ messages in thread
From: Laszlo Ersek @ 2014-01-29 14:11 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi, afaerber
On 01/28/14 07:22, 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 | 28 ++++++++++++++++++++++++++++
> include/sysemu/dump.h | 7 +++++++
> 2 files changed, 35 insertions(+), 0 deletions(-)
>
> +static void get_max_mapnr(DumpState *s)
> +{
> + GuestPhysBlock *last_block;
> +
> + last_block = QTAILQ_LAST(&s->guest_phys_blocks.head, GuestPhysBlockHead);
> + s->max_mapnr = paddr_to_pfn(last_block->target_end, s->page_shift);
> +}
> +
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 08/13] dump: add API to write dump header
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 08/13] dump: add API to write dump header qiaonuohan
2014-01-28 11:51 ` Ekaterina Tumanova
@ 2014-01-29 14:19 ` Laszlo Ersek
2014-01-30 17:14 ` Ekaterina Tumanova
2 siblings, 0 replies; 71+ messages in thread
From: Laszlo Ersek @ 2014-01-29 14:19 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi, afaerber
On 01/28/14 07:22, 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(-)
Thanks for fixing the four incorrect uses of "dh->block_size" in v7.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 09/13] dump: add API to write dump_bitmap
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 09/13] dump: add API to write dump_bitmap qiaonuohan
@ 2014-01-29 14:32 ` Laszlo Ersek
0 siblings, 0 replies; 71+ messages in thread
From: Laszlo Ersek @ 2014-01-29 14:32 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi, afaerber
On 01/28/14 07:22, 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>
> ---
> dump.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++++
> include/sysemu/dump.h | 2 +
> 2 files changed, 166 insertions(+), 0 deletions(-)
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 11/13] dump: add API to write dump pages
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 11/13] dump: add API to write dump pages qiaonuohan
@ 2014-01-29 14:39 ` Laszlo Ersek
0 siblings, 0 replies; 71+ messages in thread
From: Laszlo Ersek @ 2014-01-29 14:39 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi, afaerber
On 01/28/14 07:22, 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>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
> dump.c | 231 +++++++++++++++++++++++++++++++++++++++++++++++++
> include/sysemu/dump.h | 7 ++
> 2 files changed, 238 insertions(+), 0 deletions(-)
Again, I checked the differences with v7, and looked at my comments for
v7. Good job. My R-b stands.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 12/13] dump: make kdump-compressed format available for 'dump-guest-memory'
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 12/13] dump: make kdump-compressed format available for 'dump-guest-memory' qiaonuohan
@ 2014-01-29 14:45 ` Laszlo Ersek
0 siblings, 0 replies; 71+ messages in thread
From: Laszlo Ersek @ 2014-01-29 14:45 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, lcapitulino, anderson, kumagai-atsushi, afaerber
On 01/28/14 07:22, 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 | 131 +++++++++++++++++++++++++++++++++++++++++++++++++++---
> hmp.c | 5 ++-
> qapi-schema.json | 25 ++++++++++-
> qmp-commands.hx | 7 ++-
> 4 files changed, 158 insertions(+), 10 deletions(-)
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format
2014-01-28 6:21 [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (13 preceding siblings ...)
2014-01-28 6:37 ` [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format Qiao Nuohan
@ 2014-01-29 14:50 ` Laszlo Ersek
2014-01-30 17:01 ` [Qemu-devel] [PATCH] Define the architecture for compressed dump format Ekaterina Tumanova
2014-02-11 17:00 ` [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format Luiz Capitulino
16 siblings, 0 replies; 71+ messages in thread
From: Laszlo Ersek @ 2014-01-29 14:50 UTC (permalink / raw)
To: qiaonuohan, lcapitulino
Cc: stefanha, qemu-devel, kumagai-atsushi, anderson, afaerber
On 01/28/14 07:21, 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 v7 to v8:
> 1. rebase get_max_mapnr()
> 2. fix bug of using dh->block_size directly in create_header64()
> 3. abandon using static variables in get_next_page()
> 4. redefine 'struct PageDesc' to 'struct QEMU_PACKED PageDescriptor'
> 5. add return when format checking fails
> 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 | 966 ++++++++++++++++++++++++++++++++++++++++++++++++-
> hmp.c | 5 +-
> include/qom/cpu.h | 3 +-
> include/sysemu/dump.h | 138 +++++++
> qapi-schema.json | 38 ++-
> qmp-commands.hx | 38 ++-
> 7 files changed, 1222 insertions(+), 20 deletions(-)
Series
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Luiz, can you please add it to your queue?
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 71+ messages in thread
* [Qemu-devel] [PATCH] Define the architecture for compressed dump format.
2014-01-28 6:21 [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (14 preceding siblings ...)
2014-01-29 14:50 ` Laszlo Ersek
@ 2014-01-30 17:01 ` Ekaterina Tumanova
2014-01-30 17:20 ` Laszlo Ersek
2014-02-11 17:00 ` [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format Luiz Capitulino
16 siblings, 1 reply; 71+ messages in thread
From: Ekaterina Tumanova @ 2014-01-30 17:01 UTC (permalink / raw)
To: Public KVM Mailing List
Cc: borntraeger, qiaonuohan, lersek, Ekaterina Tumanova
If you apply this patch on top of your changes, your patches will work
on s390x as well.
---
dump.c | 8 ++++++--
target-i386/cpu.h | 2 ++
target-s390x/cpu.h | 1 +
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/dump.c b/dump.c
index 8f64aab..12ad114 100644
--- a/dump.c
+++ b/dump.c
@@ -32,6 +32,10 @@
#ifdef CONFIG_SNAPPY
#include <snappy-c.h>
#endif
+#ifndef ELF_MACHINE_UNAME
+#define ELF_MACHINE_UNAME "Unknown"
+#warning "Compressed dump is not supported on this architecture"
+#endif
static uint16_t cpu_convert_to_target16(uint16_t val, int endian)
{
@@ -817,7 +821,7 @@ static int create_header32(DumpState *s)
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);
+ strncpy(&(dh->utsname.machine), ELF_MACHINE_UNAME, sizeof(dh->utsname.machine));
if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
status |= DUMP_DH_COMPRESSED_ZLIB;
@@ -924,7 +928,7 @@ static int create_header64(DumpState *s)
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), "x86_64", 6);
+ strncpy(&(dh->utsname.machine), ELF_MACHINE_UNAME, sizeof(dh->utsname.machine));
if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
status |= DUMP_DH_COMPRESSED_ZLIB;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 1fcbc82..198743c 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -38,8 +38,10 @@
#ifdef TARGET_X86_64
#define ELF_MACHINE EM_X86_64
+#define ELF_MACHINE_UNAME "x86_64"
#else
#define ELF_MACHINE EM_386
+#define ELF_MACHINE_UNAME "i686"
#endif
#define CPUArchState struct CPUX86State
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 68b5ab7..bf7ae4c 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -28,6 +28,7 @@
#define TARGET_LONG_BITS 64
#define ELF_MACHINE EM_S390
+#define ELF_MACHINE_UNAME "S390X"
#define CPUArchState struct CPUS390XState
--
1.8.4.5
^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 08/13] dump: add API to write dump header
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 08/13] dump: add API to write dump header qiaonuohan
2014-01-28 11:51 ` Ekaterina Tumanova
2014-01-29 14:19 ` Laszlo Ersek
@ 2014-01-30 17:14 ` Ekaterina Tumanova
2 siblings, 0 replies; 71+ messages in thread
From: Ekaterina Tumanova @ 2014-01-30 17:14 UTC (permalink / raw)
To: qemu-devel, lersek, Christian Borntraeger
On 01/28/2014 10:22 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
>
...
> +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));
In this function the 3rd should argument should contain the length of
the destination argument (1st parameter).
If you place here the length of the 2nd parameter, this function call
becomes semantically the same as simple call to strcpy with all the
security implications...
There are more places like this.
Regards,
Kate.
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH] Define the architecture for compressed dump format.
2014-01-30 17:01 ` [Qemu-devel] [PATCH] Define the architecture for compressed dump format Ekaterina Tumanova
@ 2014-01-30 17:20 ` Laszlo Ersek
2014-01-31 13:45 ` [Qemu-devel] [PATCH v2] Define guest architecture for the compressed dump header Ekaterina Tumanova
0 siblings, 1 reply; 71+ messages in thread
From: Laszlo Ersek @ 2014-01-30 17:20 UTC (permalink / raw)
To: Ekaterina Tumanova; +Cc: borntraeger, qiaonuohan, Public KVM Mailing List
On 01/30/14 18:01, Ekaterina Tumanova wrote:
> If you apply this patch on top of your changes, your patches will work
> on s390x as well.
>
> ---
> dump.c | 8 ++++++--
> target-i386/cpu.h | 2 ++
> target-s390x/cpu.h | 1 +
> 3 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/dump.c b/dump.c
> index 8f64aab..12ad114 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -32,6 +32,10 @@
> #ifdef CONFIG_SNAPPY
> #include <snappy-c.h>
> #endif
> +#ifndef ELF_MACHINE_UNAME
> +#define ELF_MACHINE_UNAME "Unknown"
> +#warning "Compressed dump is not supported on this architecture"
> +#endif
>
> static uint16_t cpu_convert_to_target16(uint16_t val, int endian)
> {
> @@ -817,7 +821,7 @@ static int create_header32(DumpState *s)
> 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);
> + strncpy(&(dh->utsname.machine), ELF_MACHINE_UNAME, sizeof(dh->utsname.machine));
>
> if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
> status |= DUMP_DH_COMPRESSED_ZLIB;
> @@ -924,7 +928,7 @@ static int create_header64(DumpState *s)
> 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), "x86_64", 6);
> + strncpy(&(dh->utsname.machine), ELF_MACHINE_UNAME, sizeof(dh->utsname.machine));
>
> if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
> status |= DUMP_DH_COMPRESSED_ZLIB;
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 1fcbc82..198743c 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -38,8 +38,10 @@
>
> #ifdef TARGET_X86_64
> #define ELF_MACHINE EM_X86_64
> +#define ELF_MACHINE_UNAME "x86_64"
> #else
> #define ELF_MACHINE EM_386
> +#define ELF_MACHINE_UNAME "i686"
> #endif
>
> #define CPUArchState struct CPUX86State
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index 68b5ab7..bf7ae4c 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -28,6 +28,7 @@
> #define TARGET_LONG_BITS 64
>
> #define ELF_MACHINE EM_S390
> +#define ELF_MACHINE_UNAME "S390X"
>
> #define CPUArchState struct CPUS390XState
>
>
Looks good to me, but I wonder if the #warning will break builds on
other architectures.
>From the configure script it appears that -Werror is the default when: -
building a git working copy on Linux, and
- no relevant configure switch has been passed.
Laszlo
^ permalink raw reply [flat|nested] 71+ messages in thread
* [Qemu-devel] [PATCH v2] Define guest architecture for the compressed dump header.
2014-01-30 17:20 ` Laszlo Ersek
@ 2014-01-31 13:45 ` Ekaterina Tumanova
2014-01-31 13:45 ` [Qemu-devel] [PATCH v2] Define the architecture for compressed dump format Ekaterina Tumanova
0 siblings, 1 reply; 71+ messages in thread
From: Ekaterina Tumanova @ 2014-01-31 13:45 UTC (permalink / raw)
To: Public KVM Mailing List
Cc: borntraeger, qiaonuohan, lersek, Ekaterina Tumanova
You're right about the warning. I removed it. I also added the
cast for the destination argument, which caused a warning as well.
I tested on s390 and i386 laptop.
If you ok with the patch, can you push it on top of this series?
Thank you,
Regards,
Kate.
Ekaterina Tumanova (1):
Define the architecture for compressed dump format.
dump.c | 7 +++++--
target-i386/cpu.h | 2 ++
target-s390x/cpu.h | 1 +
3 files changed, 8 insertions(+), 2 deletions(-)
--
1.8.4.5
^ permalink raw reply [flat|nested] 71+ messages in thread
* [Qemu-devel] [PATCH v2] Define the architecture for compressed dump format.
2014-01-31 13:45 ` [Qemu-devel] [PATCH v2] Define guest architecture for the compressed dump header Ekaterina Tumanova
@ 2014-01-31 13:45 ` Ekaterina Tumanova
2014-01-31 13:56 ` Christian Borntraeger
2014-01-31 14:11 ` Laszlo Ersek
0 siblings, 2 replies; 71+ messages in thread
From: Ekaterina Tumanova @ 2014-01-31 13:45 UTC (permalink / raw)
To: Public KVM Mailing List
Cc: borntraeger, qiaonuohan, lersek, Ekaterina Tumanova
Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
---
dump.c | 7 +++++--
target-i386/cpu.h | 2 ++
target-s390x/cpu.h | 1 +
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/dump.c b/dump.c
index 8f64aab..25503bc 100644
--- a/dump.c
+++ b/dump.c
@@ -32,6 +32,9 @@
#ifdef CONFIG_SNAPPY
#include <snappy-c.h>
#endif
+#ifndef ELF_MACHINE_UNAME
+#define ELF_MACHINE_UNAME "Unknown"
+#endif
static uint16_t cpu_convert_to_target16(uint16_t val, int endian)
{
@@ -817,7 +820,7 @@ static int create_header32(DumpState *s)
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);
+ strncpy((char *)&(dh->utsname.machine), ELF_MACHINE_UNAME, sizeof(dh->utsname.machine));
if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
status |= DUMP_DH_COMPRESSED_ZLIB;
@@ -924,7 +927,7 @@ static int create_header64(DumpState *s)
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), "x86_64", 6);
+ strncpy((char *)&(dh->utsname.machine), ELF_MACHINE_UNAME, sizeof(dh->utsname.machine));
if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
status |= DUMP_DH_COMPRESSED_ZLIB;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 1fcbc82..198743c 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -38,8 +38,10 @@
#ifdef TARGET_X86_64
#define ELF_MACHINE EM_X86_64
+#define ELF_MACHINE_UNAME "x86_64"
#else
#define ELF_MACHINE EM_386
+#define ELF_MACHINE_UNAME "i686"
#endif
#define CPUArchState struct CPUX86State
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 68b5ab7..bf7ae4c 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -28,6 +28,7 @@
#define TARGET_LONG_BITS 64
#define ELF_MACHINE EM_S390
+#define ELF_MACHINE_UNAME "S390X"
#define CPUArchState struct CPUS390XState
--
1.8.4.5
^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Define the architecture for compressed dump format.
2014-01-31 13:45 ` [Qemu-devel] [PATCH v2] Define the architecture for compressed dump format Ekaterina Tumanova
@ 2014-01-31 13:56 ` Christian Borntraeger
2014-01-31 14:11 ` Laszlo Ersek
1 sibling, 0 replies; 71+ messages in thread
From: Christian Borntraeger @ 2014-01-31 13:56 UTC (permalink / raw)
To: Ekaterina Tumanova, Public KVM Mailing List; +Cc: qiaonuohan, lersek
On 31/01/14 14:45, Ekaterina Tumanova wrote:
> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> dump.c | 7 +++++--
> target-i386/cpu.h | 2 ++
> target-s390x/cpu.h | 1 +
> 3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/dump.c b/dump.c
> index 8f64aab..25503bc 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -32,6 +32,9 @@
> #ifdef CONFIG_SNAPPY
> #include <snappy-c.h>
> #endif
> +#ifndef ELF_MACHINE_UNAME
> +#define ELF_MACHINE_UNAME "Unknown"
> +#endif
>
> static uint16_t cpu_convert_to_target16(uint16_t val, int endian)
> {
> @@ -817,7 +820,7 @@ static int create_header32(DumpState *s)
> 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);
> + strncpy((char *)&(dh->utsname.machine), ELF_MACHINE_UNAME, sizeof(dh->utsname.machine));
>
> if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
> status |= DUMP_DH_COMPRESSED_ZLIB;
> @@ -924,7 +927,7 @@ static int create_header64(DumpState *s)
> 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), "x86_64", 6);
> + strncpy((char *)&(dh->utsname.machine), ELF_MACHINE_UNAME, sizeof(dh->utsname.machine));
>
> if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
> status |= DUMP_DH_COMPRESSED_ZLIB;
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 1fcbc82..198743c 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -38,8 +38,10 @@
>
> #ifdef TARGET_X86_64
> #define ELF_MACHINE EM_X86_64
> +#define ELF_MACHINE_UNAME "x86_64"
> #else
> #define ELF_MACHINE EM_386
> +#define ELF_MACHINE_UNAME "i686"
> #endif
>
> #define CPUArchState struct CPUX86State
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index 68b5ab7..bf7ae4c 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -28,6 +28,7 @@
> #define TARGET_LONG_BITS 64
>
> #define ELF_MACHINE EM_S390
> +#define ELF_MACHINE_UNAME "S390X"
>
> #define CPUArchState struct CPUS390XState
>
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Define the architecture for compressed dump format.
2014-01-31 13:45 ` [Qemu-devel] [PATCH v2] Define the architecture for compressed dump format Ekaterina Tumanova
2014-01-31 13:56 ` Christian Borntraeger
@ 2014-01-31 14:11 ` Laszlo Ersek
2014-01-31 15:04 ` [Qemu-devel] [PATCH v3 0/1] Detect arch for dump compressed header Ekaterina Tumanova
1 sibling, 1 reply; 71+ messages in thread
From: Laszlo Ersek @ 2014-01-31 14:11 UTC (permalink / raw)
To: Ekaterina Tumanova; +Cc: borntraeger, qiaonuohan, Public KVM Mailing List
one not-so-important comment below:
On 01/31/14 14:45, Ekaterina Tumanova wrote:
> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
> ---
> dump.c | 7 +++++--
> target-i386/cpu.h | 2 ++
> target-s390x/cpu.h | 1 +
> 3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/dump.c b/dump.c
> index 8f64aab..25503bc 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -32,6 +32,9 @@
> #ifdef CONFIG_SNAPPY
> #include <snappy-c.h>
> #endif
> +#ifndef ELF_MACHINE_UNAME
> +#define ELF_MACHINE_UNAME "Unknown"
> +#endif
>
> static uint16_t cpu_convert_to_target16(uint16_t val, int endian)
> {
> @@ -817,7 +820,7 @@ static int create_header32(DumpState *s)
> 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);
> + strncpy((char *)&(dh->utsname.machine), ELF_MACHINE_UNAME, sizeof(dh->utsname.machine));
"dh->utsname.machine" is actually an array of characters, if I recall
correctly (see NewUtsname in patch 08).
The expression in the first argument takes the address of the entire
array (the resultant pointer has type pointer-to-array). I didn't call
it out with memcpy(), because it didn't really matter. But now it looks
a bit gross, because as 2nd step we convert the pointer-to-array back to
pointer-to-char. It would be simpler to write
strncpy(dh->utsname.machine, ELF_MACHINE_UNAME,
sizeof(dh->utsname.machine))
where "dh->utsname.machine" decays to a pointer to its first element:
6. Language
6.3 Conversions
6.3.2 Other operands
6.3.2.1 Lvalues, arrays, and function designators
3 Except when it is the operand of the sizeof operator or the unary
& operator, or is a string literal used to initialize an array, an
expression that has type ‘‘array of type’’ is converted to an
expression with type ‘‘pointer to type’’ that points to the
initial element of the array object and is not an lvalue. If the
array object has register storage class, the behavior is undefined.
I should have probably noticed this in v1 of the followup patch. I don't
insist on upating it of course.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Nonetheless, if you want to fix that up in a v3, please keep my R-b.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 71+ messages in thread
* [Qemu-devel] [PATCH v3 0/1] Detect arch for dump compressed header.
2014-01-31 14:11 ` Laszlo Ersek
@ 2014-01-31 15:04 ` Ekaterina Tumanova
2014-01-31 15:04 ` [Qemu-devel] [PATCH v3 1/1] Define the architecture for compressed dump format Ekaterina Tumanova
` (2 more replies)
0 siblings, 3 replies; 71+ messages in thread
From: Ekaterina Tumanova @ 2014-01-31 15:04 UTC (permalink / raw)
To: Public KVM Mailing List
Cc: borntraeger, qiaonuohan, lersek, Ekaterina Tumanova
True! Fixed. Tested.
Can you please put it into push-queue?
Thanks,
Kate.
Ekaterina Tumanova (1):
Define the architecture for compressed dump format.
dump.c | 7 +++++--
target-i386/cpu.h | 2 ++
target-s390x/cpu.h | 1 +
3 files changed, 8 insertions(+), 2 deletions(-)
--
1.8.4.5
^ permalink raw reply [flat|nested] 71+ messages in thread
* [Qemu-devel] [PATCH v3 1/1] Define the architecture for compressed dump format.
2014-01-31 15:04 ` [Qemu-devel] [PATCH v3 0/1] Detect arch for dump compressed header Ekaterina Tumanova
@ 2014-01-31 15:04 ` Ekaterina Tumanova
2014-02-13 13:50 ` Luiz Capitulino
2014-01-31 17:14 ` [Qemu-devel] [PATCH v3 0/1] Detect arch for dump compressed header Laszlo Ersek
2014-02-10 3:34 ` Qiao Nuohan
2 siblings, 1 reply; 71+ messages in thread
From: Ekaterina Tumanova @ 2014-01-31 15:04 UTC (permalink / raw)
To: Public KVM Mailing List
Cc: borntraeger, qiaonuohan, lersek, Ekaterina Tumanova
Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
---
dump.c | 7 +++++--
target-i386/cpu.h | 2 ++
target-s390x/cpu.h | 1 +
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/dump.c b/dump.c
index 8f64aab..8d85255 100644
--- a/dump.c
+++ b/dump.c
@@ -32,6 +32,9 @@
#ifdef CONFIG_SNAPPY
#include <snappy-c.h>
#endif
+#ifndef ELF_MACHINE_UNAME
+#define ELF_MACHINE_UNAME "Unknown"
+#endif
static uint16_t cpu_convert_to_target16(uint16_t val, int endian)
{
@@ -817,7 +820,7 @@ static int create_header32(DumpState *s)
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);
+ strncpy(dh->utsname.machine, ELF_MACHINE_UNAME, sizeof(dh->utsname.machine));
if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
status |= DUMP_DH_COMPRESSED_ZLIB;
@@ -924,7 +927,7 @@ static int create_header64(DumpState *s)
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), "x86_64", 6);
+ strncpy(dh->utsname.machine, ELF_MACHINE_UNAME, sizeof(dh->utsname.machine));
if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
status |= DUMP_DH_COMPRESSED_ZLIB;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 1fcbc82..198743c 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -38,8 +38,10 @@
#ifdef TARGET_X86_64
#define ELF_MACHINE EM_X86_64
+#define ELF_MACHINE_UNAME "x86_64"
#else
#define ELF_MACHINE EM_386
+#define ELF_MACHINE_UNAME "i686"
#endif
#define CPUArchState struct CPUX86State
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 68b5ab7..bf7ae4c 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -28,6 +28,7 @@
#define TARGET_LONG_BITS 64
#define ELF_MACHINE EM_S390
+#define ELF_MACHINE_UNAME "S390X"
#define CPUArchState struct CPUS390XState
--
1.8.4.5
^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/1] Detect arch for dump compressed header.
2014-01-31 15:04 ` [Qemu-devel] [PATCH v3 0/1] Detect arch for dump compressed header Ekaterina Tumanova
2014-01-31 15:04 ` [Qemu-devel] [PATCH v3 1/1] Define the architecture for compressed dump format Ekaterina Tumanova
@ 2014-01-31 17:14 ` Laszlo Ersek
2014-02-10 21:23 ` Luiz Capitulino
2014-02-10 3:34 ` Qiao Nuohan
2 siblings, 1 reply; 71+ messages in thread
From: Laszlo Ersek @ 2014-01-31 17:14 UTC (permalink / raw)
To: Ekaterina Tumanova
Cc: borntraeger, qiaonuohan, Public KVM Mailing List, Luiz Capitulino
On 01/31/14 16:04, Ekaterina Tumanova wrote:
> True! Fixed. Tested.
>
> Can you please put it into push-queue?
>
> Thanks,
> Kate.
>
> Ekaterina Tumanova (1):
> Define the architecture for compressed dump format.
>
> dump.c | 7 +++++--
> target-i386/cpu.h | 2 ++
> target-s390x/cpu.h | 1 +
> 3 files changed, 8 insertions(+), 2 deletions(-)
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Also CC'ing Luiz (because the dump series belongs to his QMP tree as far
as I remember).
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/1] Detect arch for dump compressed header.
2014-01-31 15:04 ` [Qemu-devel] [PATCH v3 0/1] Detect arch for dump compressed header Ekaterina Tumanova
2014-01-31 15:04 ` [Qemu-devel] [PATCH v3 1/1] Define the architecture for compressed dump format Ekaterina Tumanova
2014-01-31 17:14 ` [Qemu-devel] [PATCH v3 0/1] Detect arch for dump compressed header Laszlo Ersek
@ 2014-02-10 3:34 ` Qiao Nuohan
2014-02-10 8:29 ` Laszlo Ersek
2 siblings, 1 reply; 71+ messages in thread
From: Qiao Nuohan @ 2014-02-10 3:34 UTC (permalink / raw)
To: Ekaterina Tumanova, lersek; +Cc: borntraeger, Public KVM Mailing List
On 01/31/2014 11:04 PM, Ekaterina Tumanova wrote:
> True! Fixed. Tested.
>
> Can you please put it into push-queue?
>
> Thanks,
> Kate.
>
> Ekaterina Tumanova (1):
> Define the architecture for compressed dump format.
>
> dump.c | 7 +++++--
> target-i386/cpu.h | 2 ++
> target-s390x/cpu.h | 1 +
> 3 files changed, 8 insertions(+), 2 deletions(-)
>
Reviewed-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Sorry for replying later. The patch seems good to me and works well on my
machine.
P.S.
I have no idea about where to find queued patches, could you give some hint?
--
Regards
Qiao Nuohan
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/1] Detect arch for dump compressed header.
2014-02-10 3:34 ` Qiao Nuohan
@ 2014-02-10 8:29 ` Laszlo Ersek
2014-02-10 8:57 ` Qiao Nuohan
0 siblings, 1 reply; 71+ messages in thread
From: Laszlo Ersek @ 2014-02-10 8:29 UTC (permalink / raw)
To: Qiao Nuohan
Cc: borntraeger, Ekaterina Tumanova, Public KVM Mailing List,
Luiz Capitulino
On 02/10/14 04:34, Qiao Nuohan wrote:
> On 01/31/2014 11:04 PM, Ekaterina Tumanova wrote:
>> True! Fixed. Tested.
>>
>> Can you please put it into push-queue?
>>
>> Thanks,
>> Kate.
>>
>> Ekaterina Tumanova (1):
>> Define the architecture for compressed dump format.
>>
>> dump.c | 7 +++++--
>> target-i386/cpu.h | 2 ++
>> target-s390x/cpu.h | 1 +
>> 3 files changed, 8 insertions(+), 2 deletions(-)
>>
>
> Reviewed-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
>
> Sorry for replying later. The patch seems good to me and works well on my
> machine.
>
> P.S.
> I have no idea about where to find queued patches, could you give some
> hint?
Last time I had submitted patches for dump, they went in via Luiz's queue:
http://repo.or.cz/w/qemu/qmp-unstable.git/shortlog/refs/heads/queue/qmp-next
It seems appropriate now as well; in my clone:
$ git diff master..qiao_v8 >squashed.patch
$ scripts/get_maintainer.pl squashed.patch
Luiz Capitulino <lcapitulino@redhat.com> (supporter:Human Monitor
(HMP),commit_signer:7/25=28%,commit_signer:1/8=12%)
[...]
It's indeed a queue, ie. Luiz can elect to rebase / reorder the buffered
patches from time to time.
Laszlo
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/1] Detect arch for dump compressed header.
2014-02-10 8:29 ` Laszlo Ersek
@ 2014-02-10 8:57 ` Qiao Nuohan
2014-02-10 13:24 ` Luiz Capitulino
0 siblings, 1 reply; 71+ messages in thread
From: Qiao Nuohan @ 2014-02-10 8:57 UTC (permalink / raw)
To: Laszlo Ersek
Cc: borntraeger, Ekaterina Tumanova, Public KVM Mailing List,
Luiz Capitulino
On 02/10/2014 04:29 PM, Laszlo Ersek wrote:
> Last time I had submitted patches for dump, they went in via Luiz's queue:
>
> http://repo.or.cz/w/qemu/qmp-unstable.git/shortlog/refs/heads/queue/qmp-next
>
> It seems appropriate now as well; in my clone:
>
> $ git diff master..qiao_v8>squashed.patch
> $ scripts/get_maintainer.pl squashed.patch
> Luiz Capitulino<lcapitulino@redhat.com> (supporter:Human Monitor
> (HMP),commit_signer:7/25=28%,commit_signer:1/8=12%)
> [...]
>
> It's indeed a queue, ie. Luiz can elect to rebase / reorder the buffered
> patches from time to time.
>
> Laszlo
Thank you. I will wait for it.
--
Regards
Qiao Nuohan
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/1] Detect arch for dump compressed header.
2014-02-10 8:57 ` Qiao Nuohan
@ 2014-02-10 13:24 ` Luiz Capitulino
0 siblings, 0 replies; 71+ messages in thread
From: Luiz Capitulino @ 2014-02-10 13:24 UTC (permalink / raw)
To: Qiao Nuohan
Cc: borntraeger, Ekaterina Tumanova, Laszlo Ersek, Public KVM Mailing List
On Mon, 10 Feb 2014 16:57:08 +0800
Qiao Nuohan <qiaonuohan@cn.fujitsu.com> wrote:
> On 02/10/2014 04:29 PM, Laszlo Ersek wrote:
> > Last time I had submitted patches for dump, they went in via Luiz's queue:
> >
> > http://repo.or.cz/w/qemu/qmp-unstable.git/shortlog/refs/heads/queue/qmp-next
> >
> > It seems appropriate now as well; in my clone:
> >
> > $ git diff master..qiao_v8>squashed.patch
> > $ scripts/get_maintainer.pl squashed.patch
> > Luiz Capitulino<lcapitulino@redhat.com> (supporter:Human Monitor
> > (HMP),commit_signer:7/25=28%,commit_signer:1/8=12%)
> > [...]
> >
> > It's indeed a queue, ie. Luiz can elect to rebase / reorder the buffered
> > patches from time to time.
> >
> > Laszlo
>
> Thank you. I will wait for it.
I'm a bit behind my queue, I'll start going over pending patches today.
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 13/13] dump: add 'query-dump-guest-memory-capability' command qiaonuohan
@ 2014-02-10 19:10 ` Luiz Capitulino
2014-02-10 22:02 ` Laszlo Ersek
0 siblings, 1 reply; 71+ messages in thread
From: Luiz Capitulino @ 2014-02-10 19:10 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, kumagai-atsushi, anderson, lersek, afaerber
On Tue, 28 Jan 2014 14:22:06 +0800
qiaonuohan <qiaonuohan@cn.fujitsu.com> 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"}
> ]
> }
I don't want to hold this series anymore, this series is long and I know it
took you and Laszlo's a long time to get it right. On the other hand we can't
allow every single command to have its own introspection protocol.
I think I'm fine accepting this one now, as long as it's fine for libvirt
too. Eric, can you confirm this please?
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.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 2ebbb23..8f64aab 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -1788,3 +1788,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 7f62007..012c70c 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2783,6 +2783,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 019dde6..e1b311a 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 formats.
> +
> +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 formats.
> +
> +EQMP
> +
> + {
> .name = "netdev_add",
> .args_type = "netdev:O",
> .mhandler.cmd_new = qmp_netdev_add,
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 03/13] dump: add API to write header of flatten format
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 03/13] dump: add API to write header of flatten format qiaonuohan
@ 2014-02-10 19:35 ` Luiz Capitulino
2014-02-10 21:57 ` Laszlo Ersek
0 siblings, 1 reply; 71+ messages in thread
From: Luiz Capitulino @ 2014-02-10 19:35 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, kumagai-atsushi, anderson, lersek, afaerber
On Tue, 28 Jan 2014 14:21:56 +0800
qiaonuohan <qiaonuohan@cn.fujitsu.com> 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>
This patch breaks git bisect:
/home/lcapitulino/work/src/upstream/qmp-unstable/dump.c:689:12: error: ‘write_start_flat_header’ defined but not used [-Werror=unused-function]
/home/lcapitulino/work/src/upstream/qmp-unstable/dump.c:715:12: error: ‘write_end_flat_header’ defined but not used [-Werror=unused-function]
cc1: all warnings being treated as errors
make[1]: *** [dump.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [subdir-x86_64-softmmu] Error 2
> ---
> 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);
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/1] Detect arch for dump compressed header.
2014-01-31 17:14 ` [Qemu-devel] [PATCH v3 0/1] Detect arch for dump compressed header Laszlo Ersek
@ 2014-02-10 21:23 ` Luiz Capitulino
2014-02-10 22:06 ` Laszlo Ersek
0 siblings, 1 reply; 71+ messages in thread
From: Luiz Capitulino @ 2014-02-10 21:23 UTC (permalink / raw)
To: Laszlo Ersek
Cc: qiaonuohan, borntraeger, Ekaterina Tumanova, Public KVM Mailing List
On Fri, 31 Jan 2014 18:14:01 +0100
Laszlo Ersek <lersek@redhat.com> wrote:
> On 01/31/14 16:04, Ekaterina Tumanova wrote:
> > True! Fixed. Tested.
> >
> > Can you please put it into push-queue?
> >
> > Thanks,
> > Kate.
> >
> > Ekaterina Tumanova (1):
> > Define the architecture for compressed dump format.
> >
> > dump.c | 7 +++++--
> > target-i386/cpu.h | 2 ++
> > target-s390x/cpu.h | 1 +
> > 3 files changed, 8 insertions(+), 2 deletions(-)
> >
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
> Also CC'ing Luiz (because the dump series belongs to his QMP tree as far
> as I remember).
Is this patch supposed to applied on top of:
[PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format
If it is, then maybe Qiao Nuohan can apply/merge it on top of v9, as a
respin of that series is needed.
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 03/13] dump: add API to write header of flatten format
2014-02-10 19:35 ` Luiz Capitulino
@ 2014-02-10 21:57 ` Laszlo Ersek
2014-02-10 22:48 ` Luiz Capitulino
0 siblings, 1 reply; 71+ messages in thread
From: Laszlo Ersek @ 2014-02-10 21:57 UTC (permalink / raw)
To: Luiz Capitulino
Cc: stefanha, qemu-devel, qiaonuohan, kumagai-atsushi, anderson, afaerber
On 02/10/14 20:35, Luiz Capitulino wrote:
> On Tue, 28 Jan 2014 14:21:56 +0800
> qiaonuohan <qiaonuohan@cn.fujitsu.com> 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>
>
> This patch breaks git bisect:
>
> /home/lcapitulino/work/src/upstream/qmp-unstable/dump.c:689:12: error: ‘write_start_flat_header’ defined but not used [-Werror=unused-function]
> /home/lcapitulino/work/src/upstream/qmp-unstable/dump.c:715:12: error: ‘write_end_flat_header’ defined but not used [-Werror=unused-function]
> cc1: all warnings being treated as errors
> make[1]: *** [dump.o] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [subdir-x86_64-softmmu] Error 2
This is one of the most useless warnings (when turned into an error).
It's common to add the building blocks first during a series, and then
to put them to use all at once, in one of the later patches. I have
faced this before (see commit 27d59ccd), and it sucks very hard.
Catching unused functions is helpful when you don't know about them. The
point of a patch series is that in patch N you know what you're going to
do in patch N+2. IOW, you know the future, while gcc doesn't.
Plus, bisection is *already* unusable with -Werror enabled. Suppose you
want to bisect a really long range, including commits that had been
written when gcc was only at, say, 4.4. At that time, gcc-4.4's -Werror
flag let some things pass that will now most certainly break your
bisection, when you compile the tree at those earlier commits with
gcc-4.8. I'm not speaking theoretically, I have factually witnessed this.
In effect, the -Werror flag *binds* a specific qemu commit to the gcc
version that is shipped by the main distros at the time of the commit.
In order to preserve all information, the commit would have to save the
gcc version to build it with, and git bisect should restore that
information (ie. require that you have that compiler version installed).
In short, -Werror and bisection don't mix. They already don't, and we
shouldn't expect them to.
In practice, in order to silence this warning, Qiao would have to move
these function definitions into the patch that uses them (bad from a
design and review standpoint), or add useless calls inside this patch
(which I did in commit 27d59ccd, and which is ridiculous).
Qiao could also play with the diagnostics pragma (ie. suppress the
warning just in this patch, just for these functions), but that pragma
only works with gcc-4.6+, if memory serves.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-10 19:10 ` Luiz Capitulino
@ 2014-02-10 22:02 ` Laszlo Ersek
2014-02-10 23:09 ` Paolo Bonzini
2014-02-10 23:09 ` Paolo Bonzini
0 siblings, 2 replies; 71+ messages in thread
From: Laszlo Ersek @ 2014-02-10 22:02 UTC (permalink / raw)
To: Luiz Capitulino
Cc: stefanha, qemu-devel, qiaonuohan, kumagai-atsushi, anderson, afaerber
On 02/10/14 20:10, Luiz Capitulino wrote:
> On Tue, 28 Jan 2014 14:22:06 +0800
> qiaonuohan <qiaonuohan@cn.fujitsu.com> 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"}
>> ]
>> }
>
> I don't want to hold this series anymore, this series is long and I know it
> took you and Laszlo's a long time to get it right. On the other hand we can't
> allow every single command to have its own introspection protocol.
>
> I think I'm fine accepting this one now, as long as it's fine for libvirt
> too. Eric, can you confirm this please?
We discussed this before, and Eric participated. In fact the custom
introspection was one of his recommendations.
http://thread.gmane.org/gmane.comp.emulators.qemu/221270/focus=246650
(Which I agreed with because it would give us the most independence.)
Of course I'm not trying to imply that this one specific interface will
doubtlessly serve all of libvirt's needs wrt. the kdump feature. We
certainly need Eric to sign off on that.
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/1] Detect arch for dump compressed header.
2014-02-10 21:23 ` Luiz Capitulino
@ 2014-02-10 22:06 ` Laszlo Ersek
0 siblings, 0 replies; 71+ messages in thread
From: Laszlo Ersek @ 2014-02-10 22:06 UTC (permalink / raw)
To: Luiz Capitulino
Cc: qiaonuohan, borntraeger, Ekaterina Tumanova, Public KVM Mailing List
On 02/10/14 22:23, Luiz Capitulino wrote:
> On Fri, 31 Jan 2014 18:14:01 +0100
> Laszlo Ersek <lersek@redhat.com> wrote:
>
>> On 01/31/14 16:04, Ekaterina Tumanova wrote:
>>> True! Fixed. Tested.
>>>
>>> Can you please put it into push-queue?
>>>
>>> Thanks,
>>> Kate.
>>>
>>> Ekaterina Tumanova (1):
>>> Define the architecture for compressed dump format.
>>>
>>> dump.c | 7 +++++--
>>> target-i386/cpu.h | 2 ++
>>> target-s390x/cpu.h | 1 +
>>> 3 files changed, 8 insertions(+), 2 deletions(-)
>>>
>>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>
>> Also CC'ing Luiz (because the dump series belongs to his QMP tree as far
>> as I remember).
>
> Is this patch supposed to applied on top of:
>
> [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format
Yes, please apply it on top.
(I think Qiao also reviewed it.)
> If it is, then maybe Qiao Nuohan can apply/merge it on top of v9, as a
> respin of that series is needed.
I disagree with the notion that a respin is needed just for the
warnings. (See my other email.)
OTOH, Qiao apparently has about ten thousand times more patience than
myself, so if he does respin, he should just keep my R-b's, because I
*will not* look at this series again, until I port it.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 03/13] dump: add API to write header of flatten format
2014-02-10 21:57 ` Laszlo Ersek
@ 2014-02-10 22:48 ` Luiz Capitulino
2014-02-10 23:20 ` Laszlo Ersek
0 siblings, 1 reply; 71+ messages in thread
From: Luiz Capitulino @ 2014-02-10 22:48 UTC (permalink / raw)
To: Laszlo Ersek
Cc: peter.maydell, stefanha, qemu-devel, qiaonuohan, kumagai-atsushi,
anderson, afaerber
On Mon, 10 Feb 2014 22:57:15 +0100
Laszlo Ersek <lersek@redhat.com> wrote:
> On 02/10/14 20:35, Luiz Capitulino wrote:
> > On Tue, 28 Jan 2014 14:21:56 +0800
> > qiaonuohan <qiaonuohan@cn.fujitsu.com> 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>
> >
> > This patch breaks git bisect:
> >
> > /home/lcapitulino/work/src/upstream/qmp-unstable/dump.c:689:12: error: ‘write_start_flat_header’ defined but not used [-Werror=unused-function]
> > /home/lcapitulino/work/src/upstream/qmp-unstable/dump.c:715:12: error: ‘write_end_flat_header’ defined but not used [-Werror=unused-function]
> > cc1: all warnings being treated as errors
> > make[1]: *** [dump.o] Error 1
> > make[1]: *** Waiting for unfinished jobs....
> > make: *** [subdir-x86_64-softmmu] Error 2
>
> This is one of the most useless warnings (when turned into an error).
> It's common to add the building blocks first during a series, and then
> to put them to use all at once, in one of the later patches. I have
> faced this before (see commit 27d59ccd), and it sucks very hard.
> Catching unused functions is helpful when you don't know about them. The
> point of a patch series is that in patch N you know what you're going to
> do in patch N+2. IOW, you know the future, while gcc doesn't.
>
> Plus, bisection is *already* unusable with -Werror enabled. Suppose you
> want to bisect a really long range, including commits that had been
> written when gcc was only at, say, 4.4. At that time, gcc-4.4's -Werror
> flag let some things pass that will now most certainly break your
> bisection, when you compile the tree at those earlier commits with
> gcc-4.8. I'm not speaking theoretically, I have factually witnessed this.
>
> In effect, the -Werror flag *binds* a specific qemu commit to the gcc
> version that is shipped by the main distros at the time of the commit.
> In order to preserve all information, the commit would have to save the
> gcc version to build it with, and git bisect should restore that
> information (ie. require that you have that compiler version installed).
>
> In short, -Werror and bisection don't mix. They already don't, and we
> shouldn't expect them to.
I understand what you're saying, and I don't want people to do needless and
endless respins, but letting bisect break at will doesn't seem a good option
either.
What other options do we have? What's the general QEMU directive in cases like
this?
1. Do what you did in commit 27d59ccd?
2. Apply it and let bisect break?
3. Drop -Werror?
>
> In practice, in order to silence this warning, Qiao would have to move
> these function definitions into the patch that uses them (bad from a
> design and review standpoint), or add useless calls inside this patch
> (which I did in commit 27d59ccd, and which is ridiculous).
>
> Qiao could also play with the diagnostics pragma (ie. suppress the
> warning just in this patch, just for these functions), but that pragma
> only works with gcc-4.6+, if memory serves.
>
> Thanks
> Laszlo
>
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-10 22:02 ` Laszlo Ersek
@ 2014-02-10 23:09 ` Paolo Bonzini
2014-02-10 23:09 ` Paolo Bonzini
1 sibling, 0 replies; 71+ messages in thread
From: Paolo Bonzini @ 2014-02-10 23:09 UTC (permalink / raw)
To: Laszlo Ersek, Luiz Capitulino
Cc: stefanha, qemu-devel, qiaonuohan, kumagai-atsushi, anderson, afaerber
Il 10/02/2014 23:02, Laszlo Ersek ha scritto:
> On 02/10/14 20:10, Luiz Capitulino wrote:
>> On Tue, 28 Jan 2014 14:22:06 +0800
>> qiaonuohan <qiaonuohan@cn.fujitsu.com> 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"}
>>> ]
>>> }
>>
>> I don't want to hold this series anymore, this series is long and I know it
>> took you and Laszlo's a long time to get it right. On the other hand we can't
>> allow every single command to have its own introspection protocol.
>>
>> I think I'm fine accepting this one now, as long as it's fine for libvirt
>> too. Eric, can you confirm this please?
>
> We discussed this before, and Eric participated. In fact the custom
> introspection was one of his recommendations.
>
> http://thread.gmane.org/gmane.comp.emulators.qemu/221270/focus=246650
>
> (Which I agreed with because it would give us the most independence.)
>
> Of course I'm not trying to imply that this one specific interface will
> doubtlessly serve all of libvirt's needs wrt. the kdump feature. We
> certainly need Eric to sign off on that.
I think the justification here is that even if you defined an Enum for
['elf','kdump-zlib', 'kdump-lzo', 'kdump-snappy'], it would not be
enough to describe which values were compiled in (as opposed to
supported by the particular QEMU version).
However, I don't see the point in having the "format-option" field.
What about:
-> { "execute": "query-dump-guest-memory-formats" }
<- { "return": [
{"available": true, "name": "elf"},
{"available": true, "name": "kdump-zlib"},
{"available": true, "name": "kdump-lzo"},
{"available": true, "name": "kdump-snappy"}
]
or just
<- { "return": ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
?
Paolo
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-10 22:02 ` Laszlo Ersek
2014-02-10 23:09 ` Paolo Bonzini
@ 2014-02-10 23:09 ` Paolo Bonzini
2014-02-10 23:30 ` Laszlo Ersek
1 sibling, 1 reply; 71+ messages in thread
From: Paolo Bonzini @ 2014-02-10 23:09 UTC (permalink / raw)
To: Laszlo Ersek, Luiz Capitulino
Cc: stefanha, qemu-devel, qiaonuohan, kumagai-atsushi, anderson, afaerber
Il 10/02/2014 23:02, Laszlo Ersek ha scritto:
> On 02/10/14 20:10, Luiz Capitulino wrote:
>> On Tue, 28 Jan 2014 14:22:06 +0800
>> qiaonuohan <qiaonuohan@cn.fujitsu.com> 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"}
>>> ]
>>> }
>>
>> I don't want to hold this series anymore, this series is long and I know it
>> took you and Laszlo's a long time to get it right. On the other hand we can't
>> allow every single command to have its own introspection protocol.
>>
>> I think I'm fine accepting this one now, as long as it's fine for libvirt
>> too. Eric, can you confirm this please?
>
> We discussed this before, and Eric participated. In fact the custom
> introspection was one of his recommendations.
>
> http://thread.gmane.org/gmane.comp.emulators.qemu/221270/focus=246650
>
> (Which I agreed with because it would give us the most independence.)
>
> Of course I'm not trying to imply that this one specific interface will
> doubtlessly serve all of libvirt's needs wrt. the kdump feature. We
> certainly need Eric to sign off on that.
I think the justification here is that even if you defined an Enum for
['elf','kdump-zlib', 'kdump-lzo', 'kdump-snappy'], it would not be
enough to describe which values were compiled in (as opposed to
supported by the particular QEMU version).
However, I don't see the point in having the "format-option" field.
What about:
-> { "execute": "query-dump-guest-memory-capabilities" }
<- { "return": { "formats":
["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
Paolo
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 03/13] dump: add API to write header of flatten format
2014-02-10 22:48 ` Luiz Capitulino
@ 2014-02-10 23:20 ` Laszlo Ersek
2014-02-11 9:06 ` Markus Armbruster
0 siblings, 1 reply; 71+ messages in thread
From: Laszlo Ersek @ 2014-02-10 23:20 UTC (permalink / raw)
To: Luiz Capitulino
Cc: peter.maydell, stefanha, qemu-devel, qiaonuohan, kumagai-atsushi,
anderson, afaerber
On 02/10/14 23:48, Luiz Capitulino wrote:
> On Mon, 10 Feb 2014 22:57:15 +0100
> Laszlo Ersek <lersek@redhat.com> wrote:
>> In short, -Werror and bisection don't mix. They already don't, and we
>> shouldn't expect them to.
>
> I understand what you're saying, and I don't want people to do needless and
> endless respins, but letting bisect break at will doesn't seem a good option
> either.
>
> What other options do we have? What's the general QEMU directive in cases like
> this?
>
> 1. Do what you did in commit 27d59ccd?
>
> 2. Apply it and let bisect break?
>
> 3. Drop -Werror?
In commit 27d59ccd, I introduced the create_blob_file() function, in
"tests/i440fx-test.c". The patch was really about nothing else than
introducing this function (which would have made no sense outside of
said C file, so I made it static).
(The new function was to be utilized in the next patch (3bcc77ae).)
Of course gcc whined at 27d59ccd, and I was forced to call
create_blob_file() from main() just to shut it up. This function call
made absolutely no sense. I only added the call in order to convince gcc
that create_blob_file() was not some abandoned, useless function.
Of course, when the tree is built at 27d59ccd, *and* "tests/i440fx-test"
is run, then create_blob_file() runs too, and it *has* an effect. A blob
file is indeed created and unlinked. In other words, in that commit the
utility function is actually exercised, *outside* its intended
scope/context, just to shut up gcc.
To follow suit, Qiao would have to call write_start_flat_header() and
write_end_flat_header() *somewhere* in patch v8 03/13. It would be a
terrible thing to do. It makes absolutely no sense to call these
functions in this patch. And I don't think we could trick gcc with an
"if (0) {...}", because the optimizer would eliminate the call and we'd
be left with the original warning/error.
Summary: in commit 27d59ccd, I did a horrible hack to shut up gcc. It
was only permissible because the hack affected just a test case. It
didn't affect production code that you actually might want to bisect.
The general qemu (and edk2) approach is to rape the code until the
*contemporary* gcc / compiler of choice shuts up. Then, at *real* bisect
time later down the road, with a new compiler release, act surprised
when the tree doesn't build. Then repeat/restart the bisection with
-Werror disabled.
My specific proposal is to test-build the tree at all patches in the
series, *except* at the last one, with -Werror disabled. Then build the
tree at the final patch with -Werror enabled.
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-10 23:09 ` Paolo Bonzini
@ 2014-02-10 23:30 ` Laszlo Ersek
2014-02-10 23:34 ` Paolo Bonzini
0 siblings, 1 reply; 71+ messages in thread
From: Laszlo Ersek @ 2014-02-10 23:30 UTC (permalink / raw)
To: Paolo Bonzini
Cc: stefanha, qemu-devel, Luiz Capitulino, qiaonuohan,
kumagai-atsushi, anderson, afaerber
On 02/11/14 00:09, Paolo Bonzini wrote:
> Il 10/02/2014 23:02, Laszlo Ersek ha scritto:
>> On 02/10/14 20:10, Luiz Capitulino wrote:
>>> On Tue, 28 Jan 2014 14:22:06 +0800
>>> qiaonuohan <qiaonuohan@cn.fujitsu.com> 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"}
>>>> ]
>>>> }
>>>
>>> I don't want to hold this series anymore, this series is long and I
>>> know it
>>> took you and Laszlo's a long time to get it right. On the other hand
>>> we can't
>>> allow every single command to have its own introspection protocol.
>>>
>>> I think I'm fine accepting this one now, as long as it's fine for
>>> libvirt
>>> too. Eric, can you confirm this please?
>>
>> We discussed this before, and Eric participated. In fact the custom
>> introspection was one of his recommendations.
>>
>> http://thread.gmane.org/gmane.comp.emulators.qemu/221270/focus=246650
>>
>> (Which I agreed with because it would give us the most independence.)
>>
>> Of course I'm not trying to imply that this one specific interface will
>> doubtlessly serve all of libvirt's needs wrt. the kdump feature. We
>> certainly need Eric to sign off on that.
>
> I think the justification here is that even if you defined an Enum for
> ['elf','kdump-zlib', 'kdump-lzo', 'kdump-snappy'], it would not be
> enough to describe which values were compiled in (as opposed to
> supported by the particular QEMU version).
>
> However, I don't see the point in having the "format-option" field. What
> about:
>
> -> { "execute": "query-dump-guest-memory-capabilities" }
> <- { "return": { "formats":
> ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
Technically you might be right. However, this partial introspection
feature is entirely a wart whose existence is exclusively justified by
non-technical reasons, such as deadlines, and not wanting to be blocked
indefinitely by architecture astronautics around full introspection. I
don't see the point of polishing it beyond bare usability, at least not
after I've reviewed three versions of the patchset.
That's of course just my opinion... :)
Laszlo
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-10 23:30 ` Laszlo Ersek
@ 2014-02-10 23:34 ` Paolo Bonzini
2014-02-11 0:22 ` Laszlo Ersek
2014-02-11 2:47 ` Luiz Capitulino
0 siblings, 2 replies; 71+ messages in thread
From: Paolo Bonzini @ 2014-02-10 23:34 UTC (permalink / raw)
To: Laszlo Ersek
Cc: stefanha, qemu-devel, Luiz Capitulino, qiaonuohan,
kumagai-atsushi, anderson, afaerber
Il 11/02/2014 00:30, Laszlo Ersek ha scritto:
>> > However, I don't see the point in having the "format-option" field. What
>> > about:
>> >
>> > -> { "execute": "query-dump-guest-memory-capabilities" }
>> > <- { "return": { "formats":
>> > ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
> Technically you might be right. However, this partial introspection
> feature is entirely a wart
I don't see it entirely like that. For example, whether paging is
supported could also be part of the capabilities and not part of the
regular QAPI introspection. Of course, quiaonuohan need not add
anything like that.
> whose existence is exclusively justified by
> non-technical reasons, such as deadlines, and not wanting to be blocked
> indefinitely by architecture astronautics around full introspection. I
> don't see the point of polishing it beyond bare usability, at least not
> after I've reviewed three versions of the patchset.
Luiz, can you apply patches 1-12 for now? I agree with Laszlo that, no
matter how unfortunate this is, the "unused" warnings are a necessary
evil and one that can be worked around easily when bisecting.
Paolo
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-10 23:34 ` Paolo Bonzini
@ 2014-02-11 0:22 ` Laszlo Ersek
2014-02-11 2:37 ` Qiao Nuohan
2014-02-11 2:47 ` Luiz Capitulino
1 sibling, 1 reply; 71+ messages in thread
From: Laszlo Ersek @ 2014-02-11 0:22 UTC (permalink / raw)
To: Paolo Bonzini
Cc: stefanha, qemu-devel, Luiz Capitulino, qiaonuohan,
kumagai-atsushi, anderson, afaerber
On 02/11/14 00:34, Paolo Bonzini wrote:
> Luiz, can you apply patches 1-12 for now? I agree with Laszlo that, no
> matter how unfortunate this is, the "unused" warnings are a necessary
> evil and one that can be worked around easily when bisecting.
Thanks, Paolo!
In addition, Luiz, please consider applying Ekaterina's v3 patch on top
of Qiao's v8 12/13 (since v8 13/13 is being postponed). Ekaterina's
patch doesn't depend on v8 13/13 and it extends the feature to s390x.
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-11 0:22 ` Laszlo Ersek
@ 2014-02-11 2:37 ` Qiao Nuohan
0 siblings, 0 replies; 71+ messages in thread
From: Qiao Nuohan @ 2014-02-11 2:37 UTC (permalink / raw)
To: Laszlo Ersek, Paolo Bonzini, Luiz Capitulino, Eric Blake
Cc: stefanha, kumagai-atsushi, qemu-devel, afaerber, anderson
On 02/11/2014 08:22 AM, Laszlo Ersek wrote:
> On 02/11/14 00:34, Paolo Bonzini wrote:
>
>> Luiz, can you apply patches 1-12 for now? I agree with Laszlo that, no
>> matter how unfortunate this is, the "unused" warnings are a necessary
>> evil and one that can be worked around easily when bisecting.
>
> Thanks, Paolo!
>
> In addition, Luiz, please consider applying Ekaterina's v3 patch on top
> of Qiao's v8 12/13 (since v8 13/13 is being postponed). Ekaterina's
> patch doesn't depend on v8 13/13 and it extends the feature to s390x.
This is a good strategy to me. And thanks to Paolo, your suggestion on 13/13 is
better, but I want to check whether Eric has some comments on it.
>
> Thanks,
> Laszlo
>
>
--
Regards
Qiao Nuohan
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-10 23:34 ` Paolo Bonzini
2014-02-11 0:22 ` Laszlo Ersek
@ 2014-02-11 2:47 ` Luiz Capitulino
2014-02-11 3:20 ` Qiao Nuohan
2014-02-11 7:19 ` Paolo Bonzini
1 sibling, 2 replies; 71+ messages in thread
From: Luiz Capitulino @ 2014-02-11 2:47 UTC (permalink / raw)
To: Paolo Bonzini
Cc: stefanha, qemu-devel, qiaonuohan, kumagai-atsushi, anderson,
Laszlo Ersek, afaerber
On Tue, 11 Feb 2014 00:34:37 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 11/02/2014 00:30, Laszlo Ersek ha scritto:
> >> > However, I don't see the point in having the "format-option" field. What
> >> > about:
> >> >
> >> > -> { "execute": "query-dump-guest-memory-capabilities" }
> >> > <- { "return": { "formats":
> >> > ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
> > Technically you might be right. However, this partial introspection
> > feature is entirely a wart
>
> I don't see it entirely like that. For example, whether paging is
> supported could also be part of the capabilities and not part of the
> regular QAPI introspection. Of course, quiaonuohan need not add
> anything like that.
>
> > whose existence is exclusively justified by
> > non-technical reasons, such as deadlines, and not wanting to be blocked
> > indefinitely by architecture astronautics around full introspection. I
> > don't see the point of polishing it beyond bare usability, at least not
> > after I've reviewed three versions of the patchset.
>
> Luiz, can you apply patches 1-12 for now? I agree with Laszlo that, no
> matter how unfortunate this is, the "unused" warnings are a necessary
> evil and one that can be worked around easily when bisecting.
Yes, I can. But what's the problem with patch 13? For me having Eric's
ACK is enough for applying it. Anything else will be done by QMP
introspection.
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-11 2:47 ` Luiz Capitulino
@ 2014-02-11 3:20 ` Qiao Nuohan
2014-02-11 7:19 ` Paolo Bonzini
1 sibling, 0 replies; 71+ messages in thread
From: Qiao Nuohan @ 2014-02-11 3:20 UTC (permalink / raw)
To: Eric Blake
Cc: stefanha, qemu-devel, Luiz Capitulino, kumagai-atsushi,
Paolo Bonzini, Laszlo Ersek, afaerber, anderson
On 02/11/2014 10:47 AM, Luiz Capitulino wrote:
> On Tue, 11 Feb 2014 00:34:37 +0100
> Paolo Bonzini<pbonzini@redhat.com> wrote:
>
>> Il 11/02/2014 00:30, Laszlo Ersek ha scritto:
>>>>> However, I don't see the point in having the "format-option" field. What
>>>>> about:
>>>>>
>>>>> -> { "execute": "query-dump-guest-memory-capabilities" }
>>>>> <- { "return": { "formats":
>>>>> ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
>>> Technically you might be right. However, this partial introspection
>>> feature is entirely a wart
>>
>> I don't see it entirely like that. For example, whether paging is
>> supported could also be part of the capabilities and not part of the
>> regular QAPI introspection. Of course, quiaonuohan need not add
>> anything like that.
>>
>>> whose existence is exclusively justified by
>>> non-technical reasons, such as deadlines, and not wanting to be blocked
>>> indefinitely by architecture astronautics around full introspection. I
>>> don't see the point of polishing it beyond bare usability, at least not
>>> after I've reviewed three versions of the patchset.
>>
>> Luiz, can you apply patches 1-12 for now? I agree with Laszlo that, no
>> matter how unfortunate this is, the "unused" warnings are a necessary
>> evil and one that can be worked around easily when bisecting.
>
> Yes, I can. But what's the problem with patch 13? For me having Eric's
> ACK is enough for applying it. Anything else will be done by QMP
> introspection.
>
Hello Eric,
Need your review here, thanks a lot!!
--
Regards
Qiao Nuohan
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-11 2:47 ` Luiz Capitulino
2014-02-11 3:20 ` Qiao Nuohan
@ 2014-02-11 7:19 ` Paolo Bonzini
2014-02-11 13:26 ` Eric Blake
1 sibling, 1 reply; 71+ messages in thread
From: Paolo Bonzini @ 2014-02-11 7:19 UTC (permalink / raw)
To: Luiz Capitulino
Cc: stefanha, qemu-devel, qiaonuohan, kumagai-atsushi, anderson,
Laszlo Ersek, afaerber
Il 11/02/2014 03:47, Luiz Capitulino ha scritto:
> On Tue, 11 Feb 2014 00:34:37 +0100
> Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>> Il 11/02/2014 00:30, Laszlo Ersek ha scritto:
>>>>> However, I don't see the point in having the "format-option" field. What
>>>>> about:
>>>>>
>>>>> -> { "execute": "query-dump-guest-memory-capabilities" }
>>>>> <- { "return": { "formats":
>>>>> ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
>>> Technically you might be right. However, this partial introspection
>>> feature is entirely a wart
>>
>> I don't see it entirely like that. For example, whether paging is
>> supported could also be part of the capabilities and not part of the
>> regular QAPI introspection. Of course, quiaonuohan need not add
>> anything like that.
>>
>>> whose existence is exclusively justified by
>>> non-technical reasons, such as deadlines, and not wanting to be blocked
>>> indefinitely by architecture astronautics around full introspection. I
>>> don't see the point of polishing it beyond bare usability, at least not
>>> after I've reviewed three versions of the patchset.
>>
>> Luiz, can you apply patches 1-12 for now? I agree with Laszlo that, no
>> matter how unfortunate this is, the "unused" warnings are a necessary
>> evil and one that can be worked around easily when bisecting.
>
> Yes, I can. But what's the problem with patch 13? For me having Eric's
> ACK is enough for applying it. Anything else will be done by QMP
> introspection.
I'm suggesting another, more streamlined format; see above.
Paolo
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 03/13] dump: add API to write header of flatten format
2014-02-10 23:20 ` Laszlo Ersek
@ 2014-02-11 9:06 ` Markus Armbruster
0 siblings, 0 replies; 71+ messages in thread
From: Markus Armbruster @ 2014-02-11 9:06 UTC (permalink / raw)
To: Laszlo Ersek
Cc: peter.maydell, stefanha, qemu-devel, Luiz Capitulino, qiaonuohan,
kumagai-atsushi, anderson, afaerber
Laszlo Ersek <lersek@redhat.com> writes:
> On 02/10/14 23:48, Luiz Capitulino wrote:
>> On Mon, 10 Feb 2014 22:57:15 +0100
>> Laszlo Ersek <lersek@redhat.com> wrote:
>
>>> In short, -Werror and bisection don't mix. They already don't, and we
>>> shouldn't expect them to.
>>
>> I understand what you're saying, and I don't want people to do needless and
>> endless respins, but letting bisect break at will doesn't seem a good option
>> either.
>>
>> What other options do we have? What's the general QEMU directive in cases like
>> this?
>>
>> 1. Do what you did in commit 27d59ccd?
>>
>> 2. Apply it and let bisect break?
>>
>> 3. Drop -Werror?
>
> In commit 27d59ccd, I introduced the create_blob_file() function, in
> "tests/i440fx-test.c". The patch was really about nothing else than
> introducing this function (which would have made no sense outside of
> said C file, so I made it static).
>
> (The new function was to be utilized in the next patch (3bcc77ae).)
>
> Of course gcc whined at 27d59ccd, and I was forced to call
> create_blob_file() from main() just to shut it up. This function call
> made absolutely no sense. I only added the call in order to convince gcc
> that create_blob_file() was not some abandoned, useless function.
>
> Of course, when the tree is built at 27d59ccd, *and* "tests/i440fx-test"
> is run, then create_blob_file() runs too, and it *has* an effect. A blob
> file is indeed created and unlinked. In other words, in that commit the
> utility function is actually exercised, *outside* its intended
> scope/context, just to shut up gcc.
>
> To follow suit, Qiao would have to call write_start_flat_header() and
> write_end_flat_header() *somewhere* in patch v8 03/13. It would be a
> terrible thing to do. It makes absolutely no sense to call these
> functions in this patch. And I don't think we could trick gcc with an
> "if (0) {...}", because the optimizer would eliminate the call and we'd
> be left with the original warning/error.
>
> Summary: in commit 27d59ccd, I did a horrible hack to shut up gcc. It
> was only permissible because the hack affected just a test case. It
> didn't affect production code that you actually might want to bisect.
Ugh!
> The general qemu (and edk2) approach is to rape the code until the
> *contemporary* gcc / compiler of choice shuts up. Then, at *real* bisect
> time later down the road, with a new compiler release, act surprised
> when the tree doesn't build. Then repeat/restart the bisection with
> -Werror disabled.
I always configure --disable-werror. It doesn't make the sky fall.
> My specific proposal is to test-build the tree at all patches in the
> series, *except* at the last one, with -Werror disabled. Then build the
> tree at the final patch with -Werror enabled.
My proposal is a generalization of yours: use a bit of common sense for
a change :)
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-11 7:19 ` Paolo Bonzini
@ 2014-02-11 13:26 ` Eric Blake
2014-02-12 3:13 ` [Qemu-devel] [PATCH v9 " Qiao Nuohan
0 siblings, 1 reply; 71+ messages in thread
From: Eric Blake @ 2014-02-11 13:26 UTC (permalink / raw)
To: Paolo Bonzini, Luiz Capitulino
Cc: stefanha, qemu-devel, qiaonuohan, kumagai-atsushi, anderson,
Laszlo Ersek, afaerber
[-- Attachment #1: Type: text/plain, Size: 1749 bytes --]
On 02/11/2014 12:19 AM, Paolo Bonzini wrote:
> Il 11/02/2014 03:47, Luiz Capitulino ha scritto:
>> On Tue, 11 Feb 2014 00:34:37 +0100
>> Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>> Il 11/02/2014 00:30, Laszlo Ersek ha scritto:
>>>>>> However, I don't see the point in having the "format-option"
>>>>>> field. What
>>>>>> about:
>>>>>>
>>>>>> -> { "execute": "query-dump-guest-memory-capabilities" }
>>>>>> <- { "return": { "formats":
>>>>>> ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
>>>> Technically you might be right. However, this partial introspection
>>>> feature is entirely a wart
>> Yes, I can. But what's the problem with patch 13? For me having Eric's
>> ACK is enough for applying it. Anything else will be done by QMP
>> introspection.
>
> I'm suggesting another, more streamlined format; see above.
I like Paolo's suggestion of a more streamlined format.
With migration introspection, we could do two things - learn the set of
knobs available (query-migrate-capabilities), AND change those knobs
(migrate-set-capabilities). Thus, each knob had to be listed as a
struct that showed both the name of the knob and the current status.
But here, we are not changing the set of format options on the fly. The
simpler array of string names supported is sufficient; it's less work
for libvirt to get an array of strings than it is to get an array of
structs only to then determine the name from the struct and the
redundant boolean that all such names are supported.
From libvirt's perspective, I think patch 13/13 needs to be respun into
the simpler format.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format
2014-01-28 6:21 [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
` (15 preceding siblings ...)
2014-01-30 17:01 ` [Qemu-devel] [PATCH] Define the architecture for compressed dump format Ekaterina Tumanova
@ 2014-02-11 17:00 ` Luiz Capitulino
2014-02-17 17:51 ` Luiz Capitulino
16 siblings, 1 reply; 71+ messages in thread
From: Luiz Capitulino @ 2014-02-11 17:00 UTC (permalink / raw)
To: qiaonuohan
Cc: stefanha, qemu-devel, kumagai-atsushi, pbonzini, anderson,
lersek, afaerber
On Tue, 28 Jan 2014 14:21:53 +0800
qiaonuohan <qiaonuohan@cn.fujitsu.com> 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/
I did what was requested by Paolo. I applied patches 1 to 12 and will wait
for the rework on patch 13.
^ permalink raw reply [flat|nested] 71+ messages in thread
* [Qemu-devel] [PATCH v9 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-11 13:26 ` Eric Blake
@ 2014-02-12 3:13 ` Qiao Nuohan
2014-02-12 3:29 ` Eric Blake
0 siblings, 1 reply; 71+ messages in thread
From: Qiao Nuohan @ 2014-02-12 3:13 UTC (permalink / raw)
To: Eric Blake, Paolo Bonzini, Luiz Capitulino, Laszlo Ersek
Cc: stefanha, kumagai-atsushi, qemu-devel, afaerber, anderson
'query-dump-guest-memory-capability' is used to query the available formats of
'dump-guest-memory'. The output of the command will be like:
-> { "execute": "query-dump-guest-memory-capability" }
<- { "return": { "formats":
["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
---
dump.c | 33 +++++++++++++++++++++++++++++++++
qapi-schema.json | 9 +++++++++
qmp-commands.hx | 23 +++++++++++++++++++++++
3 files changed, 65 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index 2ebbb23..3a8d55e 100644
--- a/dump.c
+++ b/dump.c
@@ -1788,3 +1788,36 @@ 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)
+{
+ DumpGuestMemoryFormatList *item;
+ DumpGuestMemoryCapability *cap =
+ g_malloc0(sizeof(DumpGuestMemoryCapability));
+
+ /* elf is always available */
+ item = g_malloc0(sizeof(DumpGuestMemoryFormatList));
+ cap->formats = item;
+ item->value = DUMP_GUEST_MEMORY_FORMAT_ELF;
+
+ /* kdump-zlib is always available */
+ item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
+ item = item->next;
+ item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB;
+
+ /* add new item if kdump-lzo is available */
+#ifdef CONFIG_LZO
+ item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
+ item = item->next;
+ item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO;
+#endif
+
+ /* add new item if kdump-snappy is available */
+#ifdef CONFIG_SNAPPY
+ item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
+ item = item->next;
+ item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY;
+#endif
+
+ return cap;
+}
diff --git a/qapi-schema.json b/qapi-schema.json
index 7f62007..5d13bb3 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2783,6 +2783,15 @@
'*length': 'int', '*format': 'DumpGuestMemoryFormat' } }
##
+# Since: 2.0
+##
+{ 'type': 'DumpGuestMemoryCapability',
+ 'data': {
+ 'formats': ['DumpGuestMemoryFormat'] } }
+
+{ '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 019dde6..1f9ff69 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -829,6 +829,29 @@ 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 available format of 'dump-guest-memory'
+
+Example:
+
+-> { "execute": "query-dump-guest-memory-capability" }
+<- { "return": { "formats":
+ ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
+
+Note: This is a light-weight introspection to let management know the available
+ formats of dump-guest-memory.
+
+EQMP
+
+ {
.name = "netdev_add",
.args_type = "netdev:O",
.mhandler.cmd_new = qmp_netdev_add,
--
1.7.1
^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v9 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-12 3:13 ` [Qemu-devel] [PATCH v9 " Qiao Nuohan
@ 2014-02-12 3:29 ` Eric Blake
2014-02-12 6:34 ` [Qemu-devel] [PATCH v10 " Qiao Nuohan
0 siblings, 1 reply; 71+ messages in thread
From: Eric Blake @ 2014-02-12 3:29 UTC (permalink / raw)
To: Qiao Nuohan, Paolo Bonzini, Luiz Capitulino, Laszlo Ersek
Cc: stefanha, kumagai-atsushi, qemu-devel, afaerber, anderson
[-- Attachment #1: Type: text/plain, Size: 1696 bytes --]
On 02/11/2014 08:13 PM, Qiao Nuohan wrote:
> 'query-dump-guest-memory-capability' is used to query the available
> formats of
> 'dump-guest-memory'. The output of the command will be like:
>
> -> { "execute": "query-dump-guest-memory-capability" }
> <- { "return": { "formats":
> ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> ---
> dump.c | 33 +++++++++++++++++++++++++++++++++
> qapi-schema.json | 9 +++++++++
> qmp-commands.hx | 23 +++++++++++++++++++++++
> 3 files changed, 65 insertions(+), 0 deletions(-)
> +++ b/qapi-schema.json
> @@ -2783,6 +2783,15 @@
> '*length': 'int', '*format': 'DumpGuestMemoryFormat' } }
>
> ##
> +# Since: 2.0
A bit sparse on the documentation; at a minimum, you want a line:
# @query-dump-guest-memory-capability:
prior to the Since designation (look at @query-name for an example).
> +
> +SQMP
> +query-dump-guest-memory-capability
> +----------
> +
> +Show available format of 'dump-guest-memory'
s/format of/formats for/
> +
> +Example:
> +
> +-> { "execute": "query-dump-guest-memory-capability" }
> +<- { "return": { "formats":
> + ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
> +
> +Note: This is a light-weight introspection to let management know the
> available
> + formats of dump-guest-memory.
This note feels a bit redundant with the earlier summary; I'm okay if
you leave it in, but I also don't mind if you drop it.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 71+ messages in thread
* [Qemu-devel] [PATCH v10 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-12 3:29 ` Eric Blake
@ 2014-02-12 6:34 ` Qiao Nuohan
2014-02-12 14:49 ` Luiz Capitulino
0 siblings, 1 reply; 71+ messages in thread
From: Qiao Nuohan @ 2014-02-12 6:34 UTC (permalink / raw)
To: Eric Blake
Cc: stefanha, qemu-devel, Luiz Capitulino, kumagai-atsushi,
Paolo Bonzini, Laszlo Ersek, afaerber, anderson
'query-dump-guest-memory-capability' is used to query the available formats for
'dump-guest-memory'. The output of the command will be like:
-> { "execute": "query-dump-guest-memory-capability" }
<- { "return": { "formats":
["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
---
dump.c | 33 +++++++++++++++++++++++++++++++++
qapi-schema.json | 23 +++++++++++++++++++++++
qmp-commands.hx | 20 ++++++++++++++++++++
3 files changed, 76 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index 2ebbb23..3a8d55e 100644
--- a/dump.c
+++ b/dump.c
@@ -1788,3 +1788,36 @@ 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)
+{
+ DumpGuestMemoryFormatList *item;
+ DumpGuestMemoryCapability *cap =
+ g_malloc0(sizeof(DumpGuestMemoryCapability));
+
+ /* elf is always available */
+ item = g_malloc0(sizeof(DumpGuestMemoryFormatList));
+ cap->formats = item;
+ item->value = DUMP_GUEST_MEMORY_FORMAT_ELF;
+
+ /* kdump-zlib is always available */
+ item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
+ item = item->next;
+ item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB;
+
+ /* add new item if kdump-lzo is available */
+#ifdef CONFIG_LZO
+ item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
+ item = item->next;
+ item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO;
+#endif
+
+ /* add new item if kdump-snappy is available */
+#ifdef CONFIG_SNAPPY
+ item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
+ item = item->next;
+ item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY;
+#endif
+
+ return cap;
+}
diff --git a/qapi-schema.json b/qapi-schema.json
index 7f62007..a097e6c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2783,6 +2783,29 @@
'*length': 'int', '*format': 'DumpGuestMemoryFormat' } }
##
+# @DumpGuestMemoryCapability:
+#
+# A list of the available formats for dump-guest-memory
+#
+# Since: 2.0
+##
+{ 'type': 'DumpGuestMemoryCapability',
+ 'data': {
+ 'formats': ['DumpGuestMemoryFormat'] } }
+
+##
+# @query-dump-guest-memory-capability:
+#
+# Returns the available formats for dump-guest-memory
+#
+# Returns: A @DumpGuestMemoryCapability object listing available formats for
+# dump-guest-memory
+#
+# Since: 2.0
+##
+{ '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 019dde6..029cb3d 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -829,6 +829,26 @@ 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 available formats for 'dump-guest-memory'
+
+Example:
+
+-> { "execute": "query-dump-guest-memory-capability" }
+<- { "return": { "formats":
+ ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
+
+EQMP
+
+ {
.name = "netdev_add",
.args_type = "netdev:O",
.mhandler.cmd_new = qmp_netdev_add,
--
1.7.1
^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v10 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-12 6:34 ` [Qemu-devel] [PATCH v10 " Qiao Nuohan
@ 2014-02-12 14:49 ` Luiz Capitulino
2014-02-13 1:48 ` Qiao Nuohan
0 siblings, 1 reply; 71+ messages in thread
From: Luiz Capitulino @ 2014-02-12 14:49 UTC (permalink / raw)
To: Qiao Nuohan
Cc: stefanha, qemu-devel, Laszlo Ersek, kumagai-atsushi,
Paolo Bonzini, afaerber, anderson
On Wed, 12 Feb 2014 14:34:19 +0800
Qiao Nuohan <qiaonuohan@cn.fujitsu.com> wrote:
> 'query-dump-guest-memory-capability' is used to query the available formats for
> 'dump-guest-memory'. The output of the command will be like:
>
> -> { "execute": "query-dump-guest-memory-capability" }
> <- { "return": { "formats":
> ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
The new command looks good to me, but the patch itself is broken. It doesn't
apply and I could identify at least one breakage below...
> ---
> dump.c | 33 +++++++++++++++++++++++++++++++++
> qapi-schema.json | 23 +++++++++++++++++++++++
> qmp-commands.hx | 20 ++++++++++++++++++++
> 3 files changed, 76 insertions(+), 0 deletions(-)
>
> diff --git a/dump.c b/dump.c
> index 2ebbb23..3a8d55e 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -1788,3 +1788,36 @@ 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)
> +{
> + DumpGuestMemoryFormatList *item;
> + DumpGuestMemoryCapability *cap =
> + g_malloc0(sizeof(DumpGuestMemoryCapability));
> +
> + /* elf is always available */
> + item = g_malloc0(sizeof(DumpGuestMemoryFormatList));
> + cap->formats = item;
> + item->value = DUMP_GUEST_MEMORY_FORMAT_ELF;
> +
> + /* kdump-zlib is always available */
> + item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
> + item = item->next;
> + item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB;
> +
> + /* add new item if kdump-lzo is available */
> +#ifdef CONFIG_LZO
> + item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
> + item = item->next;
> + item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO;
> +#endif
> +
> + /* add new item if kdump-snappy is available */
> +#ifdef CONFIG_SNAPPY
> + item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
> + item = item->next;
> + item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY;
> +#endif
> +
> + return cap;
> +}
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 7f62007..a097e6c 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2783,6 +2783,29 @@
> '*length': 'int', '*format': 'DumpGuestMemoryFormat' } }
>
> ##
> +# @DumpGuestMemoryCapability:
> +#
> +# A list of the available formats for dump-guest-memory
> +#
> +# Since: 2.0
> +##
> +{ 'type': 'DumpGuestMemoryCapability',
> + 'data': {
> + 'formats': ['DumpGuestMemoryFormat'] } }
> +
> +##
> +# @query-dump-guest-memory-capability:
> +#
> +# Returns the available formats for dump-guest-memory
> +#
> +# Returns: A @DumpGuestMemoryCapability object listing available formats for
> +# dump-guest-memory
> +#
> +# Since: 2.0
> +##
> +{ 'command': 'query-dump-guest-memory-capability', 'returns':
> 'DumpGuestMemoryCapability' }
Here.
> +
> +##
> # @netdev_add:
> #
> # Add a network backend.
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 019dde6..029cb3d 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -829,6 +829,26 @@ 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 available formats for 'dump-guest-memory'
> +
> +Example:
> +
> +-> { "execute": "query-dump-guest-memory-capability" }
> +<- { "return": { "formats":
> + ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
> +
> +EQMP
> +
> + {
> .name = "netdev_add",
> .args_type = "netdev:O",
> .mhandler.cmd_new = qmp_netdev_add,
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v10 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-12 14:49 ` Luiz Capitulino
@ 2014-02-13 1:48 ` Qiao Nuohan
2014-02-13 2:57 ` Luiz Capitulino
` (2 more replies)
0 siblings, 3 replies; 71+ messages in thread
From: Qiao Nuohan @ 2014-02-13 1:48 UTC (permalink / raw)
To: Luiz Capitulino
Cc: stefanha, qemu-devel, Laszlo Ersek, kumagai-atsushi,
Paolo Bonzini, afaerber, anderson
[-- Attachment #1: Type: text/plain, Size: 4276 bytes --]
On 02/12/2014 10:49 PM, Luiz Capitulino wrote:
> On Wed, 12 Feb 2014 14:34:19 +0800
> Qiao Nuohan<qiaonuohan@cn.fujitsu.com> wrote:
>
>> 'query-dump-guest-memory-capability' is used to query the available formats for
>> 'dump-guest-memory'. The output of the command will be like:
>>
>> -> { "execute": "query-dump-guest-memory-capability" }
>> <- { "return": { "formats":
>> ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
>>
>> Signed-off-by: Qiao Nuohan<qiaonuohan@cn.fujitsu.com>
>
> The new command looks good to me, but the patch itself is broken. It doesn't
> apply and I could identify at least one breakage below...
I think it is because of the 80 character limit.If I am wrong, please correct
me. And I have attach the new patch with this mail, please check.
>
>> ---
>> dump.c | 33 +++++++++++++++++++++++++++++++++
>> qapi-schema.json | 23 +++++++++++++++++++++++
>> qmp-commands.hx | 20 ++++++++++++++++++++
>> 3 files changed, 76 insertions(+), 0 deletions(-)
>>
>> diff --git a/dump.c b/dump.c
>> index 2ebbb23..3a8d55e 100644
>> --- a/dump.c
>> +++ b/dump.c
>> @@ -1788,3 +1788,36 @@ 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)
>> +{
>> + DumpGuestMemoryFormatList *item;
>> + DumpGuestMemoryCapability *cap =
>> + g_malloc0(sizeof(DumpGuestMemoryCapability));
>> +
>> + /* elf is always available */
>> + item = g_malloc0(sizeof(DumpGuestMemoryFormatList));
>> + cap->formats = item;
>> + item->value = DUMP_GUEST_MEMORY_FORMAT_ELF;
>> +
>> + /* kdump-zlib is always available */
>> + item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
>> + item = item->next;
>> + item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB;
>> +
>> + /* add new item if kdump-lzo is available */
>> +#ifdef CONFIG_LZO
>> + item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
>> + item = item->next;
>> + item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO;
>> +#endif
>> +
>> + /* add new item if kdump-snappy is available */
>> +#ifdef CONFIG_SNAPPY
>> + item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
>> + item = item->next;
>> + item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY;
>> +#endif
>> +
>> + return cap;
>> +}
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 7f62007..a097e6c 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -2783,6 +2783,29 @@
>> '*length': 'int', '*format': 'DumpGuestMemoryFormat' } }
>>
>> ##
>> +# @DumpGuestMemoryCapability:
>> +#
>> +# A list of the available formats for dump-guest-memory
>> +#
>> +# Since: 2.0
>> +##
>> +{ 'type': 'DumpGuestMemoryCapability',
>> + 'data': {
>> + 'formats': ['DumpGuestMemoryFormat'] } }
>> +
>> +##
>> +# @query-dump-guest-memory-capability:
>> +#
>> +# Returns the available formats for dump-guest-memory
>> +#
>> +# Returns: A @DumpGuestMemoryCapability object listing available formats for
>> +# dump-guest-memory
>> +#
>> +# Since: 2.0
>> +##
>> +{ 'command': 'query-dump-guest-memory-capability', 'returns':
>> 'DumpGuestMemoryCapability' }
>
> Here.
>
>> +
>> +##
>> # @netdev_add:
>> #
>> # Add a network backend.
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index 019dde6..029cb3d 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -829,6 +829,26 @@ 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 available formats for 'dump-guest-memory'
>> +
>> +Example:
>> +
>> +-> { "execute": "query-dump-guest-memory-capability" }
>> +<- { "return": { "formats":
>> + ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
>> +
>> +EQMP
>> +
>> + {
>> .name = "netdev_add",
>> .args_type = "netdev:O",
>> .mhandler.cmd_new = qmp_netdev_add,
>
>
--
Regards
Qiao Nuohan
[-- Attachment #2: 0013-dump-add-query-dump-guest-memory-capability-command.patch --]
[-- Type: text/plain, Size: 3671 bytes --]
>From c729625031c88b0bf32bae50c39f81585303cc52 Mon Sep 17 00:00:00 2001
From: qiaonuohan <qiaonuohan@cn.fujitsu.com>
Date: Thu, 13 Feb 2014 09:39:26 +0800
Subject: [PATCH v11 13/13] dump: add 'query-dump-guest-memory-capability' command
'query-dump-guest-memory-capability' is used to query the available formats for
'dump-guest-memory'. The output of the command will be like:
-> { "execute": "query-dump-guest-memory-capability" }
<- { "return": { "formats":
["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
---
dump.c | 33 +++++++++++++++++++++++++++++++++
qapi-schema.json | 24 ++++++++++++++++++++++++
qmp-commands.hx | 20 ++++++++++++++++++++
3 files changed, 77 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index 2ebbb23..3a8d55e 100644
--- a/dump.c
+++ b/dump.c
@@ -1788,3 +1788,36 @@ 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)
+{
+ DumpGuestMemoryFormatList *item;
+ DumpGuestMemoryCapability *cap =
+ g_malloc0(sizeof(DumpGuestMemoryCapability));
+
+ /* elf is always available */
+ item = g_malloc0(sizeof(DumpGuestMemoryFormatList));
+ cap->formats = item;
+ item->value = DUMP_GUEST_MEMORY_FORMAT_ELF;
+
+ /* kdump-zlib is always available */
+ item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
+ item = item->next;
+ item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB;
+
+ /* add new item if kdump-lzo is available */
+#ifdef CONFIG_LZO
+ item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
+ item = item->next;
+ item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO;
+#endif
+
+ /* add new item if kdump-snappy is available */
+#ifdef CONFIG_SNAPPY
+ item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
+ item = item->next;
+ item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY;
+#endif
+
+ return cap;
+}
diff --git a/qapi-schema.json b/qapi-schema.json
index 905a8af..3eeb261 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2783,6 +2783,30 @@
'*length': 'int', '*format': 'DumpGuestMemoryFormat' } }
##
+# @DumpGuestMemoryCapability:
+#
+# A list of the available formats for dump-guest-memory
+#
+# Since: 2.0
+##
+{ 'type': 'DumpGuestMemoryCapability',
+ 'data': {
+ 'formats': ['DumpGuestMemoryFormat'] } }
+
+##
+# @query-dump-guest-memory-capability:
+#
+# Returns the available formats for dump-guest-memory
+#
+# Returns: A @DumpGuestMemoryCapability object listing available formats for
+# dump-guest-memory
+#
+# Since: 2.0
+##
+{ '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 019dde6..029cb3d 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -829,6 +829,26 @@ 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 available formats for 'dump-guest-memory'
+
+Example:
+
+-> { "execute": "query-dump-guest-memory-capability" }
+<- { "return": { "formats":
+ ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
+
+EQMP
+
+ {
.name = "netdev_add",
.args_type = "netdev:O",
.mhandler.cmd_new = qmp_netdev_add,
--
1.7.1
^ permalink raw reply related [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v10 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-13 1:48 ` Qiao Nuohan
@ 2014-02-13 2:57 ` Luiz Capitulino
2014-02-13 13:13 ` Eric Blake
2014-02-13 13:45 ` Luiz Capitulino
2 siblings, 0 replies; 71+ messages in thread
From: Luiz Capitulino @ 2014-02-13 2:57 UTC (permalink / raw)
To: Qiao Nuohan
Cc: stefanha, qemu-devel, Laszlo Ersek, kumagai-atsushi,
Paolo Bonzini, afaerber, anderson
On Thu, 13 Feb 2014 09:48:33 +0800
Qiao Nuohan <qiaonuohan@cn.fujitsu.com> wrote:
> On 02/12/2014 10:49 PM, Luiz Capitulino wrote:
> > On Wed, 12 Feb 2014 14:34:19 +0800
> > Qiao Nuohan<qiaonuohan@cn.fujitsu.com> wrote:
> >
> >> 'query-dump-guest-memory-capability' is used to query the available formats for
> >> 'dump-guest-memory'. The output of the command will be like:
> >>
> >> -> { "execute": "query-dump-guest-memory-capability" }
> >> <- { "return": { "formats":
> >> ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
> >>
> >> Signed-off-by: Qiao Nuohan<qiaonuohan@cn.fujitsu.com>
> >
> > The new command looks good to me, but the patch itself is broken. It doesn't
> > apply and I could identify at least one breakage below...
>
> I think it is because of the 80 character limit.If I am wrong, please correct
> me. And I have attach the new patch with this mail, please check.
Yes, works now. Paolo, Eric, any acks here?
>
> >
> >> ---
> >> dump.c | 33 +++++++++++++++++++++++++++++++++
> >> qapi-schema.json | 23 +++++++++++++++++++++++
> >> qmp-commands.hx | 20 ++++++++++++++++++++
> >> 3 files changed, 76 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/dump.c b/dump.c
> >> index 2ebbb23..3a8d55e 100644
> >> --- a/dump.c
> >> +++ b/dump.c
> >> @@ -1788,3 +1788,36 @@ 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)
> >> +{
> >> + DumpGuestMemoryFormatList *item;
> >> + DumpGuestMemoryCapability *cap =
> >> + g_malloc0(sizeof(DumpGuestMemoryCapability));
> >> +
> >> + /* elf is always available */
> >> + item = g_malloc0(sizeof(DumpGuestMemoryFormatList));
> >> + cap->formats = item;
> >> + item->value = DUMP_GUEST_MEMORY_FORMAT_ELF;
> >> +
> >> + /* kdump-zlib is always available */
> >> + item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
> >> + item = item->next;
> >> + item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB;
> >> +
> >> + /* add new item if kdump-lzo is available */
> >> +#ifdef CONFIG_LZO
> >> + item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
> >> + item = item->next;
> >> + item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO;
> >> +#endif
> >> +
> >> + /* add new item if kdump-snappy is available */
> >> +#ifdef CONFIG_SNAPPY
> >> + item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
> >> + item = item->next;
> >> + item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY;
> >> +#endif
> >> +
> >> + return cap;
> >> +}
> >> diff --git a/qapi-schema.json b/qapi-schema.json
> >> index 7f62007..a097e6c 100644
> >> --- a/qapi-schema.json
> >> +++ b/qapi-schema.json
> >> @@ -2783,6 +2783,29 @@
> >> '*length': 'int', '*format': 'DumpGuestMemoryFormat' } }
> >>
> >> ##
> >> +# @DumpGuestMemoryCapability:
> >> +#
> >> +# A list of the available formats for dump-guest-memory
> >> +#
> >> +# Since: 2.0
> >> +##
> >> +{ 'type': 'DumpGuestMemoryCapability',
> >> + 'data': {
> >> + 'formats': ['DumpGuestMemoryFormat'] } }
> >> +
> >> +##
> >> +# @query-dump-guest-memory-capability:
> >> +#
> >> +# Returns the available formats for dump-guest-memory
> >> +#
> >> +# Returns: A @DumpGuestMemoryCapability object listing available formats for
> >> +# dump-guest-memory
> >> +#
> >> +# Since: 2.0
> >> +##
> >> +{ 'command': 'query-dump-guest-memory-capability', 'returns':
> >> 'DumpGuestMemoryCapability' }
> >
> > Here.
> >
> >> +
> >> +##
> >> # @netdev_add:
> >> #
> >> # Add a network backend.
> >> diff --git a/qmp-commands.hx b/qmp-commands.hx
> >> index 019dde6..029cb3d 100644
> >> --- a/qmp-commands.hx
> >> +++ b/qmp-commands.hx
> >> @@ -829,6 +829,26 @@ 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 available formats for 'dump-guest-memory'
> >> +
> >> +Example:
> >> +
> >> +-> { "execute": "query-dump-guest-memory-capability" }
> >> +<- { "return": { "formats":
> >> + ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
> >> +
> >> +EQMP
> >> +
> >> + {
> >> .name = "netdev_add",
> >> .args_type = "netdev:O",
> >> .mhandler.cmd_new = qmp_netdev_add,
> >
> >
>
>
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v10 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-13 1:48 ` Qiao Nuohan
2014-02-13 2:57 ` Luiz Capitulino
@ 2014-02-13 13:13 ` Eric Blake
2014-02-13 13:45 ` Luiz Capitulino
2 siblings, 0 replies; 71+ messages in thread
From: Eric Blake @ 2014-02-13 13:13 UTC (permalink / raw)
To: Qiao Nuohan, Luiz Capitulino
Cc: stefanha, qemu-devel, kumagai-atsushi, Paolo Bonzini,
Laszlo Ersek, afaerber, anderson
[-- Attachment #1: Type: text/plain, Size: 1059 bytes --]
On 02/12/2014 06:48 PM, Qiao Nuohan wrote:
>>From c729625031c88b0bf32bae50c39f81585303cc52 Mon Sep 17 00:00:00 2001
> From: qiaonuohan <qiaonuohan@cn.fujitsu.com>
> Date: Thu, 13 Feb 2014 09:39:26 +0800
> Subject: [PATCH v11 13/13] dump: add 'query-dump-guest-memory-capability' command
>
> 'query-dump-guest-memory-capability' is used to query the available formats for
> 'dump-guest-memory'. The output of the command will be like:
>
> -> { "execute": "query-dump-guest-memory-capability" }
> <- { "return": { "formats":
> ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> ---
> dump.c | 33 +++++++++++++++++++++++++++++++++
> qapi-schema.json | 24 ++++++++++++++++++++++++
> qmp-commands.hx | 20 ++++++++++++++++++++
> 3 files changed, 77 insertions(+), 0 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v10 13/13] dump: add 'query-dump-guest-memory-capability' command
2014-02-13 1:48 ` Qiao Nuohan
2014-02-13 2:57 ` Luiz Capitulino
2014-02-13 13:13 ` Eric Blake
@ 2014-02-13 13:45 ` Luiz Capitulino
2 siblings, 0 replies; 71+ messages in thread
From: Luiz Capitulino @ 2014-02-13 13:45 UTC (permalink / raw)
To: Qiao Nuohan
Cc: stefanha, qemu-devel, Laszlo Ersek, kumagai-atsushi,
Paolo Bonzini, afaerber, anderson
On Thu, 13 Feb 2014 09:48:33 +0800
Qiao Nuohan <qiaonuohan@cn.fujitsu.com> wrote:
> On 02/12/2014 10:49 PM, Luiz Capitulino wrote:
> > On Wed, 12 Feb 2014 14:34:19 +0800
> > Qiao Nuohan<qiaonuohan@cn.fujitsu.com> wrote:
> >
> >> 'query-dump-guest-memory-capability' is used to query the available formats for
> >> 'dump-guest-memory'. The output of the command will be like:
> >>
> >> -> { "execute": "query-dump-guest-memory-capability" }
> >> <- { "return": { "formats":
> >> ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
> >>
> >> Signed-off-by: Qiao Nuohan<qiaonuohan@cn.fujitsu.com>
> >
> > The new command looks good to me, but the patch itself is broken. It doesn't
> > apply and I could identify at least one breakage below...
>
> I think it is because of the 80 character limit.If I am wrong, please correct
> me. And I have attach the new patch with this mail, please check.
Applied to the qmp branch, thanks.
>
> >
> >> ---
> >> dump.c | 33 +++++++++++++++++++++++++++++++++
> >> qapi-schema.json | 23 +++++++++++++++++++++++
> >> qmp-commands.hx | 20 ++++++++++++++++++++
> >> 3 files changed, 76 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/dump.c b/dump.c
> >> index 2ebbb23..3a8d55e 100644
> >> --- a/dump.c
> >> +++ b/dump.c
> >> @@ -1788,3 +1788,36 @@ 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)
> >> +{
> >> + DumpGuestMemoryFormatList *item;
> >> + DumpGuestMemoryCapability *cap =
> >> + g_malloc0(sizeof(DumpGuestMemoryCapability));
> >> +
> >> + /* elf is always available */
> >> + item = g_malloc0(sizeof(DumpGuestMemoryFormatList));
> >> + cap->formats = item;
> >> + item->value = DUMP_GUEST_MEMORY_FORMAT_ELF;
> >> +
> >> + /* kdump-zlib is always available */
> >> + item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
> >> + item = item->next;
> >> + item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_ZLIB;
> >> +
> >> + /* add new item if kdump-lzo is available */
> >> +#ifdef CONFIG_LZO
> >> + item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
> >> + item = item->next;
> >> + item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO;
> >> +#endif
> >> +
> >> + /* add new item if kdump-snappy is available */
> >> +#ifdef CONFIG_SNAPPY
> >> + item->next = g_malloc0(sizeof(DumpGuestMemoryFormatList));
> >> + item = item->next;
> >> + item->value = DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY;
> >> +#endif
> >> +
> >> + return cap;
> >> +}
> >> diff --git a/qapi-schema.json b/qapi-schema.json
> >> index 7f62007..a097e6c 100644
> >> --- a/qapi-schema.json
> >> +++ b/qapi-schema.json
> >> @@ -2783,6 +2783,29 @@
> >> '*length': 'int', '*format': 'DumpGuestMemoryFormat' } }
> >>
> >> ##
> >> +# @DumpGuestMemoryCapability:
> >> +#
> >> +# A list of the available formats for dump-guest-memory
> >> +#
> >> +# Since: 2.0
> >> +##
> >> +{ 'type': 'DumpGuestMemoryCapability',
> >> + 'data': {
> >> + 'formats': ['DumpGuestMemoryFormat'] } }
> >> +
> >> +##
> >> +# @query-dump-guest-memory-capability:
> >> +#
> >> +# Returns the available formats for dump-guest-memory
> >> +#
> >> +# Returns: A @DumpGuestMemoryCapability object listing available formats for
> >> +# dump-guest-memory
> >> +#
> >> +# Since: 2.0
> >> +##
> >> +{ 'command': 'query-dump-guest-memory-capability', 'returns':
> >> 'DumpGuestMemoryCapability' }
> >
> > Here.
> >
> >> +
> >> +##
> >> # @netdev_add:
> >> #
> >> # Add a network backend.
> >> diff --git a/qmp-commands.hx b/qmp-commands.hx
> >> index 019dde6..029cb3d 100644
> >> --- a/qmp-commands.hx
> >> +++ b/qmp-commands.hx
> >> @@ -829,6 +829,26 @@ 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 available formats for 'dump-guest-memory'
> >> +
> >> +Example:
> >> +
> >> +-> { "execute": "query-dump-guest-memory-capability" }
> >> +<- { "return": { "formats":
> >> + ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
> >> +
> >> +EQMP
> >> +
> >> + {
> >> .name = "netdev_add",
> >> .args_type = "netdev:O",
> >> .mhandler.cmd_new = qmp_netdev_add,
> >
> >
>
>
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/1] Define the architecture for compressed dump format.
2014-01-31 15:04 ` [Qemu-devel] [PATCH v3 1/1] Define the architecture for compressed dump format Ekaterina Tumanova
@ 2014-02-13 13:50 ` Luiz Capitulino
0 siblings, 0 replies; 71+ messages in thread
From: Luiz Capitulino @ 2014-02-13 13:50 UTC (permalink / raw)
To: Ekaterina Tumanova
Cc: borntraeger, qiaonuohan, lersek, Public KVM Mailing List
On Fri, 31 Jan 2014 16:04:41 +0100
Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> wrote:
> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
Applied to the qmp branch, thanks.
> ---
> dump.c | 7 +++++--
> target-i386/cpu.h | 2 ++
> target-s390x/cpu.h | 1 +
> 3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/dump.c b/dump.c
> index 8f64aab..8d85255 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -32,6 +32,9 @@
> #ifdef CONFIG_SNAPPY
> #include <snappy-c.h>
> #endif
> +#ifndef ELF_MACHINE_UNAME
> +#define ELF_MACHINE_UNAME "Unknown"
> +#endif
>
> static uint16_t cpu_convert_to_target16(uint16_t val, int endian)
> {
> @@ -817,7 +820,7 @@ static int create_header32(DumpState *s)
> 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);
> + strncpy(dh->utsname.machine, ELF_MACHINE_UNAME, sizeof(dh->utsname.machine));
>
> if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
> status |= DUMP_DH_COMPRESSED_ZLIB;
> @@ -924,7 +927,7 @@ static int create_header64(DumpState *s)
> 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), "x86_64", 6);
> + strncpy(dh->utsname.machine, ELF_MACHINE_UNAME, sizeof(dh->utsname.machine));
>
> if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
> status |= DUMP_DH_COMPRESSED_ZLIB;
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 1fcbc82..198743c 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -38,8 +38,10 @@
>
> #ifdef TARGET_X86_64
> #define ELF_MACHINE EM_X86_64
> +#define ELF_MACHINE_UNAME "x86_64"
> #else
> #define ELF_MACHINE EM_386
> +#define ELF_MACHINE_UNAME "i686"
> #endif
>
> #define CPUArchState struct CPUX86State
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index 68b5ab7..bf7ae4c 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -28,6 +28,7 @@
> #define TARGET_LONG_BITS 64
>
> #define ELF_MACHINE EM_S390
> +#define ELF_MACHINE_UNAME "S390X"
>
> #define CPUArchState struct CPUS390XState
>
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format
2014-02-11 17:00 ` [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format Luiz Capitulino
@ 2014-02-17 17:51 ` Luiz Capitulino
2014-02-18 6:16 ` Qiao Nuohan
0 siblings, 1 reply; 71+ messages in thread
From: Luiz Capitulino @ 2014-02-17 17:51 UTC (permalink / raw)
To: Luiz Capitulino
Cc: stefanha, qemu-devel, qiaonuohan, kumagai-atsushi, pbonzini,
anderson, lersek, afaerber
On Tue, 11 Feb 2014 12:00:56 -0500
Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Tue, 28 Jan 2014 14:21:53 +0800
> qiaonuohan <qiaonuohan@cn.fujitsu.com> 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/
>
> I did what was requested by Paolo. I applied patches 1 to 12 and will wait
> for the rework on patch 13.
Unfortunately this series doesn't build on 32-bit hosts, so I had to drop it
from my queue (see error below). Can you please do the following:
1. Fix the build
2. Add new patch 13/13 to the series
3. Add Ekaterina's patch to your series
Thanks. Here's the error message:
ar: creating libfdt/libfdt.a
/root/qmp-unstable/dump.c: In function ‘write_dump_pages’:
/root/qmp-unstable/dump.c:1356:21: error: passing argument 2 of ‘compress2’ from incompatible pointer type [-Werror]
Z_BEST_SPEED) == Z_OK) && (size_out < s->page_size)) {
^
In file included from /root/qmp-unstable/dump.c:28:0:
/usr/include/zlib.h:1174:21: note: expected ‘uLongf *’ but argument is of type ‘size_t *’
ZEXTERN int ZEXPORT compress2 OF((Bytef *dest, uLongf *destLen,
^
cc1: all warnings being treated as errors
make[1]: *** [dump.o] Error 1
make: *** [subdir-aarch64-softmmu] Error 2
make: *** Waiting for unfinished jobs....
/root/qmp-unstable/dump.c: In function ‘write_dump_pages’:
/root/qmp-unstable/dump.c:1356:21: error: passing argument 2 of ‘compress2’ from incompatible pointer type [-Werror]
Z_BEST_SPEED) == Z_OK) && (size_out < s->page_size)) {
^
In file included from /root/qmp-unstable/dump.c:28:0:
/usr/include/zlib.h:1174:21: note: expected ‘uLongf *’ but argument is of type ‘size_t *’
ZEXTERN int ZEXPORT compress2 OF((Bytef *dest, uLongf *destLen,
^
cc1: all warnings being treated as errors
make[1]: *** [dump.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [subdir-alpha-softmmu] Error 2
^ permalink raw reply [flat|nested] 71+ messages in thread
* Re: [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format
2014-02-17 17:51 ` Luiz Capitulino
@ 2014-02-18 6:16 ` Qiao Nuohan
0 siblings, 0 replies; 71+ messages in thread
From: Qiao Nuohan @ 2014-02-18 6:16 UTC (permalink / raw)
To: Luiz Capitulino
Cc: stefanha, qemu-devel, anderson, pbonzini, kumagai-atsushi,
lersek, afaerber
On 02/18/2014 01:51 AM, Luiz Capitulino wrote:
> On Tue, 11 Feb 2014 12:00:56 -0500
> Luiz Capitulino<lcapitulino@redhat.com> wrote:
>
>> On Tue, 28 Jan 2014 14:21:53 +0800
>> qiaonuohan<qiaonuohan@cn.fujitsu.com> 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/
>>
>> I did what was requested by Paolo. I applied patches 1 to 12 and will wait
>> for the rework on patch 13.
>
> Unfortunately this series doesn't build on 32-bit hosts, so I had to drop it
> from my queue (see error below). Can you please do the following:
>
> 1. Fix the build
> 2. Add new patch 13/13 to the series
> 3. Add Ekaterina's patch to your series
>
> Thanks. Here's the error message:
>
> ar: creating libfdt/libfdt.a
> /root/qmp-unstable/dump.c: In function ‘write_dump_pages’:
> /root/qmp-unstable/dump.c:1356:21: error: passing argument 2 of ‘compress2’ from incompatible pointer type [-Werror]
> Z_BEST_SPEED) == Z_OK)&& (size_out< s->page_size)) {
> ^
> In file included from /root/qmp-unstable/dump.c:28:0:
> /usr/include/zlib.h:1174:21: note: expected ‘uLongf *’ but argument is of type ‘size_t *’
> ZEXTERN int ZEXPORT compress2 OF((Bytef *dest, uLongf *destLen,
> ^
> cc1: all warnings being treated as errors
> make[1]: *** [dump.o] Error 1
> make: *** [subdir-aarch64-softmmu] Error 2
> make: *** Waiting for unfinished jobs....
> /root/qmp-unstable/dump.c: In function ‘write_dump_pages’:
> /root/qmp-unstable/dump.c:1356:21: error: passing argument 2 of ‘compress2’ from incompatible pointer type [-Werror]
> Z_BEST_SPEED) == Z_OK)&& (size_out< s->page_size)) {
> ^
> In file included from /root/qmp-unstable/dump.c:28:0:
> /usr/include/zlib.h:1174:21: note: expected ‘uLongf *’ but argument is of type ‘size_t *’
> ZEXTERN int ZEXPORT compress2 OF((Bytef *dest, uLongf *destLen,
> ^
> cc1: all warnings being treated as errors
> make[1]: *** [dump.o] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [subdir-alpha-softmmu] Error 2
>
>
Hello Luiz,
It seems both zlib and lzo will output the error message because of the
incompatible type. And I have fixed patches and sent them to the qemu list.
P.S.
lzo/snappy will need '--enable-lzo' or '--enable-snappy' at configure.
--
Regards
Qiao Nuohan
^ permalink raw reply [flat|nested] 71+ messages in thread
end of thread, other threads:[~2014-02-18 6:18 UTC | newest]
Thread overview: 71+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-28 6:21 [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format qiaonuohan
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 01/13] dump: const-qualify the buf of WriteCoreDumpFunction qiaonuohan
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 02/13] dump: add argument to write_elfxx_notes qiaonuohan
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 03/13] dump: add API to write header of flatten format qiaonuohan
2014-02-10 19:35 ` Luiz Capitulino
2014-02-10 21:57 ` Laszlo Ersek
2014-02-10 22:48 ` Luiz Capitulino
2014-02-10 23:20 ` Laszlo Ersek
2014-02-11 9:06 ` Markus Armbruster
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 04/13] dump: add API to write vmcore qiaonuohan
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 05/13] dump: add API to write elf notes to buffer qiaonuohan
2014-01-28 6:21 ` [Qemu-devel] [PATCH v8 06/13] dump: add support for lzo/snappy qiaonuohan
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 07/13] dump: add members to DumpState and init some of them qiaonuohan
2014-01-29 14:11 ` Laszlo Ersek
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 08/13] dump: add API to write dump header qiaonuohan
2014-01-28 11:51 ` Ekaterina Tumanova
2014-01-28 13:37 ` Laszlo Ersek
2014-01-28 14:42 ` Ekaterina Tumanova
2014-01-28 14:55 ` Laszlo Ersek
2014-01-29 1:39 ` Qiao Nuohan
2014-01-29 14:19 ` Laszlo Ersek
2014-01-30 17:14 ` Ekaterina Tumanova
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 09/13] dump: add API to write dump_bitmap qiaonuohan
2014-01-29 14:32 ` Laszlo Ersek
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 10/13] dump: add APIs to operate DataCache qiaonuohan
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 11/13] dump: add API to write dump pages qiaonuohan
2014-01-29 14:39 ` Laszlo Ersek
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 12/13] dump: make kdump-compressed format available for 'dump-guest-memory' qiaonuohan
2014-01-29 14:45 ` Laszlo Ersek
2014-01-28 6:22 ` [Qemu-devel] [PATCH v8 13/13] dump: add 'query-dump-guest-memory-capability' command qiaonuohan
2014-02-10 19:10 ` Luiz Capitulino
2014-02-10 22:02 ` Laszlo Ersek
2014-02-10 23:09 ` Paolo Bonzini
2014-02-10 23:09 ` Paolo Bonzini
2014-02-10 23:30 ` Laszlo Ersek
2014-02-10 23:34 ` Paolo Bonzini
2014-02-11 0:22 ` Laszlo Ersek
2014-02-11 2:37 ` Qiao Nuohan
2014-02-11 2:47 ` Luiz Capitulino
2014-02-11 3:20 ` Qiao Nuohan
2014-02-11 7:19 ` Paolo Bonzini
2014-02-11 13:26 ` Eric Blake
2014-02-12 3:13 ` [Qemu-devel] [PATCH v9 " Qiao Nuohan
2014-02-12 3:29 ` Eric Blake
2014-02-12 6:34 ` [Qemu-devel] [PATCH v10 " Qiao Nuohan
2014-02-12 14:49 ` Luiz Capitulino
2014-02-13 1:48 ` Qiao Nuohan
2014-02-13 2:57 ` Luiz Capitulino
2014-02-13 13:13 ` Eric Blake
2014-02-13 13:45 ` Luiz Capitulino
2014-01-28 6:37 ` [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format Qiao Nuohan
2014-01-29 14:50 ` Laszlo Ersek
2014-01-30 17:01 ` [Qemu-devel] [PATCH] Define the architecture for compressed dump format Ekaterina Tumanova
2014-01-30 17:20 ` Laszlo Ersek
2014-01-31 13:45 ` [Qemu-devel] [PATCH v2] Define guest architecture for the compressed dump header Ekaterina Tumanova
2014-01-31 13:45 ` [Qemu-devel] [PATCH v2] Define the architecture for compressed dump format Ekaterina Tumanova
2014-01-31 13:56 ` Christian Borntraeger
2014-01-31 14:11 ` Laszlo Ersek
2014-01-31 15:04 ` [Qemu-devel] [PATCH v3 0/1] Detect arch for dump compressed header Ekaterina Tumanova
2014-01-31 15:04 ` [Qemu-devel] [PATCH v3 1/1] Define the architecture for compressed dump format Ekaterina Tumanova
2014-02-13 13:50 ` Luiz Capitulino
2014-01-31 17:14 ` [Qemu-devel] [PATCH v3 0/1] Detect arch for dump compressed header Laszlo Ersek
2014-02-10 21:23 ` Luiz Capitulino
2014-02-10 22:06 ` Laszlo Ersek
2014-02-10 3:34 ` Qiao Nuohan
2014-02-10 8:29 ` Laszlo Ersek
2014-02-10 8:57 ` Qiao Nuohan
2014-02-10 13:24 ` Luiz Capitulino
2014-02-11 17:00 ` [Qemu-devel] [PATCH v8 00/13] Make 'dump-guest-memory' dump in kdump-compressed format Luiz Capitulino
2014-02-17 17:51 ` Luiz Capitulino
2014-02-18 6:16 ` Qiao Nuohan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.