On 22.12.19 12:37, 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 <berto@igalia.com>
> ---
> block/qcow2-cluster.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 0a40944667..ed291a4042 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -986,6 +986,23 @@ 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)) {
> + unsigned written_from = m->cow_start.offset;
> + unsigned written_to = m->cow_end.offset + m->cow_end.nb_bytes ?:
> + m->nb_clusters << s->cluster_bits;
I suppose we could also calculate both at the beginning of the function
(I’m not sure whether the compiler can optimize these calculations to
happen only once if we don’t).
> + uint64_t l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index + i);
> + int sc;
> + for (sc = 0; sc < s->subclusters_per_cluster; sc++) {
> + int sc_off = i * s->cluster_size + sc * s->subcluster_size;
> + if (sc_off >= written_from && sc_off < written_to) {
> + l2_bitmap |= QCOW_OFLAG_SUB_ALLOC(sc);
> + l2_bitmap &= ~QCOW_OFLAG_SUB_ZERO(sc);
Works, but maybe a QCOW_OFLAG_SUB_MASK(sc) would be better for:
l2_bitmap &= ~QCOW_OFLAG_SUB_MASK(sc);
l2_bitmap |= QCOW_OFLAG_SUB_ALLOC(sc);
Nothing wrong though, so:
Reviewed-by: Max Reitz <mreitz@redhat.com>
> + }
> + }
> + set_l2_bitmap(s, l2_slice, l2_index + i, l2_bitmap);
> + }
> }
>
>
>