Re: [Qemu-devel] [PATCH REPOST 0/2] Add basic "detach" support for dump-guest-memory

[Andrew Jones <drjones@redhat.com>]

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.)
> 
> 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.
> 
> 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.

Thanks,
drew