Am 22.11.2011 16:10, schrieb Stefan Hajnoczi:
> On Tue, Nov 22, 2011 at 3:06 PM, Kevin Wolf <kw...@redhat.com> wrote:
>> Am 17.11.2011 14:40, schrieb Stefan Hajnoczi:
>>> Detect overlapping requests and remember to align to cluster boundaries
>>> if the image format uses them. This assumes that allocating I/O is
>>> performed in cluster granularity - which is true for qcow2, qed, etc.
>>>
>>> Signed-off-by: Stefan Hajnoczi <stefa...@linux.vnet.ibm.com>
>>
>>> static void coroutine_fn wait_for_overlapping_requests(BlockDriverState
>>> *bs,
>>> int64_t sector_num, int nb_sectors)
>>> {
>>> BdrvTrackedRequest *req;
>>> + int64_t cluster_sector_num;
>>> + int cluster_nb_sectors;
>>> bool retry;
>>>
>>> + /* If we touch the same cluster it counts as an overlap */
>>> + round_to_clusters(bs, sector_num, nb_sectors,
>>> + &cluster_sector_num, &cluster_nb_sectors);
>>
>> Is this really required? Image formats must be able to deal with two
>> concurrent write requests to the same cluster, and I don't think it
>> makes a difference whether it's a guest write request or a COR one.
>>
>> Or does the queuing protect more than just that a COR never takes
>> precedence over a guest write?
>
> It guarantees that a write request and a copy-on-read request racing
> for the same cluster will be serialized. Either:
> 1. CoR, then write.
> 2. Allocating write, then normal read.
>
> If we don't do this we risk:
> 3. CoR (part 1: read), allocating write, CoR (part 2: write)
>
> The result is that we dropped the write request!
Right, this requirement comes in with the next patch that makes COR
round clusters for optimisation. I think this relationship deserves a
comment here.
Anyway, I merged the series into the block branch (Only to notice that
it would have been better to merge bdrv_co_is_allocated first... Will
review that tomorrow.) If you send another version for this patch to add
a comment, I'll replace it.
Kevin
