[v4] migration: Improve error reporting

[PATCH v4 24/25] vfio: Also trace event failures in vfio_save_complete_precopy()

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

Diff against v1 v2 v3 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>, Thomas Huth <thuth@redhat.com>, Eric Farman <farman@linux.ibm.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 v4 24/25] vfio: Also trace event failures in vfio_save_complete_precopy()

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

vfio_save_complete_precopy() currently returns before doing the trace
event. Change that.

Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/vfio/migration.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index bd48f2ee472a5230c2c84bff829dae1e217db33f..c8aeb43b4249ec76ded2542d62792e8c469d5f97 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -580,9 +580,6 @@ static int vfio_save_complete_precopy(QEMUFile *f, void *opaque)
 
     qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
     ret = qemu_file_get_error(f);
-    if (ret) {
-        return ret;
-    }
 
     trace_vfio_save_complete_precopy(vbasedev->name, ret);
 
-- 
2.44.0

Re: [PATCH v4 24/25] vfio: Also trace event failures in vfio_save_complete_precopy()

Posted by Eric Auger 8 months, 3 weeks ago


On 3/6/24 14:34, Cédric Le Goater wrote:
> vfio_save_complete_precopy() currently returns before doing the trace
> event. Change that.
> 
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>  hw/vfio/migration.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index bd48f2ee472a5230c2c84bff829dae1e217db33f..c8aeb43b4249ec76ded2542d62792e8c469d5f97 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -580,9 +580,6 @@ static int vfio_save_complete_precopy(QEMUFile *f, void *opaque)
>  
>      qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
>      ret = qemu_file_get_error(f);
> -    if (ret) {
> -        return ret;
> -    }
>  
>      trace_vfio_save_complete_precopy(vbasedev->name, ret);
it is arguable if you want to trace if an error occured. If you want to
unconditionally trace the function entry, want don't we put the trace at
the beginning of the function?

Eric
>

Re: [PATCH v4 24/25] vfio: Also trace event failures in vfio_save_complete_precopy()

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

On 3/7/24 10:28, Eric Auger wrote:
> 
> 
> On 3/6/24 14:34, Cédric Le Goater wrote:
>> vfio_save_complete_precopy() currently returns before doing the trace
>> event. Change that.
>>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>>   hw/vfio/migration.c | 3 ---
>>   1 file changed, 3 deletions(-)
>>
>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>> index bd48f2ee472a5230c2c84bff829dae1e217db33f..c8aeb43b4249ec76ded2542d62792e8c469d5f97 100644
>> --- a/hw/vfio/migration.c
>> +++ b/hw/vfio/migration.c
>> @@ -580,9 +580,6 @@ static int vfio_save_complete_precopy(QEMUFile *f, void *opaque)
>>   
>>       qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
>>       ret = qemu_file_get_error(f);
>> -    if (ret) {
>> -        return ret;
>> -    }
>>   
>>       trace_vfio_save_complete_precopy(vbasedev->name, ret);
> it is arguable if you want to trace if an error occured. If you want to
> unconditionally trace the function entry, want don't we put the trace at
> the beginning of the function?

But, then, the 'ret' value is not set and the trace event is less useful.
I'd rather keep it that way.

Thanks,

C.

Re: [PATCH v4 24/25] vfio: Also trace event failures in vfio_save_complete_precopy()

Posted by Eric Auger 8 months, 3 weeks ago


On 3/7/24 14:36, Cédric Le Goater wrote:
> On 3/7/24 10:28, Eric Auger wrote:
>>
>>
>> On 3/6/24 14:34, Cédric Le Goater wrote:
>>> vfio_save_complete_precopy() currently returns before doing the trace
>>> event. Change that.
>>>
>>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>>> ---
>>>   hw/vfio/migration.c | 3 ---
>>>   1 file changed, 3 deletions(-)
>>>
>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>>> index
>>> bd48f2ee472a5230c2c84bff829dae1e217db33f..c8aeb43b4249ec76ded2542d62792e8c469d5f97 100644
>>> --- a/hw/vfio/migration.c
>>> +++ b/hw/vfio/migration.c
>>> @@ -580,9 +580,6 @@ static int vfio_save_complete_precopy(QEMUFile
>>> *f, void *opaque)
>>>         qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
>>>       ret = qemu_file_get_error(f);
>>> -    if (ret) {
>>> -        return ret;
>>> -    }
>>>         trace_vfio_save_complete_precopy(vbasedev->name, ret);
>> it is arguable if you want to trace if an error occured. If you want to
>> unconditionally trace the function entry, want don't we put the trace at
>> the beginning of the function?
> 
> But, then, the 'ret' value is not set and the trace event is less useful.
> I'd rather keep it that way.
ah I did not notice the returned value was traced too. OK then

Sorry for the noise

Eric
> 
> Thanks,
> 
> C.
>