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

Olaf Hering
2012-07-30T17:37:05Z

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.