>>> We'd like to remove the assert_hvf_ok() calls, so adding more isn't >>> really helping. Anyhow, >>> >>> Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org> >>> Tested-by: Philippe Mathieu-Daudé <phi...@linaro.org> >> >> What is the preferred method going forward? >> >> Apple designed the HV API to be able to fail at every function, and it's >> rarely something that can be recovered from. >> >> In [PATCH v4 04/15] of this series, we saw an issue that was hidden >> because the return code was not properly checked (not calling from the >> owning thread). Previously, I submitted a patch (d5bd8d8267) for the >> same issue, because Apple had changed a function's behavior. This was >> caught by an assert_hvf_ok. >> >> I agree with you, that generally adding asserts is a bad design, but at >> the same time, HV is designed in a way, that the code would be littered >> with checks otherwise. > > This suggestion was I think mine, and I think partly I was > misled by the name of assert_hvf_ok(), because in fact it > isn't an assert(). (assert() should be specifically "if we > hit this this is a bug in QEMU", and "hvf returned an error" > is generally not that. It does call abort(), though, which > isn't much better.) > > But also I think the existence of assert_hvf_ok() encourages > it to be used in cases where actually we should be doing > something more sensible with an error return. For instance, > in hvf_accel_init() if we can't init HVF then we should > return an error code to the caller, which might fall back > to the TCG accelerator -- but instead we have an assert_hvf_ok(), > so fallback won't work because we'll just bomb out. > > The KVM API is also one where any API call (ioctl) can return an > error, but we don't generally assert() that they succeeded, except > in a few cases of "given where we are, for this to fail would > mean the kernel was broken". Instead we propagate error values > upwards when the function has a mechanism to do that, > and where appropriate we print an error message that's > hopefully reasonably comprehensible to the user and exit. > > -- PMM
Thanks for the clarification. I think I understand the sentiment. Propagating the error codes when possible, is much preferred. But letting the calls go unchecked is problematic too, so I don't know if we can get around assert_hvf_ok in some shape or form. You're right, that it might encourage use as an easy fix. I'll keep an eye on it in my reviews.
