Re: [PATCH 2/3] KVM: selftests: Introduce kvm_vm_dead_free

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

On Fri, Nov 08, 2024 at 10:00:06AM +0100, Eric Auger wrote:
> Hi Oliver,
> 
> On 11/7/24 21:12, Oliver Upton wrote:
> > On Thu, Nov 07, 2024 at 11:56:32AM -0800, Sean Christopherson wrote:
> >> On Thu, Nov 07, 2024, Oliver Upton wrote:
> >>> On Thu, Nov 07, 2024 at 09:55:52AM -0800, Sean Christopherson wrote:
> >>>> On Thu, Nov 07, 2024, Eric Auger wrote:
> >>>>> In case a KVM_REQ_VM_DEAD request was sent to a VM, subsequent
> >>>>> KVM ioctls will fail and cause test failure. This now happens
> >>>>> with an aarch64 vgic test where the kvm_vm_free() fails. Let's
> >>>>> add a new kvm_vm_dead_free() helper that does all the deallocation
> >>>>> besides the KVM_SET_USER_MEMORY_REGION2 ioctl.
> >>>> Please no.  I don't want to bleed the kvm->vm_dead behavior all over selftests.
> >>>> The hack in __TEST_ASSERT_VM_VCPU_IOCTL() is there purely to provide users with
> >>>> a more helpful error message, it is most definitely not intended to be an "official"
> >>>> way to detect and react to the VM being dead.
> >>>>
> >>>> IMO, tests that intentionally result in a dead VM should assert that subsequent
> >>>> VM/vCPU ioctls return -EIO, and that's all.  Attempting to gracefully free
> >>>> resources adds complexity and pollutes the core selftests APIs, with very little
> >>>> benefit.
> >>> Encouraging tests to explicitly leak resources to fudge around assertions
> >>> in the selftests library seems off to me.
> >> I don't disagree, but I really, really don't want to add vm_dead().
> > It'd still be valuable to test that the VM is properly dead and
> > subsequent ioctls also return EIO, but I understand the hesitation.
> 
> Currently the vgic_test does not check that the VM is dead, it just
> checks the first expected errno according to the uapi documentation.
> Besides AFAIK this latter has not been updated according to the new VM
> dead implementation.

Ah yep, of course. We'll only return EIO for subsequent ioctls. It'd be
nice to assert that is the case, but it isn't _that_ big of a deal.

-- 
Thanks,
Oliver