[Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[qiaonuohan]

From: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>

Hi, all

The last version is here:
http://lists.gnu.org/archive/html/qemu-devel/2013-05/msg02280.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

For migration, 'dump-guest-memory' is supposed to support compression feature.
Because of the regression, the missing of compression feature, 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 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.

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 used with configure to make lzo or snappy available.

Changelog:
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.


Qiao Nuohan (9):
  dump: Add API to manipulate dump_bitmap
  dump: Add API to manipulate cache_data
  dump: Move struct definition into dump_memroy.h
  dump: Add API to create header of vmcore
  dump: Add API to create data of dump bitmap
  dump: Add API to create page
  dump: Add API to free memory used by creating header, bitmap and page
  dump: Add API to write header, bitmap and page into vmcore
  dump: Make kdump-compressed format available for 'dump-guest-memory'

 Makefile.target              |    1 +
 cache_data.c                 |  121 ++++++
 configure                    |   50 +++
 dump.c                       |  866 ++++++++++++++++++++++++++++++++++++++++--
 dump_bitmap.c                |  171 +++++++++
 hmp-commands.hx              |   12 +-
 hmp.c                        |   23 +-
 include/cache_data.h         |   56 +++
 include/dump_bitmap.h        |   60 +++
 include/sysemu/dump_memory.h |  178 +++++++++
 qapi-schema.json             |   22 +-
 qmp-commands.hx              |    6 +-
 12 files changed, 1526 insertions(+), 40 deletions(-)
 create mode 100644 cache_data.c
 create mode 100644 dump_bitmap.c
 create mode 100644 include/cache_data.h
 create mode 100644 include/dump_bitmap.h
 create mode 100644 include/sysemu/dump_memory.h

[Qemu-devel] [PATCH v4 1/9] dump: Add API to manipulate dump_bitmap

[qiaonuohan]

From: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>

Struct dump_bitmap is associated with a tmp file which is used to store bitmap
in kdump-compressed format temporarily. The following patch will use these
functions to gather data of bitmap and cache them into tmp files.

Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
---
 Makefile.target       |    1 +
 dump_bitmap.c         |  171 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/dump_bitmap.h |   60 +++++++++++++++++
 3 files changed, 232 insertions(+), 0 deletions(-)
 create mode 100644 dump_bitmap.c
 create mode 100644 include/dump_bitmap.h

diff --git a/Makefile.target b/Makefile.target
index ce4391f..8e557f9 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -110,6 +110,7 @@ obj-y += qtest.o
 obj-y += hw/
 obj-$(CONFIG_FDT) += device_tree.o
 obj-$(CONFIG_KVM) += kvm-all.o
+obj-y += dump_bitmap.o
 obj-y += memory.o savevm.o cputlb.o
 obj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) += memory_mapping.o
 obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o
diff --git a/dump_bitmap.c b/dump_bitmap.c
new file mode 100644
index 0000000..f35f39d
--- /dev/null
+++ b/dump_bitmap.c
@@ -0,0 +1,171 @@
+/*
+ * QEMU dump bitmap
+ *
+ * Copyright (C) 2013 FUJITSU LIMITED
+ *
+ * Authors:
+ *     Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu-common.h"
+#include "dump_bitmap.h"
+
+int init_dump_bitmap(struct dump_bitmap *db, const char *filename)
+{
+    int fd;
+    char *tmpname;
+
+    /* init the tmp file */
+    tmpname = getenv("TMPDIR");
+    if (!tmpname) {
+        tmpname = (char *)P_tmpdir;
+    }
+
+    db->file_name = (char *)g_strdup_printf("%s/%s", tmpname, filename);
+
+    fd = mkstemp(db->file_name);
+    if (fd < 0) {
+        return -1;
+    }
+
+    db->fd = fd;
+    unlink(db->file_name);
+
+    /* num_block should be set to -1, for nothing is store in buf now */
+    db->num_block = -1;
+    memset(db->buf, 0, BUFSIZE_BITMAP);
+    /* the tmp file starts to write at the very beginning */
+    db->offset = 0;
+
+    return 0;
+}
+
+int clear_dump_bitmap(struct dump_bitmap *db, size_t len_db)
+{
+    int ret;
+    char buf[BUFSIZE_BITMAP];
+    off_t offset_bit;
+
+    /* use buf to clear the tmp file block by block */
+    memset(buf, 0, sizeof(buf));
+
+    ret = lseek(db->fd, db->offset, SEEK_SET);
+    if (ret < 0) {
+        return -1;
+    }
+
+    offset_bit = 0;
+    while (offset_bit < len_db) {
+        if (write(db->fd, buf, BUFSIZE_BITMAP) != BUFSIZE_BITMAP) {
+            return -1;
+        }
+
+        offset_bit += BUFSIZE_BITMAP;
+    }
+
+    return 0;
+}
+
+int set_dump_bitmap(struct dump_bitmap *db, unsigned long long pfn, int val)
+{
+    int byte, bit;
+    off_t old_offset, new_offset;
+    old_offset = db->offset + BUFSIZE_BITMAP * db->num_block;
+    new_offset = db->offset + BUFSIZE_BITMAP * (pfn / PFN_BUFBITMAP);
+
+    /*
+     * if the block needed to be set is not same as the one cached in buf,
+     * write the block back to the tmp file, then cache new block to the buf
+     */
+    if (0 <= db->num_block && old_offset != new_offset) {
+        if (lseek(db->fd, old_offset, SEEK_SET) < 0) {
+            return -1;
+        }
+
+        if (write(db->fd, db->buf, BUFSIZE_BITMAP) != BUFSIZE_BITMAP) {
+            return -1;
+        }
+    }
+
+    if (old_offset != new_offset) {
+        if (lseek(db->fd, new_offset, SEEK_SET) < 0) {
+            return -1;
+        }
+
+        if (read(db->fd, db->buf, BUFSIZE_BITMAP) != BUFSIZE_BITMAP) {
+            return -1;
+        }
+
+        db->num_block = pfn / PFN_BUFBITMAP;
+    }
+
+    /* get the exact place of the bit in the buf, set it */
+    byte = (pfn % PFN_BUFBITMAP) >> 3;
+    bit  = (pfn % PFN_BUFBITMAP) & 7;
+    if (val) {
+        db->buf[byte] |= 1<<bit;
+    } else {
+        db->buf[byte] &= ~(1<<bit);
+    }
+
+    return 0;
+}
+
+int sync_dump_bitmap(struct dump_bitmap *db)
+{
+    off_t offset;
+    offset = db->offset + BUFSIZE_BITMAP * db->num_block;
+
+    /* db has not been used yet */
+    if (db->num_block < 0) {
+        return 0;
+    }
+
+    if (lseek(db->fd, offset, SEEK_SET) < 0) {
+        return -1;
+    }
+
+    if (write(db->fd, db->buf, BUFSIZE_BITMAP) != BUFSIZE_BITMAP) {
+        return -1;
+    }
+
+    return 0;
+}
+
+/*
+ * check if the bit is set to 1
+ */
+static inline int is_on(char *bitmap, int i)
+{
+    return bitmap[i>>3] & (1 << (i & 7));
+}
+
+int is_bit_set(struct dump_bitmap *db, unsigned long long pfn)
+{
+    off_t offset;
+
+    /* cache the block pfn belongs to, then check the block */
+    if (pfn == 0 || db->num_block != pfn/PFN_BUFBITMAP) {
+        offset = db->offset + BUFSIZE_BITMAP * (pfn/PFN_BUFBITMAP);
+        lseek(db->fd, offset, SEEK_SET);
+        read(db->fd, db->buf, BUFSIZE_BITMAP);
+        db->num_block = pfn/PFN_BUFBITMAP;
+    }
+
+    return is_on(db->buf, pfn%PFN_BUFBITMAP);
+}
+
+void free_dump_bitmap(struct dump_bitmap *db)
+{
+    if (db) {
+        if (db->file_name) {
+            g_free(db->file_name);
+        }
+
+        g_free(db);
+    }
+}
diff --git a/include/dump_bitmap.h b/include/dump_bitmap.h
new file mode 100644
index 0000000..f81106c
--- /dev/null
+++ b/include/dump_bitmap.h
@@ -0,0 +1,60 @@
+/*
+ * QEMU dump bitmap
+ *
+ * Copyright (C) 2013 FUJITSU LIMITED
+ *
+ * Authors:
+ *     Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef DUMP_BITMAP_H
+#define DUMP_BITMAP_H
+
+#define BUFSIZE_BITMAP              (4096)
+#define PFN_BUFBITMAP               (CHAR_BIT * BUFSIZE_BITMAP)
+
+struct dump_bitmap {
+    int fd;                     /* fd of the tmp file */
+    int num_block;              /* number of blocks cached in buf */
+    char *file_name;            /* name of the tmp file */
+    char buf[BUFSIZE_BITMAP];   /* used to cache blocks */
+    off_t offset;               /* offset of the tmp file */
+};
+
+/*
+ * create a tmp file used to store dump bitmap, then init buf of dump_bitmap
+ */
+int init_dump_bitmap(struct dump_bitmap *db, const char *filename);
+
+/*
+ * clear the content of the tmp file with all bits set to 0
+ */
+int clear_dump_bitmap(struct dump_bitmap *db, size_t len_db);
+
+/*
+ * 'pfn' is the number of bit needed to be set, use buf to cache the block which
+ * 'pfn' belongs to, then set 'val' to the bit
+ */
+int set_dump_bitmap(struct dump_bitmap *db, unsigned long long pfn, int val);
+
+/*
+ * when buf is caching a block, sync_dump_bitmap is needed to write the cached
+ * block to the tmp file
+ */
+int sync_dump_bitmap(struct dump_bitmap *db);
+
+/*
+ * check whether 'pfn' is set
+ */
+int is_bit_set(struct dump_bitmap *db, unsigned long long pfn);
+
+/*
+ * free the space used by dump_bitmap
+ */
+void free_dump_bitmap(struct dump_bitmap *db);
+
+#endif
-- 
1.7.1

[Qemu-devel] [PATCH v4 2/9] dump: Add API to manipulate cache_data

[qiaonuohan]

From: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>

Struct cache_data is associated with a tmp file which is used to store page
desc and page data in kdump-compressed format temporarily. The following patch
will use these function to gather data of page and cache them in tmp files.

Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
---
 Makefile.target      |    2 +-
 cache_data.c         |  121 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/cache_data.h |   56 +++++++++++++++++++++++
 3 files changed, 178 insertions(+), 1 deletions(-)
 create mode 100644 cache_data.c
 create mode 100644 include/cache_data.h

diff --git a/Makefile.target b/Makefile.target
index 8e557f9..298ec84 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -110,7 +110,7 @@ obj-y += qtest.o
 obj-y += hw/
 obj-$(CONFIG_FDT) += device_tree.o
 obj-$(CONFIG_KVM) += kvm-all.o
-obj-y += dump_bitmap.o
+obj-y += dump_bitmap.o cache_data.o
 obj-y += memory.o savevm.o cputlb.o
 obj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) += memory_mapping.o
 obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o
diff --git a/cache_data.c b/cache_data.c
new file mode 100644
index 0000000..6e91538
--- /dev/null
+++ b/cache_data.c
@@ -0,0 +1,121 @@
+/*
+ * QEMU cache data
+ *
+ * Copyright (C) 2013 FUJITSU LIMITED
+ *
+ * Authors:
+ *     Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu-common.h"
+#include "cache_data.h"
+
+int init_cache_data(struct cache_data *cd, const char *filename)
+{
+    int fd;
+    char *tmpname;
+
+    /* init the tmp file */
+    tmpname = getenv("TMPDIR");
+    if (!tmpname) {
+        tmpname = (char *)P_tmpdir;
+    }
+
+    cd->file_name = (char *)g_strdup_printf("%s/%s", tmpname, filename);
+
+    fd = mkstemp(cd->file_name);
+    if (fd < 0) {
+        return -1;
+    }
+
+    cd->fd = fd;
+    unlink(cd->file_name);
+
+    /* init buf */
+    cd->buf_size = BUFSIZE_CACHE_DATA;
+    cd->cache_size = 0;
+    cd->buf = g_malloc0(BUFSIZE_CACHE_DATA);
+
+    cd->offset = 0;
+
+    return 0;
+}
+
+int write_cache(struct cache_data *cd, void *buf, size_t size)
+{
+    /*
+     * check if the space is enough to cache data, if not write cached
+     * data to the tmp file
+     */
+    if (cd->cache_size + size > cd->buf_size) {
+        if (lseek(cd->fd, cd->offset, SEEK_SET) < 0) {
+            return -1;
+        }
+
+        if (write(cd->fd, cd->buf, cd->cache_size) != cd->cache_size) {
+            return -1;
+        }
+
+        cd->offset += cd->cache_size;
+        cd->cache_size = 0;
+    }
+
+    memcpy(cd->buf + cd->cache_size, buf, size);
+    cd->cache_size += size;
+
+    return 0;
+}
+
+int sync_cache(struct cache_data *cd)
+{
+    /* no data is cached in cache_data */
+    if (cd->cache_size == 0) {
+        return 0;
+    }
+
+    if (lseek(cd->fd, cd->offset, SEEK_SET) < 0) {
+        return -1;
+    }
+
+    if (write(cd->fd, cd->buf, cd->cache_size) != cd->cache_size) {
+        return -1;
+    }
+
+    cd->offset += cd->cache_size;
+
+    return 0;
+}
+
+int read_cache(struct cache_data *cd)
+{
+    if (lseek(cd->fd, cd->offset, SEEK_SET) < 0) {
+        return -1;
+    }
+
+    if (read(cd->fd, cd->buf, cd->cache_size) != cd->cache_size) {
+        return -1;
+    }
+
+    cd->offset += cd->cache_size;
+
+    return 0;
+}
+
+void free_cache_data(struct cache_data *cd)
+{
+    if (cd) {
+        if (cd->file_name) {
+            g_free(cd->file_name);
+        }
+
+        if (cd->buf) {
+            g_free(cd->buf);
+        }
+
+        g_free(cd);
+    }
+}
diff --git a/include/cache_data.h b/include/cache_data.h
new file mode 100644
index 0000000..18e8680
--- /dev/null
+++ b/include/cache_data.h
@@ -0,0 +1,56 @@
+/*
+ * QEMU cache data
+ *
+ * Copyright (C) 2013 FUJITSU LIMITED
+ *
+ * Authors:
+ *     Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef CACHE_DATA_H
+#define CACHE_DATA_H
+
+#define BUFSIZE_CACHE_DATA          (4096 * 4)
+
+struct cache_data {
+    int fd;             /* fd of the tmp file used to store cache data */
+    char *file_name;    /* name of the tmp file */
+    char *buf;          /* used to cache data */
+    size_t buf_size;    /* size of the buf */
+    size_t cache_size;  /* size of cached data in buf */
+    off_t offset;       /* offset of the tmp file */
+};
+
+/*
+ * create a tmp file used to store cache data, then init the buf
+ */
+int init_cache_data(struct cache_data *cd, const char *filename);
+
+/*
+ * write data to the tmp file, the data may first be cached in the buf of
+ * cache_data
+ */
+int write_cache(struct cache_data *cd, void *buf, size_t size);
+
+/*
+ * when cache_data is caching data in the buf, sync_cache is needed to write the
+ * data back to tmp file
+ */
+int sync_cache(struct cache_data *cd);
+
+/*  read data from the tmp file to the buf of 'cd', the start place is set by
+ *  cd->offset, and the size is set by cd->cache_size. cd->offset is changed
+ *  automaticlly according to the size of data read this time.
+ */
+int read_cache(struct cache_data *cd);
+
+/*
+ * free the space used by cache_data
+ */
+void free_cache_data(struct cache_data *cd);
+
+#endif
-- 
1.7.1

