* [Qemu-devel] wrong ioctl error handling on dirty pages sync? @ 2017-08-25 11:42 Ján Poctavek 2017-08-29 10:08 ` Stefan Hajnoczi 0 siblings, 1 reply; 6+ messages in thread From: Ján Poctavek @ 2017-08-25 11:42 UTC (permalink / raw) To: qemu-devel Hi guys, Maybe it is just my lack of understanding, this seems like a bug to me: To get list of dirty pages, qemu calls kvm_vm_ioctl() with KVM_GET_DIRTY_LOG: https://github.com/qemu/qemu/blob/v2.10.0-rc4/accel/kvm/kvm-all.c#L494 and considers the ioctl call failed when -1 is returned. But the kvm_vm_ioctl() itself returns -errno, not the -1 on error: https://github.com/qemu/qemu/blob/v2.10.0-rc4/accel/kvm/kvm-all.c#L2142 Thanks in advance for sheding some light into this for me. Jan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] wrong ioctl error handling on dirty pages sync? 2017-08-25 11:42 [Qemu-devel] wrong ioctl error handling on dirty pages sync? Ján Poctavek @ 2017-08-29 10:08 ` Stefan Hajnoczi 2017-09-04 12:52 ` Ján Poctavek 0 siblings, 1 reply; 6+ messages in thread From: Stefan Hajnoczi @ 2017-08-29 10:08 UTC (permalink / raw) To: Ján Poctavek; +Cc: qemu-devel On Fri, Aug 25, 2017 at 01:42:55PM +0200, Ján Poctavek wrote: > Hi guys, > > Maybe it is just my lack of understanding, this seems like a bug to me: > > To get list of dirty pages, qemu calls kvm_vm_ioctl() with > KVM_GET_DIRTY_LOG: > https://github.com/qemu/qemu/blob/v2.10.0-rc4/accel/kvm/kvm-all.c#L494 > > and considers the ioctl call failed when -1 is returned. > > But the kvm_vm_ioctl() itself returns -errno, not the -1 on error: > https://github.com/qemu/qemu/blob/v2.10.0-rc4/accel/kvm/kvm-all.c#L2142 > > Thanks in advance for sheding some light into this for me. Looks like a bug to me. Do you want to send a patch? Guidelines on how to submit a patch are here: http://wiki.qemu.org/Contribute/SubmitAPatch Stefan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] wrong ioctl error handling on dirty pages sync? 2017-08-29 10:08 ` Stefan Hajnoczi @ 2017-09-04 12:52 ` Ján Poctavek 2017-09-05 10:33 ` Stefan Hajnoczi 0 siblings, 1 reply; 6+ messages in thread From: Ján Poctavek @ 2017-09-04 12:52 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: qemu-devel On 29. 8. 2017 12:08, Stefan Hajnoczi wrote: > On Fri, Aug 25, 2017 at 01:42:55PM +0200, Ján Poctavek wrote: >> Hi guys, >> >> Maybe it is just my lack of understanding, this seems like a bug to me: >> >> To get list of dirty pages, qemu calls kvm_vm_ioctl() with >> KVM_GET_DIRTY_LOG: >> https://github.com/qemu/qemu/blob/v2.10.0-rc4/accel/kvm/kvm-all.c#L494 >> >> and considers the ioctl call failed when -1 is returned. >> >> But the kvm_vm_ioctl() itself returns -errno, not the -1 on error: >> https://github.com/qemu/qemu/blob/v2.10.0-rc4/accel/kvm/kvm-all.c#L2142 >> >> Thanks in advance for sheding some light into this for me. > Looks like a bug to me. Do you want to send a patch? > > Guidelines on how to submit a patch are here: > http://wiki.qemu.org/Contribute/SubmitAPatch > > Stefan It seems that the patch has already been created a long time ago. But it is still not included: https://lists.nongnu.org/archive/html/qemu-devel/2014-03/msg03996.html Can I help with this somehow? Jan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] wrong ioctl error handling on dirty pages sync? 2017-09-04 12:52 ` Ján Poctavek @ 2017-09-05 10:33 ` Stefan Hajnoczi 2017-09-05 11:57 ` Michael Tokarev 0 siblings, 1 reply; 6+ messages in thread From: Stefan Hajnoczi @ 2017-09-05 10:33 UTC (permalink / raw) To: Ján Poctavek; +Cc: qemu-devel, Michael Tokarev On Mon, Sep 04, 2017 at 02:52:57PM +0200, Ján Poctavek wrote: > > On 29. 8. 2017 12:08, Stefan Hajnoczi wrote: > > On Fri, Aug 25, 2017 at 01:42:55PM +0200, Ján Poctavek wrote: > > > Hi guys, > > > > > > Maybe it is just my lack of understanding, this seems like a bug to me: > > > > > > To get list of dirty pages, qemu calls kvm_vm_ioctl() with > > > KVM_GET_DIRTY_LOG: > > > https://github.com/qemu/qemu/blob/v2.10.0-rc4/accel/kvm/kvm-all.c#L494 > > > > > > and considers the ioctl call failed when -1 is returned. > > > > > > But the kvm_vm_ioctl() itself returns -errno, not the -1 on error: > > > https://github.com/qemu/qemu/blob/v2.10.0-rc4/accel/kvm/kvm-all.c#L2142 > > > > > > Thanks in advance for sheding some light into this for me. > > Looks like a bug to me. Do you want to send a patch? > > > > Guidelines on how to submit a patch are here: > > http://wiki.qemu.org/Contribute/SubmitAPatch > > > > Stefan > > It seems that the patch has already been created a long time ago. But it is > still not included: > > https://lists.nongnu.org/archive/html/qemu-devel/2014-03/msg03996.html > > Can I help with this somehow? Michael: Do you know what happened to this -trivial patch? Thanks, Stefan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] wrong ioctl error handling on dirty pages sync? 2017-09-05 10:33 ` Stefan Hajnoczi @ 2017-09-05 11:57 ` Michael Tokarev 2017-09-05 11:59 ` Michael Tokarev 0 siblings, 1 reply; 6+ messages in thread From: Michael Tokarev @ 2017-09-05 11:57 UTC (permalink / raw) To: Stefan Hajnoczi, Ján Poctavek, Peter Maydell; +Cc: qemu-devel 05.09.2017 13:33, Stefan Hajnoczi пишет: > On Mon, Sep 04, 2017 at 02:52:57PM +0200, Ján Poctavek wrote: >> >> On 29. 8. 2017 12:08, Stefan Hajnoczi wrote: >>> On Fri, Aug 25, 2017 at 01:42:55PM +0200, Ján Poctavek wrote: >>>> Hi guys, >>>> >>>> Maybe it is just my lack of understanding, this seems like a bug to me: >>>> >>>> To get list of dirty pages, qemu calls kvm_vm_ioctl() with >>>> KVM_GET_DIRTY_LOG: >>>> https://github.com/qemu/qemu/blob/v2.10.0-rc4/accel/kvm/kvm-all.c#L494 >>>> >>>> and considers the ioctl call failed when -1 is returned. >>>> >>>> But the kvm_vm_ioctl() itself returns -errno, not the -1 on error: >>>> https://github.com/qemu/qemu/blob/v2.10.0-rc4/accel/kvm/kvm-all.c#L2142 >>>> >>>> Thanks in advance for sheding some light into this for me. >>> Looks like a bug to me. Do you want to send a patch? >>> >>> Guidelines on how to submit a patch are here: >>> http://wiki.qemu.org/Contribute/SubmitAPatch >>> >>> Stefan >> >> It seems that the patch has already been created a long time ago. But it is >> still not included: >> >> https://lists.nongnu.org/archive/html/qemu-devel/2014-03/msg03996.html >> >> Can I help with this somehow? > > Michael: Do you know what happened to this -trivial patch? https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg05347.html https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg05346.html https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg05402.html -- according to the emails, it has been applied, but I don't see it in the git tree. Hmm.. Peter& /mjt ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] wrong ioctl error handling on dirty pages sync? 2017-09-05 11:57 ` Michael Tokarev @ 2017-09-05 11:59 ` Michael Tokarev 0 siblings, 0 replies; 6+ messages in thread From: Michael Tokarev @ 2017-09-05 11:59 UTC (permalink / raw) To: Stefan Hajnoczi, Ján Poctavek, Peter Maydell; +Cc: qemu-devel 05.09.2017 14:57, Michael Tokarev пишет: > 05.09.2017 13:33, Stefan Hajnoczi пишет: >> On Mon, Sep 04, 2017 at 02:52:57PM +0200, Ján Poctavek wrote: >>> >>> On 29. 8. 2017 12:08, Stefan Hajnoczi wrote: >>>> On Fri, Aug 25, 2017 at 01:42:55PM +0200, Ján Poctavek wrote: >>>>> Hi guys, >>>>> >>>>> Maybe it is just my lack of understanding, this seems like a bug to me: >>>>> >>>>> To get list of dirty pages, qemu calls kvm_vm_ioctl() with >>>>> KVM_GET_DIRTY_LOG: >>>>> https://github.com/qemu/qemu/blob/v2.10.0-rc4/accel/kvm/kvm-all.c#L494 >>>>> >>>>> and considers the ioctl call failed when -1 is returned. >>>>> >>>>> But the kvm_vm_ioctl() itself returns -errno, not the -1 on error: >>>>> https://github.com/qemu/qemu/blob/v2.10.0-rc4/accel/kvm/kvm-all.c#L2142 >>>>> >>>>> Thanks in advance for sheding some light into this for me. >>>> Looks like a bug to me. Do you want to send a patch? >>>> >>>> Guidelines on how to submit a patch are here: >>>> http://wiki.qemu.org/Contribute/SubmitAPatch >>>> >>>> Stefan >>> >>> It seems that the patch has already been created a long time ago. But it is >>> still not included: >>> >>> https://lists.nongnu.org/archive/html/qemu-devel/2014-03/msg03996.html >>> >>> Can I help with this somehow? >> >> Michael: Do you know what happened to this -trivial patch? > > https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg05347.html > https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg05346.html > https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg05402.html > > -- according to the emails, it has been applied, but I don't see it in > the git tree. Hmm.. Aha. Here we go: commit 50212d6346f33d6e19489e81b025b5c3bb8759be Author: Michael Tokarev <mjt@tls.msk.ru> Date: Mon Apr 14 16:14:04 2014 +0400 Revert "fix return check for KVM_GET_DIRTY_LOG ioctl" This reverts commit b533f658a98325d0e47b36113bd9f5bcc046fdae. The original code was wrong, because effectively it ignored errors from kernel, because kernel does not return -1 on error case but returns -errno, and does not return -EPERM for this particular ioctl. But in some cases kernel actually returned unsuccessful result, namely, when the dirty bitmap in requested slot does not exist it returns -ENOENT. With new code this condition becomes an error when it shouldn't be. Revert that patch instead of fixing it properly this late in the release process. I disagree with this approach, but let's make things move _somewhere_, instead of arguing endlessly whch of the 2 proposed fixes is better. Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> Message-id: 1397477644-902-1-git-send-email-mjt@msgid.tls.msk.ru Signed-off-by: Peter Maydell <peter.maydell@linaro.org> /mjt ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-09-05 12:00 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-08-25 11:42 [Qemu-devel] wrong ioctl error handling on dirty pages sync? Ján Poctavek 2017-08-29 10:08 ` Stefan Hajnoczi 2017-09-04 12:52 ` Ján Poctavek 2017-09-05 10:33 ` Stefan Hajnoczi 2017-09-05 11:57 ` Michael Tokarev 2017-09-05 11:59 ` Michael Tokarev
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.