Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: PATCH v6 6/6] livepatch: Add atomic replace
To:     Josh Poimboeuf <jpoimboe@redhat.com>
Cc:     Petr Mladek <pmladek@suse.com>, jikos@kernel.org, mbenes@suse.cz,
        Jason Baron <jbaron@akamai.com>, jeyu@kernel.org,
        linux-kernel@vger.kernel.org, live-patching@vger.kernel.org
References: <20180125160203.28959-1-pmladek@suse.com>
 <20180125160203.28959-7-pmladek@suse.com>
 <3e93e7b5-3077-9f03-ae89-55394e70da39@virtuozzo.com>
 <20180131215518.cwcxs2atfuzimyxt@treble>
From:   Evgenii Shatokhin <eshatokhin@virtuozzo.com>
Message-ID: <e7c51e65-e961-1df3-6d63-93dc94fe5acb@virtuozzo.com>
Date:   Thu, 1 Feb 2018 11:15:43 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.5.2
MIME-Version: 1.0
In-Reply-To: <20180131215518.cwcxs2atfuzimyxt@treble>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Received-SPF: None (protection.outlook.com: virtuozzo.com does not designate
 permitted sender hosts)
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Feb 2018 08:15:46.5957 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 8e7fbc08-ceed-4ee1-6966-08d5694bff95
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 0bc7f26d-0264-416e-a6fc-8352af79c58f
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR0802MB2438
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 01.02.2018 00:55, Josh Poimboeuf wrote:
> On Fri, Jan 26, 2018 at 01:33:04PM +0300, Evgenii Shatokhin wrote:
>>>     + The callbacks from the replaced patches are not called. It would be
>>>       pretty hard to define a reasonable semantic and implement it.
>>
>> At least, it surely simplifies error handling, if these callbacks are not
>> called.
>>
>> Anyway, I guess, this restriction should be mentioned explicitly in the
>> docs. I think this is not obvious for the patch developers (esp. those
>> familiar with RPM spec files and such ;-) ).
>>
>> What concerns me is that downgrading of the cumulative patches with
>> callbacks becomes much more difficult this way.
>>
>> I mean, suppose a user has v1 of a cumulative patch installed. Then a newer
>> version, v2, is released. They install it and find that it is buggy (very
>> unfortunate but might still happen). Now they cannot atomically replace v2
>> back with v1, because the callbacks from v1 cannot clean up after v2.
>>
>> It will be needed to unload v2 explicitly and then load v1 back, which is
>> more fragile. The loading failures are much more unlikely with livepatch
>> than with the old kpatch, but they are still possible.
>>
>> I have no good solution to this though.
> 
> I think the solution is to build a v3, which is basically identical to
> v1, except it also has callbacks for cleaning up after v2, if necessary.
> 
> It should also be smart enough to deal with the case that v2 was not
> installed beforehand.
> 

I thought about this, but such patches would be very difficult to 
maintain. It seems better to explicitly unload v2 and then load v1 back 
in such cases.

In addition, releasing v3 takes some time (build + appropriate QA 
procedures) while the users may want to do something about the faulty 
patch "right here, right now". This is what "downgrade" option is for.

Anyway, livepatch is much more likely to apply the patches successfully 
than the old kpatch core. So it should be OK to simply unload v2 and 
then load v1 in such (hopefully rare) scenarios.

As I said, the current behaviour is acceptable, esp. when it is 
documented. Implementation of livepatch is already complex enough.

Regards,
Evgenii