[Qemu-devel] [PATCH v4 3/9] dump: Move struct definition into dump_memroy.h

[qiaonuohan]

From: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>

Move definition of struct DumpState into include/sysemu/dump_memory.h.

Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
---
 dump.c                       |   22 +---------------------
 include/sysemu/dump_memory.h |   40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 21 deletions(-)
 create mode 100644 include/sysemu/dump_memory.h

diff --git a/dump.c b/dump.c
index c0d3da5..9ac66be 100644
--- a/dump.c
+++ b/dump.c
@@ -15,10 +15,9 @@
 #include "elf.h"
 #include "cpu.h"
 #include "exec/cpu-all.h"
-#include "exec/hwaddr.h"
 #include "monitor/monitor.h"
 #include "sysemu/kvm.h"
-#include "sysemu/dump.h"
+#include "sysemu/dump_memory.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/memory_mapping.h"
 #include "qapi/error.h"
@@ -57,25 +56,6 @@ static uint64_t cpu_convert_to_target64(uint64_t val, int endian)
     return val;
 }
 
-typedef struct DumpState {
-    ArchDumpInfo dump_info;
-    MemoryMappingList list;
-    uint16_t phdr_num;
-    uint32_t sh_info;
-    bool have_section;
-    bool resume;
-    size_t note_size;
-    hwaddr memory_offset;
-    int fd;
-
-    RAMBlock *block;
-    ram_addr_t start;
-    bool has_filter;
-    int64_t begin;
-    int64_t length;
-    Error **errp;
-} DumpState;
-
 static int dump_cleanup(DumpState *s)
 {
     int ret = 0;
diff --git a/include/sysemu/dump_memory.h b/include/sysemu/dump_memory.h
new file mode 100644
index 0000000..ce22c05
--- /dev/null
+++ b/include/sysemu/dump_memory.h
@@ -0,0 +1,40 @@
+/*
+ * QEMU dump memory
+ *
+ * Copyright (C) 2013 FUJITSU LIMITED
+ *
+ * Authors:
+ *     Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef DUMP_MEMORY_H
+#define DUMP_MEMORY_H
+
+#include "exec/cpu-all.h"
+#include "sysemu/memory_mapping.h"
+#include "sysemu/dump.h"
+
+typedef struct DumpState {
+    ArchDumpInfo dump_info;
+    MemoryMappingList list;
+    uint16_t phdr_num;
+    uint32_t sh_info;
+    bool have_section;
+    bool resume;
+    size_t note_size;
+    hwaddr memory_offset;
+    int fd;
+
+    RAMBlock *block;
+    ram_addr_t start;
+    bool has_filter;
+    int64_t begin;
+    int64_t length;
+    Error **errp;
+} DumpState;
+
+#endif
-- 
1.7.1

[Qemu-devel] [PATCH v4 4/9] dump: Add API to create header of vmcore

[qiaonuohan]

From: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>

Functions in this patch are used to gather data of header and sub header in
kdump-compressed format. The following patch will use these functions to gather
data of header, then cache them into struct DumpState.

Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
---
 dump.c                       |  107 ++++++++++++++++++++++++++++++++++++++++++
 include/sysemu/dump_memory.h |   93 ++++++++++++++++++++++++++++++++++++
 2 files changed, 200 insertions(+), 0 deletions(-)

diff --git a/dump.c b/dump.c
index 9ac66be..3b9d4ca 100644
--- a/dump.c
+++ b/dump.c
@@ -681,6 +681,113 @@ static ram_addr_t get_start_block(DumpState *s)
     return -1;
 }
 
+static int create_header32(DumpState *s)
+{
+    struct  disk_dump_header32 *dh;
+    struct  kdump_sub_header32 *kh;
+
+    /* create common header, the version of kdump-compressed format is 5th */
+    dh = g_malloc0(sizeof(struct disk_dump_header32));
+
+    strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
+    dh->header_version = 5;
+    dh->block_size = s->page_size;
+    dh->sub_hdr_size = sizeof(struct kdump_sub_header32) + s->note_size;
+    dh->sub_hdr_size = divideup(dh->sub_hdr_size, dh->block_size);
+    dh->max_mapnr = s->max_mapnr;
+    dh->nr_cpus = s->nr_cpus;
+    dh->bitmap_blocks = divideup(s->len_dump_bitmap, s->page_size);
+
+    memcpy(&(dh->utsname.machine), "i686", 4);
+
+    s->dh = dh;
+
+    /* create sub header */
+    kh = g_malloc0(sizeof(struct kdump_sub_header32));
+
+    kh->phys_base = PHYS_BASE;
+    kh->dump_level = DUMP_LEVEL;
+
+    kh->offset_note = DISKDUMP_HEADER_BLOCKS * dh->block_size +
+                        sizeof(struct kdump_sub_header32);
+    kh->note_size = s->note_size;
+
+    s->kh = kh;
+
+    /* get gap between header and sub header */
+    s->offset_sub_header = DISKDUMP_HEADER_BLOCKS * dh->block_size -
+                            sizeof(struct disk_dump_header32);
+
+    /* get gap between header and dump_bitmap */
+    s->offset_dump_bitmap = dh->sub_hdr_size * dh->block_size -
+                            (sizeof(struct kdump_sub_header32) + s->note_size);
+
+    /* get offset of page desc */
+    s->offset_page = (DISKDUMP_HEADER_BLOCKS + dh->sub_hdr_size +
+                        dh->bitmap_blocks) * dh->block_size;
+
+    return 0;
+}
+
+static int create_header64(DumpState *s)
+{
+    struct  disk_dump_header64 *dh;
+    struct  kdump_sub_header64 *kh;
+
+    /* create common header, the version of kdump-compressed format is 5th */
+    dh = g_malloc0(sizeof(struct disk_dump_header64));
+
+    strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
+    dh->header_version = 5;
+    dh->block_size = s->page_size;
+    dh->sub_hdr_size = sizeof(struct kdump_sub_header64) + s->note_size;
+    dh->sub_hdr_size = divideup(dh->sub_hdr_size, dh->block_size);
+    dh->max_mapnr = s->max_mapnr;
+    dh->nr_cpus = s->nr_cpus;
+    dh->bitmap_blocks = divideup(s->len_dump_bitmap, s->page_size);
+
+    memcpy(&(dh->utsname.machine), "x86_64", 6);
+
+    s->dh = dh;
+
+    /* create sub header */
+    kh = g_malloc0(sizeof(struct kdump_sub_header64));
+
+    kh->phys_base = PHYS_BASE;
+    kh->dump_level = DUMP_LEVEL;
+
+    kh->offset_note = DISKDUMP_HEADER_BLOCKS * dh->block_size +
+                        sizeof(struct kdump_sub_header64);
+    kh->note_size = s->note_size;
+
+    s->kh = kh;
+
+    /* get gap between header and sub header */
+    s->offset_sub_header = DISKDUMP_HEADER_BLOCKS * dh->block_size -
+                            sizeof(struct disk_dump_header64);
+
+    /* get gap between header and dump_bitmap */
+    s->offset_dump_bitmap = dh->sub_hdr_size * dh->block_size -
+                            (sizeof(struct kdump_sub_header64) + s->note_size);
+
+    /* get offset of page desc */
+    s->offset_page = (DISKDUMP_HEADER_BLOCKS + dh->sub_hdr_size +
+                        dh->bitmap_blocks) * dh->block_size;
+
+    return 0;
+}
+
+/*
+ * gather data of header and sub header
+ */
+static int create_header(DumpState *s)
+{
+    if (s->dump_info.d_machine == EM_386)
+        return create_header32(s);
+    else
+        return create_header64(s);
+}
+
 static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
                      int64_t begin, int64_t length, Error **errp)
 {
diff --git a/include/sysemu/dump_memory.h b/include/sysemu/dump_memory.h
index ce22c05..56e0f40 100644
--- a/include/sysemu/dump_memory.h
+++ b/include/sysemu/dump_memory.h
@@ -18,6 +18,87 @@
 #include "sysemu/memory_mapping.h"
 #include "sysemu/dump.h"
 
+#define KDUMP_SIGNATURE             "KDUMP   "
+#define SIG_LEN                     (sizeof(KDUMP_SIGNATURE) - 1)
+#define DISKDUMP_HEADER_BLOCKS      (1)
+#define PHYS_BASE                   (0)
+#define DUMP_LEVEL                  (1)
+
+#define divideup(x, y)              (((x) + ((y) - 1)) / (y))
+
+struct new_utsname {
+    char sysname[65];
+    char nodename[65];
+    char release[65];
+    char version[65];
+    char machine[65];
+    char domainname[65];
+};
+
+struct disk_dump_header32 {
+    char signature[SIG_LEN];        /* = "KDUMP   " */
+    int header_version;             /* Dump header version */
+    struct new_utsname utsname;     /* copy of system_utsname */
+    char timestamp[8];              /* Time stamp */
+    unsigned int status;            /* Above flags */
+    int block_size;                 /* Size of a block in byte */
+    int sub_hdr_size;               /* Size of arch dependent header in block */
+    unsigned int bitmap_blocks;     /* Size of Memory bitmap in block */
+    unsigned int max_mapnr;         /* = max_mapnr */
+    unsigned int total_ram_blocks;  /* Number of blocks should be written */
+    unsigned int device_blocks;     /* Number of total blocks in dump device */
+    unsigned int written_blocks;    /* Number of written blocks */
+    unsigned int current_cpu;       /* CPU# which handles dump */
+    int nr_cpus;                    /* Number of CPUs */
+    struct task_struct *tasks[0];
+};
+
+struct disk_dump_header64 {
+    char signature[SIG_LEN];        /* = "KDUMP   " */
+    int header_version;             /* Dump header version */
+    struct new_utsname utsname;     /* copy of system_utsname */
+    char timestamp[20];             /* Time stamp */
+    unsigned int status;            /* Above flags */
+    int block_size;                 /* Size of a block in byte */
+    int sub_hdr_size;               /* Size of arch dependent header in block */
+    unsigned int bitmap_blocks;     /* Size of Memory bitmap in block */
+    unsigned int max_mapnr;         /* = max_mapnr */
+    unsigned int total_ram_blocks;  /* Number of blocks should be written */
+    unsigned int device_blocks;     /* Number of total blocks in dump device */
+    unsigned int written_blocks;    /* Number of written blocks */
+    unsigned int current_cpu;       /* CPU# which handles dump */
+    int nr_cpus;                    /* Number of CPUs */
+    struct task_struct *tasks[0];
+};
+
+struct kdump_sub_header32 {
+    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 */
+    uint32_t end_pfn;               /* header_version 2 and later */
+    uint32_t offset_vmcoreinfo;     /* header_version 3 and later */
+    uint32_t size_vmcoreinfo;       /* header_version 3 and later */
+    uint32_t offset_note;           /* header_version 4 and later */
+    uint32_t note_size;             /* header_version 4 and later */
+    uint32_t offset_eraseinfo;      /* header_version 5 and later */
+    uint32_t size_eraseinfo;        /* header_version 5 and later */
+};
+
+struct kdump_sub_header64 {
+    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 */
+    uint64_t end_pfn;               /* header_version 2 and later */
+    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 */
+};
+
 typedef struct DumpState {
     ArchDumpInfo dump_info;
     MemoryMappingList list;
@@ -35,6 +116,18 @@ typedef struct DumpState {
     int64_t begin;
     int64_t length;
     Error **errp;
+
+    int page_size;
+    unsigned long long max_mapnr;
+    int nr_cpus;
+    void *dh;
+    void *kh;
+    off_t offset_sub_header;
+
+    off_t offset_dump_bitmap;
+    unsigned long len_dump_bitmap;
+
+    off_t offset_page;
 } DumpState;
 
 #endif
-- 
1.7.1

[Qemu-devel] [PATCH v4 5/9] dump: Add API to create data of dump bitmap

[qiaonuohan]

From: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>

Functions in this patch are used to gather data of 1st and 2nd dump bitmap in
kdump-compressed format. The following patch will use these functions to gather
data of dump bitmap, then cache them into tmp files.

Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
---
 dump.c                       |   98 ++++++++++++++++++++++++++++++++++++++++++
 include/sysemu/dump_memory.h |   11 +++++
 2 files changed, 109 insertions(+), 0 deletions(-)

diff --git a/dump.c b/dump.c
index 3b9d4ca..dc214b7 100644
--- a/dump.c
+++ b/dump.c
@@ -788,6 +788,104 @@ static int create_header(DumpState *s)
         return create_header64(s);
 }
 
+/*
+ * create two tmpfile and save 1st and 2nd bitmap separately
+ */
+static int prepare_dump_bitmap(DumpState *s)
+{
+    int ret;
+    struct dump_bitmap *db1;
+    struct dump_bitmap *db2;
+
+    db1 = g_malloc0(sizeof(struct dump_bitmap));
+
+    db2 = g_malloc0(sizeof(struct dump_bitmap));
+
+    ret = init_dump_bitmap(db1, FILENAME_BITMAP1);
+    if (ret < 0) {
+        dump_error(s, "dump: failed to init db1.");
+        return -1;
+    }
+    s->dump_bitmap1 = db1;
+
+    ret = init_dump_bitmap(db2, FILENAME_BITMAP2);
+    if (ret < 0) {
+        dump_error(s, "dump: failed to init db1.");
+        return -1;
+    }
+    s->dump_bitmap2 = db2;
+
+    return 0;
+}
+
+static int create_dump_bitmap(DumpState *s)
+{
+    int ret;
+    unsigned long long num_dumpable;
+    MemoryMapping *memory_mapping;
+    unsigned long long pfn_start, pfn_end, pfn;
+
+    ret = prepare_dump_bitmap(s);
+    if (ret < 0) {
+        dump_error(s, "dump: failed to prepare dump_bitmap.\n");
+        return -1;
+    }
+
+    ret = clear_dump_bitmap(s->dump_bitmap1, s->len_dump_bitmap / 2);
+    if (ret < 0) {
+        dump_error(s, "dump: failed to clear dump_bitmap1.\n");
+        return -1;
+    }
+
+    ret = clear_dump_bitmap(s->dump_bitmap2, s->len_dump_bitmap / 2);
+    if (ret < 0) {
+        dump_error(s, "dump: failed to clear dump_bitmap2.\n");
+        return -1;
+    }
+
+    /* write dump bitmap to tmp files */
+    num_dumpable = 0;
+
+    QTAILQ_FOREACH(memory_mapping, &s->list.head, next) {
+        pfn_start = paddr_to_pfn(memory_mapping->phys_addr, s->page_shift);
+        pfn_end = paddr_to_pfn(memory_mapping->phys_addr +
+                    memory_mapping->length, s->page_shift);
+
+        for (pfn = pfn_start; pfn < pfn_end; pfn++) {
+            ret = set_dump_bitmap(s->dump_bitmap1, pfn, 1);
+            if (ret < 0) {
+                dump_error(s, "dump: failed to set dump_bitmap1.\n");
+                return -1;
+            }
+            /* set dump_bitmap2, same as dump_bitmap1 */
+            ret = set_dump_bitmap(s->dump_bitmap2, pfn, 1);
+            if (ret < 0) {
+                dump_error(s, "dump: failed to set dump_bitmap2.\n");
+                return -1;
+            }
+            num_dumpable++;
+        }
+    }
+
+    /* write cached data to tmp files */
+    ret = sync_dump_bitmap(s->dump_bitmap1);
+    if (ret < 0) {
+        dump_error(s, "dump: failed to sync dump_bitmap1.\n");
+        return -1;
+    }
+
+    ret = sync_dump_bitmap(s->dump_bitmap2);
+    if (ret < 0) {
+        dump_error(s, "dump: failed to sync dump_bitmap2.\n");
+        return -1;
+    }
+
+    /* get the number of dumpable page */
+    s->num_dumpable = num_dumpable;
+
+    return 0;
+}
+
 static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
                      int64_t begin, int64_t length, Error **errp)
 {
diff --git a/include/sysemu/dump_memory.h b/include/sysemu/dump_memory.h
index 56e0f40..2b60af6 100644
--- a/include/sysemu/dump_memory.h
+++ b/include/sysemu/dump_memory.h
@@ -18,13 +18,20 @@
 #include "sysemu/memory_mapping.h"
 #include "sysemu/dump.h"
 
+#include "dump_bitmap.h"
+
 #define KDUMP_SIGNATURE             "KDUMP   "
 #define SIG_LEN                     (sizeof(KDUMP_SIGNATURE) - 1)
 #define DISKDUMP_HEADER_BLOCKS      (1)
 #define PHYS_BASE                   (0)
 #define DUMP_LEVEL                  (1)
+#define ARCH_PFN_OFFSET             (0)
+#define FILENAME_BITMAP1            "kdump_bitmap1_XXXXXX"
+#define FILENAME_BITMAP2            "kdump_bitmap2_XXXXXX"
 
 #define divideup(x, y)              (((x) + ((y) - 1)) / (y))
+#define paddr_to_pfn(X, page_shift) \
+    (((unsigned long long)(X) >> (page_shift)) - ARCH_PFN_OFFSET)
 
 struct new_utsname {
     char sysname[65];
@@ -118,14 +125,18 @@ typedef struct DumpState {
     Error **errp;
 
     int page_size;
+    int page_shift;
     unsigned long long max_mapnr;
     int nr_cpus;
     void *dh;
     void *kh;
+    unsigned long long num_dumpable;
     off_t offset_sub_header;
 
     off_t offset_dump_bitmap;
     unsigned long len_dump_bitmap;
+    struct dump_bitmap *dump_bitmap1;
+    struct dump_bitmap *dump_bitmap2;
 
     off_t offset_page;
 } DumpState;
-- 
1.7.1

[Qemu-devel] [PATCH v4 6/9] dump: Add API to create page

[qiaonuohan]

From: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>

Functions in this patch are used to gather data of page desc and page data in
kdump-compressed format. The following patch will use these functions to gather
data of page, then cache them into tmp files

Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
---
 dump.c                       |  259 ++++++++++++++++++++++++++++++++++++++++++
 include/sysemu/dump_memory.h |   32 +++++
 2 files changed, 291 insertions(+), 0 deletions(-)

diff --git a/dump.c b/dump.c
index dc214b7..405eeb1 100644
--- a/dump.c
+++ b/dump.c
@@ -886,6 +886,265 @@ static int create_dump_bitmap(DumpState *s)
     return 0;
 }
 
+/*
+ * create two tmpfile and save page_desc and page_data
+ */
+static int prepare_pages(DumpState *s)
+{
+    int ret;
+    struct cache_data *page_desc;
+    struct cache_data *page_data;
+
+    page_desc = g_malloc0(sizeof(struct cache_data));
+
+    page_data = g_malloc0(sizeof(struct cache_data));
+
+    ret = init_cache_data(page_desc, FILENAME_PAGE_DESC);
+    if (ret < 0) {
+        dump_error(s, "dump: failed to init page_desc.\n");
+        return -1;
+    }
+    s->page_desc = page_desc;
+
+    ret = init_cache_data(page_data, FILENAME_PAGE_DATA);
+    if (ret < 0) {
+        dump_error(s, "dump: failed to init page_desc.\n");
+        return -1;
+    }
+    s->page_data = page_data;
+
+    return 0;
+}
+
+/*
+ * memory should be read page by page, or it may exceed the boundary and
+ * fail to read
+ */
+static int readmem(void *bufptr, ram_addr_t addr, size_t size, DumpState *s)
+{
+    RAMBlock *block;
+
+    block = s->block;
+
+    while (block) {
+        if ((addr >= block->offset) &&
+            (addr + size <= block->offset + block->length)) {
+            memcpy(bufptr, block->host + (addr - block->offset), size);
+            return 0;
+        } else {
+            block = QTAILQ_NEXT(block, next);
+        }
+    }
+
+    return -1;
+}
+
+/*
+ * check if the page is all 0
+ */
+static inline int is_zero_page(unsigned char *buf, long page_size)
+{
+    size_t i;
+
+    for (i = 0; i < page_size; i++) {
+        if (buf[i]) {
+            return 0;
+        }
+    }
+
+    return 1;
+}
+
+static int create_pages(DumpState *s)
+{
+    int ret;
+    unsigned long long pfn;
+    unsigned char buf[s->page_size];
+    unsigned char *buf_out = NULL;
+    unsigned long len_buf_out;
+    unsigned long size_out;
+    int zero_page;
+    struct page_desc pd, pd_zero;
+    off_t offset_desc, offset_data;
+    unsigned long len_buf_out_zlib, len_buf_out_lzo, len_buf_out_snappy;
+
+    ret = 0;
+
+    ret = prepare_pages(s);
+    if (ret < 0) {
+        dump_error(s, "dump: failed to prepare pages.\n");
+        goto 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(s->page_size);
+
+    /* buf size for lzo */
+#ifdef CONFIG_LZO
+    if (s->flag_compress & DUMP_DH_COMPRESSED_LZO) {
+        if (lzo_init() != LZO_E_OK) {
+            ret = -1;
+            dump_error(s, "dump: failed to use lzo compression");
+            goto out;
+        }
+    }
+
+    lzo_bytep wrkmem;
+
+    wrkmem = g_malloc(LZO1X_1_MEM_COMPRESS);
+
+    len_buf_out_lzo = s->page_size + s->page_size / 16 + 64 + 3;
+#endif
+
+    /* buf size for snappy */
+#ifdef CONFIG_SNAPPY
+    len_buf_out_snappy = snappy_max_compressed_length(s->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));
+
+    buf_out = g_malloc(len_buf_out);
+
+    /* get offset of page_desc and page_data in dump file */
+    offset_desc = s->offset_page;
+    offset_data = offset_desc + sizeof(page_desc_t) * s->num_dumpable;
+
+    /*
+     * init zero page's page_desc and page_data, and all zero pages
+     * will use the same page_data
+     */
+    pd_zero.size = s->page_size;
+    pd_zero.flags = 0;
+    pd_zero.offset = offset_data;
+    pd_zero.page_flags = 0;
+    memset(buf, 0, pd_zero.size);
+    write_cache(s->page_data, buf, pd_zero.size);
+    offset_data += pd_zero.size;
+
+    for (pfn = 0; pfn < s->max_mapnr; pfn++) {
+        /* check whether the page is dumpable */
+        if (!is_bit_set(s->dump_bitmap2, pfn)) {
+            continue;
+        }
+
+        memset(buf, 0, s->page_size);
+        ret = readmem(buf, pfn_to_paddr(pfn, s->page_shift), s->page_size, s);
+        if (ret < 0) {
+            dump_error(s, "dump: failed to read memory ");
+            goto out;
+        }
+
+        /* check zero page */
+        zero_page = is_zero_page(buf, s->page_size);
+        if (zero_page) {
+            write_cache(s->page_desc, &pd_zero, sizeof(page_desc_t));
+        } 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
+             */
+            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 = DUMP_DH_COMPRESSED_ZLIB;
+                pd.size  = size_out;
+
+                ret = write_cache(s->page_data, buf_out, pd.size);
+                if (ret < 0) {
+                    dump_error(s, "dump: failed to write page data(zlib).\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 = DUMP_DH_COMPRESSED_LZO;
+                pd.size  = size_out;
+
+                ret = write_cache(s->page_data, buf_out, pd.size);
+                if (ret < 0) {
+                    dump_error(s, "dump: failed to write page data(lzo).\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_t *)&size_out) == SNAPPY_OK) &&
+                    (size_out < s->page_size)) {
+                pd.flags = DUMP_DH_COMPRESSED_SNAPPY;
+                pd.size  = size_out;
+
+                ret = write_cache(s->page_data, buf_out, pd.size);
+                if (ret < 0) {
+                    dump_error(s, "dump: failed to write page data(snappy).\n");
+                    goto out;
+                }
+#endif
+            } else {
+                pd.flags = 0;
+                pd.size = s->page_size;
+
+                ret = write_cache(s->page_data, buf, pd.size);
+                if (ret < 0) {
+                    dump_error(s, "dump: failed to write page data.\n");
+                    goto out;
+                }
+            }
+
+            /* get and write page desc here */
+            pd.page_flags = 0;
+            pd.offset = offset_data;
+            offset_data += pd.size;
+
+            ret = write_cache(s->page_desc, &pd, sizeof(page_desc_t));
+            if (ret < 0) {
+                dump_error(s, "dump: failed to write page desc.\n");
+                goto out;
+            }
+        }
+    }
+
+    ret = sync_cache(s->page_desc);
+    if (ret < 0) {
+        dump_error(s, "dump: failed to sync cache for page_desc.\n");
+        goto out;
+    }
+    ret = sync_cache(s->page_data);
+    if (ret < 0) {
+        dump_error(s, "dump: failed to sync cache for page_data.\n");
+        goto out;
+    }
+
+   /* get size of page_desc and page_data, then reset their offset */
+    s->page_desc_size = s->page_desc->offset;
+    s->page_data_size = s->page_data->offset;
+    s->page_desc->offset = 0;
+    s->page_data->offset = 0;
+
+out:
+#ifdef CONFIG_LZO
+    if (wrkmem) {
+        g_free(wrkmem);
+    }
+#endif
+
+    if (buf_out) {
+        g_free(buf_out);
+    }
+
+    return ret;
+}
+
 static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
                      int64_t begin, int64_t length, Error **errp)
 {
diff --git a/include/sysemu/dump_memory.h b/include/sysemu/dump_memory.h
index 2b60af6..d843e61 100644
--- a/include/sysemu/dump_memory.h
+++ b/include/sysemu/dump_memory.h
@@ -19,6 +19,21 @@
 #include "sysemu/dump.h"
 
 #include "dump_bitmap.h"
+#include "cache_data.h"
+#include <zlib.h>
+#ifdef CONFIG_LZO
+#include <lzo/lzo1x.h>
+#endif
+#ifdef CONFIG_SNAPPY
+#include <snappy-c.h>
+#endif
+
+/*
+ * flag used in page desc of kdump-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)
@@ -28,10 +43,14 @@
 #define ARCH_PFN_OFFSET             (0)
 #define FILENAME_BITMAP1            "kdump_bitmap1_XXXXXX"
 #define FILENAME_BITMAP2            "kdump_bitmap2_XXXXXX"
+#define FILENAME_PAGE_DESC          "kdump_page_desc_XXXXXX"
+#define FILENAME_PAGE_DATA          "kdump_page_data_XXXXXX"
 
 #define divideup(x, y)              (((x) + ((y) - 1)) / (y))
 #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))
 
 struct new_utsname {
     char sysname[65];
@@ -106,6 +125,14 @@ struct kdump_sub_header64 {
     uint64_t size_eraseinfo;        /* header_version 5 and later */
 };
 
+/* descriptor of each page for vmcore */
+typedef struct page_desc {
+    off_t offset;                   /* the offset of the page data*/
+    unsigned int size;              /* the size of this dump page */
+    unsigned int flags;             /* flags */
+    unsigned long long page_flags;  /* page flags */
+} page_desc_t;
+
 typedef struct DumpState {
     ArchDumpInfo dump_info;
     MemoryMappingList list;
@@ -132,6 +159,7 @@ typedef struct DumpState {
     void *kh;
     unsigned long long num_dumpable;
     off_t offset_sub_header;
+    int flag_compress;
 
     off_t offset_dump_bitmap;
     unsigned long len_dump_bitmap;
@@ -139,6 +167,10 @@ typedef struct DumpState {
     struct dump_bitmap *dump_bitmap2;
 
     off_t offset_page;
+    unsigned long long page_desc_size;
+    unsigned long long page_data_size;
+    struct cache_data *page_desc;
+    struct cache_data *page_data;
 } DumpState;
 
 #endif
-- 
1.7.1

[Qemu-devel] [PATCH v4 7/9] dump: Add API to free memory used by creating header, bitmap and page

[qiaonuohan]

From: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>

When calling create_header, create_dump_bitmap and create_pages, some memory
spaces are allocated. The following patch will use this function to free these
memory.

Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
---
 dump.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/dump.c b/dump.c
index 405eeb1..91a2384 100644
--- a/dump.c
+++ b/dump.c
@@ -1261,6 +1261,25 @@ cleanup:
     return -1;
 }
 
+static void clean_state(DumpState *s)
+{
+    if (s->dh) {
+        g_free(s->dh);
+    }
+
+    if (s->kh) {
+        g_free(s->kh);
+    }
+
+    free_dump_bitmap(s->dump_bitmap1);
+
+    free_dump_bitmap(s->dump_bitmap2);
+
+    free_cache_data(s->page_desc);
+
+    free_cache_data(s->page_data);
+}
+
 void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
                            int64_t begin, bool has_length, int64_t length,
                            Error **errp)
-- 
1.7.1

[Qemu-devel] [PATCH v4 8/9] dump: Add API to write header, bitmap and page into vmcore

[qiaonuohan]

From: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>

The following patch will use these functions to write cached data into vmcore.
Header is cached in DumpState, and bitmap and page are cached in tmp files.

Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
---
 dump.c                       |  259 ++++++++++++++++++++++++++++++++++++++++++
 include/sysemu/dump_memory.h |    1 +
 2 files changed, 260 insertions(+), 0 deletions(-)

diff --git a/dump.c b/dump.c
index 91a2384..17686ad 100644
--- a/dump.c
+++ b/dump.c
@@ -76,6 +76,36 @@ static void dump_error(DumpState *s, const char *reason)
     dump_cleanup(s);
 }
 
+/*
+ * write 'size' of 'c' to dump file
+ */
+static int fill_space(size_t size, int c, void *opaque)
+{
+    DumpState *s = opaque;
+    char tmpbuf[TMP_BUF_SIZE];
+    size_t fill_size;
+    size_t written_size;
+
+    memset(&tmpbuf, c, TMP_BUF_SIZE);
+
+    while (size) {
+        if (size > TMP_BUF_SIZE) {
+            fill_size = TMP_BUF_SIZE;
+        } else {
+            fill_size = size;
+        }
+
+        size -= fill_size;
+
+        written_size = qemu_write_full(s->fd, tmpbuf, fill_size);
+        if (written_size != fill_size) {
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
 static int fd_write_vmcore(void *buf, size_t size, void *opaque)
 {
     DumpState *s = opaque;
@@ -681,6 +711,235 @@ static ram_addr_t get_start_block(DumpState *s)
     return -1;
 }
 
+static int write_dump_header(DumpState *s)
+{
+    int ret;
+    void *dh;
+    void *kh;
+
+    /* write common header */
+    dh = s->dh;
+
+    if (s->dump_info.d_machine == EM_386) {
+        ret = fd_write_vmcore(dh, sizeof(struct disk_dump_header32), s);
+        if (ret < 0) {
+            dump_error(s, "dump: failed to write disk dump header.\n");
+            return -1;
+        }
+    } else {
+        ret = fd_write_vmcore(dh, sizeof(struct disk_dump_header64), s);
+        if (ret < 0) {
+            dump_error(s, "dump: failed to write disk dump header.\n");
+            return -1;
+        }
+    }
+
+    /* fill gap between command header and sub header */
+    ret = fill_space(s->offset_sub_header, 0, s);
+    if (ret < 0) {
+        dump_error(s, "dump: failed to fill the space between header and\
+                    sub header.\n");
+        return -1;
+    }
+
+    /* write sub header */
+    kh = s->kh;
+
+    if (s->dump_info.d_machine == EM_386) {
+        ret = fd_write_vmcore(kh, sizeof(struct kdump_sub_header32), s);
+        if (ret < 0) {
+            dump_error(s, "dump: failed to write kdump sub header.\n");
+            return -1;
+        }
+    } else {
+        ret = fd_write_vmcore(kh, sizeof(struct kdump_sub_header64), s);
+        if (ret < 0) {
+            dump_error(s, "dump: failed to write kdump sub header.\n");
+            return -1;
+        }
+    }
+
+    /* write note */
+    if (s->dump_info.d_class == ELFCLASS64) {
+        if (write_elf64_notes(s) < 0) {
+            return -1;
+        }
+    } else {
+        if (write_elf32_notes(s) < 0) {
+            return -1;
+        }
+   }
+
+    return 0;
+}
+
+static int write_dump_bitmap(DumpState *s)
+{
+    struct cache_data bm;
+    long buf_size;
+    int ret;
+    int no_bitmap;
+
+    /* fill gap between header and dump_bitmap */
+    ret = fill_space(s->offset_dump_bitmap, 0, s);
+    if (ret < 0) {
+        dump_error(s, "dump: failed to fill the space between header and\
+                    dump_bitmap.\n");
+        goto out;
+    }
+
+    bm.buf = g_malloc0(TMP_BUF_SIZE);
+
+    /* write dump_bitmap1 */
+    bm.fd = s->dump_bitmap1->fd;
+    no_bitmap = 1;
+
+again:
+    buf_size = s->len_dump_bitmap / 2;
+    bm.offset = 0;
+
+    while (buf_size > 0) {
+        if (buf_size >= TMP_BUF_SIZE) {
+            bm.cache_size = TMP_BUF_SIZE;
+        } else {
+            bm.cache_size = buf_size;
+        }
+
+        ret = read_cache(&bm);
+        if (ret < 0) {
+            goto out;
+        }
+
+        ret = fd_write_vmcore(bm.buf, bm.cache_size, s);
+        if (ret < 0) {
+            goto out;
+        }
+
+        buf_size -= bm.cache_size;
+    }
+
+    /* switch to dump_bitmap2 */
+    if (no_bitmap == 1) {
+        no_bitmap = 2;
+        bm.fd = s->dump_bitmap2->fd;
+        goto again;
+    }
+
+    return 0;
+
+out:
+    if (bm.buf) {
+        g_free(bm.buf);
+    }
+
+    return -1;
+}
+
+static int write_dump_pages(DumpState *s)
+{
+    struct cache_data page;
+    unsigned long long total_size;
+    int is_page_desc;
+    int ret;
+
+    page.buf = g_malloc0(TMP_BUF_SIZE);
+
+    /* write page_desc */
+    is_page_desc = 1;
+    total_size = s->page_desc_size;
+    page.fd = s->page_desc->fd;
+    page.offset = s->page_desc->offset;
+
+again:
+    while (total_size > 0) {
+        if (total_size > TMP_BUF_SIZE) {
+            page.cache_size = TMP_BUF_SIZE;
+        } else {
+            page.cache_size = total_size;
+        }
+
+        ret = read_cache(&page);
+        if (ret < 0) {
+            goto out;
+        }
+
+        ret = fd_write_vmcore(page.buf, page.cache_size, s);
+        if (ret < 0) {
+            goto out;
+        }
+
+        total_size -= page.cache_size;
+    }
+
+    /* switch to page_data */
+    if (is_page_desc) {
+        is_page_desc = 0;
+        total_size = s->page_data_size;
+        page.fd = s->page_data->fd;
+        page.offset = s->page_data->offset;
+        goto again;
+    }
+
+    return 0;
+
+out:
+    if (page.buf) {
+        g_free(page.buf);
+    }
+
+    return -1;
+}
+
+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)  |
+     *  |                    :                     |
+     *  |  page desc for pfn Z (struct page_desc)  |
+     *  |------------------------------------------| (not aligned by block)
+     *  |         page data (pfn 0)                |
+     *  |         page data (pfn 1)                |
+     *  |                        :                 |
+     *  |         page data (pfn Z)                |
+     *  +------------------------------------------+ offset_eraseinfo
+     *  |                    :                     |
+     *  +------------------------------------------+
+     */
+
+    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;
+    }
+
+    dump_completed(s);
+
+    return 0;
+}
+
 static int create_header32(DumpState *s)
 {
     struct  disk_dump_header32 *dh;
diff --git a/include/sysemu/dump_memory.h b/include/sysemu/dump_memory.h
index d843e61..6848f91 100644
--- a/include/sysemu/dump_memory.h
+++ b/include/sysemu/dump_memory.h
@@ -40,6 +40,7 @@
 #define DISKDUMP_HEADER_BLOCKS      (1)
 #define PHYS_BASE                   (0)
 #define DUMP_LEVEL                  (1)
+#define TMP_BUF_SIZE                (1024)
 #define ARCH_PFN_OFFSET             (0)
 #define FILENAME_BITMAP1            "kdump_bitmap1_XXXXXX"
 #define FILENAME_BITMAP2            "kdump_bitmap2_XXXXXX"
-- 
1.7.1

[Qemu-devel] [PATCH v4 9/9] dump: Make kdump-compressed format available for 'dump-guest-memory'

[qiaonuohan]

From: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>

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. 'zlib': kdump-compressed format, with zlib-compressed
3. 'lzo': kdump-compressed format, with lzo-compressed
4. 'snappy': kdump-compressed format, with snappy-compressed
And without 'format' being set, it is same as 'elf'.

Note:
  1. 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.
  2. The kdump-compressed format is the 5th edition.

Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Signed-off-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
---
 configure                    |   50 ++++++++++++++++++++
 dump.c                       |  102 +++++++++++++++++++++++++++++++++++++-----
 hmp-commands.hx              |   12 +++--
 hmp.c                        |   23 +++++++++-
 include/sysemu/dump_memory.h |    1 +
 qapi-schema.json             |   22 +++++++++-
 qmp-commands.hx              |    6 ++-
 7 files changed, 197 insertions(+), 19 deletions(-)

diff --git a/configure b/configure
index eb74510..5f459a5 100755
--- a/configure
+++ b/configure
@@ -230,6 +230,8 @@ libusb=""
 usb_redir=""
 glx=""
 zlib="yes"
+lzo="no"
+snappy="no"
 guest_agent="yes"
 want_tools="yes"
 libiscsi=""
@@ -902,6 +904,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"
@@ -1499,6 +1505,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
@@ -3901,6 +3943,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
 fi
diff --git a/dump.c b/dump.c
index 17686ad..274249f 100644
--- a/dump.c
+++ b/dump.c
@@ -1036,6 +1036,16 @@ static int create_header64(DumpState *s)
     return 0;
 }
 
+static void get_max_mapnr(DumpState *s)
+{
+    MemoryMapping *memory_mapping;
+
+    QTAILQ_FOREACH(memory_mapping, &s->list.head, next) {
+        s->max_mapnr = paddr_to_pfn(memory_mapping->phys_addr +
+                        memory_mapping->length, s->page_shift);
+    }
+}
+
 /*
  * gather data of header and sub header
  */
@@ -1404,12 +1414,14 @@ out:
     return ret;
 }
 
