Re: [PATCH v4 23/30] qcow2: Update L2 bitmap in qcow2_alloc_cluster_link_l2()

[Alberto Garcia]

On Fri 24 Apr 2020 09:39:25 PM CEST, Eric Blake wrote:
>> +        /* 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;
>> +            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);
>> +                }
>> +            }
>
> Are there more efficient ways to set this series of bits than iterating 
> one bit at a time, while still remaining legible?  For example, what if 
> we had something like:
>
> l2_bitmap = get_l2_bitmap(...);
> int sc_from = OFFSET_TO_SC(written_from);
> int sc_to = OFFSET_TO_SC(written_to - 1);
> l2_bitmap |= QCOW_OFLAG_SUB_ALLOC_RANGE(sc_from, sc_to);
> l2_bitmap &= ~QCOW_OFLAG_SUB_ZERO_RANGE(sc_from, sc_to);

That's a very good suggestion, thanks!

Berto