diff mbox

[v4,03/11] dump-guest-memory: using static DumpState, add DumpStatus

Message ID 1448976530-15984-4-git-send-email-peterx@redhat.com
State New
Headers show

Commit Message

Peter Xu Dec. 1, 2015, 1:28 p.m. UTC
Instead of malloc/free each time for DumpState, make it
static. Added DumpStatus to show status for dump.

This is to be used for detached dump.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 dump.c                | 20 +++++++++++++++++---
 include/sysemu/dump.h |  2 ++
 qapi-schema.json      | 18 ++++++++++++++++++
 3 files changed, 37 insertions(+), 3 deletions(-)

Comments

Fam Zheng Dec. 2, 2015, 12:46 a.m. UTC | #1
On Tue, 12/01 21:28, Peter Xu wrote:
> Instead of malloc/free each time for DumpState, make it
> static. Added DumpStatus to show status for dump.
> 
> This is to be used for detached dump.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  dump.c                | 20 +++++++++++++++++---
>  include/sysemu/dump.h |  2 ++
>  qapi-schema.json      | 18 ++++++++++++++++++
>  3 files changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/dump.c b/dump.c
> index d79e0ed..f5f3c59 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -1418,6 +1418,14 @@ static void get_max_mapnr(DumpState *s)
>      s->max_mapnr = paddr_to_pfn(last_block->target_end);
>  }
>  
> +static DumpState dump_state_global = { .status = DUMP_STATUS_NONE };
> +
> +static void dump_state_prepare(DumpState *s)
> +{
> +    /* zero the struct, setting status to active */
> +    *s = (DumpState) { .status = DUMP_STATUS_ACTIVE };
> +}
> +
>  static void dump_init(DumpState *s, int fd, bool has_format,
>                        DumpGuestMemoryFormat format, bool paging, bool has_filter,
>                        int64_t begin, int64_t length, Error **errp)
> @@ -1647,13 +1655,14 @@ void qmp_dump_guest_memory(bool paging, const char *file,
>          return;
>      }
>  
> -    s = g_malloc0(sizeof(DumpState));
> +    s = &dump_state_global;
> +    dump_state_prepare(s);
>  
>      dump_init(s, fd, has_format, format, paging, has_begin,
>                begin, length, &local_err);
>      if (local_err) {
> -        g_free(s);
>          error_propagate(errp, local_err);
> +        s->status = DUMP_STATUS_FAILED;
>          return;
>      }
>  
> @@ -1663,8 +1672,13 @@ void qmp_dump_guest_memory(bool paging, const char *file,
>          create_vmcore(s, errp);
>      }
>  
> +    if (*errp) {
> +        s->status = DUMP_STATUS_FAILED;
> +    } else {
> +        s->status = DUMP_STATUS_COMPLETED;
> +    }
> +

To detect error, it's better to use local_err plus error_propagate like a few
lines above. errp _can_ be NULL depending on callers, though in practice qmp
functions should get a non-NULL.

Fam

>      dump_cleanup(s);
> -    g_free(s);
>  }
>  
>  DumpGuestMemoryCapability *qmp_query_dump_guest_memory_capability(Error **errp)
> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
> index 7e4ec5c..affef38 100644
> --- a/include/sysemu/dump.h
> +++ b/include/sysemu/dump.h
> @@ -45,6 +45,7 @@
>  
>  #include "sysemu/dump-arch.h"
>  #include "sysemu/memory_mapping.h"
> +#include "qapi-types.h"
>  
>  typedef struct QEMU_PACKED MakedumpfileHeader {
>      char signature[16];     /* = "makedumpfile" */
> @@ -183,6 +184,7 @@ typedef struct DumpState {
>      off_t offset_page;          /* offset of page part in vmcore */
>      size_t num_dumpable;        /* number of page that can be dumped */
>      uint32_t flag_compress;     /* indicate the compression format */
> +    DumpStatus status;          /* current dump status */
>  } DumpState;
>  
>  uint16_t cpu_to_dump16(DumpState *s, uint16_t val);
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 97c3ac4..691a130 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2139,6 +2139,24 @@
>              '*format': 'DumpGuestMemoryFormat'} }
>  
>  ##
> +# @DumpStatus
> +#
> +# Describe the status of a long-running background guest memory dump.
> +#
> +# @none: no dump-guest-memory has started yet.
> +#
> +# @active: there is one dump running in background.
> +#
> +# @completed: the last dump has finished successfully.
> +#
> +# @failed: the last dump has failed.
> +#
> +# Since 2.6
> +##
> +{ 'enum': 'DumpStatus',
> +  'data': [ 'none', 'active', 'completed', 'failed' ] }
> +
> +##
>  # @DumpGuestMemoryCapability:
>  #
>  # A list of the available formats for dump-guest-memory
> -- 
> 2.4.3
>
Peter Xu Dec. 2, 2015, 3 a.m. UTC | #2
On Wed, Dec 02, 2015 at 08:46:49AM +0800, Fam Zheng wrote:
> On Tue, 12/01 21:28, Peter Xu wrote:
> > +    if (*errp) {
> > +        s->status = DUMP_STATUS_FAILED;
> > +    } else {
> > +        s->status = DUMP_STATUS_COMPLETED;
> > +    }
> > +
> 
> To detect error, it's better to use local_err plus error_propagate like a few
> lines above. errp _can_ be NULL depending on callers, though in practice qmp
> functions should get a non-NULL.

