qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v4 2/3] job: introduce dump guest memory job
@ 2022-08-02  1:44 Wangjing(Hogan) via
  0 siblings, 0 replies; 3+ messages in thread
From: Wangjing(Hogan) via @ 2022-08-02  1:44 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: kwolf@redhat.com, berrange@redhat.com,
	marcandre.lureau@redhat.com, qemu-devel@nongnu.org,
	Wangxin (Alexander)

> Hogan Wang <hogan.wang@huawei.com> writes:
> 
> > There's no way to cancel the current executing dump process, lead to 
> > the virtual machine manager daemon((e.g. libvirtd) cannot restore the 
> > dump job after daemon restart.
> >
> > Introduce dump guest memory job type, and add an optional 'job-id'
> > argument for dump-guest-memory QMP to make use of jobs framework.
> >
> > Signed-off-by: Hogan Wang <hogan.wang@huawei.com>
> > ---
> >  dump/dump-hmp-cmds.c | 12 ++++++++++--
> >  dump/dump.c          |  1 +
> >  qapi/dump.json       |  6 +++++-
> >  qapi/job.json        |  5 ++++-
> >  4 files changed, 20 insertions(+), 4 deletions(-)
> >
> > @@ -62,10 +64,16 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
> >          detach = qdict_get_bool(qdict, "detach");
> >      }
> >  
> > +    if (has_job_id) {
> > +        job_id = qdict_get_str(qdict, "job-id");
> > +    }
> > +
> 
> Simpler:
> 
>        const char *job_id = qdict_get_try_str(qdict, "job-id");
> 
> >      prot = g_strconcat("file:", file, NULL);
> >  
> > -    qmp_dump_guest_memory(paging, prot, true, detach, has_begin, begin,
> > -                          has_length, length, true, dump_format, &err);
> > +    qmp_dump_guest_memory(paging, prot, has_job_id, job_id,
> 
> This becomes
> 
>        qmp_dump_guest_memory(paging, prot, !!job_id, job_id,
> 
> then.
> 
Accept the improvements.

> > --- a/qapi/dump.json
> > +++ b/qapi/dump.json
> > @@ -59,6 +59,9 @@
> >  #            2. fd: the protocol starts with "fd:", and the following string
> >  #               is the fd's name.
> >  #
> > +# @job-id: identifier for the newly-created memory dump job. To be compatible
> > +#          with legacy dump process, @job-id should omitted. (Since 7.2)
> > +#
> 
> I think we need to describe things in more detail.
> 
> What are the behavioral differences between dumping with and without @job-id?
> 
> Why would you want to pass @job-id?  I figure it's to gain the ability to monitor and control dump task with query-job, job-cancel, ...
> 
Thanks for your review comments, I will write the detailed comment for @job-id in patch set for the next version.

> >  # @detach: if true, QMP will return immediately rather than
> >  #          waiting for the dump to finish. The user can track progress
> >  #          using "query-dump". (since 2.6).
> 
> Hmm, does "detach": false make any sense when "job-id" is present?
> 
I had tested in my environment,  "detach": false haven't got any sense when "job-id" is present,cann't cancel and query the job.
I will delete the code condition branch for non-detach dump job.

> Preexisting: @detach's default is undocumented.

Hogan Wang


^ permalink raw reply	[flat|nested] 3+ messages in thread
* [PATCH v4 1/3] dump: support cancel dump process
@ 2022-08-01  8:07 Hogan Wang via
  2022-08-01  8:07 ` [PATCH v4 2/3] job: introduce dump guest memory job Hogan Wang via
  0 siblings, 1 reply; 3+ messages in thread
From: Hogan Wang via @ 2022-08-01  8:07 UTC (permalink / raw)
  To: kwolf, berrange, armbru, marcandre.lureau, qemu-devel
  Cc: wangxinxin.wang, hogan.wang

Break saving pages or dump iterate when dump job in cancel state,
make sure dump process exits as soon as possible.

Signed-off-by: Hogan Wang <hogan.wang@huawei.com>
---
 dump/dump.c           | 23 +++++++++++++++++++++++
 include/sysemu/dump.h |  2 ++
 2 files changed, 25 insertions(+)

diff --git a/dump/dump.c b/dump/dump.c
index 4d9658ffa2..a57c580b12 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -54,6 +54,8 @@ static Error *dump_migration_blocker;
       DIV_ROUND_UP((name_size), 4) +                    \
       DIV_ROUND_UP((desc_size), 4)) * 4)
 
+static bool dump_cancelling(void);
+
 static inline bool dump_is_64bit(DumpState *s)
 {
     return s->dump_info.d_class == ELFCLASS64;
@@ -118,6 +120,10 @@ static int fd_write_vmcore(const void *buf, size_t size, void *opaque)
     DumpState *s = opaque;
     size_t written_size;
 
+    if (dump_cancelling()) {
+        return -ECANCELED;
+    }
+
     written_size = qemu_write_full(s->fd, buf, size);
     if (written_size != size) {
         return -errno;
@@ -627,6 +633,10 @@ static void dump_iterate(DumpState *s, Error **errp)
 
     do {
         block = s->next_block;
+        if (dump_cancelling()) {
+            error_setg(errp, "dump: job cancelled");
+            return;
+        }
 
         size = block->target_end - block->target_start;
         if (s->has_filter) {
@@ -1321,6 +1331,10 @@ static void write_dump_pages(DumpState *s, Error **errp)
      * first page of page section
      */
     while (get_next_page(&block_iter, &pfn_iter, &buf, s)) {
+        if (dump_cancelling()) {
+            error_setg(errp, "dump: job cancelled");
+            goto out;
+        }
         /* check zero page */
         if (buffer_is_zero(buf, s->dump_info.page_size)) {
             ret = write_cache(&page_desc, &pd_zero, sizeof(PageDescriptor),
@@ -1540,6 +1554,15 @@ bool qemu_system_dump_in_progress(void)
     return (qatomic_read(&state->status) == DUMP_STATUS_ACTIVE);
 }
 
+static bool dump_cancelling(void)
+{
+    DumpState *state = &dump_state_global;
+    if (state->job && job_is_cancelled(state->job)) {
+        return true;
+    }
+    return false;
+}
+
 /* calculate total size of memory to be dumped (taking filter into
  * acoount.) */
 static int64_t dump_calculate_size(DumpState *s)
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index ffc2ea1072..41bdbe595f 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -15,6 +15,7 @@
 #define DUMP_H
 
 #include "qapi/qapi-types-dump.h"
+#include "qemu/job.h"
 
 #define MAKEDUMPFILE_SIGNATURE      "makedumpfile"
 #define MAX_SIZE_MDF_HEADER         (4096) /* max size of makedumpfile_header */
@@ -154,6 +155,7 @@ typedef struct DumpState {
     GuestPhysBlockList guest_phys_blocks;
     ArchDumpInfo dump_info;
     MemoryMappingList list;
+    Job *job;
     uint32_t phdr_num;
     uint32_t shdr_num;
     bool resume;
-- 
2.33.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-02  1:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-02  1:44 [PATCH v4 2/3] job: introduce dump guest memory job Wangjing(Hogan) via
  -- strict thread matches above, loose matches on Subject: below --
2022-08-01  8:07 [PATCH v4 1/3] dump: support cancel dump process Hogan Wang via
2022-08-01  8:07 ` [PATCH v4 2/3] job: introduce dump guest memory job Hogan Wang via
2022-08-01 13:01   ` Markus Armbruster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).