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.
>> 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:"
> There were some conflicts, I was careful, but one never knows. So you
> are wellcome to take a look when the PULL cames to the tree.
>
> Thanks, Juan.
