Re: [Qemu-devel] [PATCHv2 6/7] cleanup qemu_co_sendv(), qemu_co_recvv() and friends

[Paolo Bonzini]

Il 12/03/2012 17:29, Michael Tokarev ha scritto:
>> For example, qemu_co_recvv has handling for receiving 0, but
>> > qemu_co_sendv does not.
> This is a bug, which I dind't notice, it shouldn't
> be there.  Somehow I overlooked this difference, but
> I really wondered how they're differ!  By using common
> code here it becomes much more obvious (whith the bug
> actually fixed).

write should never return 0, read does for end-of-file.

So your code was actually correct in some sense.

>> This is what I don't really like in the second part of these patches.
>> You are doing changes for the sake of other changes which are not
>> upstream yet, for which there is no clear vision, and for which there is
>> no clear benefit.
> I already posted the example of this.  I can complete whole thing
> and send it all in one huge chunk if you prefer that 
> 
>> While I agree that there is a lot of duplicated code in block.c and
>> block/*, I don't think that what we need is more parameters to the
>> functions.  We have places where we need to know the request flags, for
>> example, but the methods are already quite unwieldy and have a lot of
>> arguments.  So I'm not sure that this kind of unification buys anything.
> Which places are these?

If we turn zero-write into a special case of discard, we will need it as
a flag in discard.  Block mirroring would like to have access to
copy-on-read flags.

> Also, how about dropping nr_sectors?
> 
> If you need flags, well, the extra argument being
> added can really be used for that if necessary.

Or we can actually clean up everything, and create a real "request"
structure that can be passed along.  See how flush and discard are
really similar (which do not have all the accumulated stuff for
throttling, copy-on-read, bounce buffering, etc.).  Perhaps that's the
place to start.

Paolo