lkml.org 
[lkml]   [2022]   [Aug]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/5] selftests: KVM/x86: Fix vcpu_{save,load}_state() by adding APIC state into kvm_x86_state
On Wed, Aug 03, 2022 at 07:21:43PM +0000, Sean Christopherson wrote:
> KVM: selftests: for the shortlog.
>
> On Wed, Aug 03, 2022, Oliver Upton wrote:
> > Hi Mingwei,
> >
> > On Tue, Aug 02, 2022 at 11:07:15PM +0000, Mingwei Zhang wrote:
> > > Fix vcpu_{save,load}_state() by adding APIC state into kvm_x86_state and
> > > properly save/restore it in vcpu_{save,load}_state(). When vcpu resets,
> > > APIC state become software disabled in kernel and thus the corresponding
> > > vCPU is not able to receive posted interrupts [1]. So, add APIC
> > > save/restore in userspace in selftest library code.
> >
> > Of course, there are no hard rules around it but IMO a changelog is
> > easier to grok if it first describes the what/why of the problem, then
> > afterwards how it is fixed by the commit.
>
> I strongly disagree. :-) To some extent, it's a personal preference, e.g. I
> find it easier to understand the details (why something is a problem) if I have
> the extra context of how a problem is fixed (or: what code was broken).
>

Sorry, what I wrote definitely was asking for strict ordering. Thank you
for rightly calling that out.

My actual issue if I had been bothered to articulate it well was that $WHAT
was effectively restated in different terms which can be confusing.
Where possible, atomically addressing what, why and how can lead to a
crisper changelog.

[...]

> KVM: selftests: Save/restore vAPIC state in "migration" tests
>
> Save/restore vAPIC state as part of vCPU save/load so that it's preserved
> across VM "migration". This will allow testing that posted interrupts
> are properly handled across VM migration.
>
> With that, the first sentence covers both the "what's changing" and provides a
> high-level description of the "bug" it's fixing. And the second sentence covers
> (a) "why do we want this patch", (b) "why wasn't this a problem before", and (c)
> "what's the urgency of this patch".

LGTM.

--
Thanks,
Oliver

\
 
 \ /
  Last update: 2022-08-04 01:55    [W:0.049 / U:0.272 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site