Yes, you are right. I will make sure all the error handlings in the
patch set are using local error variables, and call
error_propagate() afterward.

Thanks.
Peter

> > Fam
diff mbox

Patch

diff --git a/dump.c b/dump.c
index d79e0ed..f5f3c59 100644
--- a/dump.c
+++ b/dump.c
@@ -1418,6 +1418,14 @@  static void get_max_mapnr(DumpState *s)
     s->max_mapnr = paddr_to_pfn(last_block->target_end);
 }
 
+static DumpState dump_state_global = { .status = DUMP_STATUS_NONE };
+
+static void dump_state_prepare(DumpState *s)
+{
+    /* zero the struct, setting status to active */
+    *s = (DumpState) { .status = DUMP_STATUS_ACTIVE };
+}
+
 static void dump_init(DumpState *s, int fd, bool has_format,
                       DumpGuestMemoryFormat format, bool paging, bool has_filter,
                       int64_t begin, int64_t length, Error **errp)
@@ -1647,13 +1655,14 @@  void qmp_dump_guest_memory(bool paging, const char *file,
         return;
     }
 
-    s = g_malloc0(sizeof(DumpState));
+    s = &dump_state_global;
+    dump_state_prepare(s);
 
     dump_init(s, fd, has_format, format, paging, has_begin,
               begin, length, &local_err);
     if (local_err) {
-        g_free(s);
         error_propagate(errp, local_err);
+        s->status = DUMP_STATUS_FAILED;
         return;
     }
 
@@ -1663,8 +1672,13 @@  void qmp_dump_guest_memory(bool paging, const char *file,
         create_vmcore(s, errp);
     }
 
+    if (*errp) {
+        s->status = DUMP_STATUS_FAILED;
+    } else {
+        s->status = DUMP_STATUS_COMPLETED;
+    }
+
     dump_cleanup(s);
-    g_free(s);
 }
 
 DumpGuestMemoryCapability *qmp_query_dump_guest_memory_capability(Error **errp)
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 7e4ec5c..affef38 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -45,6 +45,7 @@ 
 
 #include "sysemu/dump-arch.h"
 #include "sysemu/memory_mapping.h"
+#include "qapi-types.h"
 
 typedef struct QEMU_PACKED MakedumpfileHeader {
     char signature[16];     /* = "makedumpfile" */
@@ -183,6 +184,7 @@  typedef struct DumpState {
     off_t offset_page;          /* offset of page part in vmcore */
     size_t num_dumpable;        /* number of page that can be dumped */
     uint32_t flag_compress;     /* indicate the compression format */
+    DumpStatus status;          /* current dump status */
 } DumpState;
 
 uint16_t cpu_to_dump16(DumpState *s, uint16_t val);
diff --git a/qapi-schema.json b/qapi-schema.json
index 97c3ac4..691a130 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2139,6 +2139,24 @@ 
             '*format': 'DumpGuestMemoryFormat'} }
 
 ##
+# @DumpStatus
+#
+# Describe the status of a long-running background guest memory dump.
+#
+# @none: no dump-guest-memory has started yet.
+#
+# @active: there is one dump running in background.
+#
+# @completed: the last dump has finished successfully.
+#
+# @failed: the last dump has failed.
+#
+# Since 2.6
+##
+{ 'enum': 'DumpStatus',
+  'data': [ 'none', 'active', 'completed', 'failed' ] }
+
+##
 # @DumpGuestMemoryCapability:
 #
 # A list of the available formats for dump-guest-memory