On Thu, 1 Dec 2022 at 10:33, Akihiko Odaki <akihiko.odaki@xxxxxxxxxx> 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
