Patchwork [V7,1/5] runstate: introduce prelaunch-migrate state

login
register
mail settings
Submitter Jason Wang
Date March 7, 2013, 8:23 a.m.
Message ID <1362644631-23113-2-git-send-email-jasowang@redhat.com>
Download mbox | patch
Permalink /patch/225762/
State New
Headers show

Comments

Jason Wang - March 7, 2013, 8:23 a.m.
Sometimes, we need track the state when guest is just about to start after
migration. There's not a accurate state available which do this accurately
(consider qemu may started with -S in destination).

So this patch introduces a new state prelaunch-migrate which just tracks this
state, it covers the case both w/ and w/o -S in destination. The first user of
this is the support of doing announce by guest.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 migration.c      |    3 +--
 qapi-schema.json |    5 ++++-
 vl.c             |    4 +++-
 3 files changed, 8 insertions(+), 4 deletions(-)
Eric Blake - March 7, 2013, 3:37 p.m.
On 03/07/2013 01:23 AM, Jason Wang wrote:
> Sometimes, we need track the state when guest is just about to start after
> migration. There's not a accurate state available which do this accurately
> (consider qemu may started with -S in destination).

s/may/may be/

and yes, libvirt _always_ starts qemu with -S in the destination.

> 
> So this patch introduces a new state prelaunch-migrate which just tracks this
> state, it covers the case both w/ and w/o -S in destination. The first user of
> this is the support of doing announce by guest.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  migration.c      |    3 +--
>  qapi-schema.json |    5 ++++-
>  vl.c             |    4 +++-
>  3 files changed, 8 insertions(+), 4 deletions(-)

I'm not sure if this patch will have any negative effects on existing
libvirt migration or state reporting; adding Jirka to cc.

> 
> diff --git a/migration.c b/migration.c
> index 11725ae..ecdf2c5 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -107,10 +107,9 @@ static void process_incoming_migration_co(void *opaque)
>      /* Make sure all file formats flush their mutable metadata */
>      bdrv_invalidate_cache_all();
>  
> +    runstate_set(RUN_STATE_PRELAUNCH_MIGRATE);
>      if (autostart) {
>          vm_start();
> -    } else {
> -        runstate_set(RUN_STATE_PAUSED);
>      }
>  }
>  
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 28b070f..baa6361 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -174,11 +174,14 @@
>  # @suspended: guest is suspended (ACPI S3)
>  #
>  # @watchdog: the watchdog action is configured to pause and has been triggered
> +#
> +# @migrate-prelaunch: migration is completed and QEMU were started with -S

s/were/was/

>  ##
>  { 'enum': 'RunState',
>    'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
>              'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
> -            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] }
> +            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
> +            'prelaunch-migrate'] }
>  
>  ##
>  # @SnapshotInfo
> diff --git a/vl.c b/vl.c
> index c03edf1..5dd2e0e 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -534,7 +534,7 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RUN_STATE_DEBUG, RUN_STATE_RUNNING },
>  
>      { RUN_STATE_INMIGRATE, RUN_STATE_RUNNING },
> -    { RUN_STATE_INMIGRATE, RUN_STATE_PAUSED },
> +    { RUN_STATE_INMIGRATE, RUN_STATE_PRELAUNCH_MIGRATE },
>  
>      { RUN_STATE_INTERNAL_ERROR, RUN_STATE_PAUSED },
>      { RUN_STATE_INTERNAL_ERROR, RUN_STATE_FINISH_MIGRATE },
> @@ -580,6 +580,8 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
>      { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
>  
> +    { RUN_STATE_PRELAUNCH_MIGRATE, RUN_STATE_RUNNING },
> +
>      { RUN_STATE_MAX, RUN_STATE_MAX },
>  };
>  
>
Jiri Denemark - March 11, 2013, 2:31 p.m.
On Thu, Mar 07, 2013 at 08:37:17 -0700, Eric Blake wrote:
> On 03/07/2013 01:23 AM, Jason Wang wrote:
> > Sometimes, we need track the state when guest is just about to start after
> > migration. There's not a accurate state available which do this accurately
> > (consider qemu may started with -S in destination).
> 
> s/may/may be/
> 
> and yes, libvirt _always_ starts qemu with -S in the destination.
> 
> > 
> > So this patch introduces a new state prelaunch-migrate which just tracks this
> > state, it covers the case both w/ and w/o -S in destination. The first user of
> > this is the support of doing announce by guest.
> > 
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > ---
> >  migration.c      |    3 +--
> >  qapi-schema.json |    5 ++++-
> >  vl.c             |    4 +++-
> >  3 files changed, 8 insertions(+), 4 deletions(-)
> 
> I'm not sure if this patch will have any negative effects on existing
> libvirt migration or state reporting; adding Jirka to cc.

