Markus Armbruster <arm...@redhat.com> wrote:
> Juan Quintela <quint...@redhat.com> writes:
>
>> Markus Armbruster <arm...@redhat.com> wrote:
>>> Oh dear, where to start. There's so much wrong, and in pretty obvious
>>> ways. This code should never have passed review. I'm refraining from
>>> saying more; see the commit messages instead.
>>>
>>> Issues remaining after this series include:
>>>
>>> * Terrible error messages
>>>
>>> * Some error message cascades remain
>>>
>>> * There is no written contract for QEMUFileHooks, and the
>>> responsibility for reporting errors is unclear
>>>
>>> * There seem to be no tests whatsoever
>>>
>>> PATCH 29 is arguably a matter of taste. I made my case for it during
>>> review of v1. If maintainers don't want it, I'll drop it.
>>>
>>> Related: [PATCH 1/7] migration/rdma: Fix save_page method to fail on
>>> polling error
>>
>> Hi Markus
>>
>> I integrated everything except:
>>
>>> migration/rdma: Fix or document problematic uses of errno
>>
>> Most of them are dropped on following patches.
>
> The hunks that are dropped in later patches are:
>
> * Four FIXME comments about incorrect or problematic use of perror().
>
> If you drop the patch, you have to adjust the later patches that
> remove these hunks. Resolving the conflicts is *not* enough; you also
> have to correct the commit messages.
>
> The hunks that are not dropped are:
>
> * Three comments about bugs (either library doc bug or incorrect use of
> @errno here). I'd hate to lose them.
>
> * One bug fix, in qemu_rdma_advise_prefetch_mr(). Losing this one would
> be foolish.
>
> Please consider keeping the patch.
And here I am, having to redo the merge from this patch O:-)
>>> migration/rdma: Use error_report() & friends instead of stderr
>>
>> You said you have to resend this one.
>
> Can do, but since the change is trivial, perhaps you could make it in
> your tree without a resend. Change the line
>
> warn_report("WARN: migrations may fail:"
>
> to
>
> warn_report("migrations may fail:"
I thought it was more complicated.
Ok doing both.
Thanks, Juan.
