Re: [PATCH v10 05/10] livepatch: Support separate list for replaced patches.

From: Miroslav Benes
Date: Tue Mar 20 2018 - 09:32:20 EST



> > I don't know, does anybody really care about this case (patching on top
> > of a disabled patch)? It just adds to the crazy matrix of possible
> > scenarios we have to keep in our heads, which means more bugs, for very
> > little (hypothetical) gain.
>
> It depends if the we remove the replaced patches or not. If they are
> removed then replacing disabled patches is rather trivial from both
> coding and understanding side.

I agree. Since we already have the code, there is no point not to have the
feature. It is not that complicated after all.

> I am going to add this as a separate patch as well. Let's discuss
> it with the code.
>
>
> > > White the atomic replace could make things easier for both developers
> > > and users.
> >
> > I agree that atomic replace is a useful feature and I'm not arguing
> > against it, so maybe I missed your point?
>
> Your suggestion allows easier code but it reduces the advantages of
> the atomic replace feature. We would achieve almost the same results
> with a normal livepatch where the functions behave like in
> the original code.
>
> Also removing replaced patches can be seen as a clean up after
> each patch. It might be more code but the target system might be
> easier to debug. Also we do not need to mind about various
> disable scenarios.

I agree with this as well.

Yes, it was a bit painful to review, but I was quite content with the
result. I don't want to go halfway and be stuck with NOPs when it is
not complicated to remove them completely after the transition. It'd be
odd in my opinion.

Regards,
Miroslav