xfs
[Top] [All Lists]

Re: [PATCH 2/2] xfsprogs: don't warn about log sunit size if it was auto

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH 2/2] xfsprogs: don't warn about log sunit size if it was auto-discovered
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Wed, 29 Oct 2014 15:57:36 -0400
Cc: Eric Sandeen <sandeen@xxxxxxxxxx>, Stan Hoeppner <stan@xxxxxxxxxxxxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <545135D4.6010908@xxxxxxxxxxx>
References: <544FD3E1.1060000@xxxxxxxxxx> <544FD47D.8020409@xxxxxxxxxxx> <20141029183841.GB4226@xxxxxxxxxxxxxx> <545135D4.6010908@xxxxxxxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
On Wed, Oct 29, 2014 at 01:45:40PM -0500, Eric Sandeen wrote:
> On 10/29/14 1:38 PM, Brian Foster wrote:
> > On Tue, Oct 28, 2014 at 12:38:05PM -0500, Eric Sandeen wrote:
> >> Today, users doing a bare mkfs on storage with a large default
> >> stripe size may be surprised to get this warning:
> >>
> >>  log stripe unit (%d bytes) is too large (maximum is 256KiB
> >>  log stripe unit adjusted to 32KiB
> >>
> >> through no fault of their own.  The fallback is appropriate
> >> and harmless, and there's no need to warn about this in the
> >> defaults case.
> >>
> >> However, we keep the warning if a large log stripe unit was
> >> specified by the user on the commandline.
> >>
> >> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> >> ---
> ...
> 
> >> @@ -2379,11 +2383,15 @@ an AG size that is one stripe unit smaller, for 
> >> example %llu.\n"),
> >>    }
> >>  
> >>    if (logversion == 2 && (lsunit * blocksize) > 256 * 1024) {
> >> -          fprintf(stderr,
> >> +          /* Warn only if specified on commandline */
> >> +          if (lsuflag || lsunitflag) {
> >> +                  fprintf(stderr,
> >>    _("log stripe unit (%d bytes) is too large (maximum is 256KiB)\n"),
> >> -                  (lsunit * blocksize));
> >> +                          (lsunit * blocksize));
> >> +                  fprintf(stderr,
> >> +  _("log stripe unit adjusted to 32KiB\n"));
> >> +          }
> > 
> > Right above this particular hunk we have the possibility of lsunit
> > inheriting a value from dsunit. If the latter is specified by the user,
> > we don't print the message for an arguably user-specified lsunit. Hmm,
> > do we care about that case?
> 
> I don't think we do care.  I think we only care if the user manually
> specified a log stripe unit and we're changing what they asked for.
> 
> I'd put "inheritance" in the "using default behaviors" case, and not
> warn about it.
> 

Ok, sounds sane. I was thinking there might be some utility to the
message for people with larger auto-detected stripe units, but we print
the geometry anyways and those users are clearly Ok with default
settings (Dave points out that 32k is the default).

The code looks Ok to me:

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

> -Eric
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

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