[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 3/5] migration: Create load_setup()/cleanup()
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v4 3/5] migration: Create load_setup()/cleanup() methods |
Date: |
Thu, 29 Jun 2017 12:37:59 +0100 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
* Dr. David Alan Gilbert (address@hidden) wrote:
> * Juan Quintela (address@hidden) wrote:
> > "Dr. David Alan Gilbert" <address@hidden> wrote:
> > > * Juan Quintela (address@hidden) wrote:
> > >> We need to do things at load time and at cleanup time.
> > >>
> > >> Signed-off-by: Juan Quintela <address@hidden>
> > >>
> > >> --
> > >>
> > >> Move the printing of the error message so we can print the device
> > >> giving the error.
> > >> Add call to postcopy stuff
> > >> ---
> > >> include/migration/register.h | 2 ++
> > >> migration/savevm.c | 45
> > >> +++++++++++++++++++++++++++++++++++++++++++-
> > >> migration/savevm.h | 1 +
> > >> migration/trace-events | 2 ++
> > >> 4 files changed, 49 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/include/migration/register.h b/include/migration/register.h
> > >> index 938ea2b..a0f1edd 100644
> > >> --- a/include/migration/register.h
> > >> +++ b/include/migration/register.h
> > >> @@ -39,6 +39,8 @@ typedef struct SaveVMHandlers {
> > >> uint64_t *non_postcopiable_pending,
> > >> uint64_t *postcopiable_pending);
> > >> LoadStateHandler *load_state;
> > >> + int (*load_setup)(QEMUFile *f, void *opaque);
> > >> + int (*load_cleanup)(void *opaque);
> > >> } SaveVMHandlers;
> > >>
> > >> int register_savevm_live(DeviceState *dev,
> > >> diff --git a/migration/savevm.c b/migration/savevm.c
> > >> index fee11c5..fdd15fa 100644
> > >> --- a/migration/savevm.c
> > >> +++ b/migration/savevm.c
> > >> @@ -1541,7 +1541,7 @@ static void *postcopy_ram_listen_thread(void
> > >> *opaque)
> > >> * got a bad migration state).
> > >> */
> > >> migration_incoming_state_destroy();
> > >> -
> > >> + qemu_loadvm_state_cleanup();
> > >
> > > Is that order right? It seems wrong to call the cleanup
> > > code after MIS is destroyed.
> > > (The precopy path seems to call mis_destroy at the end of
> > > process_incoming_migration_bh which is much later).
> >
> > we can do either way, for now it don't matters.
> >
> > Once there, it got me thinking that we are doing things in a very
> > "interesting" way on the incoming side:
> >
> > (postcopy)
> >
> > postcopy_ram_incoming_cleanup()
> > migration_incoming_state_destroy()
> > qemu_loadvm_state_cleanup()
> >
> > (Ok, probably it is better to exchange the last two).
> >
> > But I *think* that we should move the postcopy_ram_incoming_cleanup()
> > inside ram_load_cleanup(), no?
>
> postcopy_ram_incoming_cleanup shuts down a thread that's shared across
> all RAMBlock's, so I don't think it can all be merged into
> ram_load_cleanup. You might be able to do the equivalent of the
> cleanup_range function.
Actually that's wrong, we only call ram_load_cleanup once - because
RAM is special and is only register_savevm_live once, not per device.
So yes you probably can do that.
Dave
> > And we don't have a postcopy_ram_incoming_setup() We could put there the
> > mmap of mis->postcopy_tmp_zero_page and mis->largest_page_size, no?
>
> Again that's a single shared zero page, not per RAMBlock.
>
> > I am trying to understand if the postcopy_ram_incoming_init() can be
> > moved soon, but I think no.
>
> Dave
>
> >
> > Later, Juan.
> >
> >
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
[Qemu-devel] [PATCH v4 4/5] migration: Convert ram to use new load_setup()/load_cleanup(), Juan Quintela, 2017/06/28
[Qemu-devel] [PATCH v4 5/5] migration: Make compression_threads use save/load_setup/cleanup(), Juan Quintela, 2017/06/28