diff mbox

[v4,03/11] savevm: add error parameter to qemu_savevm_state_begin()

Message ID 00094f70bb9dcc5e680e721f0758fdb590e45d81.1364565637.git.phrdina@redhat.com
State New
Headers show

Commit Message

Pavel Hrdina March 29, 2013, 2:12 p.m. UTC 
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 include/sysemu/sysemu.h | 3 ++-
 migration.c             | 2 +-
 savevm.c                | 5 +++--
 3 files changed, 6 insertions(+), 4 deletions(-)

Comments

Markus Armbruster April 9, 2013, 1:34 p.m. UTC | #1 
Pavel Hrdina <phrdina@redhat.com> writes:

> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  include/sysemu/sysemu.h | 3 ++-
>  migration.c             | 2 +-
>  savevm.c                | 5 +++--
>  3 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 6578782..2f35a05 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -74,7 +74,8 @@ void qemu_announce_self(void);
>  
>  bool qemu_savevm_state_blocked(Error **errp);
>  void qemu_savevm_state_begin(QEMUFile *f,
> -                             const MigrationParams *params);
> +                             const MigrationParams *params,
> +                             Error **errp);
>  int qemu_savevm_state_iterate(QEMUFile *f);
>  void qemu_savevm_state_complete(QEMUFile *f);
>  void qemu_savevm_state_cancel(void);
> diff --git a/migration.c b/migration.c
> index 7fb2147..d517dd6 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -505,7 +505,7 @@ static void *migration_thread(void *opaque)
>      bool old_vm_running = false;
>  
>      DPRINTF("beginning savevm\n");
> -    qemu_savevm_state_begin(s->file, &s->params);
> +    qemu_savevm_state_begin(s->file, &s->params, NULL);
>  
>      while (s->state == MIG_STATE_ACTIVE) {
>          int64_t current_time;
> diff --git a/savevm.c b/savevm.c
> index dc1f4a4..56da096 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1731,7 +1731,8 @@ bool qemu_savevm_state_blocked(Error **errp)
>  }
>  
>  void qemu_savevm_state_begin(QEMUFile *f,
> -                             const MigrationParams *params)
> +                             const MigrationParams *params,
> +                             Error **errp)
>  {
>      SaveStateEntry *se;
>      int ret;
> @@ -1921,7 +1922,7 @@ static int qemu_savevm_state(QEMUFile *f)
>      }
>  
>      qemu_mutex_unlock_iothread();
> -    qemu_savevm_state_begin(f, &params);
> +    qemu_savevm_state_begin(f, &params, NULL);
>      qemu_mutex_lock_iothread();
>  
>      while (qemu_file_get_error(f) == 0) {

Unlike PATCH 01+02, this one only adds Error parameters, no actual
errors.  Do they come later in the series?
Markus Armbruster April 9, 2013, 1:37 p.m. UTC | #2 
Markus Armbruster <armbru@redhat.com> writes:

> Pavel Hrdina <phrdina@redhat.com> writes:
>
>> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> ---
>>  include/sysemu/sysemu.h | 3 ++-
>>  migration.c             | 2 +-
>>  savevm.c                | 5 +++--
>>  3 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index 6578782..2f35a05 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -74,7 +74,8 @@ void qemu_announce_self(void);
>>  
>>  bool qemu_savevm_state_blocked(Error **errp);
>>  void qemu_savevm_state_begin(QEMUFile *f,
>> -                             const MigrationParams *params);
>> +                             const MigrationParams *params,
>> +                             Error **errp);
>>  int qemu_savevm_state_iterate(QEMUFile *f);
>>  void qemu_savevm_state_complete(QEMUFile *f);
>>  void qemu_savevm_state_cancel(void);
>> diff --git a/migration.c b/migration.c
>> index 7fb2147..d517dd6 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -505,7 +505,7 @@ static void *migration_thread(void *opaque)
>>      bool old_vm_running = false;
>>  
>>      DPRINTF("beginning savevm\n");
>> -    qemu_savevm_state_begin(s->file, &s->params);
>> +    qemu_savevm_state_begin(s->file, &s->params, NULL);
>>  
>>      while (s->state == MIG_STATE_ACTIVE) {
>>          int64_t current_time;
>> diff --git a/savevm.c b/savevm.c
>> index dc1f4a4..56da096 100644
>> --- a/savevm.c
>> +++ b/savevm.c
>> @@ -1731,7 +1731,8 @@ bool qemu_savevm_state_blocked(Error **errp)
>>  }
>>  
>>  void qemu_savevm_state_begin(QEMUFile *f,
>> -                             const MigrationParams *params)
>> +                             const MigrationParams *params,
>> +                             Error **errp)
>>  {
>>      SaveStateEntry *se;
>>      int ret;
>> @@ -1921,7 +1922,7 @@ static int qemu_savevm_state(QEMUFile *f)
>>      }
>>  
>>      qemu_mutex_unlock_iothread();
>> -    qemu_savevm_state_begin(f, &params);
>> +    qemu_savevm_state_begin(f, &params, NULL);
>>      qemu_mutex_lock_iothread();
>>  
>>      while (qemu_file_get_error(f) == 0) {
>
> Unlike PATCH 01+02, this one only adds Error parameters, no actual
> errors.  Do they come later in the series?

Unlikely, because qemu_savevm_state_begin() can't fail.  Why add an
Error parameter then?
Pavel Hrdina April 9, 2013, 1:47 p.m. UTC | #3 
On 9.4.2013 15:37, Markus Armbruster wrote:
> Markus Armbruster <armbru@redhat.com> writes:
>
>> Pavel Hrdina <phrdina@redhat.com> writes:
>>
>>> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>> ---
>>>   include/sysemu/sysemu.h | 3 ++-
>>>   migration.c             | 2 +-
>>>   savevm.c                | 5 +++--
>>>   3 files changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>>> index 6578782..2f35a05 100644
>>> --- a/include/sysemu/sysemu.h
>>> +++ b/include/sysemu/sysemu.h
>>> @@ -74,7 +74,8 @@ void qemu_announce_self(void);
>>>
>>>   bool qemu_savevm_state_blocked(Error **errp);
>>>   void qemu_savevm_state_begin(QEMUFile *f,
>>> -                             const MigrationParams *params);
>>> +                             const MigrationParams *params,
>>> +                             Error **errp);
>>>   int qemu_savevm_state_iterate(QEMUFile *f);
>>>   void qemu_savevm_state_complete(QEMUFile *f);
>>>   void qemu_savevm_state_cancel(void);
>>> diff --git a/migration.c b/migration.c
>>> index 7fb2147..d517dd6 100644
>>> --- a/migration.c
>>> +++ b/migration.c
>>> @@ -505,7 +505,7 @@ static void *migration_thread(void *opaque)
>>>       bool old_vm_running = false;
>>>
>>>       DPRINTF("beginning savevm\n");
>>> -    qemu_savevm_state_begin(s->file, &s->params);
>>> +    qemu_savevm_state_begin(s->file, &s->params, NULL);
>>>
>>>       while (s->state == MIG_STATE_ACTIVE) {
>>>           int64_t current_time;
>>> diff --git a/savevm.c b/savevm.c
>>> index dc1f4a4..56da096 100644
>>> --- a/savevm.c
>>> +++ b/savevm.c
>>> @@ -1731,7 +1731,8 @@ bool qemu_savevm_state_blocked(Error **errp)
>>>   }
>>>
>>>   void qemu_savevm_state_begin(QEMUFile *f,
>>> -                             const MigrationParams *params)
>>> +                             const MigrationParams *params,
>>> +                             Error **errp)
>>>   {
>>>       SaveStateEntry *se;
>>>       int ret;
>>> @@ -1921,7 +1922,7 @@ static int qemu_savevm_state(QEMUFile *f)
>>>       }
>>>
>>>       qemu_mutex_unlock_iothread();
>>> -    qemu_savevm_state_begin(f, &params);
>>> +    qemu_savevm_state_begin(f, &params, NULL);
>>>       qemu_mutex_lock_iothread();
>>>
>>>       while (qemu_file_get_error(f) == 0) {
>>
>> Unlike PATCH 01+02, this one only adds Error parameters, no actual
>> errors.  Do they come later in the series?
>
> Unlikely, because qemu_savevm_state_begin() can't fail.  Why add an
> Error parameter then?
>

Thanks for pointing this out. I somehow dropped the error message from 
this function and this also applies for the next patch. I'll fix it.
Markus Armbruster April 9, 2013, 1:49 p.m. UTC | #4 
Markus Armbruster <armbru@redhat.com> writes:

> Markus Armbruster <armbru@redhat.com> writes:
>
>> Pavel Hrdina <phrdina@redhat.com> writes:
>>
>>> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>> ---
>>>  include/sysemu/sysemu.h | 3 ++-
>>>  migration.c             | 2 +-
>>>  savevm.c                | 5 +++--
>>>  3 files changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>>> index 6578782..2f35a05 100644
>>> --- a/include/sysemu/sysemu.h
>>> +++ b/include/sysemu/sysemu.h
>>> @@ -74,7 +74,8 @@ void qemu_announce_self(void);
>>>  
>>>  bool qemu_savevm_state_blocked(Error **errp);
>>>  void qemu_savevm_state_begin(QEMUFile *f,
>>> -                             const MigrationParams *params);
>>> +                             const MigrationParams *params,
>>> +                             Error **errp);
>>>  int qemu_savevm_state_iterate(QEMUFile *f);
>>>  void qemu_savevm_state_complete(QEMUFile *f);
>>>  void qemu_savevm_state_cancel(void);
>>> diff --git a/migration.c b/migration.c
>>> index 7fb2147..d517dd6 100644
>>> --- a/migration.c
>>> +++ b/migration.c
>>> @@ -505,7 +505,7 @@ static void *migration_thread(void *opaque)
>>>      bool old_vm_running = false;
>>>  
>>>      DPRINTF("beginning savevm\n");
>>> -    qemu_savevm_state_begin(s->file, &s->params);
>>> +    qemu_savevm_state_begin(s->file, &s->params, NULL);
>>>  
>>>      while (s->state == MIG_STATE_ACTIVE) {
>>>          int64_t current_time;
>>> diff --git a/savevm.c b/savevm.c
>>> index dc1f4a4..56da096 100644
>>> --- a/savevm.c
>>> +++ b/savevm.c
>>> @@ -1731,7 +1731,8 @@ bool qemu_savevm_state_blocked(Error **errp)
>>>  }
>>>  
>>>  void qemu_savevm_state_begin(QEMUFile *f,
>>> -                             const MigrationParams *params)
>>> +                             const MigrationParams *params,
>>> +                             Error **errp)
>>>  {
>>>      SaveStateEntry *se;
>>>      int ret;
>>> @@ -1921,7 +1922,7 @@ static int qemu_savevm_state(QEMUFile *f)
>>>      }
>>>  
>>>      qemu_mutex_unlock_iothread();
>>> -    qemu_savevm_state_begin(f, &params);
>>> +    qemu_savevm_state_begin(f, &params, NULL);
>>>      qemu_mutex_lock_iothread();
>>>  
>>>      while (qemu_file_get_error(f) == 0) {
>>
>> Unlike PATCH 01+02, this one only adds Error parameters, no actual
>> errors.  Do they come later in the series?
>
> Unlikely, because qemu_savevm_state_begin() can't fail.  Why add an
> Error parameter then?

Bear with me, I'm confused...

Actually, it can fail, sort of: it calls qemu_file_set_error() then.
But you don't return that error via your new Error parameter.  Perhaps
that comes later in the series.
diff mbox

Patch

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 6578782..2f35a05 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -74,7 +74,8 @@  void qemu_announce_self(void);
 
 bool qemu_savevm_state_blocked(Error **errp);
 void qemu_savevm_state_begin(QEMUFile *f,
-                             const MigrationParams *params);
+                             const MigrationParams *params,
+                             Error **errp);
 int qemu_savevm_state_iterate(QEMUFile *f);
 void qemu_savevm_state_complete(QEMUFile *f);
 void qemu_savevm_state_cancel(void);
