[SLOF] [PATCH 1/7] net-snk: Improve printed text when booting via network

[Thomas Huth]

On 17/12/15 23:18, David Gibson wrote:
> On Thu, Dec 17, 2015 at 09:18:50PM +0100, Thomas Huth wrote:
>> The output of net-snk is bad in a couple of places:
>> 1) The hard-coded "Bootloader 1.6" message is pretty useless
>>    for the users (we never changed the version number in the
>>    past five years) - so let's change that into printing
>>    "Initializing NIC" instead.
>> 2) "Requesting IP address via DHCP" is wrong for DHCPv6 since
>>    in this case, the net-snk might only request the boot file
>>    name via DHCPv6 - so change that to print "information"
>>    instead of "IP address"
>> 3) The client IPv6 address is never printed out, so do that now,
>>    too.
>>
>> Signed-off-by: Thomas Huth <thuth at redhat.com>
>> ---
>>  clients/net-snk/app/netapps/netboot.c | 13 ++++++++-----
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/clients/net-snk/app/netapps/netboot.c b/clients/net-snk/app/netapps/netboot.c
>> index cf20b59..76a4b4f 100644
>> --- a/clients/net-snk/app/netapps/netboot.c
>> +++ b/clients/net-snk/app/netapps/netboot.c
>> @@ -353,7 +353,6 @@ int dhcp(char *ret_buffer, filename_ip_t * fn_ip, unsigned int retries, int flag
>>  			set_ipv6_address(fn_ip->fd, 0);
>>  			rc = dhcpv6(ret_buffer, fn_ip);
>>  			if (rc == 0) {
>> -				printf("\n");
>>  				memcpy(&fn_ip->own_ip6, get_ipv6_address(), 16);
>>  				break;
>>  			}
>> @@ -388,8 +387,7 @@ netboot(int argc, char *argv[])
>>  	int32_t block_size = strtol(argv[5], 0, 10);
>>  	uint8_t own_mac[6];
>>  
>> -	printf("\n");
>> -	printf(" Bootloader 1.6 \n");
>> +	puts("\n Initializing NIC");
>>  	memset(&fn_ip, 0, sizeof(filename_ip_t));
>>  
>>  	/***********************************************************
>> @@ -511,7 +509,7 @@ netboot(int argc, char *argv[])
>>  		set_ipv6_address(fn_ip.fd, &obp_tftp_args.ci6addr);
>>  		break;
>>  	case IP_INIT_DEFAULT:
>> -		printf("  Requesting IP address via DHCP: ");
>> +		printf("  Requesting information via DHCP: ");
>>  		rc = dhcp(ret_buffer, &fn_ip, obp_tftp_args.bootp_retries, 0);
>>  		break;
>>  	case IP_INIT_NONE:
>> @@ -548,10 +546,15 @@ netboot(int argc, char *argv[])
>>  		return -101;
>>  	}
>>  
>> -	if(ip_version == 4)
>> +	if (ip_version == 4) {
>>  		printf("%d.%d.%d.%d\n",
>>  			((fn_ip.own_ip >> 24) & 0xFF), ((fn_ip.own_ip >> 16) & 0xFF),
>>  			((fn_ip.own_ip >>  8) & 0xFF), ( fn_ip.own_ip        & 0xFF));
>> +	} else if (ip_version == 6) {
>> +		char ip6_str[40];
>> +		ipv6_to_str(fn_ip.own_ip6.addr, ip6_str);
>> +		printf("done\n  Using IPv6 address: %s\n", ip6_str);
>> +	}
> 
> The messages seem a bit gratuitously different in the IPv4 vs. IPv6
> case.  Why not just print the address in each case?

Depending on the way that should be used (IPv4 vs IPv6), SLOF prints
currently (without my patch) one of these messages before starting the
DHCP transaction:

 "Requesting IP address via DHCPv4:"
 "Requesting information via DHCPv6:"
 "Requesting IP address via DHCP:"

Then, after the colon, it counts down from 10 to 0 while trying to get
information via DHCP. In the IPv4 case, the count-down number is then
replaced with the IP address. In the IPv6 case, the address is not
printed out yet - and it would also be inappropriate to print it in the
same line since SLOF currently does not get the IPv6 address via DHCPv6
yet, it only gets the bootfile URL that way.

So if we want to have this text output in a more unified way, I'd say we
should maybe go the other way round instead, and always print
"Requesting information via ...: " first (i.e. not talking about "IP
address" here), then replace the number with "done" when the DHCP
transactions have been successful, and finally print
"Using [IPv4|IPv6] address: " in a new line.

Does that sound appropriate? If yes, I'll rework my patch ... if no, I'm
open for other suggestions!

 Thomas


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.ozlabs.org/pipermail/slof/attachments/20151218/93abe156/attachment.sig>