* [Qemu-devel] [PATCH 1/9] Add API to manipulate dump_bitmap
2013-05-07 7:16 [Qemu-devel] [PATCH 0/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format Qiao Nuohan
@ 2013-05-07 7:16 ` Qiao Nuohan
2013-05-07 16:14 ` Eric Blake
2013-05-07 7:16 ` [Qemu-devel] [PATCH 2/9] Add API to manipulate cache_data Qiao Nuohan
` (8 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Qiao Nuohan @ 2013-05-07 7:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Qiao Nuohan
Struct dump_bitmap is associated with a tmp file, and the tmp file can be used
to save data of bitmap in kdump-compressed format temporarily.
The following patch will use these functions to get the 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 | 2 +-
dump_bitmap.c | 162 +++++++++++++++++++++++++++++++++++++++++++++++++
include/dump_bitmap.h | 57 +++++++++++++++++
3 files changed, 220 insertions(+), 1 deletions(-)
create mode 100644 dump_bitmap.c
create mode 100644 include/dump_bitmap.h
diff --git a/Makefile.target b/Makefile.target
index ce4391f..00d4f13 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -112,7 +112,7 @@ obj-$(CONFIG_FDT) += device_tree.o
obj-$(CONFIG_KVM) += kvm-all.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
+obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o dump_bitmap.o
obj-$(CONFIG_NO_GET_MEMORY_MAPPING) += memory_mapping-stub.o
obj-$(CONFIG_NO_CORE_DUMP) += dump-stub.o
LIBS+=$(libs_softmmu)
diff --git a/dump_bitmap.c b/dump_bitmap.c
new file mode 100644
index 0000000..117597f
--- /dev/null
+++ b/dump_bitmap.c
@@ -0,0 +1,162 @@
+/*
+ * QEMU dump bitmap
+ *
+ * Copyright Fujitsu, Corp. 2013
+ *
+ * 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 tmpdir[] = TMP_DIR;
+ char *tmpname;
+
+ /* init the tmp file */
+ tmpname = getenv("TMPDIR");
+ if (!tmpname)
+ tmpname = tmpdir;
+
+ db->file_name = (char *)g_malloc(strlen(filename) + strlen(tmpname) + 1);
+
+ strcpy(db->file_name, tmpname);
+ strcat(db->file_name, "/");
+ strcat(db->file_name, filename);
+
+ if ((fd = mkstemp(db->file_name)) < 0)
+ return -1;
+
+ db->fd = fd;
+ unlink(db->file_name);
+
+ /* init buf, no_block should be set to -1, for nothing is store in buf now */
+ db->no_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->no_block;
+ new_offset = db->offset + BUFSIZE_BITMAP * (pfn / PFN_BUFBITMAP);
+
+ /*
+ * if the block need to be set is not same as the one cached in buf, write the
+ * block back to the tmp file, then cache the new block to the buf
+ */
+ if (0 <= db->no_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->no_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->no_block;
+
+ /* db has not been used yet */
+ if (db->no_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->no_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->no_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..a12c481
--- /dev/null
+++ b/include/dump_bitmap.h
@@ -0,0 +1,57 @@
+/*
+ * QEMU dump bitmap
+ *
+ * Copyright Fujitsu, Corp. 2013
+ *
+ * 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.
+ *
+ */
+
+#define TMP_DIR "/tmp"
+#define BITPERBYTE (8)
+#define BUFSIZE_BITMAP (4096)
+#define PFN_BUFBITMAP (BITPERBYTE * BUFSIZE_BITMAP)
+
+struct dump_bitmap {
+ int fd; /* fd of the tmp file used to store dump bitmap */
+ int no_block; /* number of block 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);
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH 1/9] Add API to manipulate dump_bitmap
2013-05-07 7:16 ` [Qemu-devel] [PATCH 1/9] Add API to manipulate dump_bitmap Qiao Nuohan
@ 2013-05-07 16:14 ` Eric Blake
2013-05-07 16:23 ` Daniel P. Berrange
2013-05-08 8:39 ` qiaonuohan
0 siblings, 2 replies; 21+ messages in thread
From: Eric Blake @ 2013-05-07 16:14 UTC (permalink / raw)
To: Qiao Nuohan; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2058 bytes --]
On 05/07/2013 01:16 AM, Qiao Nuohan wrote:
> Struct dump_bitmap is associated with a tmp file, and the tmp file can be used
> to save data of bitmap in kdump-compressed format temporarily.
> The following patch will use these functions to get the 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>
> ---
> + db->file_name = (char *)g_malloc(strlen(filename) + strlen(tmpname) + 1);
> +
> + strcpy(db->file_name, tmpname);
> + strcat(db->file_name, "/");
> + strcat(db->file_name, filename);
Off-by-one buffer overflow, since you forgot space for the NUL byte. We
use C, not C++, so you don't need to cast the result of g_malloc().
> +++ b/include/dump_bitmap.h
> @@ -0,0 +1,57 @@
> +/*
> + * QEMU dump bitmap
> + *
> + * Copyright Fujitsu, Corp. 2013
> + *
> + * 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.
> + *
> + */
> +
No double-inclusion guard?
> +#define TMP_DIR "/tmp"
Why not reuse P_tmpdir from <stdio.h> instead of reinventing a new name
for this constant?
> +#define BITPERBYTE (8)
Why not use CHAR_BIT from <limits.h> instead of reinventing a new name
for this constant?
> +#define BUFSIZE_BITMAP (4096)
> +#define PFN_BUFBITMAP (BITPERBYTE * BUFSIZE_BITMAP)
> +
> +struct dump_bitmap {
> + int fd; /* fd of the tmp file used to store dump bitmap */
> + int no_block; /* number of block cached in buf */
Trailing whitespace. Run your patch series through scripts/checkpatch.pl.
The name no_block sounds like there aren't any blocks. You probably
want the name num_block instead.
--
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 --]
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH 1/9] Add API to manipulate dump_bitmap
2013-05-07 16:14 ` Eric Blake
@ 2013-05-07 16:23 ` Daniel P. Berrange
2013-05-08 8:39 ` qiaonuohan
1 sibling, 0 replies; 21+ messages in thread
From: Daniel P. Berrange @ 2013-05-07 16:23 UTC (permalink / raw)
To: Eric Blake; +Cc: Qiao Nuohan, qemu-devel
On Tue, May 07, 2013 at 10:14:11AM -0600, Eric Blake wrote:
> On 05/07/2013 01:16 AM, Qiao Nuohan wrote:
> > Struct dump_bitmap is associated with a tmp file, and the tmp file can be used
> > to save data of bitmap in kdump-compressed format temporarily.
> > The following patch will use these functions to get the 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>
> > ---
>
> > + db->file_name = (char *)g_malloc(strlen(filename) + strlen(tmpname) + 1);
> > +
> > + strcpy(db->file_name, tmpname);
> > + strcat(db->file_name, "/");
> > + strcat(db->file_name, filename);
>
> Off-by-one buffer overflow, since you forgot space for the NUL byte. We
> use C, not C++, so you don't need to cast the result of g_malloc().
Using g_strdup_printf("%s/%s", tmpname, filename); avoids the
allocation size problems entirely.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH 1/9] Add API to manipulate dump_bitmap
2013-05-07 16:14 ` Eric Blake
2013-05-07 16:23 ` Daniel P. Berrange
@ 2013-05-08 8:39 ` qiaonuohan
1 sibling, 0 replies; 21+ messages in thread
From: qiaonuohan @ 2013-05-08 8:39 UTC (permalink / raw)
To: 'Eric Blake', 'Daniel P. Berrange'; +Cc: qemu-devel
> -----Original Message-----
> From: Eric Blake [mailto:eblake@redhat.com]
> Sent: Wednesday, May 08, 2013 12:14 AM
> To: Qiao Nuohan
> Cc: qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH 1/9] Add API to manipulate dump_bitmap
>
> On 05/07/2013 01:16 AM, Qiao Nuohan wrote:
> > Struct dump_bitmap is associated with a tmp file, and the tmp file can be
> used
> > to save data of bitmap in kdump-compressed format temporarily.
> > The following patch will use these functions to get the 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>
> > ---
>
> > + db->file_name = (char *)g_malloc(strlen(filename) + strlen(tmpname) +
> 1);
> > +
> > + strcpy(db->file_name, tmpname);
> > + strcat(db->file_name, "/");
> > + strcat(db->file_name, filename);
>
> Off-by-one buffer overflow, since you forgot space for the NUL byte. We
> use C, not C++, so you don't need to cast the result of g_malloc().
I will fix it as Daniel suggested.
>
> > +++ b/include/dump_bitmap.h
> > @@ -0,0 +1,57 @@
> > +/*
> > + * QEMU dump bitmap
> > + *
> > + * Copyright Fujitsu, Corp. 2013
> > + *
> > + * 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.
> > + *
> > + */
> > +
>
> No double-inclusion guard?
>
> > +#define TMP_DIR "/tmp"
>
> Why not reuse P_tmpdir from <stdio.h> instead of reinventing a new name
> for this constant?
>
> > +#define BITPERBYTE (8)
>
> Why not use CHAR_BIT from <limits.h> instead of reinventing a new name
> for this constant?
>
> > +#define BUFSIZE_BITMAP (4096)
> > +#define PFN_BUFBITMAP (BITPERBYTE * BUFSIZE_BITMAP)
> > +
> > +struct dump_bitmap {
> > + int fd; /* fd of the tmp file used to store dump
> bitmap */
> > + int no_block; /* number of block cached in buf */
>
> Trailing whitespace. Run your patch series through scripts/checkpatch.pl.
>
> The name no_block sounds like there aren't any blocks. You probably
> want the name num_block instead.
Got it.
Thanks for your comments.
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH 2/9] Add API to manipulate cache_data
2013-05-07 7:16 [Qemu-devel] [PATCH 0/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format Qiao Nuohan
2013-05-07 7:16 ` [Qemu-devel] [PATCH 1/9] Add API to manipulate dump_bitmap Qiao Nuohan
@ 2013-05-07 7:16 ` Qiao Nuohan
2013-05-07 7:16 ` [Qemu-devel] [PATCH 3/9] Move include and struct definition to dump.h Qiao Nuohan
` (7 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Qiao Nuohan @ 2013-05-07 7:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Qiao Nuohan
Struct dump_bitmap is associated with a tmp file, and the tmp file can be used
to save data of page desc and page data in kdump-compressed format temporarily.
The following patch will use these function to get the data of page desc and
page data 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 | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++
include/cache_data.h | 52 +++++++++++++++++++++++
3 files changed, 167 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 00d4f13..b579aff 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -112,7 +112,7 @@ obj-$(CONFIG_FDT) += device_tree.o
obj-$(CONFIG_KVM) += kvm-all.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 dump_bitmap.o
+obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o dump_bitmap.o cache_data.o
obj-$(CONFIG_NO_GET_MEMORY_MAPPING) += memory_mapping-stub.o
obj-$(CONFIG_NO_CORE_DUMP) += dump-stub.o
LIBS+=$(libs_softmmu)
diff --git a/cache_data.c b/cache_data.c
new file mode 100644
index 0000000..8602ef9
--- /dev/null
+++ b/cache_data.c
@@ -0,0 +1,114 @@
+/*
+ * QEMU cache data
+ *
+ * Copyright Fujitsu, Corp. 2013
+ *
+ * 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 tmpdir[] = TMP_DIR;
+ char *tmpname;
+
+ /* init the tmp file */
+ tmpname = getenv("TMPDIR");
+ if (!tmpname)
+ tmpname = tmpdir;
+
+ cd->file_name = (char *)g_malloc(strlen(filename) + strlen(tmpname) + 1);
+
+ strcpy(cd->file_name, tmpname);
+ strcat(cd->file_name, "/");
+ strcat(cd->file_name, filename);
+
+ if ((fd = mkstemp(cd->file_name)) < 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..4d6656f
--- /dev/null
+++ b/include/cache_data.h
@@ -0,0 +1,52 @@
+/*
+ * QEMU cache data
+ *
+ * Copyright Fujitsu, Corp. 2013
+ *
+ * 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.
+ *
+ */
+
+#define TMP_DIR "/tmp"
+#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);
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH 3/9] Move include and struct definition to dump.h
2013-05-07 7:16 [Qemu-devel] [PATCH 0/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format Qiao Nuohan
2013-05-07 7:16 ` [Qemu-devel] [PATCH 1/9] Add API to manipulate dump_bitmap Qiao Nuohan
2013-05-07 7:16 ` [Qemu-devel] [PATCH 2/9] Add API to manipulate cache_data Qiao Nuohan
@ 2013-05-07 7:16 ` Qiao Nuohan
2013-05-11 13:36 ` Andreas Färber
2013-05-07 7:16 ` [Qemu-devel] [PATCH 4/9] Add API to create header of vmcore Qiao Nuohan
` (6 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Qiao Nuohan @ 2013-05-07 7:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Qiao Nuohan
Move includes and definition of struct DumpState into include/sysemu/dump.h.
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
---
dump.c | 30 ------------------------------
include/sysemu/dump.h | 31 +++++++++++++++++++++++++++++++
2 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/dump.c b/dump.c
index c0d3da5..3785a68 100644
--- a/dump.c
+++ b/dump.c
@@ -11,18 +11,7 @@
*
*/
-#include "qemu-common.h"
-#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/sysemu.h"
-#include "sysemu/memory_mapping.h"
-#include "qapi/error.h"
-#include "qmp-commands.h"
static uint16_t cpu_convert_to_target16(uint16_t val, int endian)
{
@@ -57,25 +46,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.h b/include/sysemu/dump.h
index b8c770f..c4c90c4 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -14,12 +14,43 @@
#ifndef DUMP_H
#define DUMP_H
+#include "qemu-common.h"
+#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/sysemu.h"
+#include "sysemu/memory_mapping.h"
+#include "qapi/error.h"
+#include "qmp-commands.h"
+
typedef struct ArchDumpInfo {
int d_machine; /* Architecture */
int d_endian; /* ELFDATA2LSB or ELFDATA2MSB */
int d_class; /* ELFCLASS32 or ELFCLASS64 */
} ArchDumpInfo;
+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;
+
int cpu_get_dump_info(ArchDumpInfo *info);
ssize_t cpu_get_note_size(int class, int machine, int nr_cpus);
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH 3/9] Move include and struct definition to dump.h
2013-05-07 7:16 ` [Qemu-devel] [PATCH 3/9] Move include and struct definition to dump.h Qiao Nuohan
@ 2013-05-11 13:36 ` Andreas Färber
0 siblings, 0 replies; 21+ messages in thread
From: Andreas Färber @ 2013-05-11 13:36 UTC (permalink / raw)
To: Qiao Nuohan; +Cc: qemu-devel, Eduardo Habkost
Am 07.05.2013 09:16, schrieb Qiao Nuohan:
> Move includes and definition of struct DumpState into include/sysemu/dump.h.
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
> ---
> dump.c | 30 ------------------------------
> include/sysemu/dump.h | 31 +++++++++++++++++++++++++++++++
> 2 files changed, 31 insertions(+), 30 deletions(-)
>
> diff --git a/dump.c b/dump.c
> index c0d3da5..3785a68 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -11,18 +11,7 @@
> *
> */
>
> -#include "qemu-common.h"
> -#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/sysemu.h"
> -#include "sysemu/memory_mapping.h"
> -#include "qapi/error.h"
> -#include "qmp-commands.h"
>
> static uint16_t cpu_convert_to_target16(uint16_t val, int endian)
> {
> @@ -57,25 +46,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.h b/include/sysemu/dump.h
> index b8c770f..c4c90c4 100644
> --- a/include/sysemu/dump.h
> +++ b/include/sysemu/dump.h
> @@ -14,12 +14,43 @@
> #ifndef DUMP_H
> #define DUMP_H
>
> +#include "qemu-common.h"
Please place only the needed includes in a header, not qemu-common.h.
This avoids circular includes and resulting problems.
Regards,
Andreas
> +#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/sysemu.h"
> +#include "sysemu/memory_mapping.h"
> +#include "qapi/error.h"
> +#include "qmp-commands.h"
> +
> typedef struct ArchDumpInfo {
> int d_machine; /* Architecture */
> int d_endian; /* ELFDATA2LSB or ELFDATA2MSB */
> int d_class; /* ELFCLASS32 or ELFCLASS64 */
> } ArchDumpInfo;
>
> +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;
> +
> int cpu_get_dump_info(ArchDumpInfo *info);
> ssize_t cpu_get_note_size(int class, int machine, int nr_cpus);
>
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH 4/9] Add API to create header of vmcore
2013-05-07 7:16 [Qemu-devel] [PATCH 0/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format Qiao Nuohan
` (2 preceding siblings ...)
2013-05-07 7:16 ` [Qemu-devel] [PATCH 3/9] Move include and struct definition to dump.h Qiao Nuohan
@ 2013-05-07 7:16 ` Qiao Nuohan
2013-05-07 7:16 ` [Qemu-devel] [PATCH 5/9] Add API to create data of dump bitmap Qiao Nuohan
` (5 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Qiao Nuohan @ 2013-05-07 7:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Qiao Nuohan
Add API to create header, sub header of vmcore in kdump-compressed format.
The data is store in struct DumpState.
The following patch will use this function to gather data of header, then
write them into vmcore.
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
---
dump.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++
include/sysemu/dump.h | 95 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 202 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index 3785a68..183199f 100644
--- a/dump.c
+++ b/dump.c
@@ -671,6 +671,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.h b/include/sysemu/dump.h
index c4c90c4..0dfd4f0 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -26,12 +26,95 @@
#include "qapi/error.h"
#include "qmp-commands.h"
+#include <sys/utsname.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))
+
typedef struct ArchDumpInfo {
int d_machine; /* Architecture */
int d_endian; /* ELFDATA2LSB or ELFDATA2MSB */
int d_class; /* ELFCLASS32 or ELFCLASS64 */
} ArchDumpInfo;
+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 blocks */
+ 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 the 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 blocks */
+ 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 the 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;
@@ -49,6 +132,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;
int cpu_get_dump_info(ArchDumpInfo *info);
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH 5/9] Add API to create data of dump bitmap
2013-05-07 7:16 [Qemu-devel] [PATCH 0/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format Qiao Nuohan
` (3 preceding siblings ...)
2013-05-07 7:16 ` [Qemu-devel] [PATCH 4/9] Add API to create header of vmcore Qiao Nuohan
@ 2013-05-07 7:16 ` Qiao Nuohan
2013-05-07 7:16 ` [Qemu-devel] [PATCH 6/9] Add API to create page Qiao Nuohan
` (4 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Qiao Nuohan @ 2013-05-07 7:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Qiao Nuohan
Add API to get data of the 1st and 2nd dump bitmap and save them into tmp files.
The following patch will use these functions to gather data of dump bitmap,
then write them into vmcore.
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
---
dump.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++
include/sysemu/dump.h | 10 +++++
2 files changed, 108 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index 183199f..ccb7361 100644
--- a/dump.c
+++ b/dump.c
@@ -778,6 +778,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.h b/include/sysemu/dump.h
index 0dfd4f0..0be01e8 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -25,6 +25,7 @@
#include "sysemu/memory_mapping.h"
#include "qapi/error.h"
#include "qmp-commands.h"
+#include "dump_bitmap.h"
#include <sys/utsname.h>
@@ -33,8 +34,13 @@
#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)
typedef struct ArchDumpInfo {
int d_machine; /* Architecture */
@@ -134,14 +140,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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH 6/9] Add API to create page
2013-05-07 7:16 [Qemu-devel] [PATCH 0/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format Qiao Nuohan
` (4 preceding siblings ...)
2013-05-07 7:16 ` [Qemu-devel] [PATCH 5/9] Add API to create data of dump bitmap Qiao Nuohan
@ 2013-05-07 7:16 ` Qiao Nuohan
2013-05-07 7:16 ` [Qemu-devel] [PATCH 7/9] Add API to free buf used by creating header, bitmap and page Qiao Nuohan
` (3 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Qiao Nuohan @ 2013-05-07 7:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Qiao Nuohan
Add API to get data of page desc and page data and save them into tmp files.
The following patch will use these functions to gather data of page desc and
page data, then write them into vmcore
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
---
dump.c | 254 +++++++++++++++++++++++++++++++++++++++++++++++++
include/sysemu/dump.h | 32 ++++++
2 files changed, 286 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index ccb7361..6f86359 100644
--- a/dump.c
+++ b/dump.c
@@ -876,6 +876,260 @@ 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) &&
+ ((size_out = len_buf_out),
+ 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) &&
+ ((size_out = s->page_size),
+ 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) &&
+ ((size_out = len_buf_out_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.h b/include/sysemu/dump.h
index 0be01e8..b61c9ba 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -26,8 +26,23 @@
#include "qapi/error.h"
#include "qmp-commands.h"
#include "dump_bitmap.h"
+#include "cache_data.h"
#include <sys/utsname.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)
@@ -37,10 +52,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))
typedef struct ArchDumpInfo {
int d_machine; /* Architecture */
@@ -121,6 +140,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;
@@ -147,6 +174,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;
@@ -154,6 +182,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;
int cpu_get_dump_info(ArchDumpInfo *info);
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH 7/9] Add API to free buf used by creating header, bitmap and page
2013-05-07 7:16 [Qemu-devel] [PATCH 0/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format Qiao Nuohan
` (5 preceding siblings ...)
2013-05-07 7:16 ` [Qemu-devel] [PATCH 6/9] Add API to create page Qiao Nuohan
@ 2013-05-07 7:16 ` Qiao Nuohan
2013-05-07 7:16 ` [Qemu-devel] [PATCH 8/9] Add API to write header, bitmap and page into vmcore Qiao Nuohan
` (2 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Qiao Nuohan @ 2013-05-07 7:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Qiao Nuohan
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 spaces.
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Reviewed-by: Zhang Xiaohe <zhangxh@cn.fujitsu.com>
---
dump.c | 17 +++++++++++++++++
1 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index 6f86359..c372113 100644
--- a/dump.c
+++ b/dump.c
@@ -1246,6 +1246,23 @@ 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH 8/9] Add API to write header, bitmap and page into vmcore
2013-05-07 7:16 [Qemu-devel] [PATCH 0/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format Qiao Nuohan
` (6 preceding siblings ...)
2013-05-07 7:16 ` [Qemu-devel] [PATCH 7/9] Add API to free buf used by creating header, bitmap and page Qiao Nuohan
@ 2013-05-07 7:16 ` Qiao Nuohan
2013-05-07 7:16 ` [Qemu-devel] [PATCH 9/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format Qiao Nuohan
2013-05-08 9:03 ` [Qemu-devel] [PATCH 0/9] " HATAYAMA Daisuke
9 siblings, 0 replies; 21+ messages in thread
From: Qiao Nuohan @ 2013-05-07 7:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Qiao Nuohan
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 | 247 +++++++++++++++++++++++++++++++++++++++++++++++++
include/sysemu/dump.h | 1 +
2 files changed, 248 insertions(+), 0 deletions(-)
diff --git a/dump.c b/dump.c
index c372113..aa83744 100644
--- a/dump.c
+++ b/dump.c
@@ -66,6 +66,34 @@ 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;
@@ -671,6 +699,225 @@ 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.h b/include/sysemu/dump.h
index b61c9ba..4913468 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -49,6 +49,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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH 9/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format
2013-05-07 7:16 [Qemu-devel] [PATCH 0/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format Qiao Nuohan
` (7 preceding siblings ...)
2013-05-07 7:16 ` [Qemu-devel] [PATCH 8/9] Add API to write header, bitmap and page into vmcore Qiao Nuohan
@ 2013-05-07 7:16 ` Qiao Nuohan
2013-05-07 17:13 ` Eric Blake
2013-05-08 9:03 ` [Qemu-devel] [PATCH 0/9] " HATAYAMA Daisuke
9 siblings, 1 reply; 21+ messages in thread
From: Qiao Nuohan @ 2013-05-07 7:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Qiao Nuohan, Zhang Xiaohe
Make monitor command 'dump-guest-memory' dump in kdump-compressed format.
The command's usage:
dump [-p] protocol [flags] [begin] [length]
With 'flags' set, the core file will be in kdump-compress format, and without
it, the format is ELF. 'flags' can be:
1. FLAG_DUMP_COMPRESS_ZLIB(0x1), compress vmcore with zlib
2. FLAG_DUMP_COMPRESS_LZO(0x2), compress vmcore with lzo
3. FLAG_DUMP_COMPRESS_SNAPPY(0x4), compress vmcore with snappy
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 | 96 ++++++++++++++++++++++++++++++++++++++++++++-----
hmp-commands.hx | 8 +++--
hmp.c | 9 ++++-
include/sysemu/dump.h | 8 ++++
qapi-schema.json | 6 ++-
qmp-commands.hx | 5 ++-
7 files changed, 164 insertions(+), 18 deletions(-)
diff --git a/configure b/configure
index e818e8b..f4415c9 100755
--- a/configure
+++ b/configure
@@ -229,6 +229,8 @@ libusb=""
usb_redir=""
glx=""
zlib="yes"
+lzo="no"
+snappy="no"
guest_agent="yes"
want_tools="yes"
libiscsi=""
@@ -898,6 +900,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"
@@ -1494,6 +1500,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
@@ -3883,6 +3925,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 aa83744..fd81c07 100644
--- a/dump.c
+++ b/dump.c
@@ -1014,6 +1014,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
*/
@@ -1377,12 +1387,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_flags, int64_t flags,
+ 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);
@@ -1437,6 +1449,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_flags) {
+ switch (flags) {
+ case FLAG_DUMP_COMPRESS_ZLIB:
+ s->flag_compress = DUMP_DH_COMPRESSED_ZLIB;
+ break;
+ case FLAG_DUMP_COMPRESS_LZO:
+ s->flag_compress = DUMP_DH_COMPRESSED_LZO;
+ break;
+ case FLAG_DUMP_COMPRESS_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, BITPERBYTE), 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);
}
@@ -1510,15 +1572,22 @@ 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 *file,
+ bool has_flags, int64_t flags,
+ bool has_begin, int64_t begin,
+ bool has_length, int64_t length, Error **errp)
{
const char *p;
int fd = -1;
DumpState *s;
int ret;
+ /* kdump-compressed format is invalid with paging or filter */
+ if (has_flags && (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;
@@ -1550,17 +1619,26 @@ 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_flags, flags, 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_flags) {
+ 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..c278a50 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -992,9 +992,10 @@ 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,flags:i?,begin:i?,length:i?",
+ .params = "[-p] filename [flags] [begin] [length]",
.help = "dump guest memory to file"
+ "\n\t\t\t flags: the type of compression"
"\n\t\t\t begin(optional): the starting physical address"
"\n\t\t\t length(optional): the memory size, in bytes",
.mhandler.cmd = hmp_dump_guest_memory,
@@ -1002,12 +1003,13 @@ ETEXI
STEXI
-@item dump-guest-memory [-p] @var{protocol} @var{begin} @var{length}
+@item dump-guest-memory [-p] @var{protocol} @var{flags} @var{begin} @var{length}
@findex dump-guest-memory
Dump guest memory to @var{protocol}. The file can be processed with crash or
gdb.
filename: dump file name
paging: do paging to get guest's memory mapping
+ flags: an optional parameter which describes the format of dump.
begin: the starting physical address. It's optional, and should be
specified with length together.
length: the memory size, in bytes. It's optional, and should be specified
diff --git a/hmp.c b/hmp.c
index 4fb76ec..c7a7c2f 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1183,12 +1183,17 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
Error *errp = NULL;
int paging = qdict_get_try_bool(qdict, "paging", 0);
const char *file = qdict_get_str(qdict, "filename");
+ bool has_flags = qdict_haskey(qdict, "flags");
bool has_begin = qdict_haskey(qdict, "begin");
bool has_length = qdict_haskey(qdict, "length");
+ int64_t flags = 0;
int64_t begin = 0;
int64_t length = 0;
char *prot;
+ if (has_flags) {
+ flags = qdict_get_int(qdict, "flags");
+ }
if (has_begin) {
begin = qdict_get_int(qdict, "begin");
}
@@ -1198,8 +1203,8 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
prot = g_strconcat("file:", file, NULL);
- qmp_dump_guest_memory(paging, prot, has_begin, begin, has_length, length,
- &errp);
+ qmp_dump_guest_memory(paging, prot, has_flags, flags, has_begin, begin,
+ has_length, length, &errp);
hmp_handle_error(mon, &errp);
g_free(prot);
}
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 4913468..b860ff0 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -38,6 +38,13 @@
#endif
/*
+ * dump format
+ */
+#define FLAG_DUMP_COMPRESS_ZLIB (0x1) /* compressed with zlib */
+#define FLAG_DUMP_COMPRESS_LZO (0x2) /* compressed with lzo */
+#define FLAG_DUMP_COMPRESS_SNAPPY (0x4) /* compressed with snappy */
+
+/*
* flag used in page desc of kdump-compressed format
*/
#define DUMP_DH_COMPRESSED_ZLIB (0x1)
@@ -47,6 +54,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 7797400..b1d0c5d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2410,6 +2410,8 @@
# 2. fd: the protocol starts with "fd:", and the following string
# is the fd's name.
#
+# @flags: #optional if specified, the format of dump file.
+#
# @begin: #optional if specified, the starting physical address.
#
# @length: #optional if specified, the memory size, in bytes. If you don't
@@ -2421,8 +2423,8 @@
# Since: 1.2
##
{ 'command': 'dump-guest-memory',
- 'data': { 'paging': 'bool', 'protocol': 'str', '*begin': 'int',
- '*length': 'int' } }
+ 'data': { 'paging': 'bool', 'protocol': 'str', '*flags': 'int',
+ '*begin': 'int', '*length': 'int' } }
##
# @netdev_add:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ffd130e..93dd61c 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,flags:i?,begin:i?,length:i?",
+ .params = "-p protocol [flags] [begin] [length]",
.help = "dump guest memory to file",
.user_print = monitor_user_noop,
.mhandler.cmd_new = qmp_marshal_input_dump_guest_memory,
@@ -806,6 +806,7 @@ Arguments:
- "paging": do paging to get guest's memory mapping (json-bool)
- "protocol": destination file(started with "file:") or destination file
descriptor (started with "fd:") (json-string)
+- "flags": dump flag. It's optional, and describes the format of dump (json-int)
- "begin": the starting physical address. It's optional, and should be specified
with length together (json-int)
- "length": the memory size, in bytes. It's optional, and should be specified
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH 9/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format
2013-05-07 7:16 ` [Qemu-devel] [PATCH 9/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format Qiao Nuohan
@ 2013-05-07 17:13 ` Eric Blake
2013-05-08 8:50 ` qiaonuohan
0 siblings, 1 reply; 21+ messages in thread
From: Eric Blake @ 2013-05-07 17:13 UTC (permalink / raw)
To: Qiao Nuohan; +Cc: Zhang Xiaohe, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3715 bytes --]
On 05/07/2013 01:16 AM, Qiao Nuohan wrote:
> Make monitor command 'dump-guest-memory' dump in kdump-compressed format.
> The command's usage:
> dump [-p] protocol [flags] [begin] [length]
> With 'flags' set, the core file will be in kdump-compress format, and without
> it, the format is ELF. 'flags' can be:
> 1. FLAG_DUMP_COMPRESS_ZLIB(0x1), compress vmcore with zlib
> 2. FLAG_DUMP_COMPRESS_LZO(0x2), compress vmcore with lzo
> 3. FLAG_DUMP_COMPRESS_SNAPPY(0x4), compress vmcore with snappy
>
> 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>
> +++ b/hmp-commands.hx
> @@ -992,9 +992,10 @@ 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,flags:i?,begin:i?,length:i?",
This is a backwards-incompatible change to HMP ('dump-guest-memory file
0' used to treat 0 as begin, now it treats it as flags), but I don't
care (we've already declared that HMP is unstable). If you want
back-compat, then stick flags last, not in the middle.
See below about the naming and semantics (I hate the name "flags" for a
non-flags item, and making a human user learn flag bit values is a lousy
interface). If you fix QMP to have a sane interface, then HMP should
probably have a similar change.
> + .params = "[-p] filename [flags] [begin] [length]",
> .help = "dump guest memory to file"
> + "\n\t\t\t flags: the type of compression"
That documentation does nothing for me. What types are valid?
> +++ b/qapi-schema.json
> @@ -2410,6 +2410,8 @@
> # 2. fd: the protocol starts with "fd:", and the following string
> # is the fd's name.
> #
> +# @flags: #optional if specified, the format of dump file.
> +#
Missing a "(since 1.6)" tag to declare that it was added after the fact.
We probably also ought to solve the introspection issue before adding
this feature, so that QMP clients like libvirt know when this optional
parameter is available for use.
> # @begin: #optional if specified, the starting physical address.
> #
> # @length: #optional if specified, the memory size, in bytes. If you don't
> @@ -2421,8 +2423,8 @@
> # Since: 1.2
> ##
> { 'command': 'dump-guest-memory',
> - 'data': { 'paging': 'bool', 'protocol': 'str', '*begin': 'int',
> - '*length': 'int' } }
> + 'data': { 'paging': 'bool', 'protocol': 'str', '*flags': 'int',
> + '*begin': 'int', '*length': 'int' } }
EWWWWW - this is a LOUSY interface. Don't make '*flags' an 'int', and
for that matter, don't name it 'flags' (that implies that we can
bitwise-or multiple compressions together, but it really doesn't make
sense to do both lzo and snappy at the same time - compression really
only makes sense as a single format at a time). Name it for what it
represents (compression type), and provide an enum that lists the valid
types. Something like:
{ 'enum': 'DumpGuestMemoryFormat',
'data': [ 'uncompressed', 'zlib', 'lzo', 'snappy' ] }
{ 'command': 'dump-guest-memory',
'data': { '*format': 'DumpGuestMemoryFormat', ... }}
--
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 --]
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH 9/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format
2013-05-07 17:13 ` Eric Blake
@ 2013-05-08 8:50 ` qiaonuohan
2013-05-08 17:16 ` Eric Blake
0 siblings, 1 reply; 21+ messages in thread
From: qiaonuohan @ 2013-05-08 8:50 UTC (permalink / raw)
To: 'Eric Blake'; +Cc: 'Zhang Xiaohe', qemu-devel
> -----Original Message-----
> From: Eric Blake [mailto:eblake@redhat.com]
> Sent: Wednesday, May 08, 2013 1:13 AM
> To: Qiao Nuohan
> Cc: qemu-devel@nongnu.org; Zhang Xiaohe
> Subject: Re: [Qemu-devel] [PATCH 9/9] Make monitor command 'dump-guest-memory'
> dump in kdump-compressed format
>
>
> > + .params = "[-p] filename [flags] [begin] [length]",
> > .help = "dump guest memory to file"
> > + "\n\t\t\t flags: the type of compression"
>
> That documentation does nothing for me. What types are valid?
>
> > +++ b/qapi-schema.json
> > @@ -2410,6 +2410,8 @@
> > # 2. fd: the protocol starts with "fd:", and the following
> string
> > # is the fd's name.
> > #
> > +# @flags: #optional if specified, the format of dump file.
> > +#
>
> Missing a "(since 1.6)" tag to declare that it was added after the fact.
> We probably also ought to solve the introspection issue before adding
> this feature, so that QMP clients like libvirt know when this optional
> parameter is available for use.
Got it.
>
> > # @begin: #optional if specified, the starting physical address.
> > #
> > # @length: #optional if specified, the memory size, in bytes. If you don't
> > @@ -2421,8 +2423,8 @@
> > # Since: 1.2
> > ##
> > { 'command': 'dump-guest-memory',
> > - 'data': { 'paging': 'bool', 'protocol': 'str', '*begin': 'int',
> > - '*length': 'int' } }
> > + 'data': { 'paging': 'bool', 'protocol': 'str', '*flags': 'int',
> > + '*begin': 'int', '*length': 'int' } }
>
> EWWWWW - this is a LOUSY interface. Don't make '*flags' an 'int', and
> for that matter, don't name it 'flags' (that implies that we can
> bitwise-or multiple compressions together, but it really doesn't make
> sense to do both lzo and snappy at the same time - compression really
> only makes sense as a single format at a time). Name it for what it
> represents (compression type), and provide an enum that lists the valid
> types. Something like:
>
> { 'enum': 'DumpGuestMemoryFormat',
> 'data': [ 'uncompressed', 'zlib', 'lzo', 'snappy' ] }
>
> { 'command': 'dump-guest-memory',
> 'data': { '*format': 'DumpGuestMemoryFormat', ... }}
Thanks for your suggestion. I will fix it like:
{ 'enum': 'DumpCompressionFormat',
'data': [ 'zlib', 'lzo', 'snappy' ] }
For zlib is treated as the default compression format, and
'uncompressed' won't be an option.
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH 9/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format
2013-05-08 8:50 ` qiaonuohan
@ 2013-05-08 17:16 ` Eric Blake
2013-05-09 5:27 ` Zhang Xiaohe
0 siblings, 1 reply; 21+ messages in thread
From: Eric Blake @ 2013-05-08 17:16 UTC (permalink / raw)
To: qiaonuohan; +Cc: 'Zhang Xiaohe', qemu-devel
[-- Attachment #1: Type: text/plain, Size: 742 bytes --]
On 05/08/2013 02:50 AM, qiaonuohan wrote:
>
> Thanks for your suggestion. I will fix it like:
>
> { 'enum': 'DumpCompressionFormat',
> 'data': [ 'zlib', 'lzo', 'snappy' ] }
>
> For zlib is treated as the default compression format, and
> 'uncompressed' won't be an option.
No, I was serious that you need to provide 'uncompressed' as an explicit
enum value. It is very annoying to toggle between four states (three
compression formats and a fourth state of no compression) when the
fourth is available only by omitting a parameter. The default MUST be
'uncompressed' for backwards-compatibility, not 'zlib'.
--
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 --]
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH 9/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format
2013-05-08 17:16 ` Eric Blake
@ 2013-05-09 5:27 ` Zhang Xiaohe
2013-05-09 14:51 ` Eric Blake
0 siblings, 1 reply; 21+ messages in thread
From: Zhang Xiaohe @ 2013-05-09 5:27 UTC (permalink / raw)
To: Eric Blake; +Cc: qiaonuohan, qemu-devel
于 2013年05月09日 01:16, Eric Blake 写道:
> On 05/08/2013 02:50 AM, qiaonuohan wrote:
>>
>> Thanks for your suggestion. I will fix it like:
>>
>> { 'enum': 'DumpCompressionFormat',
>> 'data': [ 'zlib', 'lzo', 'snappy' ] }
>>
>> For zlib is treated as the default compression format, and
>> 'uncompressed' won't be an option.
>
> No, I was serious that you need to provide 'uncompressed' as an explicit
> enum value. It is very annoying to toggle between four states (three
> compression formats and a fourth state of no compression) when the
> fourth is available only by omitting a parameter. The default MUST be
> 'uncompressed' for backwards-compatibility, not 'zlib'.
>
We'd like to make sure that we understand you precisely.
The definion is like below:
{ 'enum': 'DumpGuestMemoryFormat',
'data': [ 'uncompressed', 'zlib', 'lzo', 'snappy' ] }
{ 'command': 'dump-guest-memory',
'data': { 'paging': 'bool', 'protocol': 'str', '*begin': 'int',
'*length': 'int', '*format': 'DumpCompressionFormat' } }
'format' is optional:
1. when 'format' is not specified, vmcore will be in ELF format.
2. when 'format' is specified and its parameter is 'uncompressed',
vmcore will be in ELF format as well.
3. when 'format' is specified and its parameter is 'zlib/lzo/snappy',
vmcore will be in kdump-compressed format.
If this is what you suggest, then I don't think it is necessary to
add 'uncompressed'. The backwards-compatibility is assured by case 1,
in which the interface is exactly the same as before. So why do we
add another parameter to do the same thing again?
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH 9/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format
2013-05-09 5:27 ` Zhang Xiaohe
@ 2013-05-09 14:51 ` Eric Blake
2013-05-10 1:21 ` Zhang Xiaohe
0 siblings, 1 reply; 21+ messages in thread
From: Eric Blake @ 2013-05-09 14:51 UTC (permalink / raw)
To: Zhang Xiaohe; +Cc: qiaonuohan, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3262 bytes --]
On 05/08/2013 11:27 PM, Zhang Xiaohe wrote:
> 于 2013年05月09日 01:16, Eric Blake 写道:
>> On 05/08/2013 02:50 AM, qiaonuohan wrote:
>>>
>>> Thanks for your suggestion. I will fix it like:
>>>
>>> { 'enum': 'DumpCompressionFormat',
>>> 'data': [ 'zlib', 'lzo', 'snappy' ] }
>>>
>>> For zlib is treated as the default compression format, and
>>> 'uncompressed' won't be an option.
>>
>> No, I was serious that you need to provide 'uncompressed' as an explicit
>> enum value. It is very annoying to toggle between four states (three
>> compression formats and a fourth state of no compression) when the
>> fourth is available only by omitting a parameter. The default MUST be
>> 'uncompressed' for backwards-compatibility, not 'zlib'.
>>
> We'd like to make sure that we understand you precisely.
>
> The definion is like below:
> { 'enum': 'DumpGuestMemoryFormat',
> 'data': [ 'uncompressed', 'zlib', 'lzo', 'snappy' ] }
>
> { 'command': 'dump-guest-memory',
> 'data': { 'paging': 'bool', 'protocol': 'str', '*begin': 'int',
> '*length': 'int', '*format': 'DumpCompressionFormat' } }
Closer - make sure you use the same type name in both places (the enum
name 'DumpGuestMemoryFormat' is slightly nicer than the command use of
'DumpCompressionFormat'.
>
> 'format' is optional:
> 1. when 'format' is not specified, vmcore will be in ELF format.
> 2. when 'format' is specified and its parameter is 'uncompressed',
> vmcore will be in ELF format as well.
> 3. when 'format' is specified and its parameter is 'zlib/lzo/snappy',
> vmcore will be in kdump-compressed format.
Correct. Or you could use the name 'elf' instead of 'uncompressed', if
that makes more sense - as long as the enum calls out the names you are
supporting.
>
> If this is what you suggest, then I don't think it is necessary to
> add 'uncompressed'. The backwards-compatibility is assured by case 1,
> in which the interface is exactly the same as before. So why do we
> add another parameter to do the same thing again?
Because it is nicer to apps to allow them to explicitly specify the
default. Making 'format' optional allows older apps to still work, but
for newer apps, it is easier to ALWAYS supply the format argument than
it is to make the generation of the format argument conditional on
whether the default behavior is desired.
Trust me - I'm reviewing this as a potential user of your interface.
When I ask for a fourth enum value, it's because I want to use it. When
you keep coming back and complaining that you don't want to provide the
fourth enum value because it is redundant, I feel like you aren't paying
attention to your user base. I also think that implementation-wise, it
will be easier to write your code if you have an enum supplying all four
possibilities, since it is easier to write code that defaults a missing
argument to a known enum value, and then have the rest of your code deal
with enum values, than it is to write code that has to check everywhere
whether the argument is missing (for one value) vs. one of three other
enum values.
--
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 --]
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH 9/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format
2013-05-09 14:51 ` Eric Blake
@ 2013-05-10 1:21 ` Zhang Xiaohe
0 siblings, 0 replies; 21+ messages in thread
From: Zhang Xiaohe @ 2013-05-10 1:21 UTC (permalink / raw)
To: Eric Blake; +Cc: qiaonuohan, qemu-devel
于 2013年05月09日 22:51, Eric Blake 写道:
> On 05/08/2013 11:27 PM, Zhang Xiaohe wrote:
>> 于 2013年05月09日 01:16, Eric Blake 写道:
>>> On 05/08/2013 02:50 AM, qiaonuohan wrote:
>>>>
>>>> Thanks for your suggestion. I will fix it like:
>>>>
>>>> { 'enum': 'DumpCompressionFormat',
>>>> 'data': [ 'zlib', 'lzo', 'snappy' ] }
>>>>
>>>> For zlib is treated as the default compression format, and
>>>> 'uncompressed' won't be an option.
>>>
>>> No, I was serious that you need to provide 'uncompressed' as an explicit
>>> enum value. It is very annoying to toggle between four states (three
>>> compression formats and a fourth state of no compression) when the
>>> fourth is available only by omitting a parameter. The default MUST be
>>> 'uncompressed' for backwards-compatibility, not 'zlib'.
>>>
>> We'd like to make sure that we understand you precisely.
>>
>> The definion is like below:
>> { 'enum': 'DumpGuestMemoryFormat',
>> 'data': [ 'uncompressed', 'zlib', 'lzo', 'snappy' ] }
>>
>> { 'command': 'dump-guest-memory',
>> 'data': { 'paging': 'bool', 'protocol': 'str', '*begin': 'int',
>> '*length': 'int', '*format': 'DumpCompressionFormat' } }
>
> Closer - make sure you use the same type name in both places (the enum
> name 'DumpGuestMemoryFormat' is slightly nicer than the command use of
> 'DumpCompressionFormat'.
>
>>
>> 'format' is optional:
>> 1. when 'format' is not specified, vmcore will be in ELF format.
>> 2. when 'format' is specified and its parameter is 'uncompressed',
>> vmcore will be in ELF format as well.
>> 3. when 'format' is specified and its parameter is 'zlib/lzo/snappy',
>> vmcore will be in kdump-compressed format.
>
> Correct. Or you could use the name 'elf' instead of 'uncompressed', if
> that makes more sense - as long as the enum calls out the names you are
> supporting.
>
>>
>> If this is what you suggest, then I don't think it is necessary to
>> add 'uncompressed'. The backwards-compatibility is assured by case 1,
>> in which the interface is exactly the same as before. So why do we
>> add another parameter to do the same thing again?
>
> Because it is nicer to apps to allow them to explicitly specify the
> default. Making 'format' optional allows older apps to still work, but
> for newer apps, it is easier to ALWAYS supply the format argument than
> it is to make the generation of the format argument conditional on
> whether the default behavior is desired.
>
> Trust me - I'm reviewing this as a potential user of your interface.
> When I ask for a fourth enum value, it's because I want to use it. When
> you keep coming back and complaining that you don't want to provide the
> fourth enum value because it is redundant, I feel like you aren't paying
> attention to your user base. I also think that implementation-wise, it
> will be easier to write your code if you have an enum supplying all four
> possibilities, since it is easier to write code that defaults a missing
> argument to a known enum value, and then have the rest of your code deal
> with enum values, than it is to write code that has to check everywhere
> whether the argument is missing (for one value) vs. one of three other
> enum values.
>
I think you give a good reason and we'll add the forth enum value as
'elf'. Thanks very much for your comments.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH 0/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format
2013-05-07 7:16 [Qemu-devel] [PATCH 0/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format Qiao Nuohan
` (8 preceding siblings ...)
2013-05-07 7:16 ` [Qemu-devel] [PATCH 9/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format Qiao Nuohan
@ 2013-05-08 9:03 ` HATAYAMA Daisuke
9 siblings, 0 replies; 21+ messages in thread
From: HATAYAMA Daisuke @ 2013-05-08 9:03 UTC (permalink / raw)
To: Qiao Nuohan; +Cc: qemu-devel
Please add CC list to Kumagai-san, makedumpfile maintainer, and Dave
Anderson in order to let them know this development.
(2013/05/07 16:16), Qiao Nuohan wrote:
> Hi, all
>
> 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
>
> These patches are used to make 'dump-guest-memory' be able to dump guest's
> memory in kdump-compressed format. Then vmcore can be much smaller, and
> easily be delivered.
>
It should be stressed here that compression feature is *regression*.
> 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.
>
> the kdump-compressed format is:
Rather than detail of format, qemu people should want to know more
general information about kdump-compressed format. They should think
"What this format?" now. At least the following is needed: the
kdump-compressed format is *linux specific* *linux standard* crash dump
format used in kdump framework.
> 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
> | : |
> +------------------------------------------+
Layout itself is important information. Not only describing this here,
it's better to put it anywhere of source code or point at IMPLEMENTATION
file in makedumpfile which describes orignal spec.
--
Thanks.
HATAYAMA, Daisuke
^ permalink raw reply [flat|nested] 21+ messages in thread