Re: [Qemu-devel] [PATCH 10/13 v7] dump: add APIs to operate DataCache

2014-01-23 Thread Laszlo Ersek
one comment below

On 01/17/14 08:46, qiaonuohan wrote:
 DataCache is used to store data temporarily, then the data will be written to
 vmcore. These functions will be called later when writing data of page to
 vmcore.
 
 Signed-off-by: Qiao Nuohan qiaonuo...@cn.fujitsu.com
 Reviewed-by: Laszlo Ersek ler...@redhat.com
 ---
  dump.c|   47 +++
  include/sysemu/dump.h |9 +
  2 files changed, 56 insertions(+), 0 deletions(-)
 
 diff --git a/dump.c b/dump.c
 index 26a1756..fa183cb 100644
 --- a/dump.c
 +++ b/dump.c
 @@ -1173,6 +1173,53 @@ out:
  return ret;
  }
  
 +static void prepare_data_cache(DataCache *data_cache, DumpState *s,
 +   off_t offset)
 +{
 +data_cache-fd = s-fd;
 +data_cache-data_size = 0;
 +data_cache-buf_size = BUFSIZE_DATA_CACHE;
 +data_cache-buf = g_malloc0(BUFSIZE_DATA_CACHE);
 +data_cache-offset = offset;
 +}
 +
 +static int write_cache(DataCache *dc, const void *buf, size_t size,
 +   bool flag_sync)
 +{
 +/*
 + * dc-buf_size should not be less than size, otherwise dc will never be
 + * enough
 + */
 +assert(size = dc-buf_size);
 +
 +/*
 + * if flag_sync is set, synchronize data in dc-buf into vmcore.
 + * otherwise check if the space is enough for caching data in buf, if 
 not,
 + * write the data in dc-buf to dc-fd and reset dc-buf
 + */
 +if ((!flag_sync  dc-data_size + size  dc-buf_size) ||
 +(flag_sync  dc-data_size  0)) {
 +if (write_buffer(dc-fd, dc-offset, dc-buf, dc-data_size)  0) {
 +return -1;
 +}
 +
 +dc-offset += dc-data_size;
 +dc-data_size = 0;
 +}
 +
 +if (!flag_sync) {
 +memcpy(dc-buf + dc-data_size, buf, size);
 +dc-data_size += size;
 +}

You don't need to make this block conditional on !flag_sync. By removing
that restriction, the function would become more general.

But I assume you don't need that -- I believe you feed all data to this
function with flag_sync==false, then call it one last time with
(flag_sync==false  size==0).

Thanks for adding the assert() and for the type change on buf.

My R-b stands.

Laszlo

 +
 +return 0;
 +}
 +
 +static void free_data_cache(DataCache *data_cache)
 +{
 +g_free(data_cache-buf);
 +}
 +
  static ram_addr_t get_start_block(DumpState *s)
  {
  GuestPhysBlock *block;
 diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
 index 6d4d0bc..92a95e4 100644
 --- a/include/sysemu/dump.h
 +++ b/include/sysemu/dump.h
 @@ -41,6 +41,7 @@
  #define DISKDUMP_HEADER_BLOCKS  (1)
  #define BUFSIZE_BITMAP  (TARGET_PAGE_SIZE)
  #define PFN_BUFBITMAP   (CHAR_BIT * BUFSIZE_BITMAP)
 +#define BUFSIZE_DATA_CACHE  (TARGET_PAGE_SIZE * 4)
  
  typedef struct ArchDumpInfo {
  int d_machine;  /* Architecture */
 @@ -142,6 +143,14 @@ typedef struct QEMU_PACKED KdumpSubHeader64 {
  uint64_t max_mapnr_64;  /* header_version 6 and later */
  } KdumpSubHeader64;
  
 +typedef struct DataCache {
 +int fd; /* fd of the file where to write the cached data */
 +uint8_t *buf;   /* buffer for cached data */
 +size_t buf_size;/* size of the buf */
 +size_t data_size;   /* size of cached data in buf */
 +off_t offset;   /* offset of the file */
 +} DataCache;
 +
  struct GuestPhysBlockList; /* memory_mapping.h */
  int cpu_get_dump_info(ArchDumpInfo *info,
const struct GuestPhysBlockList *guest_phys_blocks);
 




[Qemu-devel] [PATCH 10/13 v7] dump: add APIs to operate DataCache

2014-01-17 Thread qiaonuohan
DataCache is used to store data temporarily, then the data will be written to
vmcore. These functions will be called later when writing data of page to
vmcore.

Signed-off-by: Qiao Nuohan qiaonuo...@cn.fujitsu.com
Reviewed-by: Laszlo Ersek ler...@redhat.com
---
 dump.c|   47 +++
 include/sysemu/dump.h |9 +
 2 files changed, 56 insertions(+), 0 deletions(-)

diff --git a/dump.c b/dump.c
index 26a1756..fa183cb 100644
--- a/dump.c
+++ b/dump.c
@@ -1173,6 +1173,53 @@ out:
 return ret;
 }
 
+static void prepare_data_cache(DataCache *data_cache, DumpState *s,
+   off_t offset)
+{
+data_cache-fd = s-fd;
+data_cache-data_size = 0;
+data_cache-buf_size = BUFSIZE_DATA_CACHE;
+data_cache-buf = g_malloc0(BUFSIZE_DATA_CACHE);
+data_cache-offset = offset;
+}
+
+static int write_cache(DataCache *dc, const void *buf, size_t size,
+   bool flag_sync)
+{
+/*
+ * dc-buf_size should not be less than size, otherwise dc will never be
+ * enough
+ */
+assert(size = dc-buf_size);
+
+/*
+ * if flag_sync is set, synchronize data in dc-buf into vmcore.
+ * otherwise check if the space is enough for caching data in buf, if not,
+ * write the data in dc-buf to dc-fd and reset dc-buf
+ */
+if ((!flag_sync  dc-data_size + size  dc-buf_size) ||
+(flag_sync  dc-data_size  0)) {
+if (write_buffer(dc-fd, dc-offset, dc-buf, dc-data_size)  0) {
+return -1;
+}
+
+dc-offset += dc-data_size;
+dc-data_size = 0;
+}
+
+if (!flag_sync) {
+memcpy(dc-buf + dc-data_size, buf, size);
+dc-data_size += size;
+}
+
+return 0;
+}
+
+static void free_data_cache(DataCache *data_cache)
+{
+g_free(data_cache-buf);
+}
+
 static ram_addr_t get_start_block(DumpState *s)
 {
 GuestPhysBlock *block;
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 6d4d0bc..92a95e4 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -41,6 +41,7 @@
 #define DISKDUMP_HEADER_BLOCKS  (1)
 #define BUFSIZE_BITMAP  (TARGET_PAGE_SIZE)
 #define PFN_BUFBITMAP   (CHAR_BIT * BUFSIZE_BITMAP)
+#define BUFSIZE_DATA_CACHE  (TARGET_PAGE_SIZE * 4)
 
 typedef struct ArchDumpInfo {
 int d_machine;  /* Architecture */
@@ -142,6 +143,14 @@ typedef struct QEMU_PACKED KdumpSubHeader64 {
 uint64_t max_mapnr_64;  /* header_version 6 and later */
 } KdumpSubHeader64;
 
+typedef struct DataCache {
+int fd; /* fd of the file where to write the cached data */
+uint8_t *buf;   /* buffer for cached data */
+size_t buf_size;/* size of the buf */
+size_t data_size;   /* size of cached data in buf */
+off_t offset;   /* offset of the file */
+} DataCache;
+
 struct GuestPhysBlockList; /* memory_mapping.h */
 int cpu_get_dump_info(ArchDumpInfo *info,
   const struct GuestPhysBlockList *guest_phys_blocks);
-- 
1.7.1