xfs
[Top] [All Lists]

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

To: Mika Eloranta <mel@xxxxxxx>
Subject: Re: [PATCH] mkfs.xfs: add [-U uuid] option
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 22 Sep 2015 08:18:39 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1442855060-38259-1-git-send-email-mel@xxxxxxx>
References: <1442855060-38259-1-git-send-email-mel@xxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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?

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...

> @@ -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>