qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2] win32-aio fixes
@ 2013-01-16 20:19 Kevin Wolf
  0 siblings, 0 replies; 6+ messages in thread
From: Kevin Wolf @ 2013-01-16 20:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, pbonzini, stefanha

Paolo, especially the first one is worrying with respect to the test status of
this code. We should probably give it some additional testing.

Kevin Wolf (2):
  win32-aio: Fix vectored reads
  win32-aio: Fix memory leak

 block/win32-aio.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH 0/2] win32-aio fixes
@ 2013-01-16 20:19 Kevin Wolf
  2013-01-17  9:59 ` Stefan Hajnoczi
  2013-01-17 10:53 ` Paolo Bonzini
  0 siblings, 2 replies; 6+ messages in thread
From: Kevin Wolf @ 2013-01-16 20:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, pbonzini, qemu-stable, stefanha

Paolo, especially the first one is worrying with respect to the test status of
this code. We should probably give it some additional testing.

Kevin Wolf (2):
  win32-aio: Fix vectored reads
  win32-aio: Fix memory leak

 block/win32-aio.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] win32-aio fixes
  2013-01-16 20:19 Kevin Wolf
@ 2013-01-17  9:59 ` Stefan Hajnoczi
  2013-01-17 10:53 ` Paolo Bonzini
  1 sibling, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2013-01-17  9:59 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: pbonzini, qemu-devel, qemu-stable

On Wed, Jan 16, 2013 at 09:19:58PM +0100, Kevin Wolf wrote:
> Paolo, especially the first one is worrying with respect to the test status of
> this code. We should probably give it some additional testing.
> 
> Kevin Wolf (2):
>   win32-aio: Fix vectored reads
>   win32-aio: Fix memory leak
> 
>  block/win32-aio.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> -- 
> 1.7.6.5
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] win32-aio fixes
  2013-01-16 20:19 Kevin Wolf
  2013-01-17  9:59 ` Stefan Hajnoczi
@ 2013-01-17 10:53 ` Paolo Bonzini
  2013-01-17 11:35   ` Kevin Wolf
  1 sibling, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2013-01-17 10:53 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: qemu-devel, stefanha, qemu-stable

Il 16/01/2013 21:19, Kevin Wolf ha scritto:
> Paolo, especially the first one is worrying with respect to the test status of
> this code. We should probably give it some additional testing.
> 
> Kevin Wolf (2):
>   win32-aio: Fix vectored reads
>   win32-aio: Fix memory leak
> 
>  block/win32-aio.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 

Yes, the worrying part is especially that IIUC qtest does not support
Windows.  There's no way to get good coverage without qtest.

Paolo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] win32-aio fixes
  2013-01-17 10:53 ` Paolo Bonzini
@ 2013-01-17 11:35   ` Kevin Wolf
  2013-01-17 11:37     ` Paolo Bonzini
  0 siblings, 1 reply; 6+ messages in thread
From: Kevin Wolf @ 2013-01-17 11:35 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, stefanha, qemu-stable

Am 17.01.2013 11:53, schrieb Paolo Bonzini:
> Il 16/01/2013 21:19, Kevin Wolf ha scritto:
>> Paolo, especially the first one is worrying with respect to the test status of
>> this code. We should probably give it some additional testing.
>>
>> Kevin Wolf (2):
>>   win32-aio: Fix vectored reads
>>   win32-aio: Fix memory leak
>>
>>  block/win32-aio.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
> 
> Yes, the worrying part is especially that IIUC qtest does not support
> Windows.  There's no way to get good coverage without qtest.

Why that? If block drivers aren't testable from qemu-iotests with only
qemu-img and qemu-io, then there's something we did wrong. This specific
code would have been easily covered with qemu-io -k -n -c 'readv ...'
(which is how I found the bug and tested the fix).

Hm, or actually, is cache=none even needed for aio=native on Windows? In
any case I think some documentation needs to be updated.

qemu-iotests under Wine may need some polishing, though, and of course
needs someone to run it regularly with the right parameters. (In fact,
it seems we don't even run the tests with Linux AIO)

Kevin

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] win32-aio fixes
  2013-01-17 11:35   ` Kevin Wolf
@ 2013-01-17 11:37     ` Paolo Bonzini
  0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2013-01-17 11:37 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: qemu-devel, stefanha, qemu-stable

Il 17/01/2013 12:35, Kevin Wolf ha scritto:
> Am 17.01.2013 11:53, schrieb Paolo Bonzini:
>> Il 16/01/2013 21:19, Kevin Wolf ha scritto:
>>> Paolo, especially the first one is worrying with respect to the test status of
>>> this code. We should probably give it some additional testing.
>>>
>>> Kevin Wolf (2):
>>>   win32-aio: Fix vectored reads
>>>   win32-aio: Fix memory leak
>>>
>>>  block/win32-aio.c |    4 ++--
>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>
>> Yes, the worrying part is especially that IIUC qtest does not support
>> Windows.  There's no way to get good coverage without qtest.
> 
> Why that? If block drivers aren't testable from qemu-iotests with only
> qemu-img and qemu-io, then there's something we did wrong. This specific
> code would have been easily covered with qemu-io -k -n -c 'readv ...'
> (which is how I found the bug and tested the fix).

Doh, that was really stupid.

/me unsuccessfully tries to blame flu

> Hm, or actually, is cache=none even needed for aio=native on Windows? In
> any case I think some documentation needs to be updated.

Honestly I have no idea.  However, I don't think so.

Paolo

> qemu-iotests under Wine may need some polishing, though, and of course
> needs someone to run it regularly with the right parameters. (In fact,
> it seems we don't even run the tests with Linux AIO)

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-01-17 12:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-16 20:19 [Qemu-devel] [PATCH 0/2] win32-aio fixes Kevin Wolf
  -- strict thread matches above, loose matches on Subject: below --
2013-01-16 20:19 Kevin Wolf
2013-01-17  9:59 ` Stefan Hajnoczi
2013-01-17 10:53 ` Paolo Bonzini
2013-01-17 11:35   ` Kevin Wolf
2013-01-17 11:37     ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).