On 19/09/2023 08:06, Anand Jain wrote:
> [...]
>> + while (dup_fsid) {
>> + dup_fsid = false;
>> + generate_random_uuid(vfsid);
>> +
>> + list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
>> + if (!memcmp(vfsid, fs_devices->fsid, BTRFS_FSID_SIZE) ||
>> + !memcmp(vfsid, fs_devices->metadata_uuid,
>> + BTRFS_FSID_SIZE))
>> + dup_fsid = true;
>> + }
>
>
> I've noticed this section of the code a few times, but I don't believe
> I've mentioned it before. We've been using generate_random_guid() and
> generate_random_uuid() without checking for UUID clashes. Why extra
> uuid clash check here?
>
Hi Anand, what would happen if the UUID clashes here? Imagine we have
another device with the same uuid (incredibly small chance, but...), I
guess this would break in the subsequent path of fs_devices addition,
hence I added this check, which is really cheap. We need to generate a
really unique uuid here as the temp one.
Do you see any con in having this check? I'd say we should maybe even
check in the other places the code is generating a random uuid but not
checking for duplicity currently...
