Re: [Qemu-devel] [PATCH v11 01/15] migration: Set error state in case of error

[Juan Quintela <quintela@redhat.com>]

Daniel P. Berrange <berrange@redhat.com> wrote:
> On Fri, Mar 16, 2018 at 05:49:07PM +0000, Daniel P. Berrangé wrote:
>> On Fri, Mar 16, 2018 at 12:53:49PM +0100, Juan Quintela wrote:
>> > Signed-off-by: Juan Quintela <quintela@redhat.com>
>> > ---
>> >  migration/ram.c | 20 ++++++++++++++++++++
>> >  1 file changed, 20 insertions(+)
>> > 
>> > diff --git a/migration/ram.c b/migration/ram.c
>> > index 7266351fd0..1b8095a358 100644
>> > --- a/migration/ram.c
>> > +++ b/migration/ram.c
>> > @@ -414,6 +414,16 @@ static void terminate_multifd_send_threads(Error *errp)
>> >  {
>> >      int i;
>> >  
>> > +    if (errp) {
>> > +        MigrationState *s = migrate_get_current();
>> > +        migrate_set_error(s, errp);
>> 
>> This doesn't look quiet right. You're checking if 'errp' is a non-NULL,
>> which just tells you if the caller wants to collect the error, not
>> whether an error has happened. For the latter you need
>> 
>>   if (errp && *errp)
>> 
>> seems a little strange though for the caller to pass an error into this
>> method for reporting.
>
> Oh wait, I'm being mislead by the unusual parameter name.
>
> An "errp" name should only ever be used for a "Error **", but we
> only have an "Error *" here.

Copy & Paste O:-)

> So just fix the parameter name to be "err" instead of "errp".

Done.

Thanks,