| Message ID | 20240306133441.2351700-25-clg@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration: Improve error reporting | expand |
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 >
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.
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. >
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);
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(-)