Re: [PATCH 2/5] selftests: KVM/x86: Fix vcpu_{save,load}_state() by adding APIC state into kvm_x86_state

[Oliver Upton <oliver.upton@linux.dev>]

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