Am 28.05.2013 04:50, schrieb qiaonuo...@cn.fujitsu.com:
> From: Qiao Nuohan <qiaonuo...@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 <qiaonuo...@cn.fujitsu.com>
> Reviewed-by: Zhang Xiaohe <zhan...@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 <qiaonuo...@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 <qiaonuo...@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
