Re: [PATCH] target/arm: Propagate errno when writing list

[Peter Maydell]

On Thu, 1 Dec 2022 at 10:33, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> Before this change, write_kvmstate_to_list() and
> write_list_to_kvmstate() tolerated even if it failed to access some
> register, and returned a bool indicating whether one of the register
> accesses failed. However, it does not make sen not to fail early as the
> the callers check the returned value and fail early anyway.
>
> So let write_kvmstate_to_list() and write_list_to_kvmstate() fail early
> too. This will allow to propagate errno to the callers and log it if
> appropriate.

(Sorry this one didn't get reviewed earlier.)

I agree that all the callers of these functions check for
failure, so there's no major benefit from doing the
don't-fail-early logic. But is there a reason why we should
actively make this change?

In particular, these functions form part of a family with the
similar write_cpustate_to_list() and write_list_to_cpustate(),
and it's inconsistent to have the kvmstate ones return
negative-errno while the cpustate ones still return bool.
For the cpustate ones we *do* rely in some places on
the "don't fail early" behaviour. The kvmstate ones do the
same thing I think mostly for consistency.

So unless there's a specific reason why changing these
functions improves behaviour as seen by users, I think
I favour retaining the consistency.

thanks
-- PMM