diff --git a/migration.c b/migration.c
index 7fb2147..d517dd6 100644
--- a/migration.c
+++ b/migration.c
@@ -505,7 +505,7 @@  static void *migration_thread(void *opaque)
     bool old_vm_running = false;
 
     DPRINTF("beginning savevm\n");
-    qemu_savevm_state_begin(s->file, &s->params);
+    qemu_savevm_state_begin(s->file, &s->params, NULL);
 
     while (s->state == MIG_STATE_ACTIVE) {
         int64_t current_time;
diff --git a/savevm.c b/savevm.c
index dc1f4a4..56da096 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1731,7 +1731,8 @@  bool qemu_savevm_state_blocked(Error **errp)
 }
 
 void qemu_savevm_state_begin(QEMUFile *f,
-                             const MigrationParams *params)
+                             const MigrationParams *params,
+                             Error **errp)
 {
     SaveStateEntry *se;
     int ret;
@@ -1921,7 +1922,7 @@  static int qemu_savevm_state(QEMUFile *f)
     }
 
     qemu_mutex_unlock_iothread();
-    qemu_savevm_state_begin(f, &params);
+    qemu_savevm_state_begin(f, &params, NULL);
     qemu_mutex_lock_iothread();
 
     while (qemu_file_get_error(f) == 0) {