On Wed, Sep 20, 2023 at 09:03:49AM -0300, Guilherme G. Piccoli wrote:
> 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...
The duplicate uuid is unlikely so the loop would run once but making
sure it's not duplicate does not hurt so let's keep it.
