Re: [Qemu-devel] [RFC 5/6] block: core copy-on-read logic

[Stefan Hajnoczi]

On Tue, Oct 18, 2011 at 7:00 AM, Marcelo Tosatti <address@hidden> wrote:
> On Mon, Oct 17, 2011 at 04:47:31PM +0100, Stefan Hajnoczi wrote:
>> Signed-off-by: Stefan Hajnoczi <address@hidden>
>> ---
>>  block.c      |   69 
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  trace-events |    1 +
>>  2 files changed, 70 insertions(+), 0 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 0c22741..2aec6b4 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -1409,6 +1409,55 @@ int bdrv_pwrite_sync(BlockDriverState *bs, int64_t 
>> offset,
>>      return 0;
>>  }
>>
>> +static int coroutine_fn bdrv_co_copy_on_readv(BlockDriverState *bs,
>> +        int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
>> +{
>> +    void *bounce_buffer;
>> +    struct iovec iov;
>> +    QEMUIOVector bounce_qiov;
>> +    int64_t cluster_sector_num;
>> +    int cluster_nb_sectors;
>> +    size_t skip_bytes;
>> +    int ret;
>> +
>> +    /* Cover entire cluster so no additional backing file I/O is required 
>> when
>> +     * allocating cluster in the image file.
>> +     */
>> +    round_to_clusters(bs, sector_num, nb_sectors,
>> +                      &cluster_sector_num, &cluster_nb_sectors);
>> +
>> +    trace_bdrv_co_copy_on_readv(bs, sector_num, nb_sectors,
>> +                                cluster_sector_num, cluster_nb_sectors);
>> +
>> +    iov.iov_len = cluster_nb_sectors * BDRV_SECTOR_SIZE;
>> +    iov.iov_base = bounce_buffer = qemu_blockalign(bs, iov.iov_len);
>> +    qemu_iovec_init_external(&bounce_qiov, &iov, 1);
>> +
>> +    ret = bs->drv->bdrv_co_readv(bs, cluster_sector_num, cluster_nb_sectors,
>> +                                 &bounce_qiov);
>> +    if (ret < 0) {
>> +        goto err;
>> +    }
>> +
>> +    ret = bs->drv->bdrv_co_writev(bs, cluster_sector_num, 
>> cluster_nb_sectors,
>> +                                  &bounce_qiov);
>> +    if (ret < 0) {
>> +        /* It might be okay to ignore write errors for guest requests.  If 
>> this
>> +         * is a deliberate copy-on-read then we don't want to ignore the 
>> error.
>> +         * Simply report it in all cases.
>> +         */
>> +        goto err;
>> +    }
>> +
>> +    skip_bytes = (sector_num - cluster_sector_num) * BDRV_SECTOR_SIZE;
>> +    qemu_iovec_from_buffer(qiov, bounce_buffer + skip_bytes,
>> +                           nb_sectors * BDRV_SECTOR_SIZE);
>> +
>> +err:
>> +    qemu_vfree(bounce_buffer);
>> +    return ret;
>> +}
>> +
>>  /*
>>   * Handle a read request in coroutine context
>>   */
>> @@ -1431,7 +1480,27 @@ static int coroutine_fn 
>> bdrv_co_do_readv(BlockDriverState *bs,
>>      }
>>
>>      req = tracked_request_add(bs, sector_num, nb_sectors, false);
>
> The tracked request should include cluster round info?

When checking A and B for overlap, only one of them needs to be
rounded in order for overlap detection to be correct.  Therefore we
can store the original request [start, length) in tracked_requests and
only round the new request.

Stefan