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
