Retrieve the @errno set by ioctl() before we call accel_ioctl_end()
instead of afterwards, so it works whether accel_ioctl_end() preserves
@errno or not.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 accel/kvm/kvm-all.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index f9254ae654..28006d73c5 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -3373,10 +3373,10 @@ int kvm_vm_ioctl(KVMState *s, unsigned long type, ...)
     trace_kvm_vm_ioctl(type, arg);
     accel_ioctl_begin();
     ret = ioctl(s->vmfd, type, arg);
-    accel_ioctl_end();
     if (ret == -1) {
         ret = -errno;
     }
+    accel_ioctl_end();
     return ret;
 }
 
@@ -3413,10 +3413,10 @@ int kvm_device_ioctl(int fd, unsigned long type, ...)
     trace_kvm_device_ioctl(fd, type, arg);
     accel_ioctl_begin();
     ret = ioctl(fd, type, arg);
-    accel_ioctl_end();
     if (ret == -1) {
         ret = -errno;
     }
+    accel_ioctl_end();
     return ret;
 }
 
-- 
2.49.0

Superseded by

Subject: [PATCH v2] kvm: Fix kvm_vm_ioctl() and kvm_device_ioctl() return value
Date: Fri, 28 Nov 2025 16:20:50 +0100
Message-ID: <20251128152050.3417834-1-armbru@redhat.com>

Queued, thanks.

Paolo

Markus Armbruster <armbru@redhat.com> writes:

> Retrieve the @errno set by ioctl() before we call accel_ioctl_end()
> instead of afterwards, so it works whether accel_ioctl_end() preserves
> @errno or not.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

I did not check whether the assumption holds or not.  If it doesn't,
then this needs

  Fixes: a27dd2de68f3 (KVM: keep track of running ioctls)

On 25/11/25 10:03, Markus Armbruster wrote:
> Markus Armbruster <armbru@redhat.com> writes:
> 
>> Retrieve the @errno set by ioctl() before we call accel_ioctl_end()
>> instead of afterwards, so it works whether accel_ioctl_end() preserves
>> @errno or not.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> 
> I did not check whether the assumption holds or not.

Indeed, on Linux the futex syscall is called via qemu_event_set.

>  If it doesn't,
> then this needs
> 
>    Fixes: a27dd2de68f3 (KVM: keep track of running ioctls)

LGTM.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 25/11/25 10:03, Markus Armbruster wrote:
>> Markus Armbruster <armbru@redhat.com> writes:
>> 
>>> Retrieve the @errno set by ioctl() before we call accel_ioctl_end()
>>> instead of afterwards, so it works whether accel_ioctl_end() preserves
>>> @errno or not.
>>>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> 
>> I did not check whether the assumption holds or not.
>
> Indeed, on Linux the futex syscall is called via qemu_event_set.

So ...

>>  If it doesn't,
>> then this needs
>> 
>>    Fixes: a27dd2de68f3 (KVM: keep track of running ioctls)

... we definitely want this tag, and the commit message should be
clarified as well.  Here's my try:

    kvm: Fix kvm_vm_ioctl() and kvm_device_ioctl() return value

    These functions wrap ioctl().  When ioctl() fails, it sets @errno.
    The wrappers then return that @errno negated.

    Except they call accel_ioctl_end() between calling ioctl() and reading
    @errno.  accel_ioctl_end() can clobber @errno, e.g. when a futex()
    system call fails.  Seems unlikely, but it's a bug all the same.

    Fix by retrieving @errno before calling accel_ioctl_end().

    Fixes: a27dd2de68f3 (KVM: keep track of running ioctls)

> LGTM.
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks!