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:

> +static char *kvp_get_if_name(char *guid)
> +{
> +	DIR *dir;
> +	struct dirent *entry;
> +	FILE    *file;
> +	char    *p, *q, *x;
> +	char    *if_name = NULL;
> +	char    buf[256];
> +	char *kvp_net_dir = "/sys/class/net/";
> +	char dev_id[100];

Perhaps this can be written as char dev_id[100] = "short string";?

> +
> +	dir = opendir(kvp_net_dir);
> +	if (dir == NULL)
> +		return NULL;
> +
> +	memset(dev_id, 0, sizeof(dev_id));
> +	strcat(dev_id, kvp_net_dir);
> +	q = dev_id + strlen(kvp_net_dir);
> +
> +	while ((entry = readdir(dir)) != NULL) {
> +		/*
> +		 * Set the state for the next pass.
> +		 */
> +		*q = '\0';
> +		strcat(dev_id, entry->d_name);
> +		strcat(dev_id, "/device/device_id");

Maybe this (and other) strcat should be changed to snprintf?

> +
> +		file = fopen(dev_id, "r");
> +		if (file == NULL)
> +			continue;
> +
> +		p = fgets(buf, sizeof(buf), file);
> +		if (p) {
> +			x = strchr(p, '\n');
> +			if (x)
> +				*x = '\0';
> +
> +			if (!strcmp(p, guid)) {
> +				/*
> +				 * Found the guid match; return the interface
> +				 * name. The caller will free the memory.
> +				 */
> +				if_name = strdup(entry->d_name);
> +				break;

This will leave 'file' open.
I have seen it in some other place as well.

> +			}
> +		}
> +		fclose(file);
> +	}
> +
> +	closedir(dir);
> +	return if_name;
> +}
> +
> +/*
> + * Retrieve the MAC address given the interface name.
> + */
> +
> +static char *kvp_if_name_to_mac(char *if_name)
> +{
> +	FILE    *file;
> +	char    *p, *x;
> +	char    buf[256];
> +	char addr_file[100];
> +	int i;
> +	char *mac_addr = NULL;
> +
> +	memset(addr_file, 0, sizeof(addr_file));
> +	strcat(addr_file, "/sys/class/net/");
> +	strcat(addr_file, if_name);
> +	strcat(addr_file, "/address");

snprintf?

> +
> +	file = fopen(addr_file, "r");
> +	if (file == NULL)
> +		return NULL;
> +
> +	p = fgets(buf, sizeof(buf), file);
> +	if (p) {
> +		x = strchr(p, '\n');
> +		if (x)
> +			*x = '\0';
> +		for (i = 0; i < strlen(p); i++)
> +			p[i] = toupper(p[i]);
> +		mac_addr = strdup(p);
> +	}
> +
> +	fclose(file);
> +	return mac_addr;
> +}
> +
> +
>  static void kvp_process_ipconfig_file(char *cmd,
>  					char *config_buf, int len,
>  					int element_size, int offset)
> @@ -800,6 +910,314 @@ getaddr_done:
>  }
>  
>  
> +static int expand_ipv6(char *addr, int type)
> +{
> +	int ret;
> +	struct in6_addr v6_addr;
> +
> +	ret = inet_pton(AF_INET6, addr, &v6_addr);
> +
> +	if (ret != 1) {
> +		if (type == NETMASK)
> +			return 1;
> +		return 0;
> +	}
> +
> +	sprintf(addr, "%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:"
> +		"%02x%02x:%02x%02x:%02x%02x",
> +		(int)v6_addr.s6_addr[0], (int)v6_addr.s6_addr[1],
> +		(int)v6_addr.s6_addr[2], (int)v6_addr.s6_addr[3],
> +		(int)v6_addr.s6_addr[4], (int)v6_addr.s6_addr[5],
> +		(int)v6_addr.s6_addr[6], (int)v6_addr.s6_addr[7],
> +		(int)v6_addr.s6_addr[8], (int)v6_addr.s6_addr[9],
> +		(int)v6_addr.s6_addr[10], (int)v6_addr.s6_addr[11],
> +		(int)v6_addr.s6_addr[12], (int)v6_addr.s6_addr[13],
> +		(int)v6_addr.s6_addr[14], (int)v6_addr.s6_addr[15]);
> +
> +	return 1;
> +
> +}
> +
> +static int is_ipv4(char *addr)
> +{
> +	int ret;
> +	struct in_addr ipv4_addr;
> +
> +	ret = inet_pton(AF_INET, addr, &ipv4_addr);
> +
> +	if (ret == 1)
> +		return 1;
> +	return 0;
> +}
> +
> +static int parse_ip_val_buffer(char *in_buf, int *offset,
> +				char *out_buf, int out_len)
> +{
> +	char *x;
> +	char *start;
> +
> +	/*
> +	 * in_buf has sequence of characters that are seperated by
> +	 * the character ';'. The last sequence does not have the
> +	 * terminating ";" character.
> +	 */
> +	start = in_buf + *offset;
> +
> +	x = strchr(start, ';');
> +	if (x)
> +		*x = 0;
> +	else
> +		x = start + strlen(start);
> +
> +	if (strlen(start) != 0) {
> +		int i = 0;
> +		/*
> +		 * Get rid of leading spaces.
> +		 */
> +		while (start[i] == ' ')
> +			i++;
> +
> +		if ((x - start) <= out_len) {
> +			strcpy(out_buf, (start + i));
> +			*offset += (x - start) + 1;
> +			return 1;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int kvp_write_file(FILE *f, char *s1, char *s2, char *s3)
> +{
> +	char str[256];
> +	int error;
> +
> +	memset(str, 0, sizeof(str));
> +	strcat(str, s1);
> +	if (s2 != NULL)
> +		strcat(str, s2);
> +	strcat(str, "=");
> +	strcat(str, s3);
> +	strcat(str, "\n");
> +
> +	error = fputs(str, f);
> +	if (error == EOF)
> +		return HV_E_FAIL;
> +
> +	return 0;
> +}
> +
> +
> +static int process_ip_string(FILE *f, char *ip_string, int type)
> +{
> +	int error = 0;
> +	char addr[INET6_ADDRSTRLEN];
> +	int i = 0;
> +	int j = 0;
> +	char str[256];
> +	char sub_str[10];
> +	int offset = 0;
> +
> +	memset(addr, 0, sizeof(addr));
> +
> +	while (parse_ip_val_buffer(ip_string, &offset, addr,
> +					(MAX_IP_ADDR_SIZE * 2))) {
> +		memset(sub_str, 0, sizeof(sub_str));
> +		memset(str, 0, sizeof(str));
> +
> +		if (is_ipv4(addr)) {
> +			switch (type) {
> +			case IPADDR:
> +				strcat(str, "IPADDR");
> +				break;
> +			case NETMASK:
> +				strcat(str, "NETMASK");
> +				break;
> +			case GATEWAY:
> +				strcat(str, "GATEWAY");
> +				break;
> +			case DNS:
> +				strcat(str, "DNS");
> +				break;
> +			}
> +			if (i != 0) {
> +				if (type != DNS)
> +					sprintf(sub_str, "_%d", i++);
> +				else
> +					sprintf(sub_str, "%d", ++i);
> +			} else if (type == DNS) {
> +				sprintf(sub_str, "%d", ++i);
> +			}
> +
> +
> +		} else if (expand_ipv6(addr, type)) {
> +			switch (type) {
> +			case IPADDR:
> +				strcat(str, "IPV6ADDR");
> +				break;
> +			case NETMASK:
> +				strcat(str, "IPV6NETMASK");
> +				break;
> +			case GATEWAY:
> +				strcat(str, "IPV6_DEFAULTGW");
> +				break;
> +			case DNS:
> +				strcat(str, "DNS");
> +				break;
> +			}
> +			if ((j != 0) || (type == DNS)) {
> +				if (type != DNS)
> +					sprintf(sub_str, "_%d", j++);
> +				else
> +					sprintf(sub_str, "%d", ++i);
> +			} else if (type == DNS) {
> +				sprintf(sub_str, "%d", ++i);
> +			}
> +		} else {
> +			return  HV_INVALIDARG;
> +		}
> +
> +		error = kvp_write_file(f, str, sub_str, addr);
> +		if (error)
> +			return error;
> +		memset(addr, 0, sizeof(addr));
> +	}
> +
> +	return 0;
> +}
> +
> +static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
> +{
> +	int error = 0;
> +	char if_file[50];
> +	FILE *file;
> +	char cmd[512];
> +	char *mac_addr;
> +
> +	/*
> +	 * 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.
> +	 *
> +	 * We will embed the name of the interface in the configuration
> +	 * file: ifcfg-ethx (where ethx is the interface name).
> +	 *
> +	 * Here is the format of the ip configuration file:
> +	 *
> +	 * HWADDR=macaddr
> +	 * BOOTPROTO=dhcp (dhcp enabled for the interface)
> +	 * NM_CONTROLLED=no (this interface will not be controlled by NM)
> +	 * PEERDNS=yes
> +	 * IPADDR_x=ipaddr
> +	 * NETMASK_x=netmask
> +	 * GATEWAY_x=gateway
> +	 * DNSx=dns
> +	 *
> +	 * IPV6 addresses will be tagged as IPV6ADDR, IPV6 gateway will be
> +	 * tagged as IPV6_DEFAULTGW and IPV6 NETMASK will be tagged as
> +	 * IPV6NETMASK.
> +	 */
> +
> +	memset(if_file, 0, sizeof(if_file));
> +	strcat(if_file, "/var/opt/hyperv/ifcfg-");
> +	strcat(if_file, if_name);
> +
> +	file = fopen(if_file, "w");
> +
> +	if (file == NULL) {
> +		syslog(LOG_ERR, "Failed to open config file");
> +		return HV_E_FAIL;
> +	}
> +
> +	/*
> +	 * First write out the MAC address.
> +	 */
> +
> +	mac_addr = kvp_if_name_to_mac(if_name);
> +	if (mac_addr == NULL) {
> +		error = HV_E_FAIL;
> +		goto setval_error;
> +	}
> +
> +	error = kvp_write_file(file, "HWADDR", NULL, mac_addr);
> +	if (error)
> +		goto setval_error;
> +
> +	error = kvp_write_file(file, "ONBOOT", NULL, "yes");
> +	if (error)
> +		goto setval_error;
> +
> +	error = kvp_write_file(file, "IPV6INIT", NULL, "yes");
> +	if (error)
> +		goto setval_error;
> +
> +	error = kvp_write_file(file, "NM_CONTROLLED", NULL, "no");
> +	if (error)
> +		goto setval_error;
> +
> +	error = kvp_write_file(file, "PEERDNS", NULL, "yes");
> +	if (error)
> +		goto setval_error;
> +
> +	if (new_val->dhcp_enabled) {
> +		error = kvp_write_file(file, "BOOTPROTO", NULL, "dhcp");
> +		if (error)
> +			goto setval_error;
> +
> +		/*
> +		 * We are done!.
> +		 */
> +		goto setval_done;
> +	}
> +
> +	/*
> +	 * Write the configuration for ipaddress, netmask, gateway and
> +	 * name servers.
> +	 */
> +
> +	error = process_ip_string(file, (char *)new_val->ip_addr, IPADDR);
> +	if (error)
> +		goto setval_error;
> +
> +	error = process_ip_string(file, (char *)new_val->sub_net, NETMASK);
> +	if (error)
> +		goto setval_error;
> +
> +	error = process_ip_string(file, (char *)new_val->gate_way, GATEWAY);
> +	if (error)
> +		goto setval_error;
> +
> +	error = process_ip_string(file, (char *)new_val->dns_addr, DNS);
> +	if (error)
> +		goto setval_error;
> +
> +setval_done:
> +	free(mac_addr);
> +	fclose(file);
> +
> +	/*
> +	 * Now that we have populated the configuration file,
> +	 * invoke the external script to do its magic.
> +	 */
> +
> +	memset(cmd, 0, sizeof(cmd));
> +	strcat(cmd, "/sbin/hv_set_ifconfig ");
> +	strcat(cmd, if_file);

The new patch should use "%s %s", not "%s%s" as format string.