From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59437) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UpGmb-0006nb-29 for qemu-devel@nongnu.org; Wed, 19 Jun 2013 07:42:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UpGmV-0000mx-TY for qemu-devel@nongnu.org; Wed, 19 Jun 2013 07:42:37 -0400 Received: from cantor2.suse.de ([195.135.220.15]:60739 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UpGmV-0000ms-Hl for qemu-devel@nongnu.org; Wed, 19 Jun 2013 07:42:31 -0400 Message-ID: <51C19924.50804@suse.de> Date: Wed, 19 Jun 2013 13:42:28 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1369709437-24969-1-git-send-email-qiaonuohan@cn.fujitsu.com> <1369709437-24969-2-git-send-email-qiaonuohan@cn.fujitsu.com> In-Reply-To: <1369709437-24969-2-git-send-email-qiaonuohan@cn.fujitsu.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 1/9] dump: Add API to manipulate dump_bitmap List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qiaonuohan@cn.fujitsu.com Cc: qemu-devel@nongnu.org, lcapitulino@redhat.com, zhangxh@cn.fujitsu.com, Paolo Bonzini , kumagai-atsushi@mxc.nes.nec.co.jp, anderson@redhat.com Am 28.05.2013 04:50, schrieb qiaonuohan@cn.fujitsu.com: > From: Qiao Nuohan >=20 > 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 th= ese > functions to gather data of bitmap and cache them into tmp files. >=20 > Signed-off-by: Qiao Nuohan > Reviewed-by: Zhang Xiaohe > --- > 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 +=3D qtest.o > obj-y +=3D hw/ > obj-$(CONFIG_FDT) +=3D device_tree.o > obj-$(CONFIG_KVM) +=3D kvm-all.o > +obj-y +=3D dump_bitmap.o > obj-y +=3D memory.o savevm.o cputlb.o > obj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) +=3D memory_mapping.o > obj-$(CONFIG_HAVE_CORE_DUMP) +=3D 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 > + * > + * 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 =3D getenv("TMPDIR"); > + if (!tmpname) { > + tmpname =3D (char *)P_tmpdir; > + } > + > + db->file_name =3D (char *)g_strdup_printf("%s/%s", tmpname, filena= me); > + > + fd =3D 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 =3D fd; > + unlink(db->file_name); > + > + /* num_block should be set to -1, for nothing is store in buf now = */ > + db->num_block =3D -1; > + memset(db->buf, 0, BUFSIZE_BITMAP); > + /* the tmp file starts to write at the very beginning */ > + db->offset =3D 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 =3D lseek(db->fd, db->offset, SEEK_SET); > + if (ret < 0) { > + return -1; Same here. > + } > + > + offset_bit =3D 0; > + while (offset_bit < len_db) { > + if (write(db->fd, buf, BUFSIZE_BITMAP) !=3D BUFSIZE_BITMAP) { > + return -1; > + } > + > + offset_bit +=3D BUFSIZE_BITMAP; > + } > + > + return 0; > +} > + > +int set_dump_bitmap(struct dump_bitmap *db, unsigned long long pfn, in= t val) > +{ > + int byte, bit; > + off_t old_offset, new_offset; > + old_offset =3D db->offset + BUFSIZE_BITMAP * db->num_block; > + new_offset =3D 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 t= he buf > + */ > + if (0 <=3D db->num_block && old_offset !=3D new_offset) { > + if (lseek(db->fd, old_offset, SEEK_SET) < 0) { > + return -1; Ditto. > + } > + > + if (write(db->fd, db->buf, BUFSIZE_BITMAP) !=3D BUFSIZE_BITMAP= ) { > + return -1; > + } > + } > + > + if (old_offset !=3D new_offset) { > + if (lseek(db->fd, new_offset, SEEK_SET) < 0) { > + return -1; > + } > + > + if (read(db->fd, db->buf, BUFSIZE_BITMAP) !=3D BUFSIZE_BITMAP)= { > + return -1; > + } > + > + db->num_block =3D pfn / PFN_BUFBITMAP; > + } > + > + /* get the exact place of the bit in the buf, set it */ > + byte =3D (pfn % PFN_BUFBITMAP) >> 3; > + bit =3D (pfn % PFN_BUFBITMAP) & 7; > + if (val) { > + db->buf[byte] |=3D 1< + } else { > + db->buf[byte] &=3D ~(1< + } > + > + return 0; > +} > + > +int sync_dump_bitmap(struct dump_bitmap *db) > +{ > + off_t offset; > + offset =3D 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) !=3D 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 =3D=3D 0 || db->num_block !=3D pfn/PFN_BUFBITMAP) { > + offset =3D db->offset + BUFSIZE_BITMAP * (pfn/PFN_BUFBITMAP); > + lseek(db->fd, offset, SEEK_SET); > + read(db->fd, db->buf, BUFSIZE_BITMAP); > + db->num_block =3D 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 > + * > + * 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 b= lock which > + * 'pfn' belongs to, then set 'val' to the bit > + */ > +int set_dump_bitmap(struct dump_bitmap *db, unsigned long long pfn, in= t val); > + > +/* > + * when buf is caching a block, sync_dump_bitmap is needed to write th= e 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 >=20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg