xfs
[Top] [All Lists]

Re: [PATCH] xfsprogs: pick up 4k physical sector size

To: Eric Sandeen <sandeen@xxxxxxxxxx>
Subject: Re: [PATCH] xfsprogs: pick up 4k physical sector size
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 2 Mar 2012 11:20:43 +1100
Cc: xfs-oss <xfs@xxxxxxxxxxx>
In-reply-to: <4F500690.2080804@xxxxxxxxxx>
References: <4F500690.2080804@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Mar 01, 2012 at 05:30:24PM -0600, Eric Sandeen wrote:
> This splits the fs topology sectorsize into logical & physical,
> and gets both via blkid_get_topology.
> 
> After that there are various gyrations & warnings to handle
> various combinations of specified sector, blocksize, and
> what's actually found on disk.
> 
> mkfs.xfs's "sector size" gets reduced to logical if
> a block size < physical sector size is specified, for
> example.

Looks good, just a minor comment:

> +             sectorsize = ft.psectorsize ? ft.psectorsize :
> +                                           XFS_MIN_SECTORSIZE;
> +
> +             if ((blocksize < sectorsize) && (blocksize >= ft.lsectorsize)) {
> +                     fprintf(stderr, _("specified blocksize %d is less than "
> +                                       "device physical sector size %d\n"),
> +                                       blocksize, ft.psectorsize);

i wouldn't break the format string like that. Doing:

                        fprintf(stderr,
        _("specified blocksize %d is less than device physical sector size 
%d\n"),
                                blocksize, ft.psectorsize);

Is consistent with long format strings elsewhere in the xfsprogs
code, and it makes grepping easy. Same for each of the other long
format strings you broke in half...

Other than that, consider it:

Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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