-static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
-                     int64_t begin, int64_t length, Error **errp)
+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)
 {
     CPUArchState *env;
     int nr_cpus;
     int ret;
+    unsigned long tmp;
 
     if (runstate_is_running()) {
         vm_stop(RUN_STATE_SAVE_VM);
@@ -1464,6 +1476,56 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
         qemu_get_guest_simple_memory_mapping(&s->list);
     }
 
+    /* init for kdump-compressed format */
+    if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
+        switch (format) {
+        case DUMP_GUEST_MEMORY_FORMAT_ZLIB:
+            s->flag_compress = DUMP_DH_COMPRESSED_ZLIB;
+            break;
+        case DUMP_GUEST_MEMORY_FORMAT_LZO:
+            s->flag_compress = DUMP_DH_COMPRESSED_LZO;
+            break;
+        case DUMP_GUEST_MEMORY_FORMAT_SNAPPY:
+            s->flag_compress = DUMP_DH_COMPRESSED_SNAPPY;
+            break;
+        default:
+            s->flag_compress = 0;
+        }
+
+        s->nr_cpus = nr_cpus;
+        s->page_size = PAGE_SIZE;
+        s->page_shift = ffs(s->page_size) - 1;
+
+        get_max_mapnr(s);
+
+        tmp = divideup(divideup(s->max_mapnr, CHAR_BIT), s->page_size);
+        s->len_dump_bitmap = tmp * s->page_size * 2;
+
+        /*
+         * gather data of header, dump_bitmap, page_desc and page_data, then
+         * cache them in tmp files
+         */
+        ret = create_header(s);
+        if (ret < 0) {
+            error_set(errp, QERR_UNSUPPORTED);
+            goto cleanup;
+        }
+
+        ret = create_dump_bitmap(s);
+        if (ret < 0) {
+            error_set(errp, QERR_UNSUPPORTED);
+            goto cleanup;
+        }
+
+        ret = create_pages(s);
+        if (ret < 0) {
+            error_set(errp, QERR_UNSUPPORTED);
+            goto cleanup;
+        }
+
+        return 0;
+    }
+
     if (s->has_filter) {
         memory_mapping_filter(&s->list, s->begin, s->length);
     }
