[PATCH 2/5] dm: Add initial device mapper infrastructure
Ahmad Fatoum
a.fatoum at pengutronix.de
Mon Sep 8 11:40:03 PDT 2025
On 08.09.25 11:27, Tobias Waldekranz wrote:
> On fre, sep 05, 2025 at 18:14, Ahmad Fatoum <a.fatoum at pengutronix.de> wrote:
>> If someone looks at this code, because they have seen this error, they
>> won't necessarily know how to interpret the %d.
>
> My intention was that it could be correlated with the first column from
> the table generated by dm_asprint()...
Fair enough.
>
>> Can you either add a comment above the iteration or rename the i to a
>> more telling name?
>
> ...i.e., it is just an index (i). I'll add a comment describing that.
Ok. I was initially wondering if that's the "level" (e.g. dm-verity on top
of LVM) or something. A comment should be enough.
>> We don't support different block sizes right now and you hardcode
>> SECTOR_SIZE below anyway, but we like to pretend we could, so
>> blk->blockbits would be more apt here.
>
> I made a conscious decision to use hardcode these everywhere as Linux
> defines that a DM block _is_ 512B. You see these constants all over the
> place in the target implementations in the kernel as well.
>
> From dmsetup(8):
> | Devices are created by loading a table that specifies a target for
> | each sector (512 bytes) in the logical device.
>
> So while Barebox might one day support other block sizes, I do not think
> that DM ever will.
>
> What I also should have done is to have a comment detailing this, above
> the hardcoded block size. If I add this, do you think we should stick
> with the constants, or change to blk->blockbits?
Ok, let's keep it hardcoded then. A single comment in the probe function
is enough.
>>> +char *dm_asprint(struct dm_device *dm)
>>> +{
>>> + char **strv, *str, *tistr;
>>> + struct dm_target *ti;
>>> + int n = 0, strc;
>>> +
>>> + strc = 1 + dm_n_targets(dm);
>>> + strv = xmalloc(strc * sizeof(*strv));
>>
>>
>> Meh, maybe we should have a growable string type...
>
> I have taken a stab at this in v2 with rasprintf(), "the realloc()ing
> sprintf()". Let me know what you think.
Looking forward to it!
Cheers,
Ahmad
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
More information about the barebox
mailing list