Re: [PATCH v7 14/32] qcow2: Add QCow2SubclusterType and qcow2_get_subcluster_type()

[Alberto Garcia <berto@igalia.com>]

On Tue 26 May 2020 10:32:08 PM CEST, Eric Blake wrote:
>> +/* The subcluster X [0..31] is allocated */
>> +#define QCOW_OFLAG_SUB_ALLOC(X)   (1ULL << (X))
>> +/* The subcluster X [0..31] reads as zeroes */
>> +#define QCOW_OFLAG_SUB_ZERO(X)    (QCOW_OFLAG_SUB_ALLOC(X) << 32)
>> +/* Subclusters [X, Y) (0 <= X <= Y <= 32) are allocated */
>
> As you are now using a half-open range, should this be:
>   (0 <= X < Y <= 32)

I changed the macros because I wanted to allow cases where X == Y.

The reason is the new qcow2_get_subcluster_range_type() function:

    case QCOW2_SUBCLUSTER_NORMAL:
        val = l2_bitmap | QCOW_OFLAG_SUB_ALLOC_RANGE(0, sc_from);
        return cto32(val) - sc_from;

If sc_from is 0 then the result of the macro is also 0, and 'val' equals
the lower 32 bits (allocation status bits) of the L2 bitmap, as
expected.

>> +#define QCOW_OFLAG_SUB_ALLOC_RANGE(X, Y) \
>> +    (QCOW_OFLAG_SUB_ALLOC(Y) - QCOW_OFLAG_SUB_ALLOC(X))
>
> with <= instead of <, then it is impossible to distinguish between
> QCOW_OFLAG_SUB_ALLOC_RANGE(0,0) and QCOW_OFLAG_SUB_ALLOC_RANGE(31,31)
> which both resolve to 0.

Exactly, there is no difference and there should not be.

Berto