Re: [Qemu-devel] [RFC PATCH 0/5] qcow: coroutines cleanup

[Frediano Ziglio]

Il giorno ven, 22/07/2011 alle 12.10 +0200, Kevin Wolf ha scritto: 
> Am 22.07.2011 11:26, schrieb Frediano Ziglio:
> > 2011/7/22 Kevin Wolf <address@hidden>:
> >> Am 20.07.2011 15:56, schrieb Frediano Ziglio:
> >>> These patches mostly cleanup some AIO code using coroutines.
> >>> These patches apply to Kevin's repository, branch coroutine-block.
> >>> Mostly they use stack instead of allocated AIO structure.
> >>>
> >>> Frediano Ziglio (5):
> >>>   qcow: allocate QCowAIOCB structure using stack
> >>>   qcow: QCowAIOCB field cleanup
> >>>   qcow: move some blocks of code to avoid useless variable
> >>>     initialization
> >>>   avoid dandling pointers
> >>>   qcow: small optimization initializing QCowAIOCB
> >>>
> >>>  block/qcow.c  |  210 
> >>> +++++++++++++++++++++++++--------------------------------
> >>>  block/qcow2.c |   38 +++-------
> >>>  2 files changed, 102 insertions(+), 146 deletions(-)
> >>
> >> Most of it looks good now. Did you include the "RFC" in the subject just
> >> because the coroutine work is in RFC state, too, or did you intend to
> >> tell me that I shouldn't merge yet?
> >>
> >> Kevin
> >>
> > 
> > As these patches are first quite big patches I send (typo or small
> > fixes do not counts) I just want to mark that I could write something
> > really wrong. Just a way to avoid somebody having to send more patches
> > and get more attention. Some projects are quite prone to merge even
> > not that fine ones. I prefer to have some (a bit) pedantic comments
> > and a real fix/improve.
> > 
> > Now I removed the RFC from last update. The main reason is that I
> > found your qemu-iotests repository which, I think should be merged to
> > main repository, but it's just my opinion.
> > Oh... qcow fails 004 test (even origin/coroutines-block) with a I/O error.
> 
> Yup, you're right, I must have messed it up. Care to fix it or should I
> look into it?
> 

Care but I don't know if I'll have time before Thursday. However I found
the problem, really strange. bdrv_read returns <0 for errors 0 for
success and... bytes read on partial read! Now a qcow image of 128m is
560 bytes so when you read sector 1 you get 48 which is not a problem
for qcow code. But if you replace bdrv_read with a bdrv_co_readv (your
latest patch on coroutine-block) bdrv_co_readv return -EINVAL on partial
read.

Frediano