From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44140) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UpHVt-0004uz-5d for qemu-devel@nongnu.org; Wed, 19 Jun 2013 08:29:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UpHVr-0000FW-4E for qemu-devel@nongnu.org; Wed, 19 Jun 2013 08:29:25 -0400 Received: from cantor2.suse.de ([195.135.220.15]:35231 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UpHVq-0000F6-OD for qemu-devel@nongnu.org; Wed, 19 Jun 2013 08:29:23 -0400 Message-ID: <51C1A421.4030004@suse.de> Date: Wed, 19 Jun 2013 14:29:21 +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-3-git-send-email-qiaonuohan@cn.fujitsu.com> In-Reply-To: <1369709437-24969-3-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 2/9] dump: Add API to manipulate cache_data List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qiaonuohan@cn.fujitsu.com Cc: Kevin Wolf , Stefan Hajnoczi , qemu-devel@nongnu.org, lcapitulino@redhat.com, zhangxh@cn.fujitsu.com, 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 cache_data is associated with a tmp file which is used to store = page > desc and page data in kdump-compressed format temporarily. CacheData please - but I do find the English term "cache data" irritating as it sounds like data about a cache when instead it is about cached data. DataCache? CachedData? Maybe a native English speaker can advise? > The following patch > will use these function to gather data of page and cache them in tmp fi= les. >=20 > Signed-off-by: Qiao Nuohan > Reviewed-by: Zhang Xiaohe > --- > Makefile.target | 2 +- > cache_data.c | 121 ++++++++++++++++++++++++++++++++++++++++++= ++++++++ > include/cache_data.h | 56 +++++++++++++++++++++++ > 3 files changed, 178 insertions(+), 1 deletions(-) > create mode 100644 cache_data.c > create mode 100644 include/cache_data.h >=20 > diff --git a/Makefile.target b/Makefile.target > index 8e557f9..298ec84 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -110,7 +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 dump_bitmap.o cache_data.o Same comment as for dump_bitmap.o: Can we build this only once, please? > 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 > diff --git a/cache_data.c b/cache_data.c > new file mode 100644 > index 0000000..6e91538 > --- /dev/null > +++ b/cache_data.c > @@ -0,0 +1,121 @@ > +/* > + * QEMU cache data > + * > + * Copyright (C) 2013 FUJITSU LIMITED > + * > + * Authors: > + * Qiao Nuohan > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or = later. > + * See the COPYING file in the top-level directory. > + * > + */ > + > +#include "qemu-common.h" > +#include "cache_data.h" > + > +int init_cache_data(struct cache_data *cd, const char *filename) > +{ > + int fd; > + char *tmpname; > + > + /* init the tmp file */ > + tmpname =3D getenv("TMPDIR"); > + if (!tmpname) { > + tmpname =3D (char *)P_tmpdir; P_tmpdir is marked obsolescent in Open Group spec 7. Maybe Erik can comment some more? Did you verify it builds with mingw32/64? (I stumbled over it because I found the variable name odd and didn't see it defined anywhere.) > + } > + > + cd->file_name =3D (char *)g_strdup_printf("%s/%s", tmpname, filena= me); > + > + fd =3D mkstemp(cd->file_name); > + if (fd < 0) { > + return -1; Error **errp? Same below. > + } > + > + cd->fd =3D fd; > + unlink(cd->file_name); > + > + /* init buf */ > + cd->buf_size =3D BUFSIZE_CACHE_DATA; > + cd->cache_size =3D 0; > + cd->buf =3D g_malloc0(BUFSIZE_CACHE_DATA); > + > + cd->offset =3D 0; > + > + return 0; > +} I wonder if it makes sense to introduce this interface instead of going through the block layer, which would offer a number of different backends at least. CC'ing the experts. > + > +int write_cache(struct cache_data *cd, void *buf, size_t size) > +{ > + /* > + * check if the space is enough to cache data, if not write cached > + * data to the tmp file > + */ > + if (cd->cache_size + size > cd->buf_size) { > + if (lseek(cd->fd, cd->offset, SEEK_SET) < 0) { > + return -1; > + } > + > + if (write(cd->fd, cd->buf, cd->cache_size) !=3D cd->cache_size= ) { > + return -1; > + } > + > + cd->offset +=3D cd->cache_size; > + cd->cache_size =3D 0; > + } > + > + memcpy(cd->buf + cd->cache_size, buf, size); > + cd->cache_size +=3D size; > + > + return 0; > +} > + > +int sync_cache(struct cache_data *cd) > +{ > + /* no data is cached in cache_data */ > + if (cd->cache_size =3D=3D 0) { > + return 0; > + } > + > + if (lseek(cd->fd, cd->offset, SEEK_SET) < 0) { > + return -1; > + } > + > + if (write(cd->fd, cd->buf, cd->cache_size) !=3D cd->cache_size) { > + return -1; > + } > + > + cd->offset +=3D 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) !=3D cd->cache_size) { > + return -1; > + } > + > + cd->offset +=3D cd->cache_size; > + > + return 0; > +} > + > +void free_cache_data(struct cache_data *cd) > +{ > + if (cd) { > + if (cd->file_name) { > + g_free(cd->file_name); > + } > + > + if (cd->buf) { > + g_free(cd->buf); > + } > + > + g_free(cd); > + } > +} > diff --git a/include/cache_data.h b/include/cache_data.h > new file mode 100644 > index 0000000..18e8680 > --- /dev/null > +++ b/include/cache_data.h > @@ -0,0 +1,56 @@ > +/* > + * QEMU cache data > + * > + * Copyright (C) 2013 FUJITSU LIMITED > + * > + * Authors: > + * Qiao Nuohan > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or = later. > + * See the COPYING file in the top-level directory. > + * > + */ > + > +#ifndef CACHE_DATA_H > +#define CACHE_DATA_H > + > +#define BUFSIZE_CACHE_DATA (4096 * 4) > + > +struct cache_data { > + int fd; /* fd of the tmp file used to store cache data= */ > + char *file_name; /* name of the tmp file */ > + char *buf; /* used to cache data */ > + size_t buf_size; /* size of the buf */ > + size_t cache_size; /* size of cached data in buf */ > + off_t offset; /* offset of the tmp file */ > +}; CamelCase and typedef please. Regards, Andreas > + > +/* > + * create a tmp file used to store cache data, then init the buf > + */ > +int init_cache_data(struct cache_data *cd, const char *filename); > + > +/* > + * write data to the tmp file, the data may first be cached in the buf= of > + * cache_data > + */ > +int write_cache(struct cache_data *cd, void *buf, size_t size); > + > +/* > + * when cache_data is caching data in the buf, sync_cache is needed to= write the > + * data back to tmp file > + */ > +int sync_cache(struct cache_data *cd); > + > +/* read data from the tmp file to the buf of 'cd', the start place is= set by > + * cd->offset, and the size is set by cd->cache_size. cd->offset is c= hanged > + * automaticlly according to the size of data read this time. > + */ > +int read_cache(struct cache_data *cd); > + > +/* > + * free the space used by cache_data > + */ > +void free_cache_data(struct cache_data *cd); > + > +#endif >=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