Re: possible mountroot regression

On Oct 18, 2011, at 9:04 AM, Andriy Gapon wrote:

> on 14/10/2011 18:54 Arnaud Lacombe said the following:
>> Andry Gapon wrote:
>>> Simple: revert to the previous behavior.  If a user enters incorrect device name
>>> (i.e. root mounting fails), then return back to the prompt instead of panicing.
>> That should do the job.
>> 
>> - Arnaud
>> 
>> ---
>> sys/kern/vfs_mountroot.c |   45 +++++++++++++++++++++++----------------------
>> 1 files changed, 23 insertions(+), 22 deletions(-)
>> 
>> diff --git a/sys/kern/vfs_mountroot.c b/sys/kern/vfs_mountroot.c
>> index ccbcb33..ae3ffa7 100644
>> --- a/sys/kern/vfs_mountroot.c
>> +++ b/sys/kern/vfs_mountroot.c
>> _at__at_ -481,28 +481,29 _at__at_ parse_dir_ask(char **conf)
>> 	printf("\n");
>> 	printf("  ?               List valid disk boot devices\n");
>> 	printf("  .               Yield 1 second (for background tasks)\n");
>> -	printf("  <empty line>    Abort manual input\n");
>> +	printf("  x               Abort manual input)\n");
>> +
>> +	do {
>> +		error = EINVAL;
>> +		printf("\nmountroot> ");
>> +		gets(name, sizeof(name), GETS_ECHO);
>> +		if (name[0] == '?') {
>> +			printf("\nList of GEOM managed disk devices:\n  ");
>> +			g_dev_print();
>> +			continue;
>> +		}
>> +		if (name[0] == '.') {
>> +			pause("rmask", hz);
>> +			continue;
>> +		}
>> +		if (name[0] == 'x' && name[1] == '\0')
>> +			break;
>> +		mnt = name;
>> +		error = parse_mount(&mnt);
>> +		if (error < 0)
>> +			printf("Invalid specification.\n");
>> +	} while (error != 0);
>> 
>> - again:
>> -	printf("\nmountroot> ");
>> -	gets(name, sizeof(name), GETS_ECHO);
>> -	if (name[0] == '\0')
>> -		return (0);
>> -	if (name[0] == '?') {
>> -		printf("\nList of GEOM managed disk devices:\n  ");
>> -		g_dev_print();
>> -		goto again;
>> -	}
>> -	if (name[0] == '.') {
>> -		pause("rmask", hz);
>> -		goto again;
>> -	}
>> -	mnt = name;
>> -	error = parse_mount(&mnt);
>> -	if (error == -1) {
>> -		printf("Invalid specification.\n");
>> -		goto again;
>> -	}
>> 	return (error);
>> }
>> 
> 
> Arnaud,
> 
> I like how your change fixes the regression and improves code style.
> As you've said, the 'x' change is unrelated.  I like it, but it needs to be
> discussed and committed separately.
> 
> Marcel,
> 
> what do you think?

I like it.

> Would you be able to commit a variant of this patch sans the 'x' part?
> 

Yes, soonish. If people like the 'x' change I can do that in a followup
commit as well. I just need to know if people like it or not...

-- 
Marcel Moolenaar
marcel_at_xcllnt.net