@@ -1539,15 +1601,23 @@ static void clean_state(DumpState *s)
     free_cache_data(s->page_data);
 }
 
-void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
-                           int64_t begin, bool has_length, int64_t length,
-                           Error **errp)
+void qmp_dump_guest_memory(bool paging, const char *protocol, bool has_begin,
+                            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 is invalid with paging or filter */
+    if ((has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) &&
+        (paging || has_begin || has_length)) {
+        error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
+        return;
+    }
+
     if (has_begin && !has_length) {
         error_set(errp, QERR_MISSING_PARAMETER, "length");
         return;
@@ -1558,7 +1628,7 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
     }
 
 #if !defined(WIN32)
-    if (strstart(file, "fd:", &p)) {
+    if (strstart(protocol, "fd:", &p)) {
         fd = monitor_get_fd(cur_mon, p, errp);
         if (fd == -1) {
             return;
@@ -1566,7 +1636,7 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
     }
 #endif
 
-    if  (strstart(file, "file:", &p)) {
+    if  (strstart(protocol, "file:", &p)) {
         fd = qemu_open(p, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR);
         if (fd < 0) {
             error_set(errp, QERR_OPEN_FILE_FAILED, p);
@@ -1579,17 +1649,27 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
         return;
     }
 
-    s = g_malloc(sizeof(DumpState));
+    /* initialize DumpState to zero */
+    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);
+        }
     }
 
+    clean_state(s);
+
     g_free(s);
 }
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9cea415..b12e50b 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -992,17 +992,19 @@ ETEXI
 #if defined(CONFIG_HAVE_CORE_DUMP)
     {
         .name       = "dump-guest-memory",
-        .args_type  = "paging:-p,filename:F,begin:i?,length:i?",
-        .params     = "[-p] filename [begin] [length]",
+        .args_type  = "paging:-p,filename:F,begin:i?,length:i?,format:s?",
+        .params     = "[-p] filename [begin] [length] [format]",
         .help       = "dump guest memory to file"
                       "\n\t\t\t begin(optional): the starting physical address"
-                      "\n\t\t\t length(optional): the memory size, in bytes",
+                      "\n\t\t\t length(optional): the memory size, in bytes"
+                      "\n\t\t\t format(optional): the format of guest memory dump,"
+                      "\n\t\t\t it can be elf|zlib|lzo|snappy",
         .mhandler.cmd = hmp_dump_guest_memory,
     },
 
 
 STEXI
-@item dump-guest-memory [-p] @var{protocol} @var{begin} @var{length}
+@item dump-guest-memory [-p] @var{protocol} @var{begin} @var{length} @var{format}
 @findex dump-guest-memory
 Dump guest memory to @var{protocol}. The file can be processed with crash or
 gdb.
@@ -1012,6 +1014,8 @@ gdb.
             specified with length together.
     length: the memory size, in bytes. It's optional, and should be specified
             with begin together.
+    format: the format of guest memory dump. It's optional, and can be
+            elf|zlib|lzo|snappy.
 ETEXI
 #endif
 
diff --git a/hmp.c b/hmp.c
index 4fb76ec..e863267 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1185,9 +1185,12 @@ 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");
+    bool has_format = qdict_haskey(qdict, "format");
     int64_t begin = 0;
     int64_t length = 0;
+    const char *format = NULL;
     char *prot;
+    enum DumpGuestMemoryFormat dump_format;
 
     if (has_begin) {
         begin = qdict_get_int(qdict, "begin");
@@ -1195,11 +1198,29 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
     if (has_length) {
         length = qdict_get_int(qdict, "length");
     }
+    if (has_format) {
+        format = qdict_get_str(qdict, "format");
+    }
+
+    if (strcmp(format, "elf") == 0) {
+        dump_format = DUMP_GUEST_MEMORY_FORMAT_ELF;
+    } else if (strcmp(format, "zlib") == 0) {
+        dump_format = DUMP_GUEST_MEMORY_FORMAT_ZLIB;
+    } else if (strcmp(format, "lzo") == 0) {
+        dump_format = DUMP_GUEST_MEMORY_FORMAT_LZO;
+    } else if (strcmp(format, "snappy") == 0) {
+        dump_format = DUMP_GUEST_MEMORY_FORMAT_SNAPPY;
+    } else {
+        error_set(&errp, QERR_INVALID_PARAMETER_VALUE,
+                  "format", "elf|zlib|lzo|snappy");
+        hmp_handle_error(mon, &errp);
+        return;
+    }
 
     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/include/sysemu/dump_memory.h b/include/sysemu/dump_memory.h
index 6848f91..f7c5f4d 100644
--- a/include/sysemu/dump_memory.h
+++ b/include/sysemu/dump_memory.h
@@ -38,6 +38,7 @@
 #define KDUMP_SIGNATURE             "KDUMP   "
 #define SIG_LEN                     (sizeof(KDUMP_SIGNATURE) - 1)
 #define DISKDUMP_HEADER_BLOCKS      (1)
+#define PAGE_SIZE                   (4096)
 #define PHYS_BASE                   (0)
 #define DUMP_LEVEL                  (1)
 #define TMP_BUF_SIZE                (1024)
diff --git a/qapi-schema.json b/qapi-schema.json
index ef1f657..95f7e20 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2380,6 +2380,24 @@
 { 'command': 'device_del', 'data': {'id': 'str'} }
 
 ##
+# @DumpGuestMemoryFormat:
+#
+# An enumeration of guest-memory-dump's format.
+#
+# @elf: elf format
+#
+# @zlib: kdump-compressed format with zlib-compressed
+#
+# @lzo: kdump-compressed format with zlib-compressed
+#
+# @snappy: kdump-compressed format with zlib-compressed
+#
+# Since: 1.6
+##
+{ 'enum': 'DumpGuestMemoryFormat',
+  'data': [ 'elf', 'zlib', 'lzo', 'snappy' ] }
+
+##
 # @dump-guest-memory
 #
 # Dump guest's memory to vmcore. It is a synchronous operation that can take
@@ -2415,13 +2433,15 @@
 #          want to dump all guest's memory, please specify the start @begin
 #          and @length
 #
+# @format: #optional if specified, the format of guest memory dump. (since 1.6)
+#
 # 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 ffd130e..5c274fb 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -788,8 +788,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?,length: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,
@@ -810,6 +810,8 @@ 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|zlib|lzo|snappy (json-string)
 
 Example:
 
-- 
1.7.1

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Qiao Nuohan]

 > I haven't reviewed it yet, but we need introspection support before merging
 > this.

Hello Luiz,

Is it possible to get this reviewed, or I am supposed to wait until
introspection support being settled?

On 05/28/2013 10:50 AM, qiaonuohan@cn.fujitsu.com wrote:
> From: Qiao Nuohan<qiaonuohan@cn.fujitsu.com>
>
> Hi, all
>
> The last version is here:
> http://lists.gnu.org/archive/html/qemu-devel/2013-05/msg02280.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
>
> For migration, 'dump-guest-memory' is supposed to support compression feature.
> Because of the regression, the missing of compression feature, 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 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.
>
> 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 used with configure to make lzo or snappy available.
>
> Changelog:
> 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.
>
>
> Qiao Nuohan (9):
>    dump: Add API to manipulate dump_bitmap
>    dump: Add API to manipulate cache_data
>    dump: Move struct definition into dump_memroy.h
>    dump: Add API to create header of vmcore
>    dump: Add API to create data of dump bitmap
>    dump: Add API to create page
>    dump: Add API to free memory used by creating header, bitmap and page
>    dump: Add API to write header, bitmap and page into vmcore
>    dump: Make kdump-compressed format available for 'dump-guest-memory'
>
>   Makefile.target              |    1 +
>   cache_data.c                 |  121 ++++++
>   configure                    |   50 +++
>   dump.c                       |  866 ++++++++++++++++++++++++++++++++++++++++--
>   dump_bitmap.c                |  171 +++++++++
>   hmp-commands.hx              |   12 +-
>   hmp.c                        |   23 +-
>   include/cache_data.h         |   56 +++
>   include/dump_bitmap.h        |   60 +++
>   include/sysemu/dump_memory.h |  178 +++++++++
>   qapi-schema.json             |   22 +-
>   qmp-commands.hx              |    6 +-
>   12 files changed, 1526 insertions(+), 40 deletions(-)
>   create mode 100644 cache_data.c
>   create mode 100644 dump_bitmap.c
>   create mode 100644 include/cache_data.h
>   create mode 100644 include/dump_bitmap.h
>   create mode 100644 include/sysemu/dump_memory.h
>
>


-- 
Regards
Qiao Nuohan

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Luiz Capitulino]

