Re: [PATCH 13/17] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO

[Olaf Hering <olaf@aepfle.de>]

On Tue, Jul 24, K. Y. Srinivasan wrote:

> +	/*
> +	 * Set the configuration for the specified interface with
> +	 * the information provided. Since there is no standard
> +	 * way to configure an interface, we will have an external
> +	 * script that does the job of configuring the interface and
> +	 * flushing the configuration.
> +	 *
> +	 * The parameters passed to this external script are:
> +	 * 1. A configuration file that has the specified configuration.

Maybe this should be written as 'A info file that has the requested
network configuration' or something like that.

> +	 *
> +	 * We will embed the name of the interface in the configuration
> +	 * file: ifcfg-ethx (where ethx is the interface name).

I think the intention here is to use the generated file as is. Depending
on the distro in the guest the file may need some processing. So I think
the actual interface name should also be part of the file.

> +	 *
> +	 * Here is the format of the ip configuration file:
> +	 *
> +	 * HWADDR=macaddr
> +	 * BOOTPROTO=dhcp (dhcp enabled for the interface)

While BOOTPROTO= is used in current network config files, its meaning
there is unrelated to what its meant here. Here it means DHCP=yes/no, so
I think the file should contain just that. And as the code is written
now BOOTPROTO= is optional, which is not mentioned in the comment.

> +	 * NM_CONTROLLED=no (this interface will not be controlled by NM)

I think this is not up to kvp_deamon to decide what controls the
interface. Maybe one day NM is sufficiently advanced technology that it
can cope with such requests?
The helper script should decide if the NM flag should be written to the
final config file.

> +	 * PEERDNS=yes
> +	 * IPADDR_x=ipaddr
> +	 * NETMASK_x=netmask
> +	 * GATEWAY_x=gateway

This should mention that its either IPADDR=ipaddr1 or 'IPADDR=ipaddr1;
IPADDR_1=ipaddr2'

> +	 * DNSx=dns

This should mention that its either DNS=ipaddr1 or 'DNS=ipaddr1;
DNS1=ipaddr2'

Olaf