[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-guest-memory |
Date: |
Tue, 24 Nov 2015 11:10:27 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, 11/24 09:57, Peter Xu wrote:
> On 11/24/2015 01:57 AM, Andrew Jones wrote:
> > On Mon, Nov 23, 2015 at 05:22:29PM +0100, Laszlo Ersek wrote:
> >> On 11/23/15 11:07, Peter Xu wrote:
> >>> Currently, dump-guest-memory supports synchronous operation only. This
> >>> patch
> >>> sets are adding "detach" support for it (just like "migrate -d" for
> >>> migration). When "-d" is provided, dump-guest-memory command will return
> >>> immediately without hanging user. This should be useful when the backend
> >>> storage for the dump file is very slow.
> >>>
> >>> Peter Xu (2):
> >>> dump-guest-memory: add "detach" flag for QMP/HMP interfaces
> >>> dump-guest-memory: add basic "detach" support.
> >>>
> >>> dump.c | 62
> >>> ++++++++++++++++++++++++++++++++++++++++++++++-----
> >>> hmp-commands.hx | 5 +++--
> >>> hmp.c | 3 ++-
> >>> include/sysemu/dump.h | 4 ++++
> >>> qapi-schema.json | 3 ++-
> >>> qmp-commands.hx | 4 ++--
> >>> qmp.c | 9 ++++++++
> >>> vl.c | 3 +++
> >>> 8 files changed, 81 insertions(+), 12 deletions(-)
> >>>
> >>
> >> I'm not seeing anything that would prevent races between the new thread
> >> and any other existing threads that manipulate the MemoryRegion objects
> >> (in response to guest actions), or the guest RAM contents (by way of
> >> executing guest code).
> >>
> >> The dump_init() function has
> >>
> >> if (runstate_is_running()) {
> >> vm_stop(RUN_STATE_SAVE_VM);
> >> s->resume = true;
> >> } else {
> >> s->resume = false;
> >> }
> >>
> >> Whereas dump_cleanup() has:
> >>
> >> if (s->resume) {
> >> vm_start();
> >> }
> >>
> >> If you return control to the QEMU monitor's user before the dump
> >> completes, they could issue the "cont" command, and unleash the VCPU
> >> threads again. (Of course, this is just one example where things could
> >> go wrong.)
>
> Yes, I added the global flag "dump_in_progress_p" to do this. For
> now, what I found might be affected was "dump-guest-memory" itself,
> and "cont". Please check patch 2/2 modification for qmp_cont(). I
> failed to find any other place that might be influenced by this
> asynchronous operation (you are right, maybe it still exists, and it
> might introduce extra bugs, actually that's what I was looking for
> to see whether I missed something in the review session).
What about all the hot-plug commands that changes the memory layout?
Another question is what if user issued "stop" during dump, should you still
resume when dump completes?
>
> >>
> >> Also, the live migration analogy is not a good one IMO. For live
> >> migration, a whole infrastructure exists for tracking asynchronous guest
> >> state changes (dirty bitmap, locking, whatever).
> >>
> >> The good analogy with live migration would be continuous streaming of
> >> guest memory changes into the dump file, until it converges, or a cutoff
> >> is reached (at which point the guest would be frozen, same as now). Of
> >> course, such streaming could generate huge amounts of traffic and
> >> entirely defeat the original purpose.
>
> Yes, I see that migration is much more complex scenario, so that's
> why I am trying to add "basic detach" support, just as I mentioned
> in the patch title. :)
>
> Before doing anything like that complex, I will send a mail asking
> about it, to first know whether we need to do that.
>
> >>
> >> In total, I don't think this is a good idea. I find it possible that
> >> this would lead to QEMU crashes, and/or wildly inconsistent guest memory
> >> images.
> >
> > Despite having already run through both patches giving review comments,
> > I agree with Laszlo. At first blush it seems like a good idea, but I
> > can't think of how it would be safe. Also, an admin can always background
> > the task that invokes the dump if they need that particular terminal
> > back. So, this looks more like a management tool problem to solve, if
> > anything.
> >
> >>
> >> As for the goal itself... People also tend to cope with *kdump* being
> >> slow on physical machines.
> >>
> >> My recommendation would be to use the dump compression feature
> >> (especially lzo and snappy). In my experience, even when people are
> >> aware of their existence, they don't always realize that shrinking the
> >> dump file size by a given factor also shrinks the dumping *time* by the
> >> same factor, provided that the dumping process remains IO-bound even in
> >> the compressed case.
> >>
> >> Which it should, assuming a "very slow storage" -- lzo and snappy are
> >> very CPU-efficient.
> >
> > This has been my experience, i.e. using lzo or snappy tends to be much,
> > much faster.
>
> Sorry that I am not the daily user of dump-guest-memory, so I may
> have not tried to compare how time would save when compression
> techniques are used. Thanks (Drew & Laszlo) to let me know this.
> Actually, what I am coping with is the bz:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1193826
I don't think this mode is very helpful to fix this issue unless there is a way
to query the dump progress.
>
> I just feel like it would be nice to offer something extra, when
> people are using the stdio monitor, they could have another choice
> when dump. Also, this is my first patch to QEMU. That's all I
> thought about.
>
> Thanks you all (especially Drew and Laszlo) for leaving mass review
> comments. After knowing that more than one of you would suggest not
> taking the risk comparing to the feature it brings, I'd totally
> agree to drop this patch.
>
> Thanks.
> Peter
>
> >
> > Thanks,
> > drew
> >
>
- Re: [Qemu-devel] [PATCH REPOST 2/2] dump-guest-memory: add basic "detach" support., (continued)
- [Qemu-devel] [PATCH REPOST 1/2] dump-guest-memory: add "detach" flag for QMP/HMP interfaces, Peter Xu, 2015/11/23
- Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-guest-memory, Eric Blake, 2015/11/23
- Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-guest-memory, Laszlo Ersek, 2015/11/23
- Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-guest-memory, Andrew Jones, 2015/11/23
- Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-guest-memory, Peter Xu, 2015/11/23
- Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-guest-memory,
Fam Zheng <=
- Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-guest-memory, Laszlo Ersek, 2015/11/24
- Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-guest-memory, Fam Zheng, 2015/11/24
- Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-guest-memory, Eric Blake, 2015/11/24
- Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-guest-memory, Fam Zheng, 2015/11/24
- Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-guest-memory, Peter Xu, 2015/11/24
- Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-guest-memory, Fam Zheng, 2015/11/24
- Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-guest-memory, Markus Armbruster, 2015/11/25
- Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-guest-memory, Paolo Bonzini, 2015/11/24
- Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-guest-memory, Laszlo Ersek, 2015/11/24
- Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-guest-memory, Paolo Bonzini, 2015/11/24