Re: [Qemu-devel] [PATCH 0/3] block: Handle multiple write requests at once

[Kevin Wolf]

Christoph Hellwig schrieb:
> On Tue, Sep 01, 2009 at 03:51:49PM +0200, Kevin Wolf wrote:
>> virtio often issues multiple requests in a row, but each one independently. 
>> If
>> the block drivers knew all of the requests, they could optimize the way they
>> handle the requests. See the description of patch 3 for how qcow2 can use 
>> this
>> to avoid unnecessary writes to the disk.
> 
> I think this interface is extremly awkward and the layering is wrong.
> Everyone benefits from having one large instead of multiple small
> requests, so if we do get multiple sequential write requests we should
> always merged it at a high level even before starting to issue AIO,
> e.g. do it all in virtio-blk.

While I don't agree with everything you're saying, I think you do have a
point. When you're working on one specific problem, you're sometimes
blind for the big picture.

It's true that the functionality that is implemented so far could be
useful for all block formats. I'm not sure if merging requests is the
only useful thing you could with it, but for now it seems to be the only
one. So the general merging mechanism doesn't belong into the qcow2
block driver.

On the other hand, if you look at the TODO which says that we need to
merge requests if they are not directly adjacent, but touch the same
cluster, this is something that cannot be done by the generic block
layer - it doesn't even know that things like clusters exist. We could
have a block driver function like bdrv_can_merge that takes two requests
and checks if it would make sense to merge these requests. Does that
make sense?

Your suggestion to do it all in virtio-blk I don't like at all. We need
to do it in the most generic place, not in earliest possible place. Why
shouldn't there be other devices that could profit from it? Not sure
about IDE and SCSI, but the Xen disk device should definitely be able to
make use of it. Let's not make the mistake to change the code from being
too format specific to being too device specific. It belongs in block.c.

> Of course using a sane filesystem in the guest would also fix it,
> but the point of virtualization is at least partially to keep all
> that old crap working nicely.

Not sure what your definition of "old crap" is, but ext3 seems to meet
it. I don't think it's irrelevant.

Kevin