[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH v2 20/26] qcow2: Update L2 bitmap in qcow2_alloc_cluster_
From: |
Max Reitz |
Subject: |
Re: [RFC PATCH v2 20/26] qcow2: Update L2 bitmap in qcow2_alloc_cluster_link_l2() |
Date: |
Tue, 5 Nov 2019 12:43:16 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.1 |
On 26.10.19 23:25, Alberto Garcia wrote:
> The L2 bitmap needs to be updated after each write to indicate what
> new subclusters are now allocated.
>
> This needs to happen even if the cluster was already allocated and the
> L2 entry was otherwise valid.
>
> Signed-off-by: Alberto Garcia <address@hidden>
> ---
> block/qcow2-cluster.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index fb6cf8df17..acb7226e03 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -980,6 +980,22 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs,
> QCowL2Meta *m)
>
> set_l2_entry(s, l2_slice, l2_index + i, QCOW_OFLAG_COPIED |
> (cluster_offset + (i << s->cluster_bits)));
> +
> + /* Update bitmap with the subclusters that were just written */
> + if (has_subclusters(s)) {
> + uint64_t written_from = m->cow_start.offset;
> + uint64_t written_to = m->cow_end.offset + m->cow_end.nb_bytes;
It’s a bit strange to make these uint64_t when all the fields queried
are of type unsigned, but more on that below.
> + uint64_t l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index + i);
> + int sc;
> + for (sc = 0; sc < s->subclusters_per_cluster; sc++) {
> + uint64_t sc_off = i * s->cluster_size + sc *
> s->subcluster_size;
It’s weird to give this a uint64_t type when all the variables in the
term are of type int.
I’m not sure whether it can overflow. handle_alloc() limits everything
to INT_MAX, but I’m not sure about handle_copied().
Speaking of handle_copied(); both elements of Qcow2COWRegion are of type
unsigned. handle_copied() doesn’t look like it takes any precautions to
limit the range to even UINT_MAX (and it should probably limit it to
INT_MAX).
Max
> + if (sc_off >= written_from && sc_off < written_to) {
> + l2_bitmap |= QCOW_OFLAG_SUB_ALLOC(sc);
> + l2_bitmap &= ~QCOW_OFLAG_SUB_ZERO(sc);
> + }
> + }
> + set_l2_bitmap(s, l2_slice, l2_index + i, l2_bitmap);
> + }
> }
>
>
>
signature.asc
Description: OpenPGP digital signature
- Re: [RFC PATCH v2 20/26] qcow2: Update L2 bitmap in qcow2_alloc_cluster_link_l2(),
Max Reitz <=