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

[David Gibson]

On Fri, Dec 18, 2015 at 08:46:09AM +0100, Thomas Huth wrote:
> 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!

Ok, I'm not that fussed.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/slof/attachments/20151221/774cc4d6/attachment.sig>