[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC V6 04/33] qcow2: Add qcow2_dedup_read_missing_and_
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [RFC V6 04/33] qcow2: Add qcow2_dedup_read_missing_and_concatenate |
Date: |
Wed, 6 Feb 2013 17:45:41 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Wed, Feb 06, 2013 at 01:31:37PM +0100, Benoît Canet wrote:
> +/*
> + * Prepare a buffer containing all the required data required to compute
> cluster
Easier to read this way:
s/all the required data required/everything required/
> + * sized deduplication hashes.
> + * If sector_num or nb_sectors are not cluster-aligned, missing data
> + * before/after the qiov will be read.
> + *
> + * @qiov: the qiov for which missing data must be read
> + * @sector_num: the first sectors that must be read into the qiov
> + * @nb_sectors: the number of sectors to read into the qiov
> + * @data: the place where the data will be concatenated and
> stored
> + * @nb_data_sectors: the resulting size of the contatenated data (in
> sectors)
> + * @ret: negative on error
> + */
> +int qcow2_dedup_read_missing_and_concatenate(BlockDriverState *bs,
> + QEMUIOVector *qiov,
> + uint64_t sector_num,
> + int nb_sectors,
> + uint8_t **data,
> + int *nb_data_sectors)
> +{
> + BDRVQcowState *s = bs->opaque;
> + int ret = 0;
> + uint64_t cluster_beginning_sector;
> + uint64_t first_sector_after_qiov;
> + int cluster_beginning_nr;
> + int cluster_ending_nr;
> + int unaligned_ending_nr;
> + uint64_t max_cluster_ending_nr;
> +
> + /* compute how much and where to read at the beginning */
> + cluster_beginning_nr = sector_num & (s->cluster_sectors - 1);
> + cluster_beginning_sector = sector_num - cluster_beginning_nr;
> +
> + /* for the ending */
> + first_sector_after_qiov = sector_num + nb_sectors;
> + unaligned_ending_nr = first_sector_after_qiov & (s->cluster_sectors - 1);
> + cluster_ending_nr = unaligned_ending_nr ?
> + s->cluster_sectors - unaligned_ending_nr : 0;
> +
> + /* compute total size in sectors and allocate memory */
> + *nb_data_sectors = cluster_beginning_nr + nb_sectors + cluster_ending_nr;
> + *data = qemu_blockalign(bs, *nb_data_sectors * BDRV_SECTOR_SIZE);
> +
> + /* read beginning */
> + if (cluster_beginning_nr) {
> + ret = qcow2_read_cluster_data(bs,
> + *data,
> + cluster_beginning_sector,
> + cluster_beginning_nr);
> + }
> +
> + if (ret < 0) {
> + goto fail;
> + }
> +
> + /* append qiov content */
> + qemu_iovec_to_buf(qiov, 0, *data + cluster_beginning_nr *
> BDRV_SECTOR_SIZE,
> + qiov->size);
> +
> + /* Fix cluster_ending_nr if we are at risk of reading outside the image
> + * (Cluster unaligned image size)
> + */
> + max_cluster_ending_nr = bs->total_sectors - first_sector_after_qiov;
> + cluster_ending_nr = max_cluster_ending_nr < (uint64_t) cluster_ending_nr
> ?
> + (int) max_cluster_ending_nr : cluster_ending_nr;
Is there a test case for the cluster unaligned image size scenario?
> +
> + /* read and add ending */
> + if (cluster_ending_nr) {
> + ret = qcow2_read_cluster_data(bs,
> + *data +
> + (cluster_beginning_nr +
> + nb_sectors) *
> + BDRV_SECTOR_SIZE,
> + first_sector_after_qiov,
> + cluster_ending_nr);
> + }
> +
> + if (ret < 0) {
> + goto fail;
> + }
> +
> + return 0;
> +
> +fail:
> + qemu_vfree(*data);
> + *data = NULL;
> + return ret;
> +}
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 7610e56..ecbe352 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1110,6 +1110,41 @@ fail:
> return ret;
> }
>
> +/**
> + * Read some data from the QCOW2 file
> + *
> + * Important: s->lock is dropped. Things can change before the function
> returns
> + * to the caller.
> + *
> + * @data: the buffer where the data must be stored
> + * @sector_num: the sector number to read in the QCOW2 file
> + * @nb_sectors: the number of sectors to read
> + * @ret: negative on error
> + */
> +int qcow2_read_cluster_data(BlockDriverState *bs,
> + uint8_t *data,
> + uint64_t sector_num,
> + int nb_sectors)
> +{
> + BDRVQcowState *s = bs->opaque;
> + QEMUIOVector qiov;
> + struct iovec iov;
> + int ret;
> +
> + iov.iov_len = nb_sectors * BDRV_SECTOR_SIZE;
> + iov.iov_base = data;
> + qemu_iovec_init_external(&qiov, &iov, 1);
> + qemu_co_mutex_unlock(&s->lock);
> + ret = bdrv_co_readv(bs, sector_num, nb_sectors, &qiov);
This function should be marked coroutine_fn - it may only be called from
inside a coroutine. It's good to mark all coroutine functions so the
reader knows immediately this will run in coroutine context.
bdrv_co_readv() is does I/O throttling. This is wrong here since we
don't want to charge for internal I/O.
- [Qemu-devel] [RFC V6 00/33] QCOW2 deduplication core functionality, Benoît Canet, 2013/02/06
- [Qemu-devel] [RFC V6 02/33] qmp: Add DedupStatus enum., Benoît Canet, 2013/02/06
- [Qemu-devel] [RFC V6 01/33] qcow2: Add deduplication to the qcow2 specification., Benoît Canet, 2013/02/06
- [Qemu-devel] [RFC V6 03/33] qcow2: Add deduplication structures and fields., Benoît Canet, 2013/02/06
- [Qemu-devel] [RFC V6 04/33] qcow2: Add qcow2_de dup_read_missing_and_concatenate, Benoît Canet, 2013/02/06
- Re: [Qemu-devel] [RFC V6 04/33] qcow2: Add qcow2_dedup_read_missing_and_concatenate,
Stefan Hajnoczi <=
- Re: [Qemu-devel] [RFC V6 04/33] qcow2: Add qcow2_dedup_read_missing_and_concatenate, Eric Blake, 2013/02/06
- [Qemu-devel] [RFC V6 06/33] qcow2: Create a way to link to l2 tables when deduplicating., Benoît Canet, 2013/02/06
- [Qemu-devel] [RFC V6 05/33] qcow2: Make update_refcount public., Benoît Canet, 2013/02/06
- [Qemu-devel] [RFC V6 11/33] qcow2: Add qcow2_dedup_grow_table and use it., Benoît Canet, 2013/02/06
- [Qemu-devel] [RFC V6 12/33] qcow2: Makes qcow2_alloc_cluster_link_l2 mark to deduplicate clusters., Benoît Canet, 2013/02/06