[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 11:38:58 +0100 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
* 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.
> 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
[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