[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 01/12] qcow2: Allow "full" discard
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v5 01/12] qcow2: Allow "full" discard |
Date: |
Tue, 22 Apr 2014 16:04:29 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 17.04.2014 um 23:59 hat Max Reitz geschrieben:
> Normally, discarded sectors should read back as zero. However, there are
> cases in which a sector (or rather cluster) should be discarded as if
> they were never written in the first place, that is, reading them should
> fall through to the backing file again.
>
> Signed-off-by: Max Reitz <address@hidden>
> ---
> block/qcow2-cluster.c | 26 ++++++++++++++++----------
> block/qcow2-snapshot.c | 2 +-
> block/qcow2.c | 2 +-
> block/qcow2.h | 2 +-
> 4 files changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 331ab08..9b73d97 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1343,7 +1343,7 @@ int qcow2_decompress_cluster(BlockDriverState *bs,
> uint64_t cluster_offset)
> * clusters.
> */
> static int discard_single_l2(BlockDriverState *bs, uint64_t offset,
> - unsigned int nb_clusters, enum qcow2_discard_type type)
> + unsigned int nb_clusters, enum qcow2_discard_type type, bool
> full_discard)
> {
> BDRVQcowState *s = bs->opaque;
> uint64_t *l2_table;
> @@ -1365,25 +1365,31 @@ static int discard_single_l2(BlockDriverState *bs,
> uint64_t offset,
> old_offset = be64_to_cpu(l2_table[l2_index + i]);
>
> /*
> - * Make sure that a discarded area reads back as zeroes for v3 images
> - * (we cannot do it for v2 without actually writing a zero-filled
> - * buffer). We can skip the operation if the cluster is already
> marked
> - * as zero, or if it's unallocated and we don't have a backing file.
> + * If full_discard is false, make sure that a discarded area reads
> back
> + * as zeroes for v3 images (we cannot do it for v2 without actually
> + * writing a zero-filled buffer). We can skip the operation if the
> + * cluster is already marked as zero, or if it's unallocated and we
> + * don't have a backing file.
> *
> * TODO We might want to use bdrv_get_block_status(bs) here, but
> we're
> * holding s->lock, so that doesn't work today.
> + *
> + * In case of full_discard being true, the sector should not be read
> + * back as zeroes, but rather fall through to the backing file.
> */
> - if (old_offset & QCOW_OFLAG_ZERO) {
> + if (!full_discard && (old_offset & QCOW_OFLAG_ZERO)) {
> continue;
> }
>
> - if ((old_offset & L2E_OFFSET_MASK) == 0 && !bs->backing_hd) {
> + if ((old_offset & L2E_OFFSET_MASK) == 0 &&
> + (full_discard || !bs->backing_hd))
> + {
> continue;
> }
I don't think that's right. You wouldn't discard (non-preallocated) zero
clusters with this code. You should probably check the cluster type.
Kevin
- [Qemu-devel] [PATCH v5 00/12] qemu-img: Implement commit like QMP, Max Reitz, 2014/04/17
- [Qemu-devel] [PATCH v5 01/12] qcow2: Allow "full" discard, Max Reitz, 2014/04/17
- Re: [Qemu-devel] [PATCH v5 01/12] qcow2: Allow "full" discard,
Kevin Wolf <=
- [Qemu-devel] [PATCH v5 04/12] blockjob: Add "ready" field, Max Reitz, 2014/04/17
- [Qemu-devel] [PATCH v5 03/12] blockjob: Introduce block_job_complete_sync(), Max Reitz, 2014/04/17
- [Qemu-devel] [PATCH v5 05/12] block/mirror: Improve progress report, Max Reitz, 2014/04/17
- [Qemu-devel] [PATCH v5 06/12] qemu-img: Implement commit like QMP, Max Reitz, 2014/04/17
- [Qemu-devel] [PATCH v5 07/12] qemu-img: Empty images after commit, Max Reitz, 2014/04/17