Thank you for the suggestions, v2 patch posted which incorporates them.
Regards,
Tony
On 01/08/2018 05:16 AM, Karel Zak wrote:
> On Fri, Jan 05, 2018 at 11:15:39AM -0600, Tony Asleson wrote:
>> +static int stratis_valid_sb(blkid_probe pr, uint8_t *p)
>> +{
>> + const struct stratis_sb *stratis = (const struct stratis_sb *)p;
>> + uint32_t crc = 0;
>> +
>> + // generate CRC from byte position 4 for length 508 == 512 byte sector
>> + crc = crc32c(~0L, p + sizeof(stratis->crc32),
>> + BS - sizeof(stratis->crc32));
>> + crc ^= ~0L;
>> +
>> + return blkid_probe_verify_csum(pr, crc, le32_to_cpu(stratis->crc32));
>
> Not sure that blkid_probe_verify_csum() is a good idea for your
> use-case.
>
> The function ignores (returns 1) bad checksums if BLKID_SUBLKS_BADCSUM
> flag is enabled. I guess you want to use second (backup) superblock
> if the primary one is broken. It would be better to use
>
> return crc == le32_to_cpu(stratis->crc32);
>
> and do not use blkid_probe_verify_csum() at all.
>
>> +static int probe_stratis(blkid_probe pr,
>> + const struct blkid_idmag *mag __attribute__((__unused__)))
>> +{
>> + const struct stratis_sb *stratis = NULL;
>> + uint8_t *buf = blkid_probe_get_buffer(pr, 0, SB_AREA_SIZE);
>> +
>> + if (!buf)
>> + return errno ? -errno : 1;
>> +
>> + if (stratis_valid_sb(pr, buf + FIRST_COPY_OFFSET)) {
>> + stratis = (const struct stratis_sb *)(buf + FIRST_COPY_OFFSET);
>> + } else {
>> + if (!stratis_valid_sb(pr, buf + SECOND_COPY_OFFSET))
>> + return 1;
>> +
>> + stratis = (const struct stratis_sb *)
>> + (buf + SECOND_COPY_OFFSET);
>> + }
>> +
>> + blkid_probe_strncpy_uuid(pr, (unsigned char *)stratis->dev_uuid,
>> + sizeof(stratis->dev_uuid));
>> + blkid_probe_set_value(pr, "POOL_UUID",
>> + (unsigned char *)stratis->pool_uuid,
>> + sizeof(stratis->pool_uuid));
>> +
>> + blkid_probe_sprintf_value(pr, "SECTORS", "%" PRIu64, stratis->sectors);
>
> Hmm... SECTORS sounds pretty generic. Don't forget it will be
> stored in udevd db. Would be possible to use any prefix?
>
> For example:
>
> POOL_SECTORS
>
>> + blkid_probe_sprintf_value(pr, "INITIALIZATION_TIME", "%" PRIu64,
>> + stratis->initialization_time);
>
> POOL_INITTIME
>
> Karel
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
