Re: [Qemu-block] [RFC PATCH 0/8] quorum: Implement .bdrv_co_preadv/pwritev()

[Kevin Wolf]

Am 11.11.2016 um 10:56 hat Paolo Bonzini geschrieben:
> On 10/11/2016 18:19, Kevin Wolf wrote:
> > This is part of a series that I'm working on and that aims to remove the
> > bdrv_aio_*() emulation in io.c. blkdebug and blkverify were easy, but for
> > quorum I need a few more patches, so I'm sending this out as an RFC while I
> > continue work on the rest (QED, and then possibly some polishing).
> > 
> > After the series, quorum doesn't only use the preferred interfaces, but the
> > code becomes a little easier to follow and byte granularity requests are
> > supported. There is probably still some potential for additional cleanups 
> > in a
> > follow-up series, but this should already be enough to be worth merging.
> 
> Honestly I don't see the point.  It seems easier, more practical and
> more effective to convert bdrv_aio_* to byte ranges, especially since
> QED would be basically a rewrite.

Well, it's an RFC for a reason, I want people to give their opinions
before consistently following through with this. But I do think that we
pointlessly offer too many different interfaces to do the same thing in
the block layer.

Anyway, I think that this series makes the quorum driver quite a bit
easier to follow (and it was the same thing really for blkdebug and
blkverify), even though I only made the really obvious big changes. If
someone looked into this, there are most likely more simplifications
that could be done now.

Sometimes, simplicity is in the eye of the beholder and that's why I'm
asking for more opinions, but for me this is a clear improvement. With
this, the added benefit that we can phase out at least one of the bdrv_*
interfaces and byte granularity as a side effect, my question is rather:
What would be the point in not doing it?


As for QED, that shouldn't be hard. Quorum was really the hardest
conversion because it actually uses AIO to implement parallelism.
blkverify was similar, but on a much simpler level. But QED won't need
to create any coroutines internally because each request is processed
strictly sequentially. It's a simple mechanical conversion from:

    bdrv_aio_foo(..., cb, opaque);

to:

    ret = bdrv_co_foo(...);
    cb(opaque, ret);

And that's it. The rest is cleanup work that we may or may not do.

Kevin