On 5/14/14, 5:04 PM, Goffredo Baroncelli wrote:
> On 05/14/2014 07:39 PM, Eric Sandeen wrote:
>> Allow the specification of the filesystem UUID at mkfs time.
>>
>> Non-unique unique IDs are rejected.  This includes attempting
>> to re-mkfs with the same UUID; if you really want to do that,
>> you can mkfs with a new UUID, then re-mkfs with the one you
>> wanted.  ;)
>>
>> (Implemented only for mkfs.btrfs, not btrfs-convert).
>>
>> Signed-off-by: Eric Sandeen <sand...@redhat.com>
>> ---
>>
>> NB: the prior patch didn't work well if you re-mkfs'd with
>> the same UUID; to be honest I didn't get to the bottom of it,
>> but the fact that that old UUID was already in an internal
>> list of present devices seems to have confused things.
>>
>> V2: reject non-unique unique IDs.
>> V3: test for non-unique unique IDs early in mkfs.
>>

<snip>

>> @@ -1368,6 +1374,20 @@ int main(int ac, char **av)
>>                      "The -r option is limited to a single device\n");
>>              exit(1);
>>      }
>> +
>> +    if (fs_uuid) {
>> +            uuid_t dummy_uuid;
>> +
>> +            if (uuid_parse(fs_uuid, dummy_uuid) != 0) {
>> +                    fprintf(stderr, "could not parse UUID: %s\n", fs_uuid);
>> +                    exit(1);
>> +            }
>> +            if (!test_uuid_unique(fs_uuid)) {
>> +                    fprintf(stderr, "non-unique UUID: %s\n", fs_uuid);
>> +                    exit(1);
>> +            }
> 
> My test showed that this detect a false positive when the user tries to mkfs 
> two time on the same device with the same uuid.

It's not a false positive; after the first mkfs, the UUID does exist.  :)  See 
also the commit log I wrote.

>> diff --git a/utils.c b/utils.c
>> index 3e9c527..cfac0d4 100644
>> --- a/utils.c
>> +++ b/utils.c
>> @@ -93,12 +93,41 @@ static u64 reference_root_table[] = {
>>      [6] =   BTRFS_CSUM_TREE_OBJECTID,
>>  };
>>  
>> -int make_btrfs(int fd, const char *device, const char *label,
>> +int test_uuid_unique(char *fs_uuid)
>> +{
>> +    int unique = 1;
>> +    blkid_dev_iterate iter = NULL;
>> +    blkid_dev dev = NULL;
>> +    blkid_cache cache = NULL;
>> +
>> +    if (blkid_get_cache(&cache, 0) < 0) {
>> +            printf("ERROR: lblkid cache get failed\n");
>> +            return 1;
>> +    }
>> +    blkid_probe_all(cache);
>> +    iter = blkid_dev_iterate_begin(cache);
>> +    blkid_dev_set_search(iter, "UUID", fs_uuid);
>> +
>> +    while (blkid_dev_next(iter, &dev) == 0) {
> 
> This function should skip the check for the devices involved in the mkfs.

Perhaps.  When I was doing something similar before, I ended up with 
inexplicable segfaults when a device found on initial scan (?) got recreated
with the same UUID.  Or something.

<snip>

>> @@ -125,7 +154,20 @@ int make_btrfs(int fd, const char *device, const char 
>> *label,
>>      memset(&super, 0, sizeof(super));
>>  
>>      num_bytes = (num_bytes / sectorsize) * sectorsize;
>> -    uuid_generate(super.fsid);
>> +    if (fs_uuid) {
>> +            if (uuid_parse(fs_uuid, super.fsid) != 0) {
>> +                    fprintf(stderr, "could not parse UUID: %s\n", fs_uuid);
>> +                    ret = -EINVAL;
>> +                    goto out;
>> +            }
>> +            if (!test_uuid_unique(fs_uuid)) {
>> +                    fprintf(stderr, "non-unique UUID: %s\n", fs_uuid);
>> +                    ret = -EBUSY;
>> +                    goto out;
>> +            }
> 
> Why a second call to test_uuid_unique(fs_uuid) ?

Because kdave said he thought it was worth being paranoid in an earlier email,
if I understood him correctly.

-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to