xfs
[Top] [All Lists]

Re: [PATCH] mkfs.xfs: add [-U uuid] option

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH] mkfs.xfs: add [-U uuid] option
From: Mika Eloranta <mika.eloranta@xxxxxxx>
Date: Tue, 22 Sep 2015 01:56:53 +0300
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-sasl-enc:x-sasl-enc; s=smtpout; bh=GulQrmrDHD06BFN sZb+a7LSdmqU=; b=LFkZ2HzPpLqn+j3Hb20DkVfrNiBASCdlVyX2BwbSYs5o+Hk pT1dhlooquBM2q3KVEvGIyD9Sb4FmtYjX7RlAYOD2AFr2VqWTKrUHQytBKZe2O0N 0ieK+Kgfc2J99sK4k9enM9rJz5y0vkirI1nx6ozDpi3IGbiKqPvyiK+fWjmk=
In-reply-to: <20150921221839.GC19114@dastard>
References: <1442855060-38259-1-git-send-email-mel@xxxxxxx> <20150921221839.GC19114@dastard>
On 22 Sep 2015, at 01:18, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> 
> On Mon, Sep 21, 2015 at 08:04:20PM +0300, Mika Eloranta wrote:
>> The UUID can now be optionally specified during filesystem
>> creation.
> 
> Which UUID are you wanting to set - the metadata uuid or the user
> visible UUID label? Or both? Can you explain the use case for this?
> i.e. I'm trying to work out why Why doesn't mkfs.xfs +
> xfs_admin -U <uuid> doesn't work for you?

I want to set the user visible UUID (same as xfs_admin -U/-u). Whether this 
impacts the "metadata uuidâ or not, I do not know, Iâm not an expert on the XFS 
internals, just a user.
Use case: pre-defined UUID set for the filesystem by an _external system_ so 
that a detached (network) filesystem can later be correctly identified/verified 
by its UUID (i.e. by the contents of the filesystem instead of metadata outside 
the filesystem). For example, first creating a database record for a block 
filesystem with a random UUIDv4 and creating the actual filesystem with the 
same UUID only after that.
âmkfs.ext4" already supports this with the same exact invocation.

xfs_admin -U <uuid> does not seem to be an option because:

* xfs_admin -U option is going away? 
http://oss.sgi.com/pipermail/xfs/2015-April/041265.html
* Other people have issues with it: 
https://bugzilla.redhat.com/show_bug.cgi?id=1233220
* I get inconsistent behavior with it: about half of the time 
/dev/disk/by-uuid/* or âlsblk" do NOT see the change (Fedora 22).
* Pre-assigning the UUID straight away during the filesystem creation will work 
around any current or future bugs (e.g. above) and limitations regarding 
changing the UUID.
* When you need a filesystem with a specific UUID, it is an unnecessary extra 
step to first create it with a random UUID and only afterwards set it to the 
one you need.

> We need some explaination in the commit message so that when we look
> at this in a couple of years time we know why we added this to
> mkfsâ

Sure, I can update the commit message.

Please let me know if you need more clarifications, thanks!

Cheers,

        - Mika

> 
>> @@ -948,6 +948,7 @@ main(
>>      bool                    finobtflag;
>>      int                     spinodes;
>> 
>> +    platform_uuid_clear(&uuid);
>>      progname = basename(argv[0]);
>>      setlocale(LC_ALL, "");
>>      bindtextdomain(PACKAGE, LOCALEDIR);
>> @@ -990,7 +991,7 @@ main(
>>      xi.isdirect = LIBXFS_DIRECT;
>>      xi.isreadonly = LIBXFS_EXCLUSIVELY;
>> 
>> -    while ((c = getopt(argc, argv, "b:d:i:l:L:m:n:KNp:qr:s:CfV")) != EOF) {
>> +    while ((c = getopt(argc, argv, "b:d:i:l:L:U:m:n:KNp:qr:s:CfV")) != EOF) 
>> {
>>              switch (c) {
>>              case 'C':
>>              case 'f':
>> @@ -1465,6 +1466,10 @@ main(
>>                              illegal(optarg, "L");
>>                      label = optarg;
>>                      break;
>> +            case 'U':
>> +                    if (platform_uuid_parse(optarg, &uuid))
>> +                            illegal(optarg, "U");
>> +                    break;
> 
> I'd prefer not to introduce new top level options if possible - this
> seems to fit under the -m (global metadata options) option subgroup
> (i.e. -m uuid=<UUID>).
> 
>>              case 'm':
>>                      p = optarg;
>>                      while (*p != '\0') {
>> @@ -2550,7 +2555,9 @@ _("size %s specified for log subvolume is too large, 
>> maximum is %lld blocks\n"),
>>      sbp->sb_dblocks = dblocks;
>>      sbp->sb_rblocks = rtblocks;
>>      sbp->sb_rextents = rtextents;
>> -    platform_uuid_generate(&uuid);
>> +    if (platform_uuid_is_null(&uuid)) {
>> +        platform_uuid_generate(&uuid);
>> +    }
> 
> Just generate the uuid initially and then it gets overwritten by the
> CLI option if it is set. No need for null detection and generation
> here.
> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>