On 2015年05月28日 22:55, Eric Sandeen wrote:
> On 5/28/15 2:06 AM, Wang Sheng-Hui wrote:
>> Some conditions are duplicated. Merge them.
>
> While you are correct that they are duplicated, simply combining
> them doesn't mkae a lot of sense; now we'll get this sort of
> error message:
>
>> agsize (%lld blocks) too small, need at least %lld blocks
>> too many allocation groups for size = %lld
>> need at most %lld allocation groups
>
> and what should a user do with that information? It's somewhat
> nonsensical.
>
> The code as it is today, and as it has been since commit
> 1f1b8be7926480046ead7b98c9850ace7bcd82a3, doesn't make much sense,
> because we can never hit the second conditional. But simply
> combining them doesn't look like the right answer to me.
>
> If you look at the above commit, what it used to do was something
> like:
>
> - /*
> - * If the specified agsize is too small, or too large,
> - * complain.
> - */
> - if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
> - fprintf(stderr,
> - _("agsize (%lldb) too small, need at least %lld blocks\n"),
> - (long long)agsize,
> - (long long)XFS_AG_MIN_BLOCKS(blocklog));
> - usage();
> - }
>
> <snip>
>
> - /*
> - * If the ag size is too small, complain if agcount/agsize was
> - * specified, and fix it otherwise.
> - */
> - if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
> - if (daflag || dasize) {
> - fprintf(stderr,
> - _("too many allocation groups for size = %lld\n"),
> - (long long)agsize);
> - fprintf(stderr,
> - _("need at most %lld allocation groups\n"),
> - (long long)
> - (dblocks / XFS_AG_MIN_BLOCKS(blocklog) +
> - (dblocks % XFS_AG_MIN_BLOCKS(blocklog) != 0)));
>
> ...
>
> I *think* the intent was to use different messages based on what
> was specified by the user; i.e. if agsize was specified, and is too
> small, then say:
>
>> agsize (%lld blocks) too small, need at least %lld blocks
>
> but if agcount was specified, and was too large, then say:
>
>> too many allocation groups for size = %lld
>> need at most %lld allocation groups
>
> but combining those two messages doesn't make sense to me.
>
Got it, Eric.
Regards,
Sheng-Hui
> -Eric
>
>
>> Signed-off-by: Wang Sheng-Hui <shhuiw@xxxxxxxxxxx>
>> ---
>> mkfs/xfs_mkfs.c | 24 ++++++++----------------
>> 1 file changed, 8 insertions(+), 16 deletions(-)
>>
>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
>> index e2a052d..7ec2556 100644
>> --- a/mkfs/xfs_mkfs.c
>> +++ b/mkfs/xfs_mkfs.c
>> @@ -740,22 +740,6 @@ validate_ag_geometry(
>> __uint64_t agsize,
>> __uint64_t agcount)
>> {
>> - if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
>> - fprintf(stderr,
>> - _("agsize (%lld blocks) too small, need at least %lld blocks\n"),
>> - (long long)agsize,
>> - (long long)XFS_AG_MIN_BLOCKS(blocklog));
>> - usage();
>> - }
>> -
>> - if (agsize > XFS_AG_MAX_BLOCKS(blocklog)) {
>> - fprintf(stderr,
>> - _("agsize (%lld blocks) too big, maximum is %lld blocks\n"),
>> - (long long)agsize,
>> - (long long)XFS_AG_MAX_BLOCKS(blocklog));
>> - usage();
>> - }
>> -
>> if (agsize > dblocks) {
>> fprintf(stderr,
>> _("agsize (%lld blocks) too big, data area is %lld blocks\n"),
>> @@ -765,6 +749,10 @@ validate_ag_geometry(
>>
>> if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
>> fprintf(stderr,
>> + _("agsize (%lld blocks) too small, need at least %lld blocks\n"),
>> + (long long)agsize,
>> + (long long)XFS_AG_MIN_BLOCKS(blocklog));
>> + fprintf(stderr,
>> _("too many allocation groups for size = %lld\n"),
>> (long long)agsize);
>> fprintf(stderr, _("need at most %lld allocation groups\n"),
>> @@ -775,6 +763,10 @@ validate_ag_geometry(
>>
>> if (agsize > XFS_AG_MAX_BLOCKS(blocklog)) {
>> fprintf(stderr,
>> + _("agsize (%lld blocks) too big, maximum is %lld blocks\n"),
>> + (long long)agsize,
>> + (long long)XFS_AG_MAX_BLOCKS(blocklog));
>> + fprintf(stderr,
>> _("too few allocation groups for size = %lld\n"), (long long)agsize);
>> fprintf(stderr,
>> _("need at least %lld allocation groups\n"),
>>
>
>
>
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
