Re: [Qemu-devel] [RFC 10/29] migration: new property "x-postcopy-fast"

2017-08-01 Thread Peter Xu 
On Tue, Aug 01, 2017 at 09:50:02AM +0100, Dr. David Alan Gilbert wrote:
> * Peter Xu (pet...@redhat.com) wrote:
> > On Mon, Jul 31, 2017 at 07:52:24PM +0100, Dr. David Alan Gilbert wrote:
> > > * Peter Xu (pet...@redhat.com) wrote:
> > > > This provides a way to start postcopy ASAP when migration starts. To do
> > > > this, we need both:
> > > > 
> > > >   -global migration.x-postcopy-ram=on \
> > > >   -global migration.x-postcopy-fast=on
> > > 
> > > Can you explain why this is necessary?  Both sides already know
> > > they're doing a postcopy recovery don't they?
> > 
> > What I wanted to do here is to provide a way to start postcopy at the
> > very beginning (actually it'll possibly start postcopy at the first
> > loop in migration_thread), instead of start postcopy until we trigger
> > it using "migrate_start_postcopy" command.
> > 
> > I used it for easier debugging (so I don't need to type
> > "migrate_start_postcopy" every time when I trigger postcopy
> > migration), meanwhile I think it can also be used when someone really
> > want to start postcopy from the very beginning.
> > 
> > Would such a new parameter makes sense?
> 
> Other than debugging, I don't think there's a real use for it; the
> slight delay between starting migration and triggering postcopy has
> very little cost.

Then let me drop this patch in next version. I do think I should avoid
introducing too many things "for debugging only"...

-- 
Peter Xu

Re: [Qemu-devel] [RFC 10/29] migration: new property "x-postcopy-fast"

2017-08-01 Thread Dr. David Alan Gilbert 
* Peter Xu (pet...@redhat.com) wrote:
> On Mon, Jul 31, 2017 at 07:52:24PM +0100, Dr. David Alan Gilbert wrote:
> > * Peter Xu (pet...@redhat.com) wrote:
> > > This provides a way to start postcopy ASAP when migration starts. To do
> > > this, we need both:
> > > 
> > >   -global migration.x-postcopy-ram=on \
> > >   -global migration.x-postcopy-fast=on
> > 
> > Can you explain why this is necessary?  Both sides already know
> > they're doing a postcopy recovery don't they?
> 
> What I wanted to do here is to provide a way to start postcopy at the
> very beginning (actually it'll possibly start postcopy at the first
> loop in migration_thread), instead of start postcopy until we trigger
> it using "migrate_start_postcopy" command.
> 
> I used it for easier debugging (so I don't need to type
> "migrate_start_postcopy" every time when I trigger postcopy
> migration), meanwhile I think it can also be used when someone really
> want to start postcopy from the very beginning.
> 
> Would such a new parameter makes sense?

Other than debugging, I don't think there's a real use for it; the
slight delay between starting migration and triggering postcopy has
very little cost.

Dave

> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC 10/29] migration: new property "x-postcopy-fast"

2017-07-31 Thread Peter Xu 
On Mon, Jul 31, 2017 at 07:52:24PM +0100, Dr. David Alan Gilbert wrote:
> * Peter Xu (pet...@redhat.com) wrote:
> > This provides a way to start postcopy ASAP when migration starts. To do
> > this, we need both:
> > 
> >   -global migration.x-postcopy-ram=on \
> >   -global migration.x-postcopy-fast=on
> 
> Can you explain why this is necessary?  Both sides already know
> they're doing a postcopy recovery don't they?

What I wanted to do here is to provide a way to start postcopy at the
very beginning (actually it'll possibly start postcopy at the first
loop in migration_thread), instead of start postcopy until we trigger
it using "migrate_start_postcopy" command.

I used it for easier debugging (so I don't need to type
"migrate_start_postcopy" every time when I trigger postcopy
migration), meanwhile I think it can also be used when someone really
want to start postcopy from the very beginning.

Would such a new parameter makes sense?

-- 
Peter Xu

Re: [Qemu-devel] [RFC 10/29] migration: new property "x-postcopy-fast"

2017-07-31 Thread Dr. David Alan Gilbert 
* Peter Xu (pet...@redhat.com) wrote:
> This provides a way to start postcopy ASAP when migration starts. To do
> this, we need both:
> 
>   -global migration.x-postcopy-ram=on \
>   -global migration.x-postcopy-fast=on

Can you explain why this is necessary?  Both sides already know
they're doing a postcopy recovery don't they?

Dave

> 
> Signed-off-by: Peter Xu 
> ---
>  migration/migration.c | 9 -
>  migration/migration.h | 2 ++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 5b2602e..efee87e 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1936,6 +1936,11 @@ bool migrate_colo_enabled(void)
>  return s->enabled_capabilities[MIGRATION_CAPABILITY_X_COLO];
>  }
>  
> +static bool postcopy_should_start(MigrationState *s)
> +{
> +return atomic_read(>start_postcopy) || s->start_postcopy_fast;
> +}
> +
>  /*
>   * Master migration thread on the source VM.
>   * It drives the migration and pumps the data down the outgoing channel.
> @@ -2013,7 +2018,7 @@ static void *migration_thread(void *opaque)
>  if (migrate_postcopy_ram() &&
>  s->state != MIGRATION_STATUS_POSTCOPY_ACTIVE &&
>  pend_nonpost <= threshold_size &&
> -atomic_read(>start_postcopy)) {
> +postcopy_should_start(s)) {
>  
>  if (!postcopy_start(s, _vm_running)) {
>  current_active_state = 
> MIGRATION_STATUS_POSTCOPY_ACTIVE;
> @@ -2170,6 +2175,8 @@ static Property migration_properties[] = {
>   send_configuration, true),
>  DEFINE_PROP_BOOL("send-section-footer", MigrationState,
>   send_section_footer, true),
> +DEFINE_PROP_BOOL("x-postcopy-fast", MigrationState,
> + start_postcopy_fast, false),
>  
>  /* Migration parameters */
>  DEFINE_PROP_INT64("x-compress-level", MigrationState,
> diff --git a/migration/migration.h b/migration/migration.h
> index 70e3094..e902bae 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -113,6 +113,8 @@ struct MigrationState
>  
>  /* Flag set once the migration has been asked to enter postcopy */
>  bool start_postcopy;
> +/* Set the flag if we want to start postcopy ASAP when migration starts 
> */
> +bool start_postcopy_fast;
>  /* Flag set after postcopy has sent the device state */
>  bool postcopy_after_devices;
>  
> -- 
> 2.7.4
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK