diff mbox

[V6,1/4] net: announce self after vm start

Message ID 20120328054012.34135.77090.stgit@amd-6168-8-1.englab.nay.redhat.com
State New
Headers show

Commit Message

Jason Wang March 28, 2012, 5:40 a.m. UTC 
qemu_announce_self() were moved to vm_start(). This is because we may
want to let guest to send the gratuitous packets. A global variable
need_announce were introduced to record the pending announcement, and
vm_start() would send gratuitous packet depends on this value.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 migration.c |    2 +-
 migration.h |    2 ++
 vl.c        |    5 +++++
 3 files changed, 8 insertions(+), 1 deletions(-)

Comments

Paolo Bonzini March 28, 2012, 7:22 a.m. UTC | #1 
Il 28/03/2012 07:40, Jason Wang ha scritto:
> qemu_announce_self() were moved to vm_start(). This is because we may
> want to let guest to send the gratuitous packets. A global variable
> need_announce were introduced to record the pending announcement, and
> vm_start() would send gratuitous packet depends on this value.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  migration.c |    2 +-
>  migration.h |    2 ++
>  vl.c        |    5 +++++
>  3 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/migration.c b/migration.c
> index 00fa1e3..861cce9 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -88,7 +88,7 @@ void process_incoming_migration(QEMUFile *f)
>          fprintf(stderr, "load of migration failed\n");
>          exit(0);
>      }
> -    qemu_announce_self();
> +    need_announce = true;
>      DPRINTF("successfully loaded vm state\n");
>  
>      /* Make sure all file formats flush their mutable metadata */
> diff --git a/migration.h b/migration.h
> index 372b066..0a31463 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -95,4 +95,6 @@ void migrate_add_blocker(Error *reason);
>   */
>  void migrate_del_blocker(Error *reason);
>  
> +extern bool need_announce;
> +
>  #endif
> diff --git a/vl.c b/vl.c
> index 65f11f2..05ebf57 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -231,6 +231,7 @@ int boot_menu;
>  uint8_t *boot_splash_filedata;
>  int boot_splash_filedata_size;
>  uint8_t qemu_extra_params_fw[2];
> +bool need_announce = false;
>  
>  typedef struct FWBootEntry FWBootEntry;
>  
> @@ -1266,6 +1267,10 @@ void vm_start(void)
>          vm_state_notify(1, RUN_STATE_RUNNING);
>          resume_all_vcpus();
>          monitor_protocol_event(QEVENT_RESUME, NULL);
> +        if (need_announce) {
> +            need_announce = false;
> +            qemu_announce_self();
> +        }
>      }
>  }
>  
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo
Orit Wasserman May 2, 2012, 7:49 a.m. UTC | #2 
On 03/28/2012 07:40 AM, Jason Wang wrote:
> qemu_announce_self() were moved to vm_start(). This is because we may
> want to let guest to send the gratuitous packets. A global variable
> need_announce were introduced to record the pending announcement, and
> vm_start() would send gratuitous packet depends on this value.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  migration.c |    2 +-
>  migration.h |    2 ++
>  vl.c        |    5 +++++
>  3 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/migration.c b/migration.c
> index 00fa1e3..861cce9 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -88,7 +88,7 @@ void process_incoming_migration(QEMUFile *f)
>          fprintf(stderr, "load of migration failed\n");
>          exit(0);
>      }
> -    qemu_announce_self();
> +    need_announce = true;
>      DPRINTF("successfully loaded vm state\n");
>  
>      /* Make sure all file formats flush their mutable metadata */
> diff --git a/migration.h b/migration.h
> index 372b066..0a31463 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -95,4 +95,6 @@ void migrate_add_blocker(Error *reason);
>   */
>  void migrate_del_blocker(Error *reason);
>  
> +extern bool need_announce;
> +
Hi Jason,
I don't like this external flag.
As this is only related to migration I think we can add a new state RUN_STATE_MIG_PRELAUNCH.
In vm_start call qemu_announce_self only if the state was RUN_STATE_MIG_PRELAUNCH.
This will we useful if we will need to do something else when resuming a migrated guest.

Regards,
Orit

>  #endif
> diff --git a/vl.c b/vl.c
> index 65f11f2..05ebf57 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -231,6 +231,7 @@ int boot_menu;
>  uint8_t *boot_splash_filedata;
>  int boot_splash_filedata_size;
>  uint8_t qemu_extra_params_fw[2];
> +bool need_announce = false;
>  
>  typedef struct FWBootEntry FWBootEntry;
>  
> @@ -1266,6 +1267,10 @@ void vm_start(void)
>          vm_state_notify(1, RUN_STATE_RUNNING);
>          resume_all_vcpus();
>          monitor_protocol_event(QEVENT_RESUME, NULL);
> +        if (need_announce) {
> +            need_announce = false;
> +            qemu_announce_self();
> +        }
>      }
>  }
>  
> 
>
Jason Wang May 2, 2012, 7:59 a.m. UTC | #3 
On 05/02/2012 03:49 PM, Orit Wasserman wrote:
> On 03/28/2012 07:40 AM, Jason Wang wrote:
>> qemu_announce_self() were moved to vm_start(). This is because we may
>> want to let guest to send the gratuitous packets. A global variable
>> need_announce were introduced to record the pending announcement, and
>> vm_start() would send gratuitous packet depends on this value.
>>
>> Signed-off-by: Jason Wang<jasowang@redhat.com>
>> ---
>>   migration.c |    2 +-
>>   migration.h |    2 ++
>>   vl.c        |    5 +++++
>>   3 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/migration.c b/migration.c
>> index 00fa1e3..861cce9 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -88,7 +88,7 @@ void process_incoming_migration(QEMUFile *f)
>>           fprintf(stderr, "load of migration failed\n");
>>           exit(0);
>>       }
>> -    qemu_announce_self();
>> +    need_announce = true;
>>       DPRINTF("successfully loaded vm state\n");
>>
>>       /* Make sure all file formats flush their mutable metadata */
>> diff --git a/migration.h b/migration.h
>> index 372b066..0a31463 100644
>> --- a/migration.h
>> +++ b/migration.h
>> @@ -95,4 +95,6 @@ void migrate_add_blocker(Error *reason);
>>    */
>>   void migrate_del_blocker(Error *reason);
>>
>> +extern bool need_announce;
>> +
> Hi Jason,
> I don't like this external flag.
> As this is only related to migration I think we can add a new state RUN_STATE_MIG_PRELAUNCH.
> In vm_start call qemu_announce_self only if the state was RUN_STATE_MIG_PRELAUNCH.
> This will we useful if we will need to do something else when resuming a migrated guest.
>
> Regards,
> Orit
Hi Orit:

No problem, the only reason of using global variable is simplicity, we 
thought to add one new run state in the past. I would add one like what 
you suggested.

Thanks

>>   #endif
>> diff --git a/vl.c b/vl.c
>> index 65f11f2..05ebf57 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -231,6 +231,7 @@ int boot_menu;
>>   uint8_t *boot_splash_filedata;
>>   int boot_splash_filedata_size;
>>   uint8_t qemu_extra_params_fw[2];
>> +bool need_announce = false;
>>
>>   typedef struct FWBootEntry FWBootEntry;
>>
>> @@ -1266,6 +1267,10 @@ void vm_start(void)
>>           vm_state_notify(1, RUN_STATE_RUNNING);
>>           resume_all_vcpus();
>>           monitor_protocol_event(QEVENT_RESUME, NULL);
>> +        if (need_announce) {
>> +            need_announce = false;
>> +            qemu_announce_self();
>> +        }
>>       }
>>   }
>>
>>
>>
>
Paolo Bonzini May 2, 2012, 8:32 a.m. UTC | #4 
Il 02/05/2012 09:59, Jason Wang ha scritto:
> I don't like this external flag. As this is only related to migration
> I think we can add a new state RUN_STATE_MIG_PRELAUNCH. In vm_start
> call qemu_announce_self only if the state was 
> RUN_STATE_MIG_PRELAUNCH. This will we useful if we will need to do
> something else when resuming a migrated guest.

I don't think this global is replacing a runstate.  It is simply saying
that some work has been delayed to the next time the CPU runs.

Perhaps we can instead use a runstate-change notifier and keep the
global hidden to vl.c.

Paolo
diff mbox

Patch

diff --git a/migration.c b/migration.c
index 00fa1e3..861cce9 100644
--- a/migration.c
+++ b/migration.c
@@ -88,7 +88,7 @@  void process_incoming_migration(QEMUFile *f)
         fprintf(stderr, "load of migration failed\n");
         exit(0);
     }
-    qemu_announce_self();
+    need_announce = true;
     DPRINTF("successfully loaded vm state\n");
 
     /* Make sure all file formats flush their mutable metadata */
diff --git a/migration.h b/migration.h
index 372b066..0a31463 100644
--- a/migration.h
+++ b/migration.h
@@ -95,4 +95,6 @@  void migrate_add_blocker(Error *reason);
  */
 void migrate_del_blocker(Error *reason);
 
+extern bool need_announce;
+
 #endif
diff --git a/vl.c b/vl.c
index 65f11f2..05ebf57 100644
--- a/vl.c
+++ b/vl.c
@@ -231,6 +231,7 @@  int boot_menu;
 uint8_t *boot_splash_filedata;
 int boot_splash_filedata_size;
 uint8_t qemu_extra_params_fw[2];
+bool need_announce = false;
 
 typedef struct FWBootEntry FWBootEntry;
 
@@ -1266,6 +1267,10 @@  void vm_start(void)
         vm_state_notify(1, RUN_STATE_RUNNING);
         resume_all_vcpus();
         monitor_protocol_event(QEVENT_RESUME, NULL);
+        if (need_announce) {
+            need_announce = false;
+            qemu_announce_self();
+        }
     }
 }