On 22. Aug 2019, at 17:30, Julien Grall > wrote: Hi Pawel, On 22/08/2019 12:02, Wieczorkiewicz, Pawel wrote: On 22. Aug 2019, at 12:29, Julien Grall > wrote: On 21/08/2019 09:19, Pawel Wieczorkiewicz wrote: More generally, I am not very comfortable to see panic() in the middle of the code. Could you explain why panic is the best solution over reverting the work? Yes. Production-ready hotpatches must not contain inconsistent hooks or functions-to-be-applied/reverted. The goal here is to detect such hotpatches and fail hard immediately highlighting the fact that such hotpatch is broken. Aside the len = 0 that you are going to fix. How would this condition happen? Are you going to add code that will potentially trigger the panic? The default livepatch code path is very conservative (to the extent it does not even need this check and panic). But, with the changes of this series, one can potentially construct a hotpatch that upon load would trigger the panic (or without the panic, would silently leave the Xen code in memory in an inconsistent state). This obviously must not happen in production, so the new livepatch feature should be used with care.The changes make livepatch more flexible and universal, yet when using new features, more care is needed. However, when someone is developing a hotpatch or is using the new feature for debugging or for whatever non-production-related reason, it is very helpful to detect immediately any sort of undefined state. I have been there myself when I was working on the changes presented here, and thought that would be a good idea to add this checks permanently. Also, when something changes in the future in the livepatch implementation, those checks could potentially highlight any inconsistencies. The inconsistent application of a hotpatch (some function applied, some reverted while other left behined) leaves the system in a very bad state. I think the best what we could do here is panic() to enable post-mortem analysis of what went wrong and avoid leaking such system into production. Thank you for the explanation here (and on IRC). May I ask some documentation regarding the panic in at least commit message? Ideally, this would explain why the panic the most sensible solution. Yes, certainly. I will add that. Cheers, -- Julien Grall Best Regards, Pawel Wieczorkiewicz Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879