[v3] migration: Improve error reporting

[PATCH v3 10/26] migration: Move cleanup after after error reporting in qemu_savevm_state_setup()

Cédric Le Goater posted 26 patches 8 months, 3 weeks ago

Diff against v1 v2 v4 v5
Download mbox

Maintainers: Stefano Stabellini <sstabellini@kernel.org>, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul@xen.org>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, David Gibson <david@gibson.dropbear.id.au>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Alex Williamson <alex.williamson@redhat.com>, Peter Xu <peterx@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Fabiano Rosas <farosas@suse.de>, Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <fam@euphon.net>, Eric Blake <eblake@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>, John Snow <jsnow@redhat.com>, Hyman Huang <yong.huang@smartx.com>

There is a newer version of this series

Expand all Fold all

[PATCH v3 10/26] migration: Move cleanup after after error reporting in qemu_savevm_state_setup()

Posted by Cédric Le Goater 8 months, 3 weeks ago

This will help preserving the error set by .save_setup() handlers.

Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 migration/savevm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 31ce9391d49c825d4ec835e26ac0246e192783a0..e400706e61e06d2d1d03a11aed14f30a243833f2 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1740,10 +1740,10 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
         qemu_savevm_state_complete_precopy(f, false, false);
         ret = qemu_file_get_error(f);
     }
-    qemu_savevm_state_cleanup();
     if (ret != 0) {
         error_setg_errno(errp, -ret, "Error while writing VM state");
     }
+    qemu_savevm_state_cleanup();
 
     if (ret != 0) {
         status = MIGRATION_STATUS_FAILED;
-- 
2.44.0

Re: [PATCH v3 10/26] migration: Move cleanup after after error reporting in qemu_savevm_state_setup()

Posted by Peter Xu 8 months, 3 weeks ago

On Mon, Mar 04, 2024 at 01:28:28PM +0100, Cédric Le Goater wrote:
> This will help preserving the error set by .save_setup() handlers.
> 
> Signed-off-by: Cédric Le Goater <clg@redhat.com>

IIUC this is about the next patch.  I got fully confused before reading
into the next one.  IMHO we can squash it into where it's used.

Thanks,

> ---
>  migration/savevm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 31ce9391d49c825d4ec835e26ac0246e192783a0..e400706e61e06d2d1d03a11aed14f30a243833f2 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1740,10 +1740,10 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
>          qemu_savevm_state_complete_precopy(f, false, false);
>          ret = qemu_file_get_error(f);
>      }
> -    qemu_savevm_state_cleanup();
>      if (ret != 0) {
>          error_setg_errno(errp, -ret, "Error while writing VM state");
>      }
> +    qemu_savevm_state_cleanup();
>  
>      if (ret != 0) {
>          status = MIGRATION_STATUS_FAILED;
> -- 
> 2.44.0
> 
> 

-- 
Peter Xu

Re: [PATCH v3 10/26] migration: Move cleanup after after error reporting in qemu_savevm_state_setup()

Posted by Cédric Le Goater 8 months, 3 weeks ago

On 3/5/24 04:32, Peter Xu wrote:
> On Mon, Mar 04, 2024 at 01:28:28PM +0100, Cédric Le Goater wrote:
>> This will help preserving the error set by .save_setup() handlers.
>>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> 
> IIUC this is about the next patch.  I got fully confused before reading
> into the next one.  IMHO we can squash it into where it's used.

That's where the change was initially ... I thought extracting it in its
own patch would clarify. Oh well nevermind, I will put it back and add
a comment in the commit log.

Thanks,

C.



> 
> Thanks,
> 
>> ---
>>   migration/savevm.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index 31ce9391d49c825d4ec835e26ac0246e192783a0..e400706e61e06d2d1d03a11aed14f30a243833f2 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -1740,10 +1740,10 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
>>           qemu_savevm_state_complete_precopy(f, false, false);
>>           ret = qemu_file_get_error(f);
>>       }
>> -    qemu_savevm_state_cleanup();
>>       if (ret != 0) {
>>           error_setg_errno(errp, -ret, "Error while writing VM state");
>>       }
>> +    qemu_savevm_state_cleanup();
>>   
>>       if (ret != 0) {
>>           status = MIGRATION_STATUS_FAILED;
>> -- 
>> 2.44.0
>>
>>
>