2012-08-17 01:14:37

by KY Srinivasan

[permalink] [raw]
Subject: [PATCH V3 00/14] drivers: hv: kvp

This patch set expands the KVP (Key Value Pair) functionality to
implement the mechanism to GET/SET IP addresses in the guest. This
functionality is used in Windows Server 2012 to implement VM
replication functionality. The way IP configuration information
is managed is distro specific. Based on the feedback I have gotten
from Olaf, Greg, Steve, Ben and Mairus, I have chosen to seperate
distro specific code from this patch-set. Most of the GET operation
can be implemented in a way that is completely distro independent and
I have implemented that as such and is included in this patch-set.
Some of the attributes that can only be fetched in a distro
dependent way as well the mechanism for configuring an interface
(the SET operation) that is clearly distro specific is to be
implemented via external scripts that will be invoked via the KVP
code. We define here the interface to these scripts.

Adding support for IP injection resulted in some changes to the
protocol between the user level daemon and the kernel driver.
These changes have been implemented in way that would retain
compatibility with older daemons. I would like to thank Olaf and
Greg for pointing out the compatibility issue.

This version of the patch set addresses all of the comments that I
have received to date from Olaf, Ben and others. Specifically, I have
cleanedup all of the string manipulation code. Furthermore, I have also
simplified the format of the interface configuration file that is
generated by the KVP daemon and gotten rid of some constant strings that
may not be relevant on all distros. I would like to thank Olaf and Ben
for their detailed review.

Following are the changes from V2 of this patch set:

1. Addressed the indentation issue Greg noted in the patch:
0001-Drivers-hv-kvp-Support-the-new-IP-injection-messages.patch

2. Cleaned up the Reviewed-byi line based on input from Ben and Greg.

3. Since Ben had an identical patch for properly closing the open
streams, I have not included that patch in this series.

4. Greg applied the first 3 patches of the V2 patch set. This version
has the remaining patches.


K. Y. Srinivasan (14):
Drivers: hv: kvp: Support the new IP injection messages
Tools: hv: Prepare to expand kvp_get_ip_address() functionality
Tools: hv: Further refactor kvp_get_ip_address()
Tools: hv: Gather address family information
Tools: hv: Gather subnet information
Tools: hv: Represent the ipv6 mask using CIDR notation
Tools: hv: Gather ipv[4,6] gateway information
Tools: hv: Gather DNS information
Tools: hv: Gather DHCP information
Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO
Tools: hv: Rename the function kvp_get_ip_address()
Tools: hv: Implement the KVP verb - KVP_OP_GET_IP_INFO
Tools: hv: Get rid of some unused variables
Tools: hv: Correctly type string variables

drivers/hv/hv_kvp.c | 141 +++++++-
include/linux/hyperv.h | 2 +
tools/hv/hv_kvp_daemon.c | 866 ++++++++++++++++++++++++++++++++++++++++++----
3 files changed, 942 insertions(+), 67 deletions(-)

--
1.7.4.1


2012-08-17 01:15:53

by KY Srinivasan

[permalink] [raw]
Subject: [PATCH V3 03/14] Tools: hv: Further refactor kvp_get_ip_address()

In preparation for making kvp_get_ip_address() more generic, factor out
the code for handling IP addresses.

Signed-off-by: K. Y. Srinivasan <[email protected]>
Reviewed-by: Haiyang Zhang <[email protected]>
---
tools/hv/hv_kvp_daemon.c | 94 ++++++++++++++++++++-------------------------
1 files changed, 42 insertions(+), 52 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 3af37f0..3dc989f 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -491,17 +491,50 @@ done:
return;
}

+static int kvp_process_ip_address(void *addrp,
+ int family, char *buffer,
+ int length, int *offset)
+{
+ struct sockaddr_in *addr;
+ struct sockaddr_in6 *addr6;
+ int addr_length;
+ char tmp[50];
+ const char *str;
+
+ if (family == AF_INET) {
+ addr = (struct sockaddr_in *)addrp;
+ str = inet_ntop(family, &addr->sin_addr, tmp, 50);
+ addr_length = INET_ADDRSTRLEN;
+ } else {
+ addr6 = (struct sockaddr_in6 *)addrp;
+ str = inet_ntop(family, &addr6->sin6_addr.s6_addr, tmp, 50);
+ addr_length = INET6_ADDRSTRLEN;
+ }
+
+ if ((length - *offset) < addr_length + 1)
+ return 1;
+ if (str == NULL) {
+ strcpy(buffer, "inet_ntop failed\n");
+ return 1;
+ }
+ if (*offset == 0)
+ strcpy(buffer, tmp);
+ else
+ strcat(buffer, tmp);
+ strcat(buffer, ";");
+
+ *offset += strlen(str) + 1;
+ return 0;
+}
+
static int
kvp_get_ip_address(int family, char *if_name, int op,
void *out_buffer, int length)
{
struct ifaddrs *ifap;
struct ifaddrs *curp;
- int ipv4_len = strlen("255.255.255.255") + 1;
- int ipv6_len = strlen("ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff")+1;
int offset = 0;
const char *str;
- char tmp[50];
int error = 0;
char *buffer;
struct hv_kvp_ipaddr_value *ip_buffer;
@@ -556,55 +589,12 @@ kvp_get_ip_address(int family, char *if_name, int op,
continue;
}

- if ((curp->ifa_addr->sa_family == AF_INET) &&
- ((family == AF_INET) || (family == 0))) {
- struct sockaddr_in *addr =
- (struct sockaddr_in *) curp->ifa_addr;
-
- str = inet_ntop(AF_INET, &addr->sin_addr, tmp, 50);
- if (str == NULL) {
- strcpy(buffer, "inet_ntop failed\n");
- error = 1;
- goto getaddr_done;
- }
- if (offset == 0)
- strcpy(buffer, tmp);
- else
- strcat(buffer, tmp);
- strcat(buffer, ";");
-
- offset += strlen(str) + 1;
- if ((length - offset) < (ipv4_len + 1))
- goto getaddr_done;
-
- } else if ((family == AF_INET6) || (family == 0)) {
-
- /*
- * We only support AF_INET and AF_INET6
- * and the list of addresses is separated by a ";".
- */
- struct sockaddr_in6 *addr =
- (struct sockaddr_in6 *) curp->ifa_addr;
-
- str = inet_ntop(AF_INET6,
- &addr->sin6_addr.s6_addr,
- tmp, 50);
- if (str == NULL) {
- strcpy(buffer, "inet_ntop failed\n");
- error = 1;
- goto getaddr_done;
- }
- if (offset == 0)
- strcpy(buffer, tmp);
- else
- strcat(buffer, tmp);
- strcat(buffer, ";");
- offset += strlen(str) + 1;
- if ((length - offset) < (ipv6_len + 1))
- goto getaddr_done;
-
- }
-
+ error = kvp_process_ip_address(curp->ifa_addr,
+ curp->ifa_addr->sa_family,
+ buffer,
+ length, &offset);
+ if (error)
+ goto getaddr_done;

curp = curp->ifa_next;
}
--
1.7.4.1

2012-08-17 01:15:57

by KY Srinivasan

[permalink] [raw]
Subject: [PATCH V3 05/14] Tools: hv: Gather subnet information

Now, gather sub-net information for the specified interface.

