[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 03/11] dump-guest-memory: using static DumpSt
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [PATCH v7 03/11] dump-guest-memory: using static DumpState, add DumpStatus |
Date: |
Tue, 16 Feb 2016 21:11:23 +0800 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Tue, Feb 16, 2016 at 01:32:00PM +0100, Andrew Jones wrote:
> On Tue, Feb 16, 2016 at 03:50:53PM +0800, Peter Xu wrote:
> > Instead of malloc/free each time for DumpState, make it
> > static. Added DumpStatus to show status for dump.
>
> I see that the motivation for making DumpState static is for
> dump_in_progress(). DumpState isn't massive, but it isn't tiny
> either, so maybe we should just have a global pointer instead?
> dump_in_progress() can report DUMP_STATUS_NONE when the pointer
> is NULL, or whatever the status is, when it's not.
I chose to use static dump state mostly for the simplicity of
implementation.
If I use a dynamic one, I can save some memory spaces when there is
no dump task, however since it's async dump with dedicated thread, I
need at least a lock to protect the global pointer when to
use/malloc/free it, since there can be concurrent operations (e.g.,
when dump thread finished and trying to free the dump state, one
user might be querying dump status too). So I just chose not to save
the 200+ bytes.
One more thing a static dump state could bring is that, instead of
letting the user know "there is/is't a dump running", we can let him
know something more, like: "no dump started" or "latest dump
finished, with total written bytes XXX". Also, user can always check
for all the information with the last time he/she did the dump
(which I take it as a tiny small enhancement too).
Thanks.
Peter
>
> drew
>
> >
> > This is to be used for detached dump.
> >
> > Signed-off-by: Peter Xu <address@hidden>
> > Reviewed-by: Fam Zheng <address@hidden>
> > ---
> > dump.c | 21 ++++++++++++++++-----
> > include/sysemu/dump.h | 2 ++
> > qapi-schema.json | 18 ++++++++++++++++++
> > 3 files changed, 36 insertions(+), 5 deletions(-)
> >
> > diff --git a/dump.c b/dump.c
> > index c4a62d9..434bc60 100644
> > --- a/dump.c
> > +++ b/dump.c
> > @@ -1442,6 +1442,14 @@ static void get_max_mapnr(DumpState *s)
> > s->max_mapnr = dump_paddr_to_pfn(s, 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)
> > @@ -1676,24 +1684,27 @@ 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;
> > }
> >
> > if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
> > - create_kdump_vmcore(s, errp);
> > + create_kdump_vmcore(s, &local_err);
> > } else {
> > - create_vmcore(s, errp);
> > + create_vmcore(s, &local_err);
> > }
> >
> > + s->status = (local_err ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED);
> > + error_propagate(errp, local_err);
> > +
> > 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 2f04b24..21fc02d 100644
> > --- a/include/sysemu/dump.h
> > +++ b/include/sysemu/dump.h
> > @@ -38,6 +38,7 @@
> >
> > #include "sysemu/dump-arch.h"
> > #include "sysemu/memory_mapping.h"
> > +#include "qapi-types.h"
> >
> > typedef struct QEMU_PACKED MakedumpfileHeader {
> > char signature[16]; /* = "makedumpfile" */
> > @@ -176,6 +177,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 caff580..ccd30c8 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -2219,6 +2219,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
> >
> >
- [Qemu-devel] [PATCH v7 00/11] Add basic "detach" support for dump-guest-memory, Peter Xu, 2016/02/16
- [Qemu-devel] [PATCH v7 01/11] dump-guest-memory: cleanup: removing dump_{error|cleanup}()., Peter Xu, 2016/02/16
- [Qemu-devel] [PATCH v7 02/11] dump-guest-memory: add "detach" flag for QMP/HMP interfaces., Peter Xu, 2016/02/16
- [Qemu-devel] [PATCH v7 03/11] dump-guest-memory: using static DumpState, add DumpStatus, Peter Xu, 2016/02/16
- [Qemu-devel] [PATCH v7 04/11] dump-guest-memory: add dump_in_progress() helper function, Peter Xu, 2016/02/16
- [Qemu-devel] [PATCH v7 05/11] dump-guest-memory: introduce dump_process() helper function., Peter Xu, 2016/02/16
- [Qemu-devel] [PATCH v7 06/11] dump-guest-memory: disable dump when in INMIGRATE state, Peter Xu, 2016/02/16
- [Qemu-devel] [PATCH v7 07/11] dump-guest-memory: add "detach" support, Peter Xu, 2016/02/16
- [Qemu-devel] [PATCH v7 08/11] DumpState: adding total_size and written_size fields, Peter Xu, 2016/02/16
- [Qemu-devel] [PATCH v7 09/11] Dump: add qmp command "query-dump", Peter Xu, 2016/02/16
- [Qemu-devel] [PATCH v7 10/11] Dump: add hmp command "info dump", Peter Xu, 2016/02/16