> -----Original Message----- > From: Olaf Hering [mailto:olaf@xxxxxxxxx] > Sent: Monday, July 30, 2012 1:33 PM > To: KY Srinivasan > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > devel@xxxxxxxxxxxxxxxxxxxxxx; apw@xxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; > ben@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 13/17] Tools: hv: Implement the KVP verb - > KVP_OP_SET_IP_INFO > > 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";? Why? > > > + > > + 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? Already done. > > > + > > + 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. I will audit all instances and fix it. > > > + } > > + } > > + 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? Already fixed. > > > + > > + 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. > > > Thanks Olaf, K. Y _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
