On 6/9/16 8:27 AM, Jan Tulak wrote:
> If user writes a value using b or s suffix without explicitly stating the size
> of blocks or sectors, mkfs ends with a not helpful error about the value being
> too small. It happens because we read the physical geometry after all options
> are parsed.
It's probably worth putting an example of the current error in the commitlog,
i.e.:
# mkfs.xfs -dfile,name=fsfile,size=128m -l su=10b -b
Illegal value 10b for -l su option. value is too small
...
> So, tell the user exactly what is wrong with the input.
>
> Signed-off-by: Jan Tulak <jtulak@xxxxxxxxxx>
> ---
>
> While adding entries into the mkfs input test, I found this issue, though it
> may be only a documentation thing. When I give some option block/sector suffix
> without specifying explicitly its size (for example, -l su=10b without -b
> size=4096), I get an error that value 10b is too small. Of course it is,
> because, at the time, mkfs did not read physical geometry yet, so blocksize is
> 0. And 10*0 = 0.
>
> I think that this is not something we need to change, but it should be better
> documented. Maybe not manpage (where it can be overlooked if not written to
> every option using the size and it might be that it already is somewhere down
> there), but an error message should warn the user in case of using b or s
> suffix incorrectly.
>
> I'm open to suggestions for a better solution, though.
>
> UPDATE:
> Add { and } to fix a gcc warning about ambigious else branch.
>
> Cheers,
> Jan
> ---
> mkfs/xfs_mkfs.c | 22 ++++++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index ed7800f..455bf11 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3614,10 +3614,24 @@ cvtnum(
> if (sp[1] != '\0')
> return -1LL;
>
> - if (*sp == 'b')
> - return i * blksize;
> - if (*sp == 's')
> - return i * sectsize;
> + if (*sp == 'b') {
> + if (!blksize) {
> + fprintf(stderr,
> +_("Blocksize must be explicitly provided when using 'b' suffix.\n"));
I'd fix the warning text a little bit, because:
# mkfs.xfs -dfile,name=fsfile,size=128m -l su=10b -b size=4k
Blocksize must be explicitly provided when using 'b' suffix.
But I *did* provide it! No fair! ;)
Perhaps:
+_("Blocksize must be specified prior to using the 'b' suffix.\n"));
and
+_("Sectorsize must be specified prior to using the 's' suffix.\n"));
?
(old mkfs said "blocksize not available yet." which is a bit cryptic
as well, but at least we clearly have the same behavior now, just
different messages.)
-Eric
> + usage();
> + } else {
> + return i * blksize;
> + }
> + }
> + if (*sp == 's') {
> + if (!sectsize) {
> + fprintf(stderr,
> +_("Sectorsize must be explicitly provided when using 's' suffix.\n"));
> + usage();
> + } else {
> + return i * sectsize;
> + }
> + }
>
> c = tolower(*sp);
> switch (c) {
>
|