Signed-off-by: K. Y. Srinivasan <[email protected]>
Reviewed-by: Haiyang Zhang <[email protected]>
---
tools/hv/hv_kvp_daemon.c | 31 +++++++++++++++++++++++++++++--
1 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 13ae098..de1724c 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -534,6 +534,7 @@ kvp_get_ip_address(int family, char *if_name, int op,
struct ifaddrs *ifap;
struct ifaddrs *curp;
int offset = 0;
+ int sn_offset = 0;
const char *str;
int error = 0;
char *buffer;
@@ -594,12 +595,38 @@ kvp_get_ip_address(int family, char *if_name, int op,
* Gather info other than the IP address.
* IP address info will be gathered later.
*/
- if (curp->ifa_addr->sa_family == AF_INET)
+ if (curp->ifa_addr->sa_family == AF_INET) {
ip_buffer->addr_family |= ADDR_FAMILY_IPV4;
- else
+ /*
+ * Get subnet info.
+ */
+ error = kvp_process_ip_address(
+ curp->ifa_netmask,
+ AF_INET,
+ (char *)
+ ip_buffer->sub_net,
+ length,
+ &sn_offset);
+ if (error)
+ goto gather_ipaddr;
+ } else {
ip_buffer->addr_family |= ADDR_FAMILY_IPV6;
+ /*
+ * Get subnet info.
+ */
+ error = kvp_process_ip_address(
+ curp->ifa_netmask,
+ AF_INET6,
+ (char *)
+ ip_buffer->sub_net,
+ length,
+ &sn_offset);
+ if (error)
+ goto gather_ipaddr;
+ }
}

+gather_ipaddr:
error = kvp_process_ip_address(curp->ifa_addr,
curp->ifa_addr->sa_family,
buffer,
--
1.7.4.1

2012-08-17 01:15:55

by KY Srinivasan

[permalink] [raw]
Subject: [PATCH V3 02/14] Tools: hv: Prepare to expand kvp_get_ip_address() functionality

kvp_get_ip_address() implemented the functionality to retrieve IP address info.
Make this function more generic so that we could retrieve additional
per-interface information.

Signed-off-by: K. Y. Srinivasan <[email protected]>
Reviewed-by: Haiyang Zhang <[email protected]>
---
tools/hv/hv_kvp_daemon.c | 129 ++++++++++++++++++++++++++++++----------------
1 files changed, 84 insertions(+), 45 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 069e2b3..3af37f0 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -492,7 +492,8 @@ done:
}

static int
-kvp_get_ip_address(int family, char *buffer, int length)
+kvp_get_ip_address(int family, char *if_name, int op,
+ void *out_buffer, int length)
{
struct ifaddrs *ifap;
struct ifaddrs *curp;
@@ -502,10 +503,19 @@ kvp_get_ip_address(int family, char *buffer, int length)
const char *str;
char tmp[50];
int error = 0;
-
+ char *buffer;
+ struct hv_kvp_ipaddr_value *ip_buffer;
+
+ if (op == KVP_OP_ENUMERATE) {
+ buffer = out_buffer;
+ } else {
+ ip_buffer = out_buffer;
+ buffer = (char *)ip_buffer->ip_addr;
+ ip_buffer->addr_family = 0;
+ }
/*
* On entry into this function, the buffer is capable of holding the
- * maximum key value (2048 bytes).
+ * maximum key value.
*/

if (getifaddrs(&ifap)) {
@@ -515,58 +525,87 @@ kvp_get_ip_address(int family, char *buffer, int length)

curp = ifap;
while (curp != NULL) {
- if ((curp->ifa_addr != NULL) &&
- (curp->ifa_addr->sa_family == family)) {
- if (family == AF_INET) {
- struct sockaddr_in *addr =
- (struct sockaddr_in *) curp->ifa_addr;
-
- str = inet_ntop(family, &addr->sin_addr,
- tmp, 50);
- if (str == NULL) {
- strcpy(buffer, "inet_ntop failed\n");
- error = 1;
- goto getaddr_done;
- }
- if (offset == 0)
- strcpy(buffer, tmp);
- else
- strcat(buffer, tmp);
- strcat(buffer, ";");
+ if (curp->ifa_addr == NULL) {
+ curp = curp->ifa_next;
+ continue;
+ }

- offset += strlen(str) + 1;
- if ((length - offset) < (ipv4_len + 1))
- goto getaddr_done;
+ if ((if_name != NULL) &&
+ (strncmp(curp->ifa_name, if_name, strlen(if_name)))) {
+ /*
+ * We want info about a specific interface;
+ * just continue.
+ */
+ curp = curp->ifa_next;
+ continue;
+ }

- } else {
+ /*
+ * We only support two address families: AF_INET and AF_INET6.
+ * If a family value of 0 is specified, we collect both
+ * supported address families; if not we gather info on
+ * the specified address family.
+ */
+ if ((family != 0) && (curp->ifa_addr->sa_family != family)) {
+ curp = curp->ifa_next;
+ continue;
+ }
+ if ((curp->ifa_addr->sa_family != AF_INET) &&
+ (curp->ifa_addr->sa_family != AF_INET6)) {
+ curp = curp->ifa_next;
+ continue;
+ }
+
+ if ((curp->ifa_addr->sa_family == AF_INET) &&
+ ((family == AF_INET) || (family == 0))) {
+ struct sockaddr_in *addr =
+ (struct sockaddr_in *) curp->ifa_addr;
+
+ str = inet_ntop(AF_INET, &addr->sin_addr, tmp, 50);
+ if (str == NULL) {
+ strcpy(buffer, "inet_ntop failed\n");
+ error = 1;
+ goto getaddr_done;
+ }
+ if (offset == 0)
+ strcpy(buffer, tmp);
+ else
+ strcat(buffer, tmp);
+ strcat(buffer, ";");
+
+ offset += strlen(str) + 1;
+ if ((length - offset) < (ipv4_len + 1))
+ goto getaddr_done;
+
+ } else if ((family == AF_INET6) || (family == 0)) {

/*
* We only support AF_INET and AF_INET6
* and the list of addresses is separated by a ";".
*/
- struct sockaddr_in6 *addr =
+ struct sockaddr_in6 *addr =
(struct sockaddr_in6 *) curp->ifa_addr;

- str = inet_ntop(family,
+ str = inet_ntop(AF_INET6,
&addr->sin6_addr.s6_addr,
tmp, 50);
- if (str == NULL) {
- strcpy(buffer, "inet_ntop failed\n");
- error = 1;
- goto getaddr_done;
- }
- if (offset == 0)
- strcpy(buffer, tmp);
- else
- strcat(buffer, tmp);
- strcat(buffer, ";");
- offset += strlen(str) + 1;
- if ((length - offset) < (ipv6_len + 1))
- goto getaddr_done;
-
+ if (str == NULL) {
+ strcpy(buffer, "inet_ntop failed\n");
+ error = 1;
+ goto getaddr_done;
}
+ if (offset == 0)
+ strcpy(buffer, tmp);
+ else
+ strcat(buffer, tmp);
+ strcat(buffer, ";");
+ offset += strlen(str) + 1;
+ if ((length - offset) < (ipv6_len + 1))
+ goto getaddr_done;

}
+
+
curp = curp->ifa_next;
}

@@ -811,13 +850,13 @@ int main(void)
strcpy(key_value, lic_version);
break;
case NetworkAddressIPv4:
- kvp_get_ip_address(AF_INET, key_value,
- HV_KVP_EXCHANGE_MAX_VALUE_SIZE);
+ kvp_get_ip_address(AF_INET, NULL, KVP_OP_ENUMERATE,
+ key_value, HV_KVP_EXCHANGE_MAX_VALUE_SIZE);
strcpy(key_name, "NetworkAddressIPv4");
break;
case NetworkAddressIPv6:
- kvp_get_ip_address(AF_INET6, key_value,
- HV_KVP_EXCHANGE_MAX_VALUE_SIZE);
+ kvp_get_ip_address(AF_INET6, NULL, KVP_OP_ENUMERATE,
+ key_value, HV_KVP_EXCHANGE_MAX_VALUE_SIZE);
strcpy(key_name, "NetworkAddressIPv6");
break;
case OSBuildNumber:
--
1.7.4.1

2012-08-17 01:16:46

by KY Srinivasan

[permalink] [raw]
Subject: [PATCH V3 10/14] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO

Implement the KVP verb - KVP_OP_SET_IP_INFO. This operation configures the
specified interface based on the given configuration. Since configuring
an interface is very distro specific, we invoke an external script to
configure the interface.

Signed-off-by: K. Y. Srinivasan <[email protected]>
Reviewed-by: Haiyang Zhang <[email protected]>
---
include/linux/hyperv.h | 2 +
tools/hv/hv_kvp_daemon.c | 443 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 445 insertions(+), 0 deletions(-)

diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index b587c44..588d8f2 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -191,6 +191,8 @@ enum hv_kvp_exchg_pool {
#define HV_ERROR_NOT_SUPPORTED 0x80070032
#define HV_ERROR_MACHINE_LOCKED 0x800704F7
#define HV_ERROR_DEVICE_NOT_CONNECTED 0x8007048F
+#define HV_INVALIDARG 0x80070057
+#define HV_GUID_NOTFOUND 0x80041002

#define ADDR_FAMILY_NONE 0x00
#define ADDR_FAMILY_IPV4 0x01
diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 8a18819..59669a8 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -31,6 +31,7 @@
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
+#include <ctype.h>
#include <errno.h>
#include <arpa/inet.h>
#include <linux/connector.h>
@@ -41,6 +42,7 @@
#include <syslog.h>
#include <sys/stat.h>
#include <fcntl.h>
+#include <dirent.h>

/*
* KVP protocol: The user mode component first registers with the
@@ -68,6 +70,14 @@ enum key_index {
ProcessorArchitecture
};

+
+enum {
+ IPADDR = 0,
+ NETMASK,
+ GATEWAY,
+ DNS
+};
+
static char kvp_send_buffer[4096];
static char kvp_recv_buffer[4096 * 2];
static struct sockaddr_nl addr;
@@ -81,6 +91,11 @@ static char *os_build;
static char *lic_version = "Unknown version";
static struct utsname uts_buf;

+/*
+ * The location of the interface configuration file.
+ */
+
+#define KVP_CONFIG_LOC "/var/opt/"

#define MAX_FILE_NAME 100
#define ENTRIES_PER_BLOCK 50
@@ -491,6 +506,104 @@ done:
return;
}

+
+
+/*
+ * Retrieve an interface name corresponding to the specified guid.
+ * If there is a match, the function returns a pointer
+ * to the interface name and if not, a NULL is returned.
+ * If a match is found, the caller is responsible for
+ * freeing the memory.
+ */
+
+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[256];
+
+ dir = opendir(kvp_net_dir);
+ if (dir == NULL)
+ return NULL;
+
+ snprintf(dev_id, sizeof(dev_id), "%s", 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");
+
+ 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);
+ fclose(file);
+ break;
+ }
+ }
+ 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[256];
+ int i;
+ char *mac_addr = NULL;
+
+ snprintf(addr_file, sizeof(addr_file), "%s%s%s", "/sys/class/net/",
+ if_name, "/address");
+
+ 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)
@@ -791,6 +904,315 @@ 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)
+{
+ int ret;
+
+ ret = fprintf(f, "%s%s%s%s\n", s1, s2, "=", s3);
+
+ if (ret < 0)
+ 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))) {
+
+ sub_str[0] = 0;
+ if (is_ipv4(addr)) {
+ switch (type) {
+ case IPADDR:
+ snprintf(str, sizeof(str), "%s", "IPADDR");
+ break;
+ case NETMASK:
+ snprintf(str, sizeof(str), "%s", "NETMASK");
+ break;
+ case GATEWAY:
+ snprintf(str, sizeof(str), "%s", "GATEWAY");
+ break;
+ case DNS:
+ snprintf(str, sizeof(str), "%s", "DNS");
+ break;
+ }
+ if (i != 0) {
+ if (type != DNS) {
+ snprintf(sub_str, sizeof(sub_str),
+ "_%d", i++);
+ } else {
+ snprintf(sub_str, sizeof(sub_str),
+ "%d", ++i);
+ }
+ } else if (type == DNS) {
+ snprintf(sub_str, sizeof(sub_str), "%d", ++i);
+ }
+
+
+ } else if (expand_ipv6(addr, type)) {
+ switch (type) {
+ case IPADDR:
+ snprintf(str, sizeof(str), "%s", "IPV6ADDR");
+ break;
+ case NETMASK:
+ snprintf(str, sizeof(str), "%s", "IPV6NETMASK");
+ break;
+ case GATEWAY:
+ snprintf(str, sizeof(str), "%s",
+ "IPV6_DEFAULTGW");
+ break;
+ case DNS:
+ snprintf(str, sizeof(str), "%s", "DNS");
+ break;
+ }
+ if ((j != 0) || (type == DNS)) {
+ if (type != DNS) {
+ snprintf(sub_str, sizeof(sub_str),
+ "_%d", j++);
+ } else {
+ snprintf(sub_str, sizeof(sub_str),
+ "%d", ++i);
+ }
+ } else if (type == DNS) {
+ snprintf(sub_str, sizeof(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[128];
+ 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).
+ *
+ * The information provided here may be more than what is needed
+ * in a given distro to configure the interface and so are free
+ * ignore information that may not be relevant.
+ *
+ * Here is the format of the ip configuration file:
+ *
+ * HWADDR=macaddr
+ * IF_NAME=interface name
+ * DHCP=yes (This is optional; if yes, DHCP is configured)
+ *
+ * IPADDR=ipaddr1
+ * IPADDR_1=ipaddr2
+ * IPADDR_x=ipaddry (where y = x + 1)
+ *
+ * NETMASK=netmask1
+ * NETMASK_x=netmasky (where y = x + 1)
+ *
+ * GATEWAY=ipaddr1
+ * GATEWAY_x=ipaddry (where y = x + 1)
+ *
+ * DNSx=ipaddrx (where first DNS address is tagged as DNS1 etc)
+ *
+ * IPV6 addresses will be tagged as IPV6ADDR, IPV6 gateway will be
+ * tagged as IPV6_DEFAULTGW and IPV6 NETMASK will be tagged as
+ * IPV6NETMASK.
+ *
+ * The host can specify multiple ipv4 and ipv6 addresses to be
+ * configured for the interface. Furthermore, the configuration
+ * needs to be persistent. A subsequent GET call on the interface
+ * is expected to return the configuration that is set via the SET
+ * call.
+ */
+
+ snprintf(if_file, sizeof(if_file), "%s%s%s", KVP_CONFIG_LOC,
+ "hyperv/ifcfg-", 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", "", mac_addr);
+ if (error)
+ goto setval_error;
+
+ error = kvp_write_file(file, "IF_NAME", "", if_name);
+ if (error)
+ goto setval_error;
+
+ if (new_val->dhcp_enabled) {
+ error = kvp_write_file(file, "DHCP", "", "yes");
+ 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.
+ */
+
+ snprintf(cmd, sizeof(cmd), "%s %s", "hv_set_ifconfig", if_file);
+ system(cmd);
+ return 0;
+
+setval_error:
+ syslog(LOG_ERR, "Failed to write config file");
+ free(mac_addr);
+ fclose(file);
+ return error;
+}
+
+
static int
kvp_get_domain_name(char *buffer, int length)
{
@@ -860,6 +1282,8 @@ int main(void)
char *key_name;
int op;
int pool;
+ char *if_name;
+ struct hv_kvp_ipaddr_value *kvp_ip_val;

daemon(1, 0);
openlog("KVP", 0, LOG_USER);
@@ -963,6 +1387,25 @@ int main(void)
}

switch (op) {
+ case KVP_OP_SET_IP_INFO:
+ kvp_ip_val = &hv_msg->body.kvp_ip_val;
+ if_name = kvp_get_if_name(
+ (char *)kvp_ip_val->adapter_id);
+ if (if_name == NULL) {
+ /*
+ * We could not map the guid to an
+ * interface name; return error.
+ */
+ hv_msg->error = HV_GUID_NOTFOUND;
+ break;
+ }
+ error = kvp_set_ip_info(if_name, kvp_ip_val);
+ if (error)
+ hv_msg->error = error;
+
+ free(if_name);
+ break;
+
case KVP_OP_SET:
if (kvp_key_add_or_modify(pool,
hv_msg->body.kvp_set.data.key,
--
1.7.4.1

2012-08-17 01:16:44

by KY Srinivasan

[permalink] [raw]
Subject: [PATCH V3 09/14] Tools: hv: Gather DHCP information

Collect information on dhcp setting for the specified interface.
We invoke an external script to get this information.

Signed-off-by: K. Y. Srinivasan <[email protected]>
Reviewed-by: Haiyang Zhang <[email protected]>
---
tools/hv/hv_kvp_daemon.c | 31 +++++++++++++++++++++++++++++++
1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 2f8e0b9..8a18819 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -525,6 +525,9 @@ static void kvp_get_ipconfig_info(char *if_name,
struct hv_kvp_ipaddr_value *buffer)
{
char cmd[512];
+ char dhcp_info[128];
+ char *p;
+ FILE *file;

/*
* Get the address of default gateway (ipv4).
@@ -573,6 +576,34 @@ static void kvp_get_ipconfig_info(char *if_name,
*/
kvp_process_ipconfig_file(cmd, (char *)buffer->dns_addr,
(MAX_IP_ADDR_SIZE * 2), INET_ADDRSTRLEN, 0);
+
+ /*
+ * Gather the DHCP state.
+ * We will gather this state by invoking an external script.
+ * The parameter to the script is the interface name.
+ * Here is the expected output:
+ *
+ * Enabled: DHCP enabled.
+ */
+
+ sprintf(cmd, "%s %s", "hv_get_dhcp_info", if_name);
+
+ file = popen(cmd, "r");
+ if (file == NULL)
+ return;
+
+ p = fgets(dhcp_info, sizeof(dhcp_info), file);
+ if (p == NULL) {
+ pclose(file);
+ return;
+ }
+
+ if (!strncmp(p, "Enabled", 7))
+ buffer->dhcp_enabled = 1;
+ else
+ buffer->dhcp_enabled = 0;
+
+ pclose(file);
}


--
1.7.4.1

2012-08-17 01:16:42

by KY Srinivasan

[permalink] [raw]
Subject: [PATCH V3 12/14] Tools: hv: Implement the KVP verb - KVP_OP_GET_IP_INFO

Now implement the KVP verb - KVP_OP_GET_IP_INFO. This operation retrieves IP
information for the specified interface.

Signed-off-by: K. Y. Srinivasan <[email protected]>
Reviewed-by: Haiyang Zhang <[email protected]>
---
tools/hv/hv_kvp_daemon.c | 93 ++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 90 insertions(+), 3 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index bb2a6c4..e7db9ac 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -604,6 +604,69 @@ static char *kvp_if_name_to_mac(char *if_name)
}


+/*
+ * Retrieve the interface name given tha MAC address.
+ */
+
+static char *kvp_mac_to_if_name(char *mac)
+{
+ 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[256];
+ int i;
+
+ dir = opendir(kvp_net_dir);
+ if (dir == NULL)
+ return NULL;
+
+ snprintf(dev_id, sizeof(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, "/address");
+
+ 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';
+
+ for (i = 0; i < strlen(p); i++)
+ p[i] = toupper(p[i]);
+
+ if (!strcmp(p, mac)) {
+ /*
+ * Found the MAC match; return the interface
+ * name. The caller will free the memory.
+ */
+ if_name = strdup(entry->d_name);
+ fclose(file);
+ break;
+ }
+ }
+ fclose(file);
+ }
+
+ closedir(dir);
+ return if_name;
+}
+
+
static void kvp_process_ipconfig_file(char *cmd,
char *config_buf, int len,
int element_size, int offset)
@@ -750,10 +813,10 @@ static int kvp_process_ip_address(void *addrp,
}

if ((length - *offset) < addr_length + 1)
- return 1;
+ return HV_E_FAIL;
if (str == NULL) {
strcpy(buffer, "inet_ntop failed\n");
- return 1;
+ return HV_E_FAIL;
}
if (*offset == 0)
strcpy(buffer, tmp);
@@ -797,7 +860,7 @@ kvp_get_ip_info(int family, char *if_name, int op,

if (getifaddrs(&ifap)) {
strcpy(buffer, "getifaddrs failed\n");
- return 1;
+ return HV_E_FAIL;
}

curp = ifap;
@@ -1387,6 +1450,30 @@ int main(void)
}

switch (op) {
+ case KVP_OP_GET_IP_INFO:
+ kvp_ip_val = &hv_msg->body.kvp_ip_val;
+ if_name =
+ kvp_mac_to_if_name((char *)kvp_ip_val->adapter_id);
+
+ if (if_name == NULL) {
+ /*
+ * We could not map the mac address to an
+ * interface name; return error.
+ */
+ hv_msg->error = HV_E_FAIL;
+ break;
+ }
+ error = kvp_get_ip_info(
+ 0, if_name, KVP_OP_GET_IP_INFO,
+ kvp_ip_val,
+ (MAX_IP_ADDR_SIZE * 2));
+
+ if (error)
+ hv_msg->error = error;
+
+ free(if_name);
+ break;
+
case KVP_OP_SET_IP_INFO:
kvp_ip_val = &hv_msg->body.kvp_ip_val;
if_name = kvp_get_if_name(
--
1.7.4.1

2012-08-17 01:16:41

by KY Srinivasan

[permalink] [raw]
Subject: [PATCH V3 06/14] Tools: hv: Represent the ipv6 mask using CIDR notation

Transform ipv6 subnet information to CIDR notation.

Signed-off-by: K. Y. Srinivasan <[email protected]>
Reviewed-by: Haiyang Zhang <[email protected]>
---
tools/hv/hv_kvp_daemon.c | 46 ++++++++++++++++++++++++++++++++++++----------
1 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index de1724c..a63e83a 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -491,6 +491,15 @@ done:
return;
}

+static unsigned int hweight32(unsigned int *w)
+{
+ unsigned int res = *w - ((*w >> 1) & 0x55555555);
+ res = (res & 0x33333333) + ((res >> 2) & 0x33333333);
+ res = (res + (res >> 4)) & 0x0F0F0F0F;
+ res = res + (res >> 8);
+ return (res + (res >> 16)) & 0x000000FF;
+}
+
static int kvp_process_ip_address(void *addrp,
int family, char *buffer,
int length, int *offset)
@@ -535,10 +544,15 @@ kvp_get_ip_address(int family, char *if_name, int op,
struct ifaddrs *curp;
int offset = 0;
int sn_offset = 0;
- const char *str;
int error = 0;
char *buffer;
struct hv_kvp_ipaddr_value *ip_buffer;
+ char cidr_mask[5]; /* /xyz */
+ int weight;
+ int i;
+ unsigned int *w;
+ char *sn_str;
+ struct sockaddr_in6 *addr6;

if (op == KVP_OP_ENUMERATE) {
buffer = out_buffer;
@@ -611,18 +625,30 @@ kvp_get_ip_address(int family, char *if_name, int op,
goto gather_ipaddr;
} else {
ip_buffer->addr_family |= ADDR_FAMILY_IPV6;
+
/*
- * Get subnet info.
+ * Get subnet info in CIDR format.
*/
- error = kvp_process_ip_address(
- curp->ifa_netmask,
- AF_INET6,
- (char *)
- ip_buffer->sub_net,
- length,
- &sn_offset);
- if (error)
+ weight = 0;
+ sn_str = (char *)ip_buffer->sub_net;
+ addr6 = (struct sockaddr_in6 *)
+ curp->ifa_netmask;
+ w = addr6->sin6_addr.s6_addr32;
+
+ for (i = 0; i < 4; i++)
+ weight += hweight32(&w[i]);
+
+ sprintf(cidr_mask, "/%d", weight);
+ if ((length - sn_offset) <
+ (strlen(cidr_mask) + 1))
goto gather_ipaddr;
+
+ if (sn_offset == 0)
+ strcpy(sn_str, cidr_mask);
+ else
+ strcat(sn_str, cidr_mask);
+ strcat((char *)ip_buffer->sub_net, ";");
+ sn_offset += strlen(sn_str) + 1;
}
}

--
1.7.4.1

2012-08-17 01:16:39

by KY Srinivasan

[permalink] [raw]
Subject: [PATCH V3 13/14] Tools: hv: Get rid of some unused variables

Get rid of unused variables.

Reported-by: Ben Hutchings <[email protected]>
Signed-off-by: K. Y. Srinivasan <[email protected]>
Reviewed-by: Haiyang Zhang <[email protected]>
---
tools/hv/hv_kvp_daemon.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index e7db9ac..4a1428c 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -211,7 +211,7 @@ static void kvp_update_mem_state(int pool)
}
static int kvp_file_init(void)
{
- int ret, fd;
+ int fd;
FILE *filep;
size_t records_read;
__u8 *fname;
@@ -327,7 +327,6 @@ static int kvp_key_add_or_modify(int pool, __u8 *key, int key_size, __u8 *value,
int value_size)
{
int i;
- int j, k;
int num_records;
struct kvp_record *record;
int num_blocks;
--
1.7.4.1

2012-08-17 01:16:38

by KY Srinivasan

[permalink] [raw]
Subject: [PATCH V3 14/14] Tools: hv: Correctly type string variables

Correctly type character strings.

Reported-by: Ben Hutchings <[email protected]>
Signed-off-by: K. Y. Srinivasan <[email protected]>
Reviewed-by: Haiyang Zhang <[email protected]>
---
tools/hv/hv_kvp_daemon.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 4a1428c..c8e1013 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -101,8 +101,8 @@ static struct utsname uts_buf;
#define ENTRIES_PER_BLOCK 50

struct kvp_record {
- __u8 key[HV_KVP_EXCHANGE_MAX_KEY_SIZE];
- __u8 value[HV_KVP_EXCHANGE_MAX_VALUE_SIZE];
+ char key[HV_KVP_EXCHANGE_MAX_KEY_SIZE];
+ char value[HV_KVP_EXCHANGE_MAX_VALUE_SIZE];
};

struct kvp_file_state {
@@ -110,7 +110,7 @@ struct kvp_file_state {
int num_blocks;
struct kvp_record *records;
int num_records;
- __u8 fname[MAX_FILE_NAME];
+ char fname[MAX_FILE_NAME];
};

static struct kvp_file_state kvp_file_info[KVP_POOL_COUNT];
@@ -214,7 +214,7 @@ static int kvp_file_init(void)
int fd;
FILE *filep;
size_t records_read;
- __u8 *fname;
+ char *fname;
struct kvp_record *record;
struct kvp_record *readp;
int num_blocks;
--
1.7.4.1

2012-08-17 01:18:15

by KY Srinivasan

[permalink] [raw]
Subject: [PATCH V3 11/14] Tools: hv: Rename the function kvp_get_ip_address()

Rename the function kvp_get_ip_address() to better reflect the functionality
being implemented.

Signed-off-by: K. Y. Srinivasan <[email protected]>
Reviewed-by: Haiyang Zhang <[email protected]>
---
tools/hv/hv_kvp_daemon.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 59669a8..bb2a6c4 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -766,7 +766,7 @@ static int kvp_process_ip_address(void *addrp,
}

static int
-kvp_get_ip_address(int family, char *if_name, int op,
+kvp_get_ip_info(int family, char *if_name, int op,
void *out_buffer, int length)
{
struct ifaddrs *ifap;
@@ -1469,12 +1469,12 @@ int main(void)
strcpy(key_value, lic_version);
break;
case NetworkAddressIPv4:
- kvp_get_ip_address(AF_INET, NULL, KVP_OP_ENUMERATE,
+ kvp_get_ip_info(AF_INET, NULL, KVP_OP_ENUMERATE,
key_value, HV_KVP_EXCHANGE_MAX_VALUE_SIZE);
strcpy(key_name, "NetworkAddressIPv4");
break;
case NetworkAddressIPv6:
- kvp_get_ip_address(AF_INET6, NULL, KVP_OP_ENUMERATE,
+ kvp_get_ip_info(AF_INET6, NULL, KVP_OP_ENUMERATE,
key_value, HV_KVP_EXCHANGE_MAX_VALUE_SIZE);
strcpy(key_name, "NetworkAddressIPv6");
break;
--
1.7.4.1

2012-08-17 01:18:18

by KY Srinivasan

[permalink] [raw]
Subject: [PATCH V3 07/14] Tools: hv: Gather ipv[4,6] gateway information

Gather information on the default gateways - ipv4/ipv6.

Signed-off-by: K. Y. Srinivasan <[email protected]>
Reviewed-by: Haiyang Zhang <[email protected]>
---
tools/hv/hv_kvp_daemon.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index a63e83a..65d54c8 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -491,6 +491,68 @@ done:
return;
}

+static void kvp_process_ipconfig_file(char *cmd,
+ char *config_buf, int len,
+ int element_size, int offset)
+{
+ char buf[256];
+ char *p;
+ char *x;
+ FILE *file;
+
+ /*
+ * First execute the command.
+ */
+ file = popen(cmd, "r");
+ if (file == NULL)
+ return;
+
+ if (offset == 0)
+ memset(config_buf, 0, len);
+ while ((p = fgets(buf, sizeof(buf), file)) != NULL) {
+ if ((len - strlen(config_buf)) < (element_size + 1))
+ break;
+
+ x = strchr(p, '\n');
+ *x = '\0';
+ strcat(config_buf, p);
+ strcat(config_buf, ";");
+ }
+ pclose(file);
+}
+
+static void kvp_get_ipconfig_info(char *if_name,
+ struct hv_kvp_ipaddr_value *buffer)
+{
+ char cmd[512];
+
+ /*
+ * Get the address of default gateway (ipv4).
+ */
+ sprintf(cmd, "%s %s", "ip route show dev", if_name);
+ strcat(cmd, " | awk '/default/ {print $3 }'");
+
+ /*
+ * Execute the command to gather gateway info.
+ */
+ kvp_process_ipconfig_file(cmd, (char *)buffer->gate_way,
+ (MAX_GATEWAY_SIZE * 2), INET_ADDRSTRLEN, 0);
+
+ /*
+ * Get the address of default gateway (ipv6).
+ */
+ sprintf(cmd, "%s %s", "ip -f inet6 route show dev", if_name);
+ strcat(cmd, " | awk '/default/ {print $3 }'");
+
+ /*
+ * Execute the command to gather gateway info (ipv6).
+ */
+ kvp_process_ipconfig_file(cmd, (char *)buffer->gate_way,
+ (MAX_GATEWAY_SIZE * 2), INET6_ADDRSTRLEN, 1);
+
+}
+
+
static unsigned int hweight32(unsigned int *w)
{
unsigned int res = *w - ((*w >> 1) & 0x55555555);
@@ -650,6 +712,12 @@ kvp_get_ip_address(int family, char *if_name, int op,
strcat((char *)ip_buffer->sub_net, ";");
sn_offset += strlen(sn_str) + 1;
}
+
+ /*
+ * Collect other ip related configuration info.
+ */
+
+ kvp_get_ipconfig_info(if_name, ip_buffer);
}

gather_ipaddr:
--
1.7.4.1

2012-08-17 01:18:25

by KY Srinivasan

[permalink] [raw]
Subject: [PATCH V3 01/14] Drivers: hv: kvp: Support the new IP injection messages

Implement support for the new IP injection messages in the driver code.

Signed-off-by: K. Y. Srinivasan <[email protected]>
Reviewed-by: Haiyang Zhang <[email protected]>
---
drivers/hv/hv_kvp.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 135 insertions(+), 6 deletions(-)

diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index eb4d073..d906050 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -63,7 +63,7 @@ static int dm_reg_value;
static void kvp_send_key(struct work_struct *dummy);


-static void kvp_respond_to_host(char *key, char *value, int error);
+static void kvp_respond_to_host(struct hv_kvp_msg *msg, int error);
static void kvp_work_func(struct work_struct *dummy);
static void kvp_register(int);

@@ -108,7 +108,7 @@ kvp_work_func(struct work_struct *dummy)
* If the timer fires, the user-mode component has not responded;
* process the pending transaction.
*/
- kvp_respond_to_host("Unknown key", "Guest timed out", HV_E_FAIL);
+ kvp_respond_to_host(NULL, HV_E_FAIL);
}

static int kvp_handle_handshake(struct hv_kvp_msg *msg)
@@ -199,9 +199,118 @@ kvp_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
* to the host. But first, cancel the timeout.
*/
if (cancel_delayed_work_sync(&kvp_work))
- kvp_respond_to_host(data->data.key, data->data.value, error);
+ kvp_respond_to_host(message, error);
}

+
+static int process_ob_ipinfo(void *in_msg, void *out_msg, int op)
+{
+ struct hv_kvp_msg *in = in_msg;
+ struct hv_kvp_ip_msg *out = out_msg;
+ int len;
+
+ switch (op) {
+ case KVP_OP_GET_IP_INFO:
+ /*
+ * Transform all parameters into utf16 encoding.
+ */
+ len = utf8s_to_utf16s((char *)in->body.kvp_ip_val.ip_addr,
+ strlen((char *)in->body.kvp_ip_val.ip_addr),
+ UTF16_HOST_ENDIAN,
+ (wchar_t *)out->kvp_ip_val.ip_addr,
+ MAX_IP_ADDR_SIZE);
+ if (len < 0)
+ return len;
+
+ len = utf8s_to_utf16s((char *)in->body.kvp_ip_val.sub_net,
+ strlen((char *)in->body.kvp_ip_val.sub_net),
+ UTF16_HOST_ENDIAN,
+ (wchar_t *)out->kvp_ip_val.sub_net,
+ MAX_IP_ADDR_SIZE);
+ if (len < 0)
+ return len;
+
+ len = utf8s_to_utf16s((char *)in->body.kvp_ip_val.gate_way,
+ strlen((char *)in->body.kvp_ip_val.gate_way),
+ UTF16_HOST_ENDIAN,
+ (wchar_t *)out->kvp_ip_val.gate_way,
+ MAX_GATEWAY_SIZE);
+ if (len < 0)
+ return len;
+
+ len = utf8s_to_utf16s((char *)in->body.kvp_ip_val.dns_addr,
+ strlen((char *)in->body.kvp_ip_val.dns_addr),
+ UTF16_HOST_ENDIAN,
+ (wchar_t *)out->kvp_ip_val.dns_addr,
+ MAX_IP_ADDR_SIZE);
+ if (len < 0)
+ return len;
+
+ len = utf8s_to_utf16s((char *)in->body.kvp_ip_val.adapter_id,
+ strlen((char *)in->body.kvp_ip_val.adapter_id),
+ UTF16_HOST_ENDIAN,
+ (wchar_t *)out->kvp_ip_val.adapter_id,
+ MAX_IP_ADDR_SIZE);
+ if (len < 0)
+ return len;
+
+ out->kvp_ip_val.dhcp_enabled =
+ in->body.kvp_ip_val.dhcp_enabled;
+ }
+
+ return 0;
+}
+
+static void process_ib_ipinfo(void *in_msg, void *out_msg, int op)
+{
+ struct hv_kvp_ip_msg *in = in_msg;
+ struct hv_kvp_msg *out = out_msg;
+
+ switch (op) {
+ case KVP_OP_SET_IP_INFO:
+ /*
+ * Transform all parameters into utf8 encoding.
+ */
+ utf16s_to_utf8s((wchar_t *)in->kvp_ip_val.ip_addr,
+ MAX_IP_ADDR_SIZE,
+ UTF16_LITTLE_ENDIAN,
+ (__u8 *)out->body.kvp_ip_val.ip_addr,
+ MAX_IP_ADDR_SIZE);
+
+ utf16s_to_utf8s((wchar_t *)in->kvp_ip_val.sub_net,
+ MAX_IP_ADDR_SIZE,
+ UTF16_LITTLE_ENDIAN,
+ (__u8 *)out->body.kvp_ip_val.sub_net,
+ MAX_IP_ADDR_SIZE);
+
+ utf16s_to_utf8s((wchar_t *)in->kvp_ip_val.gate_way,
+ MAX_GATEWAY_SIZE,
+ UTF16_LITTLE_ENDIAN,
+ (__u8 *)out->body.kvp_ip_val.gate_way,
+ MAX_GATEWAY_SIZE);
+
+ utf16s_to_utf8s((wchar_t *)in->kvp_ip_val.dns_addr,
+ MAX_IP_ADDR_SIZE,
+ UTF16_LITTLE_ENDIAN,
+ (__u8 *)out->body.kvp_ip_val.dns_addr,
+ MAX_IP_ADDR_SIZE);
+
+ out->body.kvp_ip_val.dhcp_enabled = in->kvp_ip_val.dhcp_enabled;
+
+ default:
+ utf16s_to_utf8s((wchar_t *)in->kvp_ip_val.adapter_id,
+ MAX_ADAPTER_ID_SIZE,
+ UTF16_LITTLE_ENDIAN,
+ (__u8 *)out->body.kvp_ip_val.adapter_id,
+ MAX_ADAPTER_ID_SIZE);
+
+ out->body.kvp_ip_val.addr_family = in->kvp_ip_val.addr_family;
+ }
+}
+
+
+
+
static void
kvp_send_key(struct work_struct *dummy)
{
@@ -237,6 +346,12 @@ kvp_send_key(struct work_struct *dummy)
*/

switch (message->kvp_hdr.operation) {
+ case KVP_OP_SET_IP_INFO:
+ process_ib_ipinfo(in_msg, message, KVP_OP_SET_IP_INFO);
+ break;
+ case KVP_OP_GET_IP_INFO:
+ process_ib_ipinfo(in_msg, message, KVP_OP_GET_IP_INFO);
+ break;
case KVP_OP_SET:
switch (in_msg->body.kvp_set.data.value_type) {
case REG_SZ:
@@ -313,17 +428,19 @@ kvp_send_key(struct work_struct *dummy)
*/

static void
-kvp_respond_to_host(char *key, char *value, int error)
+kvp_respond_to_host(struct hv_kvp_msg *msg_to_host, int error)
{
struct hv_kvp_msg *kvp_msg;
struct hv_kvp_exchg_msg_value *kvp_data;
char *key_name;
+ char *value;
struct icmsg_hdr *icmsghdrp;
int keylen = 0;
int valuelen = 0;
u32 buf_len;
struct vmbus_channel *channel;
u64 req_id;
+ int ret;

/*
* If a transaction is not active; log and return.
@@ -376,6 +493,16 @@ kvp_respond_to_host(char *key, char *value, int error)
sizeof(struct icmsg_hdr)];

switch (kvp_transaction.kvp_msg->kvp_hdr.operation) {
+ case KVP_OP_GET_IP_INFO:
+ ret = process_ob_ipinfo(msg_to_host,
+ (struct hv_kvp_ip_msg *)kvp_msg,
+ KVP_OP_GET_IP_INFO);
+ if (ret < 0)
+ icmsghdrp->status = HV_E_FAIL;
+
+ goto response_done;
+ case KVP_OP_SET_IP_INFO:
+ goto response_done;
case KVP_OP_GET:
kvp_data = &kvp_msg->body.kvp_get.data;
goto copy_value;
@@ -389,7 +516,7 @@ kvp_respond_to_host(char *key, char *value, int error)
}

kvp_data = &kvp_msg->body.kvp_enum_data.data;
- key_name = key;
+ key_name = msg_to_host->body.kvp_enum_data.data.key;

/*
* The windows host expects the key/value pair to be encoded
@@ -403,6 +530,7 @@ kvp_respond_to_host(char *key, char *value, int error)
kvp_data->key_size = 2*(keylen + 1); /* utf16 encoding */

copy_value:
+ value = msg_to_host->body.kvp_enum_data.data.value;
valuelen = utf8s_to_utf16s(value, strlen(value), UTF16_HOST_ENDIAN,
(wchar_t *) kvp_data->value,
(HV_KVP_EXCHANGE_MAX_VALUE_SIZE / 2) - 2);
@@ -455,7 +583,8 @@ void hv_kvp_onchannelcallback(void *context)
return;
}

- vmbus_recvpacket(channel, recv_buffer, PAGE_SIZE, &recvlen, &requestid);
+ vmbus_recvpacket(channel, recv_buffer, PAGE_SIZE * 2, &recvlen,
+ &requestid);

if (recvlen > 0) {
icmsghdrp = (struct icmsg_hdr *)&recv_buffer[
--
1.7.4.1

2012-08-17 01:18:21

by KY Srinivasan

[permalink] [raw]
Subject: [PATCH V3 04/14] Tools: hv: Gather address family information

Now, gather address family information for the specified interface.

Signed-off-by: K. Y. Srinivasan <[email protected]>
Reviewed-by: Haiyang Zhang <[email protected]>
---
tools/hv/hv_kvp_daemon.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 3dc989f..13ae098 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -589,6 +589,17 @@ kvp_get_ip_address(int family, char *if_name, int op,
continue;
}

+ if (op == KVP_OP_GET_IP_INFO) {
+ /*
+ * Gather info other than the IP address.
+ * IP address info will be gathered later.
+ */
+ if (curp->ifa_addr->sa_family == AF_INET)
+ ip_buffer->addr_family |= ADDR_FAMILY_IPV4;
+ else
+ ip_buffer->addr_family |= ADDR_FAMILY_IPV6;
+ }
+
error = kvp_process_ip_address(curp->ifa_addr,
curp->ifa_addr->sa_family,
buffer,
--
1.7.4.1

2012-08-17 01:19:14

by KY Srinivasan

[permalink] [raw]
Subject: [PATCH V3 08/14] Tools: hv: Gather DNS information

Now, gather DNS information. Invoke an external script (that can be
distro dependent) to gather the DNS information.

Signed-off-by: K. Y. Srinivasan <[email protected]>
Reviewed-by: Haiyang Zhang <[email protected]>
---
tools/hv/hv_kvp_daemon.c | 23 +++++++++++++++++++++++
1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 65d54c8..2f8e0b9 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -550,6 +550,29 @@ static void kvp_get_ipconfig_info(char *if_name,
kvp_process_ipconfig_file(cmd, (char *)buffer->gate_way,
(MAX_GATEWAY_SIZE * 2), INET6_ADDRSTRLEN, 1);

+
+ /*
+ * Gather the DNS state.
+ * Since there is no standard way to get this information
+ * across various distributions of interest; we just invoke
+ * an external script that needs to be ported across distros
+ * of interest.
+ *
+ * Following is the expected format of the information from the script:
+ *
+ * ipaddr1 (nameserver1)
+ * ipaddr2 (nameserver2)
+ * .
+ * .
+ */
+
+ sprintf(cmd, "%s", "hv_get_dns_info");
+
+ /*
+ * Execute the command to gather DNS info.
+ */
+ kvp_process_ipconfig_file(cmd, (char *)buffer->dns_addr,
+ (MAX_IP_ADDR_SIZE * 2), INET_ADDRSTRLEN, 0);
}


--
1.7.4.1

2012-08-17 16:51:28

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH V3 08/14] Tools: hv: Gather DNS information

On Thu, Aug 16, 2012 at 06:32:19PM -0700, K. Y. Srinivasan wrote:
> Now, gather DNS information. Invoke an external script (that can be
> distro dependent) to gather the DNS information.

Where is that script? Why a script, this really isn't in a standard
method for all Linux distros? If not, why not work to do that like the
freedesktop.org people have done for other system configuration items?

greg k-h

2012-08-17 16:55:56

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH V3 08/14] Tools: hv: Gather DNS information

On Fri, Aug 17, 2012 at 09:51:09AM -0700, Greg KH wrote:
> On Thu, Aug 16, 2012 at 06:32:19PM -0700, K. Y. Srinivasan wrote:
> > Now, gather DNS information. Invoke an external script (that can be
> > distro dependent) to gather the DNS information.
>
> Where is that script? Why a script, this really isn't in a standard
> method for all Linux distros? If not, why not work to do that like the
> freedesktop.org people have done for other system configuration items?

And please, don't include <[email protected]> in your
patches anymore, it just constantly bounces as it's no longer valid (and
hasn't been for over 6 months).

It's really annoying.

greg k-h

2012-08-17 17:17:46

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH V3 08/14] Tools: hv: Gather DNS information

On Fri, Aug 17, 2012 at 05:07:22PM +0000, KY Srinivasan wrote:
> > On Thu, Aug 16, 2012 at 06:32:19PM -0700, K. Y. Srinivasan wrote:
> > > Now, gather DNS information. Invoke an external script (that can be
> > > distro dependent) to gather the DNS information.
> >
> > Where is that script? Why a script, this really isn't in a standard
> > method for all Linux distros? If not, why not work to do that like the
> > freedesktop.org people have done for other system configuration items?
>
> We debated this issue on this mailing list for a long time. The consensus was that
> we would invoke external "distro" specific scripts for both configuring the interface
> as well as gathering information that could not be gathered in a distro independent
> fashion. That is what I have implemented here. This code specifies the interface to this
> external script. If these external scripts are not there, it is not fatal - on the SET side,
> the interface will not be configured and on the get side some attributes that are to be
> gathered via these external scripts will not be collected.

I've applied the first 7 patches here, feel free to submit the rest when
you answer the other questions I asked as well.

greg k-h

2012-08-17 17:23:18

by KY Srinivasan

[permalink] [raw]
Subject: RE: [PATCH V3 08/14] Tools: hv: Gather DNS information



> -----Original Message-----
> From: Greg KH [mailto:[email protected]]
> Sent: Friday, August 17, 2012 12:51 PM
> To: KY Srinivasan
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH V3 08/14] Tools: hv: Gather DNS information
>
> On Thu, Aug 16, 2012 at 06:32:19PM -0700, K. Y. Srinivasan wrote:
> > Now, gather DNS information. Invoke an external script (that can be
> > distro dependent) to gather the DNS information.
>
> Where is that script? Why a script, this really isn't in a standard
> method for all Linux distros? If not, why not work to do that like the
> freedesktop.org people have done for other system configuration items?

We debated this issue on this mailing list for a long time. The consensus was that
we would invoke external "distro" specific scripts for both configuring the interface
as well as gathering information that could not be gathered in a distro independent
fashion. That is what I have implemented here. This code specifies the interface to this
external script. If these external scripts are not there, it is not fatal - on the SET side,
the interface will not be configured and on the get side some attributes that are to be
gathered via these external scripts will not be collected.

Regards,

K. Y


2012-08-17 17:32:21

by Ben Hutchings

[permalink] [raw]
Subject: Re: [PATCH V3 08/14] Tools: hv: Gather DNS information

On Fri, Aug 17, 2012 at 09:51:09AM -0700, Greg KH wrote:
> On Thu, Aug 16, 2012 at 06:32:19PM -0700, K. Y. Srinivasan wrote:
> > Now, gather DNS information. Invoke an external script (that can be
> > distro dependent) to gather the DNS information.
>
> Where is that script? Why a script, this really isn't in a standard
> method for all Linux distros? If not, why not work to do that like the
> freedesktop.org people have done for other system configuration items?

I suggested using the Network Manager D-Bus API. That obviously
isn't going to work everywhere, but it seems to cover the same simple
cases that this hypervisor-guest interface allows for.

Ben.

--
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
- Albert Camus

2012-08-17 17:38:53

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH V3 08/14] Tools: hv: Gather DNS information

On Fri, Aug 17, 2012 at 06:32:09PM +0100, Ben Hutchings wrote:
> On Fri, Aug 17, 2012 at 09:51:09AM -0700, Greg KH wrote:
> > On Thu, Aug 16, 2012 at 06:32:19PM -0700, K. Y. Srinivasan wrote:
> > > Now, gather DNS information. Invoke an external script (that can be
> > > distro dependent) to gather the DNS information.
> >
> > Where is that script? Why a script, this really isn't in a standard
> > method for all Linux distros? If not, why not work to do that like the
> > freedesktop.org people have done for other system configuration items?
>
> I suggested using the Network Manager D-Bus API. That obviously
> isn't going to work everywhere, but it seems to cover the same simple
> cases that this hypervisor-guest interface allows for.

That's a great start, as I think all of the distros have now pretty much
standardized on this format, right?

greg k-h

2012-08-17 17:48:38

by KY Srinivasan

[permalink] [raw]
Subject: RE: [PATCH V3 08/14] Tools: hv: Gather DNS information



> -----Original Message-----
> From: Ben Hutchings [mailto:[email protected]]
> Sent: Friday, August 17, 2012 1:32 PM
> To: Greg KH
> Cc: KY Srinivasan; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH V3 08/14] Tools: hv: Gather DNS information
>
> On Fri, Aug 17, 2012 at 09:51:09AM -0700, Greg KH wrote:
> > On Thu, Aug 16, 2012 at 06:32:19PM -0700, K. Y. Srinivasan wrote:
> > > Now, gather DNS information. Invoke an external script (that can be
> > > distro dependent) to gather the DNS information.
> >
> > Where is that script? Why a script, this really isn't in a standard
> > method for all Linux distros? If not, why not work to do that like the
> > freedesktop.org people have done for other system configuration items?
>
> I suggested using the Network Manager D-Bus API. That obviously
> isn't going to work everywhere, but it seems to cover the same simple
> cases that this hypervisor-guest interface allows for.

The scheme Olaf proposed, which is what I have implemented here does not
preclude using the D-BUS API in the external script.

Regards,

K. Y

2012-08-17 17:52:00

by Ben Hutchings

[permalink] [raw]
Subject: Re: [PATCH V3 08/14] Tools: hv: Gather DNS information

On Fri, Aug 17, 2012 at 10:38:37AM -0700, Greg KH wrote:
> On Fri, Aug 17, 2012 at 06:32:09PM +0100, Ben Hutchings wrote:
> > On Fri, Aug 17, 2012 at 09:51:09AM -0700, Greg KH wrote:
> > > On Thu, Aug 16, 2012 at 06:32:19PM -0700, K. Y. Srinivasan wrote:
> > > > Now, gather DNS information. Invoke an external script (that can be
> > > > distro dependent) to gather the DNS information.
> > >
> > > Where is that script? Why a script, this really isn't in a standard
> > > method for all Linux distros? If not, why not work to do that like the
> > > freedesktop.org people have done for other system configuration items?
> >
> > I suggested using the Network Manager D-Bus API. That obviously
> > isn't going to work everywhere, but it seems to cover the same simple
> > cases that this hypervisor-guest interface allows for.
>
> That's a great start, as I think all of the distros have now pretty much
> standardized on this format, right?

I expect it to be available on most current distributions, but not
necessarily enabled by default. Debian only installs NM if you choose
the 'desktop' task at install time, so text-mode virtal servers
wouldn't have it. KY may also be concerned about older distributions
that never had NM.

Ben.

--
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
- Albert Camus

2012-08-17 17:55:51

by KY Srinivasan

[permalink] [raw]
Subject: RE: [PATCH V3 08/14] Tools: hv: Gather DNS information



> -----Original Message-----
> From: Greg KH [mailto:[email protected]]
> Sent: Friday, August 17, 2012 1:39 PM
> To: Ben Hutchings
> Cc: KY Srinivasan; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH V3 08/14] Tools: hv: Gather DNS information
>
> On Fri, Aug 17, 2012 at 06:32:09PM +0100, Ben Hutchings wrote:
> > On Fri, Aug 17, 2012 at 09:51:09AM -0700, Greg KH wrote:
> > > On Thu, Aug 16, 2012 at 06:32:19PM -0700, K. Y. Srinivasan wrote:
> > > > Now, gather DNS information. Invoke an external script (that can be
> > > > distro dependent) to gather the DNS information.
> > >
> > > Where is that script? Why a script, this really isn't in a standard
> > > method for all Linux distros? If not, why not work to do that like the
> > > freedesktop.org people have done for other system configuration items?
> >
> > I suggested using the Network Manager D-Bus API. That obviously
> > isn't going to work everywhere, but it seems to cover the same simple
> > cases that this hypervisor-guest interface allows for.
>
> That's a great start, as I think all of the distros have now pretty much
> standardized on this format, right?

Based on Ben's suggestion I was looking at using NM D-Bus API. Subsequently,
talking to Olaf and Mairus, I chose to go with the external script approach since
each Distro could pretty much decide how the functionality would be implemented
on a specific Distro including for instance by using the NM D-Bus API in the script.
I think the approach of having an external script while not precluding anything
makes the KVP code more generic and more widely applicable without sacrificing
anything.

Regards,

K. Y


2012-08-17 18:27:52

by Olaf Hering

[permalink] [raw]
Subject: Re: [PATCH V3 08/14] Tools: hv: Gather DNS information

On Fri, Aug 17, Ben Hutchings wrote:

> On Fri, Aug 17, 2012 at 09:51:09AM -0700, Greg KH wrote:
> > On Thu, Aug 16, 2012 at 06:32:19PM -0700, K. Y. Srinivasan wrote:
> > > Now, gather DNS information. Invoke an external script (that can be
> > > distro dependent) to gather the DNS information.
> >
> > Where is that script? Why a script, this really isn't in a standard
> > method for all Linux distros? If not, why not work to do that like the
> > freedesktop.org people have done for other system configuration items?
>
> I suggested using the Network Manager D-Bus API. That obviously
> isn't going to work everywhere, but it seems to cover the same simple
> cases that this hypervisor-guest interface allows for.

If its not going to work everywhere how can it be the solution?

The script in turn can do whatever it takes, which includes NM. However
NM (in its current state) shouldnt be thrown at everyone and everything.

Olaf

2012-08-17 18:44:48

by Ben Hutchings

[permalink] [raw]
Subject: Re: [PATCH V3 08/14] Tools: hv: Gather DNS information

On Fri, Aug 17, 2012 at 08:27:21PM +0200, Olaf Hering wrote:
> On Fri, Aug 17, Ben Hutchings wrote:
>
> > On Fri, Aug 17, 2012 at 09:51:09AM -0700, Greg KH wrote:
> > > On Thu, Aug 16, 2012 at 06:32:19PM -0700, K. Y. Srinivasan wrote:
> > > > Now, gather DNS information. Invoke an external script (that can be
> > > > distro dependent) to gather the DNS information.
> > >
> > > Where is that script? Why a script, this really isn't in a standard
> > > method for all Linux distros? If not, why not work to do that like the
> > > freedesktop.org people have done for other system configuration items?
> >
> > I suggested using the Network Manager D-Bus API. That obviously
> > isn't going to work everywhere, but it seems to cover the same simple
> > cases that this hypervisor-guest interface allows for.
>
> If its not going to work everywhere how can it be the solution?

You are never going to have a 'works everywhere' solution for
Linux guests.

How are these distribution-specific integration scripts going
to get installed in the guesta, anyway?

Ben.

> The script in turn can do whatever it takes, which includes NM. However
> NM (in its current state) shouldnt be thrown at everyone and everything.

--
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
- Albert Camus

2012-08-17 18:54:14

by Olaf Hering

[permalink] [raw]
Subject: Re: [PATCH V3 08/14] Tools: hv: Gather DNS information

On Fri, Aug 17, Ben Hutchings wrote:

> You are never going to have a 'works everywhere' solution for
> Linux guests.

The hook is the solution.

> How are these distribution-specific integration scripts going
> to get installed in the guesta, anyway?

The packager of the daemon will write the script, perhaps based on a
template included in tools/hv/script_xyz.sh?

Olaf

2012-08-17 21:56:49

by KY Srinivasan

[permalink] [raw]
Subject: RE: [PATCH V3 08/14] Tools: hv: Gather DNS information



> -----Original Message-----
> From: Greg KH [mailto:[email protected]]
> Sent: Friday, August 17, 2012 1:17 PM
> To: KY Srinivasan
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH V3 08/14] Tools: hv: Gather DNS information
>
> On Fri, Aug 17, 2012 at 05:07:22PM +0000, KY Srinivasan wrote:
> > > On Thu, Aug 16, 2012 at 06:32:19PM -0700, K. Y. Srinivasan wrote:
> > > > Now, gather DNS information. Invoke an external script (that can be
> > > > distro dependent) to gather the DNS information.
> > >
> > > Where is that script? Why a script, this really isn't in a standard
> > > method for all Linux distros? If not, why not work to do that like the
> > > freedesktop.org people have done for other system configuration items?
> >
> > We debated this issue on this mailing list for a long time. The consensus was
> that
> > we would invoke external "distro" specific scripts for both configuring the
> interface
> > as well as gathering information that could not be gathered in a distro
> independent
> > fashion. That is what I have implemented here. This code specifies the interface
> to this
> > external script. If these external scripts are not there, it is not fatal - on the SET
> side,
> > the interface will not be configured and on the get side some attributes that are
> to be
> > gathered via these external scripts will not be collected.
>
> I've applied the first 7 patches here, feel free to submit the rest when
> you answer the other questions I asked as well.

Greg,

I have tried to answer your questions in other emails I have sent out on this thread.
To summarize, the current implementation (with external scripts) does not preclude
using Network Manager where it makes sense (in the external scripts).

If you think it will be useful to have example scripts, I could send the scripts I
have used to test this code as example code. The reason I did not send the
scripts is because (a) not having these scripts is not fatal to this code and
(b) Given that this is a distro specific script, I did not want code checked
into the kernel tree that was very distro specific.

Let me know if I have addressed your concerns and if I can re-send the
remaining 7 patches.

Regards,

K. Y

2012-08-22 08:50:34

by Andy Whitcroft

[permalink] [raw]
Subject: Re: [PATCH V3 08/14] Tools: hv: Gather DNS information

On Fri, Aug 17, 2012 at 08:53:53PM +0200, Olaf Hering wrote:
> On Fri, Aug 17, Ben Hutchings wrote:
>
> > You are never going to have a 'works everywhere' solution for
> > Linux guests.
>
> The hook is the solution.
>
> > How are these distribution-specific integration scripts going
> > to get installed in the guesta, anyway?
>
> The packager of the daemon will write the script, perhaps based on a
> template included in tools/hv/script_xyz.sh?

This seems a reasonable approach and one with precident. We use a similar
scheme for how to handle 'make install' in the kernel letting the distro
provide /sbin/installkernel so as to avoid having distro smarts for every
distro in the kernel source.

It might be nice to share some examples if you have them, but it feels a
little grubby to have them in the kernel tree.

-apw

2012-08-22 15:45:56

by KY Srinivasan

[permalink] [raw]
Subject: RE: [PATCH V3 08/14] Tools: hv: Gather DNS information



> -----Original Message-----
> From: Greg KH [mailto:[email protected]]
> Sent: Friday, August 17, 2012 12:51 PM
> To: KY Srinivasan
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH V3 08/14] Tools: hv: Gather DNS information
>
> On Thu, Aug 16, 2012 at 06:32:19PM -0700, K. Y. Srinivasan wrote:
> > Now, gather DNS information. Invoke an external script (that can be
> > distro dependent) to gather the DNS information.
>
> Where is that script?

If you think it is useful, I could send you the scripts that I have used to test this
functionality as example scripts.

> Why a script, this really isn't in a standard
> method for all Linux distros? If not, why not work to do that like the
> freedesktop.org people have done for other system configuration items?

When discussing this while preparing the patches it was clear that there
really is no single way to obtain the network information, if you are
using network manager you need to use dbus, basic debian networking as
on a server you need to look in /etc/resolv.conf, if you have dnsmasq
installed you need to look in a third place. On the SLES side that I am somewhat
familiar with, if I recall correctly, none of the appliance images have Network Manager installed.
The consensus was that we should avoid pulling a vast pile of distro smarts into the source which
lead to the hook API proposed in these patches.

The approach chosen here does not preclude using Network Manager in the external scripts
if a particular distro chooses to go that route. The current implementation supports maximum
flexibility without sacrificing anything. While the current scripts I have do not use NM APIs, we could
work on external scripts using DBUS APIs.

If it is ok with you, I could send you the remaining 7 patches along with the scripts I am
currently using to test this code. These scripts should be viewed just example code.

Regards,

K. Y