On Wed, 05 Jun 2013 09:29:19 +0800
Qiao Nuohan <qiaonuohan@cn.fujitsu.com> wrote:

>  > I haven't reviewed it yet, but we need introspection support before merging
>  > this.
> 
> Hello Luiz,
> 
> Is it possible to get this reviewed, or I am supposed to wait until
> introspection support being settled?

I can review it until the end of this week. If this series is adding a new
argument (which I believe is what it does) then there's only two ways
to get this merged: either we wait for full introspection or you add this
feature as a new command.

I'd prefer to wait for full introspection, but it depends how long it's
going to take to get it merged and how much time you're willing to wait.

Amos, can you give us an update on that work?

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Luiz Capitulino]

[CC'ing Amos this time]

On Wed, 05 Jun 2013 09:29:19 +0800
Qiao Nuohan <qiaonuohan@cn.fujitsu.com> wrote:

>  > I haven't reviewed it yet, but we need introspection support before merging
>  > this.
> 
> Hello Luiz,
> 
> Is it possible to get this reviewed, or I am supposed to wait until
> introspection support being settled?

I can review it until the end of this week. If this series is adding a new
argument (which I believe is what it does) then there's only two ways
to get this merged: either we wait for full introspection or you add this
feature as a new command.

I'd prefer to wait for full introspection, but it depends how long it's
going to take to get it merged and how much time you're willing to wait.

Amos, can you give us an update on that work?

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Amos Kong]

On Tue, Jun 04, 2013 at 10:15:41PM -0400, Luiz Capitulino wrote:
> 
> [CC'ing Amos this time]
> 
> On Wed, 05 Jun 2013 09:29:19 +0800
> Qiao Nuohan <qiaonuohan@cn.fujitsu.com> wrote:
> 
> >  > I haven't reviewed it yet, but we need introspection support before merging
> >  > this.
> > 
> > Hello Luiz,
> > 
> > Is it possible to get this reviewed, or I am supposed to wait until
> > introspection support being settled?
> 
> I can review it until the end of this week. If this series is adding a new
> argument (which I believe is what it does) then there's only two ways
> to get this merged: either we wait for full introspection or you add this
> feature as a new command.
> 
> I'd prefer to wait for full introspection, but it depends how long it's
> going to take to get it merged and how much time you're willing to wait.
> 
> Amos, can you give us an update on that work?


Summary of upstream discussion: 

 1) move events to schema.json, then it can also be introspected
    (future work) (Luiz)
 2) need to support to return everyting in one shot (Eric)
 3) support filter by cmd/tyep/event name
 4) processe qapi-schema.json for a more explicit wire format
    with metadata (Eric)
 5) dynamic schema (only for compiled/loaded modules) (Kevin)
 6) the real motivation behind full introspection is to allow
    commands/enum/etc to be extended

I'm tring to implement my original throught, it's a little bit slow
because of the effect of other tasks. I will send the draft patch 
next week.

			Amos.

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Qiao Nuohan]

On 06/05/2013 10:15 AM, Luiz Capitulino wrote:
> I can review it until the end of this week. If this series is adding a new
> argument (which I believe is what it does) then there's only two ways
> to get this merged: either we wait for full introspection or you add this
> feature as a new command.
>
> I'd prefer to wait for full introspection, but it depends how long it's
> going to take to get it merged and how much time you're willing to wait.
>
> Amos, can you give us an update on that work?

Hi Luiz,

I would like to get these patches reviewed first. If introspection won't take
too much time, I will choose to wait. However, I cannot figure out how long it
will take, why not get the parts not related to introspection settled first?

Seems Amos's draft will not change qapi-schema.json.

-- 
Regards
Qiao Nuohan

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Luiz Capitulino]

On Mon, 10 Jun 2013 10:15:35 +0800
Qiao Nuohan <qiaonuohan@cn.fujitsu.com> wrote:

> On 06/05/2013 10:15 AM, Luiz Capitulino wrote:
> > I can review it until the end of this week. If this series is adding a new
> > argument (which I believe is what it does) then there's only two ways
> > to get this merged: either we wait for full introspection or you add this
> > feature as a new command.
> >
> > I'd prefer to wait for full introspection, but it depends how long it's
> > going to take to get it merged and how much time you're willing to wait.
> >
> > Amos, can you give us an update on that work?
> 
> Hi Luiz,
> 
> I would like to get these patches reviewed first. If introspection won't take
> too much time, I will choose to wait. However, I cannot figure out how long it
> will take, why not get the parts not related to introspection settled first?

What do you mean by "settled"? We can keep the review cycle going, but to
merge this we have two options: we wait for the full introspection or we
make this a different command.

> Seems Amos's draft will not change qapi-schema.json.

The point of Amos series is discovery, not conflicts. If we merge your
series w/o introspection support, then it's impossible for a mngt app like
libvirt to know whether or not a given QEMU version supports your new
argument.

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Qiao Nuohan]

On 06/10/2013 08:54 PM, Luiz Capitulino wrote:
> On Mon, 10 Jun 2013 10:15:35 +0800
> Qiao Nuohan<qiaonuohan@cn.fujitsu.com>  wrote:
>
>> On 06/05/2013 10:15 AM, Luiz Capitulino wrote:
>>> I can review it until the end of this week. If this series is adding a new
>>> argument (which I believe is what it does) then there's only two ways
>>> to get this merged: either we wait for full introspection or you add this
>>> feature as a new command.
>>>
>>> I'd prefer to wait for full introspection, but it depends how long it's
>>> going to take to get it merged and how much time you're willing to wait.
>>>
>>> Amos, can you give us an update on that work?
>>
>> Hi Luiz,
>>
>> I would like to get these patches reviewed first. If introspection won't take
>> too much time, I will choose to wait. However, I cannot figure out how long it
>> will take, why not get the parts not related to introspection settled first?
>
> What do you mean by "settled"? We can keep the review cycle going, but to
> merge this we have two options: we wait for the full introspection or we
> make this a different command.

Sorry for confusing you. I mean "reviewed enough" here.

If it won't take several months, I have no doubt about waiting for
introspection. I just want these patches reviewed first, then I may get these
patches ready for merging soon after introspection support.

>
>> Seems Amos's draft will not change qapi-schema.json.
>
> The point of Amos series is discovery, not conflicts. If we merge your
> series w/o introspection support, then it's impossible for a mngt app like
> libvirt to know whether or not a given QEMU version supports your new
> argument.
>

Thanks for your explanation.

>


-- 
Regards
Qiao Nuohan

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Luiz Capitulino]

On Tue, 11 Jun 2013 09:48:40 +0800
Qiao Nuohan <qiaonuohan@cn.fujitsu.com> wrote:

> introspection. I just want these patches reviewed first, then I may get these
> patches ready for merging soon after introspection support.

I've started reviewing this, but I'm out of cycles for this week.

Can anyone CC'ed help with this?

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Qiao Nuohan]

On 06/14/2013 02:12 AM, Luiz Capitulino wrote:
> I've started reviewing this, but I'm out of cycles for this week.

Hi Luiz,

No comments yet?

-- 
Regards
Qiao Nuohan

Re: [Qemu-devel] [PATCH v4 1/9] dump: Add API to manipulate dump_bitmap

[Andreas Färber]

Am 28.05.2013 04:50, schrieb qiaonuohan@cn.fujitsu.com:
> From: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> 
> Struct dump_bitmap is associated with a tmp file which is used to store bitmap
> in kdump-compressed format temporarily. The following patch will use these
> functions to gather data of bitmap and cache them into tmp files.
> 
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
> ---
>  Makefile.target       |    1 +
>  dump_bitmap.c         |  171 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/dump_bitmap.h |   60 +++++++++++++++++
>  3 files changed, 232 insertions(+), 0 deletions(-)
>  create mode 100644 dump_bitmap.c
>  create mode 100644 include/dump_bitmap.h

Wouldn't this fit better in include/sysemu/?

> diff --git a/Makefile.target b/Makefile.target
> index ce4391f..8e557f9 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -110,6 +110,7 @@ obj-y += qtest.o
>  obj-y += hw/
>  obj-$(CONFIG_FDT) += device_tree.o
>  obj-$(CONFIG_KVM) += kvm-all.o
> +obj-y += dump_bitmap.o
>  obj-y += memory.o savevm.o cputlb.o
>  obj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) += memory_mapping.o
>  obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o

My refactoring was merged, so this may conflict, although --3way
resolution should work.