I don't see any issues this patch could cause to libvirt. The only place
where we ask qemu for its current state is when we reconnect to existing
qemu processes after libvirtd restart. And the only thing we care about
is whether the guest is running or not. We use our own state information
to detect if we were migrating or not.

Jirka
Jason Wang - March 12, 2013, 3:09 a.m.
On 03/11/2013 10:31 PM, Jiri Denemark wrote:
> On Thu, Mar 07, 2013 at 08:37:17 -0700, Eric Blake wrote:
>> On 03/07/2013 01:23 AM, Jason Wang wrote:
>>> Sometimes, we need track the state when guest is just about to start after
>>> migration. There's not a accurate state available which do this accurately
>>> (consider qemu may started with -S in destination).
>> s/may/may be/
>>
>> and yes, libvirt _always_ starts qemu with -S in the destination.
>>
>>> So this patch introduces a new state prelaunch-migrate which just tracks this
>>> state, it covers the case both w/ and w/o -S in destination. The first user of
>>> this is the support of doing announce by guest.
>>>
>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>> ---
>>>  migration.c      |    3 +--
>>>  qapi-schema.json |    5 ++++-
>>>  vl.c             |    4 +++-
>>>  3 files changed, 8 insertions(+), 4 deletions(-)
>> I'm not sure if this patch will have any negative effects on existing
>> libvirt migration or state reporting; adding Jirka to cc.
> I don't see any issues this patch could cause to libvirt. The only place
> where we ask qemu for its current state is when we reconnect to existing
> qemu processes after libvirtd restart. And the only thing we care about
> is whether the guest is running or not. We use our own state information
> to detect if we were migrating or not.
>
> Jirka
>

Thanks for the checking. Since Michael prefers a device specific method
instead of introducing a new runstate, I plan to use post load and vm
state change handler instead of this new runstate in next version.

Patch

diff --git a/migration.c b/migration.c
index 11725ae..ecdf2c5 100644
--- a/migration.c
+++ b/migration.c
@@ -107,10 +107,9 @@  static void process_incoming_migration_co(void *opaque)
     /* Make sure all file formats flush their mutable metadata */
     bdrv_invalidate_cache_all();
 
+    runstate_set(RUN_STATE_PRELAUNCH_MIGRATE);
     if (autostart) {
         vm_start();
-    } else {
-        runstate_set(RUN_STATE_PAUSED);
     }
 }
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 28b070f..baa6361 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -174,11 +174,14 @@ 
 # @suspended: guest is suspended (ACPI S3)
 #
 # @watchdog: the watchdog action is configured to pause and has been triggered
+#
+# @migrate-prelaunch: migration is completed and QEMU were started with -S
 ##
 { 'enum': 'RunState',
   'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
             'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
-            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] }
+            'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
+            'prelaunch-migrate'] }
 
 ##
 # @SnapshotInfo
diff --git a/vl.c b/vl.c
index c03edf1..5dd2e0e 100644
--- a/vl.c
+++ b/vl.c
@@ -534,7 +534,7 @@  static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_DEBUG, RUN_STATE_RUNNING },
 
     { RUN_STATE_INMIGRATE, RUN_STATE_RUNNING },
-    { RUN_STATE_INMIGRATE, RUN_STATE_PAUSED },
+    { RUN_STATE_INMIGRATE, RUN_STATE_PRELAUNCH_MIGRATE },
 
     { RUN_STATE_INTERNAL_ERROR, RUN_STATE_PAUSED },
     { RUN_STATE_INTERNAL_ERROR, RUN_STATE_FINISH_MIGRATE },
@@ -580,6 +580,8 @@  static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
     { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
 
+    { RUN_STATE_PRELAUNCH_MIGRATE, RUN_STATE_RUNNING },
+
     { RUN_STATE_MAX, RUN_STATE_MAX },
 };