> diff --git a/dump_bitmap.c b/dump_bitmap.c
> new file mode 100644
> index 0000000..f35f39d
> --- /dev/null
> +++ b/dump_bitmap.c
> @@ -0,0 +1,171 @@
> +/*
> + * QEMU dump bitmap
> + *
> + * Copyright (C) 2013 FUJITSU LIMITED
> + *
> + * Authors:
> + *     Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu-common.h"
> +#include "dump_bitmap.h"
> +
> +int init_dump_bitmap(struct dump_bitmap *db, const char *filename)
> +{
> +    int fd;
> +    char *tmpname;
> +
> +    /* init the tmp file */
> +    tmpname = getenv("TMPDIR");
> +    if (!tmpname) {
> +        tmpname = (char *)P_tmpdir;
> +    }
> +
> +    db->file_name = (char *)g_strdup_printf("%s/%s", tmpname, filename);
> +
> +    fd = mkstemp(db->file_name);
> +    if (fd < 0) {
> +        return -1;

Given that all this is going to be used from a QMP command, I think it
were better to make the function return void and instead supply an Error
**errp argument that can return a textual error that can then be
propagated on.

> +    }
> +
> +    db->fd = fd;
> +    unlink(db->file_name);
> +
> +    /* num_block should be set to -1, for nothing is store in buf now */
> +    db->num_block = -1;
> +    memset(db->buf, 0, BUFSIZE_BITMAP);
> +    /* the tmp file starts to write at the very beginning */
> +    db->offset = 0;
> +
> +    return 0;
> +}
> +
> +int clear_dump_bitmap(struct dump_bitmap *db, size_t len_db)
> +{
> +    int ret;
> +    char buf[BUFSIZE_BITMAP];
> +    off_t offset_bit;
> +
> +    /* use buf to clear the tmp file block by block */
> +    memset(buf, 0, sizeof(buf));
> +
> +    ret = lseek(db->fd, db->offset, SEEK_SET);
> +    if (ret < 0) {
> +        return -1;

Same here.

> +    }
> +
> +    offset_bit = 0;
> +    while (offset_bit < len_db) {
> +        if (write(db->fd, buf, BUFSIZE_BITMAP) != BUFSIZE_BITMAP) {
> +            return -1;
> +        }
> +
> +        offset_bit += BUFSIZE_BITMAP;
> +    }
> +
> +    return 0;
> +}
> +
> +int set_dump_bitmap(struct dump_bitmap *db, unsigned long long pfn, int val)
> +{
> +    int byte, bit;
> +    off_t old_offset, new_offset;
> +    old_offset = db->offset + BUFSIZE_BITMAP * db->num_block;
> +    new_offset = db->offset + BUFSIZE_BITMAP * (pfn / PFN_BUFBITMAP);
> +
> +    /*
> +     * if the block needed to be set is not same as the one cached in buf,
> +     * write the block back to the tmp file, then cache new block to the buf
> +     */
> +    if (0 <= db->num_block && old_offset != new_offset) {
> +        if (lseek(db->fd, old_offset, SEEK_SET) < 0) {
> +            return -1;

Ditto.

> +        }
> +
> +        if (write(db->fd, db->buf, BUFSIZE_BITMAP) != BUFSIZE_BITMAP) {
> +            return -1;
> +        }
> +    }
> +
> +    if (old_offset != new_offset) {
> +        if (lseek(db->fd, new_offset, SEEK_SET) < 0) {
> +            return -1;
> +        }
> +
> +        if (read(db->fd, db->buf, BUFSIZE_BITMAP) != BUFSIZE_BITMAP) {
> +            return -1;
> +        }
> +
> +        db->num_block = pfn / PFN_BUFBITMAP;
> +    }
> +
> +    /* get the exact place of the bit in the buf, set it */
> +    byte = (pfn % PFN_BUFBITMAP) >> 3;
> +    bit  = (pfn % PFN_BUFBITMAP) & 7;
> +    if (val) {
> +        db->buf[byte] |= 1<<bit;
> +    } else {
> +        db->buf[byte] &= ~(1<<bit);
> +    }
> +
> +    return 0;
> +}
> +
> +int sync_dump_bitmap(struct dump_bitmap *db)
> +{
> +    off_t offset;
> +    offset = db->offset + BUFSIZE_BITMAP * db->num_block;
> +
> +    /* db has not been used yet */
> +    if (db->num_block < 0) {
> +        return 0;
> +    }
> +
> +    if (lseek(db->fd, offset, SEEK_SET) < 0) {
> +        return -1;

Ditto.

> +    }
> +
> +    if (write(db->fd, db->buf, BUFSIZE_BITMAP) != BUFSIZE_BITMAP) {
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +/*
> + * check if the bit is set to 1
> + */
> +static inline int is_on(char *bitmap, int i)

Return bool?

> +{
> +    return bitmap[i>>3] & (1 << (i & 7));
> +}
> +
> +int is_bit_set(struct dump_bitmap *db, unsigned long long pfn)

bool?

> +{
> +    off_t offset;
> +
> +    /* cache the block pfn belongs to, then check the block */
> +    if (pfn == 0 || db->num_block != pfn/PFN_BUFBITMAP) {
> +        offset = db->offset + BUFSIZE_BITMAP * (pfn/PFN_BUFBITMAP);
> +        lseek(db->fd, offset, SEEK_SET);
> +        read(db->fd, db->buf, BUFSIZE_BITMAP);
> +        db->num_block = pfn/PFN_BUFBITMAP;
> +    }
> +
> +    return is_on(db->buf, pfn%PFN_BUFBITMAP);

Coding Style asks for spaces around / and % operators.

> +}
> +
> +void free_dump_bitmap(struct dump_bitmap *db)
> +{
> +    if (db) {
> +        if (db->file_name) {
> +            g_free(db->file_name);
> +        }

The if seems unnecessary, g_free(NULL) should work fine.

> +
> +        g_free(db);
> +    }
> +}
> diff --git a/include/dump_bitmap.h b/include/dump_bitmap.h
> new file mode 100644
> index 0000000..f81106c
> --- /dev/null
> +++ b/include/dump_bitmap.h
> @@ -0,0 +1,60 @@
> +/*
> + * QEMU dump bitmap
> + *
> + * Copyright (C) 2013 FUJITSU LIMITED
> + *
> + * Authors:
> + *     Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef DUMP_BITMAP_H
> +#define DUMP_BITMAP_H
> +
> +#define BUFSIZE_BITMAP              (4096)
> +#define PFN_BUFBITMAP               (CHAR_BIT * BUFSIZE_BITMAP)
> +
> +struct dump_bitmap {
> +    int fd;                     /* fd of the tmp file */
> +    int num_block;              /* number of blocks cached in buf */
> +    char *file_name;            /* name of the tmp file */
> +    char buf[BUFSIZE_BITMAP];   /* used to cache blocks */
> +    off_t offset;               /* offset of the tmp file */
> +};

Please use CamelCase for the struct name, and please supply a typedef
and use that below.

Regards,
Andreas

> +
> +/*
> + * create a tmp file used to store dump bitmap, then init buf of dump_bitmap
> + */
> +int init_dump_bitmap(struct dump_bitmap *db, const char *filename);
> +
> +/*
> + * clear the content of the tmp file with all bits set to 0
> + */
> +int clear_dump_bitmap(struct dump_bitmap *db, size_t len_db);
> +
> +/*
> + * 'pfn' is the number of bit needed to be set, use buf to cache the block which
> + * 'pfn' belongs to, then set 'val' to the bit
> + */
> +int set_dump_bitmap(struct dump_bitmap *db, unsigned long long pfn, int val);
> +
> +/*
> + * when buf is caching a block, sync_dump_bitmap is needed to write the cached
> + * block to the tmp file
> + */
> +int sync_dump_bitmap(struct dump_bitmap *db);
> +
> +/*
> + * check whether 'pfn' is set
> + */
> +int is_bit_set(struct dump_bitmap *db, unsigned long long pfn);
> +
> +/*
> + * free the space used by dump_bitmap
> + */
> +void free_dump_bitmap(struct dump_bitmap *db);
> +
> +#endif
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v4 1/9] dump: Add API to manipulate dump_bitmap

[Andreas Färber]

Am 28.05.2013 04:50, schrieb qiaonuohan@cn.fujitsu.com:
> From: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> 
> Struct dump_bitmap is associated with a tmp file which is used to store bitmap
> in kdump-compressed format temporarily. The following patch will use these
> functions to gather data of bitmap and cache them into tmp files.
> 
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
> ---
>  Makefile.target       |    1 +
>  dump_bitmap.c         |  171 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/dump_bitmap.h |   60 +++++++++++++++++
>  3 files changed, 232 insertions(+), 0 deletions(-)
>  create mode 100644 dump_bitmap.c
>  create mode 100644 include/dump_bitmap.h

As expected only applied with --3way now.

Build fails as follows:

  CC    alpha-softmmu/dump_bitmap.o
/home/andreas/QEMU/qemu/dump_bitmap.c: In function ‘is_bit_set’:
/home/andreas/QEMU/qemu/dump_bitmap.c:155:13: error: ignoring return
value of ‘read’, declared with attribute warn_unused_result
[-Werror=unused-result]
cc1: all warnings being treated as errors
make[1]: *** [dump_bitmap.o] Fehler 1
make: *** [subdir-alpha-softmmu] Fehler 2

That brings up another question: Why compile this file per target in
Makefile.target:obj-y rather than once in Makefile.objs:common-obj-y?
I don't see any target page sizes or target types being used.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v4 2/9] dump: Add API to manipulate cache_data

[Andreas Färber]

Am 28.05.2013 04:50, schrieb qiaonuohan@cn.fujitsu.com:
> From: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> 
> Struct cache_data is associated with a tmp file which is used to store page
> desc and page data in kdump-compressed format temporarily.

CacheData please - but I do find the English term "cache data"
irritating as it sounds like data about a cache when instead it is about
cached data. DataCache? CachedData? Maybe a native English speaker can
advise?

> The following patch
> will use these function to gather data of page and cache them in tmp files.
> 
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
> ---
>  Makefile.target      |    2 +-
>  cache_data.c         |  121 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/cache_data.h |   56 +++++++++++++++++++++++
>  3 files changed, 178 insertions(+), 1 deletions(-)
>  create mode 100644 cache_data.c
>  create mode 100644 include/cache_data.h
> 
> diff --git a/Makefile.target b/Makefile.target
> index 8e557f9..298ec84 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -110,7 +110,7 @@ obj-y += qtest.o
>  obj-y += hw/
>  obj-$(CONFIG_FDT) += device_tree.o
>  obj-$(CONFIG_KVM) += kvm-all.o
> -obj-y += dump_bitmap.o
> +obj-y += dump_bitmap.o cache_data.o

Same comment as for dump_bitmap.o: Can we build this only once, please?

>  obj-y += memory.o savevm.o cputlb.o
>  obj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) += memory_mapping.o
>  obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o
> diff --git a/cache_data.c b/cache_data.c
> new file mode 100644
> index 0000000..6e91538
> --- /dev/null
> +++ b/cache_data.c
> @@ -0,0 +1,121 @@
> +/*
> + * QEMU cache data
> + *
> + * Copyright (C) 2013 FUJITSU LIMITED
> + *
> + * Authors:
> + *     Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu-common.h"
> +#include "cache_data.h"
> +
> +int init_cache_data(struct cache_data *cd, const char *filename)
> +{
> +    int fd;
> +    char *tmpname;
> +
> +    /* init the tmp file */
> +    tmpname = getenv("TMPDIR");
> +    if (!tmpname) {
> +        tmpname = (char *)P_tmpdir;

P_tmpdir is marked obsolescent in Open Group spec 7. Maybe Erik can
comment some more? Did you verify it builds with mingw32/64?
(I stumbled over it because I found the variable name odd and didn't see
it defined anywhere.)

> +    }
> +
> +    cd->file_name = (char *)g_strdup_printf("%s/%s", tmpname, filename);
> +
> +    fd = mkstemp(cd->file_name);
> +    if (fd < 0) {
> +        return -1;

Error **errp? Same below.

> +    }
> +
> +    cd->fd = fd;
> +    unlink(cd->file_name);
> +
> +    /* init buf */
> +    cd->buf_size = BUFSIZE_CACHE_DATA;
> +    cd->cache_size = 0;
> +    cd->buf = g_malloc0(BUFSIZE_CACHE_DATA);
> +
> +    cd->offset = 0;
> +
> +    return 0;
> +}

I wonder if it makes sense to introduce this interface instead of going
through the block layer, which would offer a number of different
backends at least. CC'ing the experts.

> +
> +int write_cache(struct cache_data *cd, void *buf, size_t size)
> +{
> +    /*
> +     * check if the space is enough to cache data, if not write cached
> +     * data to the tmp file
> +     */
> +    if (cd->cache_size + size > cd->buf_size) {
> +        if (lseek(cd->fd, cd->offset, SEEK_SET) < 0) {
> +            return -1;
> +        }
> +
> +        if (write(cd->fd, cd->buf, cd->cache_size) != cd->cache_size) {
> +            return -1;
> +        }
> +
> +        cd->offset += cd->cache_size;
> +        cd->cache_size = 0;
> +    }
> +
> +    memcpy(cd->buf + cd->cache_size, buf, size);
> +    cd->cache_size += size;
> +
> +    return 0;
> +}
> +
> +int sync_cache(struct cache_data *cd)
> +{
> +    /* no data is cached in cache_data */
> +    if (cd->cache_size == 0) {
> +        return 0;
> +    }
> +
> +    if (lseek(cd->fd, cd->offset, SEEK_SET) < 0) {
> +        return -1;
> +    }
> +
> +    if (write(cd->fd, cd->buf, cd->cache_size) != cd->cache_size) {
> +        return -1;
> +    }
> +
> +    cd->offset += cd->cache_size;
> +
> +    return 0;
> +}
> +
> +int read_cache(struct cache_data *cd)
> +{
> +    if (lseek(cd->fd, cd->offset, SEEK_SET) < 0) {
> +        return -1;
> +    }
> +
> +    if (read(cd->fd, cd->buf, cd->cache_size) != cd->cache_size) {
> +        return -1;
> +    }
> +
> +    cd->offset += cd->cache_size;
> +
> +    return 0;
> +}
> +
> +void free_cache_data(struct cache_data *cd)
> +{
> +    if (cd) {
> +        if (cd->file_name) {
> +            g_free(cd->file_name);
> +        }
> +
> +        if (cd->buf) {
> +            g_free(cd->buf);
> +        }
> +
> +        g_free(cd);
> +    }
> +}
> diff --git a/include/cache_data.h b/include/cache_data.h
> new file mode 100644
> index 0000000..18e8680
> --- /dev/null
> +++ b/include/cache_data.h
> @@ -0,0 +1,56 @@
> +/*
> + * QEMU cache data
> + *
> + * Copyright (C) 2013 FUJITSU LIMITED
> + *
> + * Authors:
> + *     Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef CACHE_DATA_H
> +#define CACHE_DATA_H
> +
> +#define BUFSIZE_CACHE_DATA          (4096 * 4)
> +
> +struct cache_data {
> +    int fd;             /* fd of the tmp file used to store cache data */
> +    char *file_name;    /* name of the tmp file */
> +    char *buf;          /* used to cache data */
> +    size_t buf_size;    /* size of the buf */
> +    size_t cache_size;  /* size of cached data in buf */
> +    off_t offset;       /* offset of the tmp file */
> +};

CamelCase and typedef please.

Regards,
Andreas

> +
> +/*
> + * create a tmp file used to store cache data, then init the buf
> + */
> +int init_cache_data(struct cache_data *cd, const char *filename);
> +
> +/*
> + * write data to the tmp file, the data may first be cached in the buf of
> + * cache_data
> + */
> +int write_cache(struct cache_data *cd, void *buf, size_t size);
> +
> +/*
> + * when cache_data is caching data in the buf, sync_cache is needed to write the
> + * data back to tmp file
> + */
> +int sync_cache(struct cache_data *cd);
> +
> +/*  read data from the tmp file to the buf of 'cd', the start place is set by
> + *  cd->offset, and the size is set by cd->cache_size. cd->offset is changed
> + *  automaticlly according to the size of data read this time.
> + */
> +int read_cache(struct cache_data *cd);
> +
> +/*
> + * free the space used by cache_data
> + */
> +void free_cache_data(struct cache_data *cd);
> +
> +#endif
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v4 3/9] dump: Move struct definition into dump_memroy.h

[Andreas Färber]

"dump_memory.h"

Am 28.05.2013 04:50, schrieb qiaonuohan@cn.fujitsu.com:
> From: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> 
> Move definition of struct DumpState into include/sysemu/dump_memory.h.
> 
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
> ---
>  dump.c                       |   22 +---------------------
>  include/sysemu/dump_memory.h |   40 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 21 deletions(-)
>  create mode 100644 include/sysemu/dump_memory.h
> 
> diff --git a/dump.c b/dump.c
> index c0d3da5..9ac66be 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -15,10 +15,9 @@
>  #include "elf.h"
>  #include "cpu.h"
>  #include "exec/cpu-all.h"
> -#include "exec/hwaddr.h"
>  #include "monitor/monitor.h"
>  #include "sysemu/kvm.h"
> -#include "sysemu/dump.h"
> +#include "sysemu/dump_memory.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/memory_mapping.h"
>  #include "qapi/error.h"
> @@ -57,25 +56,6 @@ static uint64_t cpu_convert_to_target64(uint64_t val, int endian)
>      return val;
>  }
>  
> -typedef struct DumpState {
> -    ArchDumpInfo dump_info;
> -    MemoryMappingList list;
> -    uint16_t phdr_num;
> -    uint32_t sh_info;
> -    bool have_section;
> -    bool resume;
> -    size_t note_size;
> -    hwaddr memory_offset;
> -    int fd;
> -
> -    RAMBlock *block;
> -    ram_addr_t start;
> -    bool has_filter;
> -    int64_t begin;
> -    int64_t length;
> -    Error **errp;
> -} DumpState;
> -
>  static int dump_cleanup(DumpState *s)
>  {
>      int ret = 0;
> diff --git a/include/sysemu/dump_memory.h b/include/sysemu/dump_memory.h
> new file mode 100644
> index 0000000..ce22c05
> --- /dev/null
> +++ b/include/sysemu/dump_memory.h
> @@ -0,0 +1,40 @@
> +/*
> + * QEMU dump memory
> + *
> + * Copyright (C) 2013 FUJITSU LIMITED
> + *
> + * Authors:
> + *     Qiao Nuohan <qiaonuohan@cn.fujitsu.com>

I wondered whether you may want to keep Wen Congyang here as original
dump.c author and the corresponding dump.c copyright - but since it's
all Fujitsu that's none of my business. ;)

Regards,
Andreas

> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef DUMP_MEMORY_H
> +#define DUMP_MEMORY_H
> +
> +#include "exec/cpu-all.h"
> +#include "sysemu/memory_mapping.h"
> +#include "sysemu/dump.h"
> +
> +typedef struct DumpState {
> +    ArchDumpInfo dump_info;
> +    MemoryMappingList list;
> +    uint16_t phdr_num;
> +    uint32_t sh_info;
> +    bool have_section;
> +    bool resume;
> +    size_t note_size;
> +    hwaddr memory_offset;
> +    int fd;
> +
> +    RAMBlock *block;
> +    ram_addr_t start;
> +    bool has_filter;
> +    int64_t begin;
> +    int64_t length;
> +    Error **errp;
> +} DumpState;
> +
> +#endif
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v4 9/9] dump: Make kdump-compressed format available for 'dump-guest-memory'

[Stefan Hajnoczi]

On Tue, May 28, 2013 at 10:50:37AM +0800, qiaonuohan@cn.fujitsu.com wrote:
> From: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> 
> 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. 'zlib': kdump-compressed format, with zlib-compressed
> 3. 'lzo': kdump-compressed format, with lzo-compressed
> 4. 'snappy': kdump-compressed format, with snappy-compressed

Please use kdump-zlib, kdump-lzo, and kdump-snappy so the format is
clear.  (There may be other formats that support zlib, lzo, or snappy in
the future.)

Re: [Qemu-devel] [PATCH v4 4/9] dump: Add API to create header of vmcore

[Andreas Färber]

Am 28.05.2013 04:50, schrieb qiaonuohan@cn.fujitsu.com:
> From: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> 
> Functions in this patch are used to gather data of header and sub header in
> kdump-compressed format. The following patch will use these functions to gather
> data of header, then cache them into struct DumpState.
> 
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
> ---
>  dump.c                       |  107 ++++++++++++++++++++++++++++++++++++++++++
>  include/sysemu/dump_memory.h |   93 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 200 insertions(+), 0 deletions(-)
> 
> diff --git a/dump.c b/dump.c
> index 9ac66be..3b9d4ca 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -681,6 +681,113 @@ static ram_addr_t get_start_block(DumpState *s)
>      return -1;
>  }
>  
> +static int create_header32(DumpState *s)
> +{
> +    struct  disk_dump_header32 *dh;
> +    struct  kdump_sub_header32 *kh;
> +
> +    /* create common header, the version of kdump-compressed format is 5th */
> +    dh = g_malloc0(sizeof(struct disk_dump_header32));
> +
> +    strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
> +    dh->header_version = 5;
> +    dh->block_size = s->page_size;
> +    dh->sub_hdr_size = sizeof(struct kdump_sub_header32) + s->note_size;
> +    dh->sub_hdr_size = divideup(dh->sub_hdr_size, dh->block_size);
> +    dh->max_mapnr = s->max_mapnr;
> +    dh->nr_cpus = s->nr_cpus;
> +    dh->bitmap_blocks = divideup(s->len_dump_bitmap, s->page_size);
> +
> +    memcpy(&(dh->utsname.machine), "i686", 4);
> +
> +    s->dh = dh;
> +
> +    /* create sub header */
> +    kh = g_malloc0(sizeof(struct kdump_sub_header32));
> +
> +    kh->phys_base = PHYS_BASE;
> +    kh->dump_level = DUMP_LEVEL;
> +
> +    kh->offset_note = DISKDUMP_HEADER_BLOCKS * dh->block_size +
> +                        sizeof(struct kdump_sub_header32);
> +    kh->note_size = s->note_size;
> +
> +    s->kh = kh;
> +
> +    /* get gap between header and sub header */
> +    s->offset_sub_header = DISKDUMP_HEADER_BLOCKS * dh->block_size -
> +                            sizeof(struct disk_dump_header32);
> +
> +    /* get gap between header and dump_bitmap */
> +    s->offset_dump_bitmap = dh->sub_hdr_size * dh->block_size -
> +                            (sizeof(struct kdump_sub_header32) + s->note_size);
> +
> +    /* get offset of page desc */
> +    s->offset_page = (DISKDUMP_HEADER_BLOCKS + dh->sub_hdr_size +
> +                        dh->bitmap_blocks) * dh->block_size;
> +
> +    return 0;
> +}

Function always return 0 - make it void?

> +
> +static int create_header64(DumpState *s)
> +{
> +    struct  disk_dump_header64 *dh;
> +    struct  kdump_sub_header64 *kh;
> +
> +    /* create common header, the version of kdump-compressed format is 5th */
> +    dh = g_malloc0(sizeof(struct disk_dump_header64));
> +
> +    strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
> +    dh->header_version = 5;
> +    dh->block_size = s->page_size;
> +    dh->sub_hdr_size = sizeof(struct kdump_sub_header64) + s->note_size;
> +    dh->sub_hdr_size = divideup(dh->sub_hdr_size, dh->block_size);
> +    dh->max_mapnr = s->max_mapnr;
> +    dh->nr_cpus = s->nr_cpus;
> +    dh->bitmap_blocks = divideup(s->len_dump_bitmap, s->page_size);
> +
> +    memcpy(&(dh->utsname.machine), "x86_64", 6);
> +
> +    s->dh = dh;
> +
> +    /* create sub header */
> +    kh = g_malloc0(sizeof(struct kdump_sub_header64));
> +
> +    kh->phys_base = PHYS_BASE;
> +    kh->dump_level = DUMP_LEVEL;
> +
> +    kh->offset_note = DISKDUMP_HEADER_BLOCKS * dh->block_size +
> +                        sizeof(struct kdump_sub_header64);
> +    kh->note_size = s->note_size;
> +
> +    s->kh = kh;
> +
> +    /* get gap between header and sub header */
> +    s->offset_sub_header = DISKDUMP_HEADER_BLOCKS * dh->block_size -
> +                            sizeof(struct disk_dump_header64);
> +
> +    /* get gap between header and dump_bitmap */
> +    s->offset_dump_bitmap = dh->sub_hdr_size * dh->block_size -
> +                            (sizeof(struct kdump_sub_header64) + s->note_size);
> +
> +    /* get offset of page desc */
> +    s->offset_page = (DISKDUMP_HEADER_BLOCKS + dh->sub_hdr_size +
> +                        dh->bitmap_blocks) * dh->block_size;
> +
> +    return 0;
> +}
> +
> +/*
> + * gather data of header and sub header
> + */
> +static int create_header(DumpState *s)
> +{
> +    if (s->dump_info.d_machine == EM_386)
> +        return create_header32(s);
> +    else
> +        return create_header64(s);
> +}

Braces for if and else missing.

Surely EM_386 is not the only 32-bit machine.

> +
>  static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
>                       int64_t begin, int64_t length, Error **errp)
>  {
> diff --git a/include/sysemu/dump_memory.h b/include/sysemu/dump_memory.h
> index ce22c05..56e0f40 100644
> --- a/include/sysemu/dump_memory.h
> +++ b/include/sysemu/dump_memory.h
> @@ -18,6 +18,87 @@
>  #include "sysemu/memory_mapping.h"
>  #include "sysemu/dump.h"
>  
> +#define KDUMP_SIGNATURE             "KDUMP   "
> +#define SIG_LEN                     (sizeof(KDUMP_SIGNATURE) - 1)
> +#define DISKDUMP_HEADER_BLOCKS      (1)
> +#define PHYS_BASE                   (0)
> +#define DUMP_LEVEL                  (1)
> +
> +#define divideup(x, y)              (((x) + ((y) - 1)) / (y))
> +
> +struct new_utsname {
> +    char sysname[65];
> +    char nodename[65];
> +    char release[65];
> +    char version[65];
> +    char machine[65];
> +    char domainname[65];
> +};
> +
> +struct disk_dump_header32 {
> +    char signature[SIG_LEN];        /* = "KDUMP   " */
> +    int header_version;             /* Dump header version */
> +    struct new_utsname utsname;     /* copy of system_utsname */
> +    char timestamp[8];              /* Time stamp */
> +    unsigned int status;            /* Above flags */
> +    int block_size;                 /* Size of a block in byte */
> +    int sub_hdr_size;               /* Size of arch dependent header in block */
> +    unsigned int bitmap_blocks;     /* Size of Memory bitmap in block */
> +    unsigned int max_mapnr;         /* = max_mapnr */
> +    unsigned int total_ram_blocks;  /* Number of blocks should be written */
> +    unsigned int device_blocks;     /* Number of total blocks in dump device */
> +    unsigned int written_blocks;    /* Number of written blocks */
> +    unsigned int current_cpu;       /* CPU# which handles dump */
> +    int nr_cpus;                    /* Number of CPUs */
> +    struct task_struct *tasks[0];
> +};
> +
> +struct disk_dump_header64 {
> +    char signature[SIG_LEN];        /* = "KDUMP   " */
> +    int header_version;             /* Dump header version */
> +    struct new_utsname utsname;     /* copy of system_utsname */
> +    char timestamp[20];             /* Time stamp */
> +    unsigned int status;            /* Above flags */
> +    int block_size;                 /* Size of a block in byte */
> +    int sub_hdr_size;               /* Size of arch dependent header in block */
> +    unsigned int bitmap_blocks;     /* Size of Memory bitmap in block */
> +    unsigned int max_mapnr;         /* = max_mapnr */
> +    unsigned int total_ram_blocks;  /* Number of blocks should be written */
> +    unsigned int device_blocks;     /* Number of total blocks in dump device */
> +    unsigned int written_blocks;    /* Number of written blocks */
> +    unsigned int current_cpu;       /* CPU# which handles dump */
> +    int nr_cpus;                    /* Number of CPUs */
> +    struct task_struct *tasks[0];
> +};

If these are headers, shouldn't they be using int32_t / uint32_t just
like the ones below? Also should tasks rather be struct task_struct
tasks[0]? An array of pointers in the file is a bit hard to imagine...

Also do any of these structs need QEMU_PACKED attribute?

> +
> +struct kdump_sub_header32 {
> +    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 */
> +    uint32_t end_pfn;               /* header_version 2 and later */
> +    uint32_t offset_vmcoreinfo;     /* header_version 3 and later */
> +    uint32_t size_vmcoreinfo;       /* header_version 3 and later */
> +    uint32_t offset_note;           /* header_version 4 and later */
> +    uint32_t note_size;             /* header_version 4 and later */
> +    uint32_t offset_eraseinfo;      /* header_version 5 and later */
> +    uint32_t size_eraseinfo;        /* header_version 5 and later */
> +};
> +
> +struct kdump_sub_header64 {
> +    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 */
> +    uint64_t end_pfn;               /* header_version 2 and later */
> +    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 */
> +};
> +
>  typedef struct DumpState {
>      ArchDumpInfo dump_info;
>      MemoryMappingList list;
> @@ -35,6 +116,18 @@ typedef struct DumpState {
>      int64_t begin;
>      int64_t length;
>      Error **errp;
> +
> +    int page_size;
> +    unsigned long long max_mapnr;
> +    int nr_cpus;
> +    void *dh;
> +    void *kh;
> +    off_t offset_sub_header;
> +
> +    off_t offset_dump_bitmap;
> +    unsigned long len_dump_bitmap;
> +
> +    off_t offset_page;
>  } DumpState;
>  
>  #endif

Why unsigned long long and unsigned long respectively?

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Stefan Hajnoczi]

On Tue, May 28, 2013 at 10:50:28AM +0800, qiaonuohan@cn.fujitsu.com wrote:
> 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.
> 
> 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 used with configure to make lzo or snappy available.

Please see the coding style docs and run scripts/checkpatch.pl on your
patches:
http://git.qemu.org/?p=qemu.git;a=blob;f=CODING_STYLE;hb=HEAD
http://git.qemu.org/?p=qemu.git;a=blob;f=HACKING;hb=HEAD

Where does that code live that writes DISKDUMP files?  I can see the
diskdump.[ch] code.

The file format is pretty bad: we need 4 temporary files and a lot of
data copying to write it out.

Why not just compress an ELF file and teach the crash utility how to
decompress while reading the ELF?

Also, did you look into simply outputting the ELF file without zero
pages?

Stefan

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Qiao Nuohan]

On 06/19/2013 09:49 PM, Stefan Hajnoczi wrote:
> Where does that code live that writes DISKDUMP files?  I can see the
> diskdump.[ch] code.

Sorry, I cannot catch what do you mean here.

>
> The file format is pretty bad: we need 4 temporary files and a lot of
> data copying to write it out.
>
> Why not just compress an ELF file and teach the crash utility how to
> decompress while reading the ELF?
>
> Also, did you look into simply outputting the ELF file without zero
> pages?

What I want is a dump file with smaller size. And compressed format and with
zero pages excluded can make it. I choose kdump-compressed format because it is
a standard format and it can realize what I want.

Why 4 temporary files are need? dump-guest-memory may be called with a fd which
is supposed to send data of dump to. If fd is opened on a pipe or etc which is
unable to seek, then I need to cache the data.

-- 
Regards
Qiao Nuohan

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Stefan Hajnoczi]

On Thu, Jun 20, 2013 at 10:18:35AM +0800, Qiao Nuohan wrote:
> On 06/19/2013 09:49 PM, Stefan Hajnoczi wrote:
> >Where does that code live that writes DISKDUMP files?  I can see the
> >diskdump.[ch] code.
> 
> Sorry, I cannot catch what do you mean here.

Please link to the code that writes DISKDUMP kdump files on a physical
machine.  I only see the crash utility code to read the DISKDUMP code
but I haven't found anything in the Linux kernel, the crash utility, or
the kexec-utils code to actually write a DISKDUMP file.

> >
> >The file format is pretty bad: we need 4 temporary files and a lot of
> >data copying to write it out.
> >
> >Why not just compress an ELF file and teach the crash utility how to
> >decompress while reading the ELF?
> >
> >Also, did you look into simply outputting the ELF file without zero
> >pages?
> 
> What I want is a dump file with smaller size. And compressed format and with
> zero pages excluded can make it. I choose kdump-compressed format because it is
> a standard format and it can realize what I want.
> 
> Why 4 temporary files are need? dump-guest-memory may be called with a fd which
> is supposed to send data of dump to. If fd is opened on a pipe or etc which is
> unable to seek, then I need to cache the data.

I understand why you need temporary files, but my questions stand:

Have you looked at using ELF more efficiently instead of duplicating
kdump code into QEMU?  kdump is not a great format for the problem
you're trying to solve - you're not filling in the Linux-specific
metadata and it's a pain to write due to its layout.

Why can't you simply omit zero pages from the ELF?

Why can't you compress the entire ELF file and add straightforward
decompression to the crash utility?

Stefan

Re: [Qemu-devel] [PATCH v4 2/9] dump: Add API to manipulate cache_data

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1549 bytes --]

On 06/19/2013 01:29 PM, Andreas Färber wrote:

>> +int init_cache_data(struct cache_data *cd, const char *filename)
>> +{
>> +    int fd;
>> +    char *tmpname;
>> +
>> +    /* init the tmp file */
>> +    tmpname = getenv("TMPDIR");
>> +    if (!tmpname) {
>> +        tmpname = (char *)P_tmpdir;
> 
> P_tmpdir is marked obsolescent in Open Group spec 7. Maybe Erik can

s/Erik/Eric/ (but don't worry, you're not the first to make that typo)

Hmm, you are correct that tempnam() is marked as an obsolescent
interface (because it has the same security flaws as mktemp(); the
standard introduced mkstemp() to overcome the security hole but did not
add a replacement for tempnam()).  I guess since nothing else in the
standard refers to P_tmpdir, it was also marked obsolecent.  And since
C99 doesn't require either the constant or the (inherently broken)
tempnam(), it may be safer to guard this line by #ifdef P_tmpdir, rather
than assuming that <stdio.h> blindly provides it.

> comment some more? Did you verify it builds with mingw32/64?
> (I stumbled over it because I found the variable name odd and didn't see
> it defined anywhere.)
> 
>> +    }
>> +
>> +    cd->file_name = (char *)g_strdup_printf("%s/%s", tmpname, filename);
>> +
>> +    fd = mkstemp(cd->file_name);

At least your use of P_tmpdir was to generate a saner template, instead
of trying to use the inherently-broken tempnam().

-- 
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: 621 bytes --]

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Qiao Nuohan]

Sorry for replying late.

On 06/20/2013 04:57 PM, Stefan Hajnoczi wrote:
>
> Please link to the code that writes DISKDUMP kdump files on a physical
> machine.  I only see the crash utility code to read the DISKDUMP code
> but I haven't found anything in the Linux kernel, the crash utility, or
> the kexec-utils code to actually write a DISKDUMP file.

I think you can refer to the following URL, and the kdump-compressed format is
described in the file called IMPLEMENTATION.

http://sourceforge.net/projects/makedumpfile/.

>
> I understand why you need temporary files, but my questions stand:
>
> Have you looked at using ELF more efficiently instead of duplicating
> kdump code into QEMU?  kdump is not a great format for the problem
> you're trying to solve - you're not filling in the Linux-specific
> metadata and it's a pain to write due to its layout.
>
> Why can't you simply omit zero pages from the ELF?
>
> Why can't you compress the entire ELF file and add straightforward
> decompression to the crash utility?
>

As I have said, the main purpose of this work is *reducing* the *size* of dump
file to make delivering dump file more conveniently.

Compared with migration, "memory only" dump has a feature regression without
compression and excluding zero pages. So the regression makes me feel it is
necessary to make these patches.


You asked about using ELF more efficiently. For implementing *excluding zero*
pages, *PT_LOAD* can be made use of. p_memsz and p_filesz fields of PT_LOAD
entry are used to describe memory size and the size of corresponding data in
dump file respectively. Blocks only get zero pages in it will have *p_filesz*
set to 0.

However, such implementation faces a problem: the number of PT_LOAD may
*exceed* the range. As zero pages occur *arbitrarily*, the number of PT_LOAD,
at the worst case, may reach the number of the total physical page frames. For
example, 256MB have 2^16 page frames may exceed the range of e_phnum. Although
sh_info is extended to store PT_LOAD when e_phnum is not enough, sh_info that
has 2^32 range may also exceed if the guest machine has more-than-16TB physical
memory (it won't occur soon, but it will happen one day).

OTOH, the reason why I chose kdump-compressed format is ELF doesn't support
compression and filtering yet. To implement compression and filtering on ELF,
we need to consider specific ABI among qemu, crash utility and makedumpfile.
After that, another work needs to be done to port them in specific
distributions.

Compared kdump-compressed format with ELF, compression and filtering is
supported and we don't need to modify tools like crash and makedumpfile.
Considering these reasons, kdump-compressed format is better than ELF. It get
more merit.


According to your comments, I think your objection first comes from the
*temporary files*. What if temporary files won't be used? Flatten kdump-
compressed format is supported by crash and makedumpfile which offers a
mechanism to avoid seek in the case of sending data through *pipe*, then I
don't need to cache pages' data in temporary files.

And about the metadata(do you mean VMCOREINFO here?) you pointed out, it
contains debugging information related to kernel memory. The debugging
information is useful if present, because we can use it to filter more kinds of
memory. But now we only need to exclude zero pages and there's formal mechanism
between qemu and linux kernel, so without metadata can also satisfy our need.

Think over all of the above, I still chose kdump-compressed format. What's your 
opinion about this?

-- 
Regards
Qiao Nuohan

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Stefan Hajnoczi]

On Thu, Jun 27, 2013 at 03:11:09PM +0800, Qiao Nuohan wrote:
> You asked about using ELF more efficiently. For implementing *excluding zero*
> pages, *PT_LOAD* can be made use of. p_memsz and p_filesz fields of PT_LOAD
> entry are used to describe memory size and the size of corresponding data in
> dump file respectively. Blocks only get zero pages in it will have *p_filesz*
> set to 0.
> 
> However, such implementation faces a problem: the number of PT_LOAD may
> *exceed* the range. As zero pages occur *arbitrarily*, the number of PT_LOAD,
> at the worst case, may reach the number of the total physical page frames. For
> example, 256MB have 2^16 page frames may exceed the range of e_phnum. Although
> sh_info is extended to store PT_LOAD when e_phnum is not enough, sh_info that
> has 2^32 range may also exceed if the guest machine has more-than-16TB physical
> memory (it won't occur soon, but it will happen one day).

A simple patch to do this would scan memory for zero regions
write_elf_loads().  It could avoid a huge number of PT_LOAD entries by
setting a minimum threshold like 64 KB or 128 KB.  This would eliminate
the big zero regions from the ELF file.

Of course it doesn't compress the memory contents so it's still going to
be bigger than a compressed kdump FILEDUMP.

The interesting question is how effective this approach is.  If it's
good enough then it would be a fairly simple modification to dump.c.

> OTOH, the reason why I chose kdump-compressed format is ELF doesn't support
> compression and filtering yet. To implement compression and filtering on ELF,
> we need to consider specific ABI among qemu, crash utility and makedumpfile.
> After that, another work needs to be done to port them in specific
> distributions.
> 
> Compared kdump-compressed format with ELF, compression and filtering is
> supported and we don't need to modify tools like crash and makedumpfile.
> Considering these reasons, kdump-compressed format is better than ELF. It get
> more merit.

I meant simply compressing the ELF output during creation.  But I guess
the tools need random access to the ELF file, which is inefficient with
zlib and friends.

> According to your comments, I think your objection first comes from the
> *temporary files*.

I'm not happy with duplicating the kdump FILEDUMP format code into QEMU
because it is relatively big and ugly (temporary files make up a large
part of that).

That said, compressed kdump FILEDUMP might really be the only option
that meets your needs.  I'm just surprised at all the extra work
necessary to make the dump compact.

> What if temporary files won't be used? Flatten kdump-
> compressed format is supported by crash and makedumpfile which offers a
> mechanism to avoid seek in the case of sending data through *pipe*, then I
> don't need to cache pages' data in temporary files.

If it makes the code simpler and smaller it would be nice.

Stefan

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Qiao Nuohan]

On 06/27/2013 04:54 PM, Stefan Hajnoczi wrote:
>
> The interesting question is how effective this approach is.  If it's
> good enough then it would be a fairly simple modification to dump.c.

I see, if excluding zero page in ELF can make a lot of size reduce, it's better
to choose this method. But think over the situation that kernel is on for a
long time, then few zero pages will be in memory, compression will do more work
to reduce size not excluding zero pages. So the approach is not always
effective.

A test on a 1GB memory, and the machine is just on:

size       format         method for reducing memory

1.1GB      ELF            no
1.1GB      kdump          no
227MB      kdump          with all zero pages excluded
  96MB      kdump          compressed with zero pages remained
  88MB      kdump          compressed with zero pages excluded

excluding zero pages does some work, but compression seems to be more effective.

>
> I meant simply compressing the ELF output during creation.  But I guess
> the tools need random access to the ELF file, which is inefficient with
> zlib and friends.

It is the reason of why I don't like entirely compression. It will make crash 
work in terrific bad performance.

>
> I'm not happy with duplicating the kdump FILEDUMP format code into QEMU
> because it is relatively big and ugly (temporary files make up a large
> part of that).

>
> If it makes the code simpler and smaller it would be nice.

That's the point. I will make the code simpler.

Thanks for your comments!

-- 
Regards
Qiao Nuohan

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Stefan Hajnoczi]

On Fri, Jun 28, 2013 at 10:57:28AM +0800, Qiao Nuohan wrote:
> On 06/27/2013 04:54 PM, Stefan Hajnoczi wrote:
> >
> >The interesting question is how effective this approach is.  If it's
> >good enough then it would be a fairly simple modification to dump.c.
> 
> I see, if excluding zero page in ELF can make a lot of size reduce, it's better
> to choose this method. But think over the situation that kernel is on for a
> long time, then few zero pages will be in memory, compression will do more work
> to reduce size not excluding zero pages. So the approach is not always
> effective.
> 
> A test on a 1GB memory, and the machine is just on:
> 
> size       format         method for reducing memory
> 
> 1.1GB      ELF            no
> 1.1GB      kdump          no
> 227MB      kdump          with all zero pages excluded
>  96MB      kdump          compressed with zero pages remained
>  88MB      kdump          compressed with zero pages excluded
> 
> excluding zero pages does some work, but compression seems to be more effective.
[...]
> >If it makes the code simpler and smaller it would be nice.
> 
> That's the point. I will make the code simpler.

I'm now convinced that kdump is worthwhile, thanks for providing data.

It would be nice to see the flattened kdump approach.

Stefan

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Qiao Nuohan]

On 07/01/2013 07:45 PM, Stefan Hajnoczi wrote:
> I'm now convinced that kdump is worthwhile, thanks for providing data.
>
> It would be nice to see the flattened kdump approach.

Hi Stefan and Luiz,

Thanks for Stefan's review again!

I am starting to implement the function using flatten format. And the idea is:

the process will use "seek" to find the place to write if it possible,
otherwise flatten format will be used to avoid "seek" operation.

I send the mail just to make sure if the idea will be accepted by qemu people
and I am spending time on useful thing. Additionally, since the introspection
is not ready yet, the 5th version of my patches will be sent later just to
see if the process of creating kdump-compressed format will be convinced by
qemu people.

P.S.

In flatten format, data will be write to dumpfile block by block, and uses the
following structure to indicate the offset and size of a data block.

struct makedumpfile_data_header {
         int64_t offset;
         int64_t buf_size;
};

For more information, please refer to makedumpfile


http://sourceforge.net/projects/makedumpfile/

-- 
Regards
Qiao Nuohan

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

[Stefan Hajnoczi]

On Wed, Jul 03, 2013 at 03:39:51PM +0800, Qiao Nuohan wrote:
> On 07/01/2013 07:45 PM, Stefan Hajnoczi wrote:
> In flatten format, data will be write to dumpfile block by block, and uses the
> following structure to indicate the offset and size of a data block.
> 
> struct makedumpfile_data_header {
>         int64_t offset;
>         int64_t buf_size;
> };
> 
> For more information, please refer to makedumpfile
> 
> 
> http://sourceforge.net/projects/makedumpfile/

I see.  From the QEMU code perspective this will be simpler.

Stefan