linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V4 00/10] Tools: hv: kvp
@ 2012-09-04 21:45 K. Y. Srinivasan
  2012-09-04 21:46 ` [PATCH V4 01/10] Tools: hv: Get rid of some unused variables K. Y. Srinivasan
  0 siblings, 1 reply; 17+ messages in thread
From: K. Y. Srinivasan @ 2012-09-04 21:45 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, ben, thozza, dcbw
  Cc: K. Y. Srinivasan

This is a resubmission of the subset of the patches that were not
accepted in an earlier submission. The main objection last time was
that the KVP daemon code invoked external scripts for some of the
operations and that I had not included those scripts in the patchset.
This current approach of not encumbering the KVP daemon code with
Distro specific details was discussed on this mailing list and here is
the summary:

1) Using Network Manager APIs in the KVP daemon code directly would
minimize environments where KVP functionality could be deployed since
Network Manager is not available in all of the Distro images of interest;
especially server images.

2) The current architecture of invoking external scripts for what is
essentially a distro specific operation retains full flexibility of
deploying KVP on all Distros of interest while not sacrificing our ability to
use Network Manager APIs. For instance a Distro could choose to implement these
external scripts using NM AMPIs.

In this patchset I have included the external scripts that I had used to test the
KVP functionality as example scripts. These are meant as just examples.


K. Y. Srinivasan (10):
  Tools: hv: Get rid of some unused variables
  Tools: hv: Correctly type string variables
  Tools: hv: Add an example script to retrieve DNS entries
  Tools: hv: Gather DNS information
  Tools: hv: Add an example script to retrieve dhcp state
  Tools: hv: Gather DHCP information
  Tools: hv: Add an example script to configure an interface
  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

 include/linux/hyperv.h       |    2 +
 tools/hv/hv_get_dhcp_info.sh |   25 ++
 tools/hv/hv_get_dns_info.sh  |   13 +
 tools/hv/hv_kvp_daemon.c     |  607 +++++++++++++++++++++++++++++++++++++++++-
 tools/hv/hv_set_ifconfig.sh  |   68 +++++
 5 files changed, 703 insertions(+), 12 deletions(-)
 create mode 100755 tools/hv/hv_get_dhcp_info.sh
 create mode 100755 tools/hv/hv_get_dns_info.sh
 create mode 100755 tools/hv/hv_set_ifconfig.sh

-- 
1.7.4.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH V4 01/10] Tools: hv: Get rid of some unused variables
  2012-09-04 21:45 [PATCH V4 00/10] Tools: hv: kvp K. Y. Srinivasan
@ 2012-09-04 21:46 ` K. Y. Srinivasan
  2012-09-04 21:46   ` [PATCH V4 02/10] Tools: hv: Correctly type string variables K. Y. Srinivasan
                     ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: K. Y. Srinivasan @ 2012-09-04 21:46 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, ben, thozza, dcbw
  Cc: K. Y. Srinivasan

Get rid of unused variables.

Reported-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 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 65d54c8..d12d714 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -196,7 +196,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;
@@ -312,7 +312,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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH V4 02/10] Tools: hv: Correctly type string variables
  2012-09-04 21:46 ` [PATCH V4 01/10] Tools: hv: Get rid of some unused variables K. Y. Srinivasan
@ 2012-09-04 21:46   ` K. Y. Srinivasan
  2012-09-04 21:46   ` [PATCH V4 03/10] Tools: hv: Add an example script to retrieve DNS entries K. Y. Srinivasan
                     ` (7 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: K. Y. Srinivasan @ 2012-09-04 21:46 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, ben, thozza, dcbw
  Cc: K. Y. Srinivasan

Correctly type character strings.

Reported-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 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 d12d714..026bf0e 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -86,8 +86,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 {
@@ -95,7 +95,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];
@@ -199,7 +199,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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH V4 03/10] Tools: hv: Add an example script to retrieve DNS entries
  2012-09-04 21:46 ` [PATCH V4 01/10] Tools: hv: Get rid of some unused variables K. Y. Srinivasan
  2012-09-04 21:46   ` [PATCH V4 02/10] Tools: hv: Correctly type string variables K. Y. Srinivasan
@ 2012-09-04 21:46   ` K. Y. Srinivasan
  2012-09-04 21:46   ` [PATCH V4 04/10] Tools: hv: Gather DNS information K. Y. Srinivasan
                     ` (6 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: K. Y. Srinivasan @ 2012-09-04 21:46 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, ben, thozza, dcbw
  Cc: K. Y. Srinivasan

To keep the KVP daemon code free of distro specific details, we invoke an
external script to retrieve the DNS entries. This is an example script that
was used to test the KVP code. This script has to be implemented in a Distro
specific fashion. For instance on distros that ship with Network Manager enabled,
this script can be based on NM APIs. 

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 tools/hv/hv_get_dns_info.sh |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)
 create mode 100755 tools/hv/hv_get_dns_info.sh

diff --git a/tools/hv/hv_get_dns_info.sh b/tools/hv/hv_get_dns_info.sh
new file mode 100755
index 0000000..058c17b
--- /dev/null
+++ b/tools/hv/hv_get_dns_info.sh
@@ -0,0 +1,13 @@
+#!/bin/bash
+
+# This example script parses /etc/resolv.conf to retrive DNS information.
+# In the interest of keeping the KVP daemon code free of distro specific
+# information; the kvp daemon code invokes this external script to gather
+# DNS information.
+# This script is expected to print the nameserver values to stdout.
+# Each Distro is expected to implement this script in a distro specific
+# fashion. For instance on Distros that ship with Network Manager enabled,
+# this script can be based on the Network Manager APIs for retrieving DNS
+# entries.
+
+cat /etc/resolv.conf 2>/dev/null | awk '/^nameserver/ { print $2 }'
-- 
1.7.4.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH V4 04/10] Tools: hv: Gather DNS information
  2012-09-04 21:46 ` [PATCH V4 01/10] Tools: hv: Get rid of some unused variables K. Y. Srinivasan
  2012-09-04 21:46   ` [PATCH V4 02/10] Tools: hv: Correctly type string variables K. Y. Srinivasan
  2012-09-04 21:46   ` [PATCH V4 03/10] Tools: hv: Add an example script to retrieve DNS entries K. Y. Srinivasan
@ 2012-09-04 21:46   ` K. Y. Srinivasan
  2012-09-04 21:46   ` [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp state K. Y. Srinivasan
                     ` (5 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: K. Y. Srinivasan @ 2012-09-04 21:46 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, ben, thozza, dcbw
  Cc: K. Y. Srinivasan

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

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 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 026bf0e..cfa23a1 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -549,6 +549,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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp state
  2012-09-04 21:46 ` [PATCH V4 01/10] Tools: hv: Get rid of some unused variables K. Y. Srinivasan
                     ` (2 preceding siblings ...)
  2012-09-04 21:46   ` [PATCH V4 04/10] Tools: hv: Gather DNS information K. Y. Srinivasan
@ 2012-09-04 21:46   ` K. Y. Srinivasan
  2012-09-04 22:59     ` Greg KH
  2012-09-04 21:46   ` [PATCH V4 06/10] Tools: hv: Gather DHCP information K. Y. Srinivasan
                     ` (4 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: K. Y. Srinivasan @ 2012-09-04 21:46 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, ben, thozza, dcbw
  Cc: K. Y. Srinivasan

To keep the KVP daemon code free of distro specific details, we invoke an
external script to retrieve the DHCP state. This is an example script that
was used to test the KVP code. This script has to be implemented in a Distro
specific fashion. For instance on distros that ship with Network Manager enabled,
this script can be based on NM APIs.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 tools/hv/hv_get_dhcp_info.sh |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)
 create mode 100755 tools/hv/hv_get_dhcp_info.sh

diff --git a/tools/hv/hv_get_dhcp_info.sh b/tools/hv/hv_get_dhcp_info.sh
new file mode 100755
index 0000000..3de4587
--- /dev/null
+++ b/tools/hv/hv_get_dhcp_info.sh
@@ -0,0 +1,25 @@
+#!/bin/bash
+
+# This example script retrieves the DHCP state of a given interface.
+# In the interest of keeping the KVP daemon code free of distro specific
+# information; the kvp daemon code invokes this external script to gather
+# DHCP setting for the specific interface.
+#
+# Input: Name of the interface
+#
+# Output: The script prints the string "Enabled" to stdout to indicate
+#	that DHCP is enabled on the interface.
+#
+# Each Distro is expected to implement this script in a distro specific
+# fashion. For instance on Distros that ship with Network Manager enabled,
+# this script can be based on the Network Manager APIs for retrieving DHCP
+# information.
+
+if_file="/etc/sysconfig/network-scripts/ifcfg-"$1
+
+dhcp=$(grep "dhcp" $if_file 2>/dev/null)
+
+if [ "$dhcp" != "" ];
+then
+echo "Enabled"
+fi
-- 
1.7.4.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH V4 06/10] Tools: hv: Gather DHCP information
  2012-09-04 21:46 ` [PATCH V4 01/10] Tools: hv: Get rid of some unused variables K. Y. Srinivasan
                     ` (3 preceding siblings ...)
  2012-09-04 21:46   ` [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp state K. Y. Srinivasan
@ 2012-09-04 21:46   ` K. Y. Srinivasan
  2012-09-04 21:46   ` [PATCH V4 07/10] Tools: hv: Add an example script to configure an interface K. Y. Srinivasan
                     ` (3 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: K. Y. Srinivasan @ 2012-09-04 21:46 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, ben, thozza, dcbw
  Cc: K. Y. Srinivasan

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

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 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 cfa23a1..6fb2c1c 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -524,6 +524,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).
@@ -572,6 +575,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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH V4 07/10] Tools: hv: Add an example script to configure an interface
  2012-09-04 21:46 ` [PATCH V4 01/10] Tools: hv: Get rid of some unused variables K. Y. Srinivasan
                     ` (4 preceding siblings ...)
  2012-09-04 21:46   ` [PATCH V4 06/10] Tools: hv: Gather DHCP information K. Y. Srinivasan
@ 2012-09-04 21:46   ` K. Y. Srinivasan
  2012-09-04 21:46   ` [PATCH V4 08/10] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO K. Y. Srinivasan
                     ` (2 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: K. Y. Srinivasan @ 2012-09-04 21:46 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, ben, thozza, dcbw
  Cc: K. Y. Srinivasan

To keep the KVP daemon code free of distro specific details, we invoke an
external script to configure the interface. This is an example script that
was used to test the KVP code. This script has to be implemented in a Distro
specific fashion. For instance on distros that ship with Network Manager enabled,
this script can be based on NM APIs.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 tools/hv/hv_set_ifconfig.sh |   68 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 68 insertions(+), 0 deletions(-)
 create mode 100755 tools/hv/hv_set_ifconfig.sh

diff --git a/tools/hv/hv_set_ifconfig.sh b/tools/hv/hv_set_ifconfig.sh
new file mode 100755
index 0000000..3e9427e
--- /dev/null
+++ b/tools/hv/hv_set_ifconfig.sh
@@ -0,0 +1,68 @@
+#!/bin/bash
+
+# This example script activates an interface based on the specified
+# configuration.
+#
+# In the interest of keeping the KVP daemon code free of distro specific
+# information; the kvp daemon code invokes this external script to configure
+# the interface.
+#
+# The only argument to this script is the configuration file that is to
+# be used to configure the interface.
+#
+# Each Distro is expected to implement this script in a distro specific
+# fashion. For instance on Distros that ship with Network Manager enabled,
+# this script can be based on the Network Manager APIs for configuring the
+# interface.
+#
+# This example script is based on a RHEL environment.
+#
+# 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.
+#
+
+
+
+echo "IPV6INIT=yes" >> $1
+echo "NM_CONTROLLED=no" >> $1
+echo "PEERDNS=yes" >> $1
+echo "ONBOOT=yes" >> $1
+
+dhcp=$(grep "DHCP" $1 2>/dev/null)
+if [ "$dhcp" != "" ];
+then
+echo "BOOTPROTO=dhcp" >> $1;
+fi
+
+cp $1 /etc/sysconfig/network-scripts/
+
+
+interface=$(echo $1 | awk -F - '{ print $2 }')
+
+/sbin/ifdown $interface 2>/dev/null
+/sbin/ifup $interfac 2>/dev/null
-- 
1.7.4.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH V4 08/10] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO
  2012-09-04 21:46 ` [PATCH V4 01/10] Tools: hv: Get rid of some unused variables K. Y. Srinivasan
                     ` (5 preceding siblings ...)
  2012-09-04 21:46   ` [PATCH V4 07/10] Tools: hv: Add an example script to configure an interface K. Y. Srinivasan
@ 2012-09-04 21:46   ` K. Y. Srinivasan
  2012-09-04 21:46   ` [PATCH V4 09/10] Tools: hv: Rename the function kvp_get_ip_address() K. Y. Srinivasan
  2012-09-04 21:46   ` [PATCH V4 10/10] Tools: hv: Implement the KVP verb - KVP_OP_GET_IP_INFO K. Y. Srinivasan
  8 siblings, 0 replies; 17+ messages in thread
From: K. Y. Srinivasan @ 2012-09-04 21:46 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, ben, thozza, dcbw
  Cc: K. Y. Srinivasan

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 (Distro specific)
script to configure the interface.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 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 7585d55..e73b852 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 6fb2c1c..ac144b9 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
@@ -490,6 +505,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)
@@ -790,6 +903,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)
 {
@@ -859,6 +1281,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);
@@ -962,6 +1386,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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH V4 09/10] Tools: hv: Rename the function kvp_get_ip_address()
  2012-09-04 21:46 ` [PATCH V4 01/10] Tools: hv: Get rid of some unused variables K. Y. Srinivasan
                     ` (6 preceding siblings ...)
  2012-09-04 21:46   ` [PATCH V4 08/10] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO K. Y. Srinivasan
@ 2012-09-04 21:46   ` K. Y. Srinivasan
  2012-09-04 21:46   ` [PATCH V4 10/10] Tools: hv: Implement the KVP verb - KVP_OP_GET_IP_INFO K. Y. Srinivasan
  8 siblings, 0 replies; 17+ messages in thread
From: K. Y. Srinivasan @ 2012-09-04 21:46 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, ben, thozza, dcbw
  Cc: K. Y. Srinivasan

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

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 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 ac144b9..849c438 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -765,7 +765,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;
@@ -1468,12 +1468,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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH V4 10/10] Tools: hv: Implement the KVP verb - KVP_OP_GET_IP_INFO
  2012-09-04 21:46 ` [PATCH V4 01/10] Tools: hv: Get rid of some unused variables K. Y. Srinivasan
                     ` (7 preceding siblings ...)
  2012-09-04 21:46   ` [PATCH V4 09/10] Tools: hv: Rename the function kvp_get_ip_address() K. Y. Srinivasan
@ 2012-09-04 21:46   ` K. Y. Srinivasan
  8 siblings, 0 replies; 17+ messages in thread
From: K. Y. Srinivasan @ 2012-09-04 21:46 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, ben, thozza, dcbw
  Cc: K. Y. Srinivasan

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 <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 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 849c438..c8e1013 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -603,6 +603,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)
@@ -749,10 +812,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);
@@ -796,7 +859,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;
@@ -1386,6 +1449,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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp state
  2012-09-04 21:46   ` [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp state K. Y. Srinivasan
@ 2012-09-04 22:59     ` Greg KH
  2012-09-04 23:39       ` KY Srinivasan
  0 siblings, 1 reply; 17+ messages in thread
From: Greg KH @ 2012-09-04 22:59 UTC (permalink / raw)
  To: K. Y. Srinivasan; +Cc: linux-kernel, devel, olaf, apw, ben, thozza, dcbw

On Tue, Sep 04, 2012 at 02:46:37PM -0700, K. Y. Srinivasan wrote:
> To keep the KVP daemon code free of distro specific details, we invoke an
> external script to retrieve the DHCP state. This is an example script that
> was used to test the KVP code. This script has to be implemented in a Distro
> specific fashion. For instance on distros that ship with Network Manager enabled,
> this script can be based on NM APIs.
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
>  tools/hv/hv_get_dhcp_info.sh |   25 +++++++++++++++++++++++++
>  1 files changed, 25 insertions(+), 0 deletions(-)
>  create mode 100755 tools/hv/hv_get_dhcp_info.sh
> 
> diff --git a/tools/hv/hv_get_dhcp_info.sh b/tools/hv/hv_get_dhcp_info.sh
> new file mode 100755
> index 0000000..3de4587
> --- /dev/null
> +++ b/tools/hv/hv_get_dhcp_info.sh
> @@ -0,0 +1,25 @@
> +#!/bin/bash
> +
> +# This example script retrieves the DHCP state of a given interface.
> +# In the interest of keeping the KVP daemon code free of distro specific
> +# information; the kvp daemon code invokes this external script to gather
> +# DHCP setting for the specific interface.
> +#
> +# Input: Name of the interface
> +#
> +# Output: The script prints the string "Enabled" to stdout to indicate
> +#	that DHCP is enabled on the interface.

What happens if DHCP is not enabled on the interface?  Shouldn't that
also return something other than "success"?

greg k-h

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp state
  2012-09-04 22:59     ` Greg KH
@ 2012-09-04 23:39       ` KY Srinivasan
  2012-09-05  0:04         ` Greg KH
  0 siblings, 1 reply; 17+ messages in thread
From: KY Srinivasan @ 2012-09-04 23:39 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, devel, olaf, apw, ben, thozza, dcbw



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, September 04, 2012 6:59 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; ben@decadent.org.uk; thozza@redhat.com;
> dcbw@redhat.com
> Subject: Re: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp
> state
> 
> On Tue, Sep 04, 2012 at 02:46:37PM -0700, K. Y. Srinivasan wrote:
> > To keep the KVP daemon code free of distro specific details, we invoke an
> > external script to retrieve the DHCP state. This is an example script that
> > was used to test the KVP code. This script has to be implemented in a Distro
> > specific fashion. For instance on distros that ship with Network Manager
> enabled,
> > this script can be based on NM APIs.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---
> >  tools/hv/hv_get_dhcp_info.sh |   25 +++++++++++++++++++++++++
> >  1 files changed, 25 insertions(+), 0 deletions(-)
> >  create mode 100755 tools/hv/hv_get_dhcp_info.sh
> >
> > diff --git a/tools/hv/hv_get_dhcp_info.sh b/tools/hv/hv_get_dhcp_info.sh
> > new file mode 100755
> > index 0000000..3de4587
> > --- /dev/null
> > +++ b/tools/hv/hv_get_dhcp_info.sh
> > @@ -0,0 +1,25 @@
> > +#!/bin/bash
> > +
> > +# This example script retrieves the DHCP state of a given interface.
> > +# In the interest of keeping the KVP daemon code free of distro specific
> > +# information; the kvp daemon code invokes this external script to gather
> > +# DHCP setting for the specific interface.
> > +#
> > +# Input: Name of the interface
> > +#
> > +# Output: The script prints the string "Enabled" to stdout to indicate
> > +#	that DHCP is enabled on the interface.
> 
> What happens if DHCP is not enabled on the interface?  Shouldn't that
> also return something other than "success"?

The script is expected to write "Enabled" to stdout to indicate if DHCP is enabled; if this 
is not the case, implicitly we assume DHCP is not enabled (since this is a binary state).

Regards,

K. Y 




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp state
  2012-09-04 23:39       ` KY Srinivasan
@ 2012-09-05  0:04         ` Greg KH
  2012-09-05  0:29           ` KY Srinivasan
  0 siblings, 1 reply; 17+ messages in thread
From: Greg KH @ 2012-09-05  0:04 UTC (permalink / raw)
  To: KY Srinivasan; +Cc: olaf, thozza, linux-kernel, dcbw, apw, devel, ben

On Tue, Sep 04, 2012 at 11:39:12PM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Tuesday, September 04, 2012 6:59 PM
> > To: KY Srinivasan
> > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> > apw@canonical.com; ben@decadent.org.uk; thozza@redhat.com;
> > dcbw@redhat.com
> > Subject: Re: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp
> > state
> > 
> > On Tue, Sep 04, 2012 at 02:46:37PM -0700, K. Y. Srinivasan wrote:
> > > To keep the KVP daemon code free of distro specific details, we invoke an
> > > external script to retrieve the DHCP state. This is an example script that
> > > was used to test the KVP code. This script has to be implemented in a Distro
> > > specific fashion. For instance on distros that ship with Network Manager
> > enabled,
> > > this script can be based on NM APIs.
> > >
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > ---
> > >  tools/hv/hv_get_dhcp_info.sh |   25 +++++++++++++++++++++++++
> > >  1 files changed, 25 insertions(+), 0 deletions(-)
> > >  create mode 100755 tools/hv/hv_get_dhcp_info.sh
> > >
> > > diff --git a/tools/hv/hv_get_dhcp_info.sh b/tools/hv/hv_get_dhcp_info.sh
> > > new file mode 100755
> > > index 0000000..3de4587
> > > --- /dev/null
> > > +++ b/tools/hv/hv_get_dhcp_info.sh
> > > @@ -0,0 +1,25 @@
> > > +#!/bin/bash
> > > +
> > > +# This example script retrieves the DHCP state of a given interface.
> > > +# In the interest of keeping the KVP daemon code free of distro specific
> > > +# information; the kvp daemon code invokes this external script to gather
> > > +# DHCP setting for the specific interface.
> > > +#
> > > +# Input: Name of the interface
> > > +#
> > > +# Output: The script prints the string "Enabled" to stdout to indicate
> > > +#	that DHCP is enabled on the interface.
> > 
> > What happens if DHCP is not enabled on the interface?  Shouldn't that
> > also return something other than "success"?
> 
> The script is expected to write "Enabled" to stdout to indicate if DHCP is enabled; if this 
> is not the case, implicitly we assume DHCP is not enabled (since this is a binary state).

It's not really "binary" given that you are expecting "Enabled" or
nothing, right?  "Disabled" would make a bit more sense perhaps?

Having a script return "nothing at all" to show the failure of a state
doesn't seem the wisest thing, does it?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp state
  2012-09-05  0:04         ` Greg KH
@ 2012-09-05  0:29           ` KY Srinivasan
  2012-09-05  2:25             ` KY Srinivasan
  0 siblings, 1 reply; 17+ messages in thread
From: KY Srinivasan @ 2012-09-05  0:29 UTC (permalink / raw)
  To: Greg KH; +Cc: olaf, thozza, linux-kernel, dcbw, apw, devel, ben



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, September 04, 2012 8:04 PM
> To: KY Srinivasan
> Cc: olaf@aepfle.de; thozza@redhat.com; linux-kernel@vger.kernel.org;
> dcbw@redhat.com; apw@canonical.com; devel@linuxdriverproject.org;
> ben@decadent.org.uk
> Subject: Re: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp
> state
> 
> On Tue, Sep 04, 2012 at 11:39:12PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Tuesday, September 04, 2012 6:59 PM
> > > To: KY Srinivasan
> > > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de;
> > > apw@canonical.com; ben@decadent.org.uk; thozza@redhat.com;
> > > dcbw@redhat.com
> > > Subject: Re: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve
> dhcp
> > > state
> > >
> > > On Tue, Sep 04, 2012 at 02:46:37PM -0700, K. Y. Srinivasan wrote:
> > > > To keep the KVP daemon code free of distro specific details, we invoke an
> > > > external script to retrieve the DHCP state. This is an example script that
> > > > was used to test the KVP code. This script has to be implemented in a Distro
> > > > specific fashion. For instance on distros that ship with Network Manager
> > > enabled,
> > > > this script can be based on NM APIs.
> > > >
> > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > > ---
> > > >  tools/hv/hv_get_dhcp_info.sh |   25 +++++++++++++++++++++++++
> > > >  1 files changed, 25 insertions(+), 0 deletions(-)
> > > >  create mode 100755 tools/hv/hv_get_dhcp_info.sh
> > > >
> > > > diff --git a/tools/hv/hv_get_dhcp_info.sh b/tools/hv/hv_get_dhcp_info.sh
> > > > new file mode 100755
> > > > index 0000000..3de4587
> > > > --- /dev/null
> > > > +++ b/tools/hv/hv_get_dhcp_info.sh
> > > > @@ -0,0 +1,25 @@
> > > > +#!/bin/bash
> > > > +
> > > > +# This example script retrieves the DHCP state of a given interface.
> > > > +# In the interest of keeping the KVP daemon code free of distro specific
> > > > +# information; the kvp daemon code invokes this external script to gather
> > > > +# DHCP setting for the specific interface.
> > > > +#
> > > > +# Input: Name of the interface
> > > > +#
> > > > +# Output: The script prints the string "Enabled" to stdout to indicate
> > > > +#	that DHCP is enabled on the interface.
> > >
> > > What happens if DHCP is not enabled on the interface?  Shouldn't that
> > > also return something other than "success"?
> >
> > The script is expected to write "Enabled" to stdout to indicate if DHCP is
> enabled; if this
> > is not the case, implicitly we assume DHCP is not enabled (since this is a binary
> state).
> 
> It's not really "binary" given that you are expecting "Enabled" or
> nothing, right?  "Disabled" would make a bit more sense perhaps?

In the KVP daemon code, I currently only check for "Enabled" to see if
DHCP is enabled.

> 
> Having a script return "nothing at all" to show the failure of a state
> doesn't seem the wisest thing, does it?

I can certainly resubmit this patch with the change you are suggesting. This patch adds a new file
and does not impact any of the other patches that follows this. Would you want me to resubmit all of the
remaining patches starting with this one.

Regards,

K. Y 



^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp state
  2012-09-05  0:29           ` KY Srinivasan
@ 2012-09-05  2:25             ` KY Srinivasan
  2012-09-05 19:12               ` Greg KH
  0 siblings, 1 reply; 17+ messages in thread
From: KY Srinivasan @ 2012-09-05  2:25 UTC (permalink / raw)
  To: KY Srinivasan, Greg KH; +Cc: olaf, thozza, linux-kernel, dcbw, apw, devel, ben



> -----Original Message-----
> From: devel [mailto:devel-bounces@linuxdriverproject.org] On Behalf Of KY
> Srinivasan
> Sent: Tuesday, September 04, 2012 8:29 PM
> To: Greg KH
> Cc: olaf@aepfle.de; thozza@redhat.com; linux-kernel@vger.kernel.org;
> dcbw@redhat.com; apw@canonical.com; devel@linuxdriverproject.org;
> ben@decadent.org.uk
> Subject: RE: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp
> state
> 
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Tuesday, September 04, 2012 8:04 PM
> > To: KY Srinivasan
> > Cc: olaf@aepfle.de; thozza@redhat.com; linux-kernel@vger.kernel.org;
> > dcbw@redhat.com; apw@canonical.com; devel@linuxdriverproject.org;
> > ben@decadent.org.uk
> > Subject: Re: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp
> > state
> >
> > On Tue, Sep 04, 2012 at 11:39:12PM +0000, KY Srinivasan wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > Sent: Tuesday, September 04, 2012 6:59 PM
> > > > To: KY Srinivasan
> > > > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > olaf@aepfle.de;
> > > > apw@canonical.com; ben@decadent.org.uk; thozza@redhat.com;
> > > > dcbw@redhat.com
> > > > Subject: Re: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve
> > dhcp
> > > > state
> > > >
> > > > On Tue, Sep 04, 2012 at 02:46:37PM -0700, K. Y. Srinivasan wrote:
> > > > > To keep the KVP daemon code free of distro specific details, we invoke an
> > > > > external script to retrieve the DHCP state. This is an example script that
> > > > > was used to test the KVP code. This script has to be implemented in a
> Distro
> > > > > specific fashion. For instance on distros that ship with Network Manager
> > > > enabled,
> > > > > this script can be based on NM APIs.
> > > > >
> > > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > > > ---
> > > > >  tools/hv/hv_get_dhcp_info.sh |   25 +++++++++++++++++++++++++
> > > > >  1 files changed, 25 insertions(+), 0 deletions(-)
> > > > >  create mode 100755 tools/hv/hv_get_dhcp_info.sh
> > > > >
> > > > > diff --git a/tools/hv/hv_get_dhcp_info.sh
> b/tools/hv/hv_get_dhcp_info.sh
> > > > > new file mode 100755
> > > > > index 0000000..3de4587
> > > > > --- /dev/null
> > > > > +++ b/tools/hv/hv_get_dhcp_info.sh
> > > > > @@ -0,0 +1,25 @@
> > > > > +#!/bin/bash
> > > > > +
> > > > > +# This example script retrieves the DHCP state of a given interface.
> > > > > +# In the interest of keeping the KVP daemon code free of distro specific
> > > > > +# information; the kvp daemon code invokes this external script to
> gather
> > > > > +# DHCP setting for the specific interface.
> > > > > +#
> > > > > +# Input: Name of the interface
> > > > > +#
> > > > > +# Output: The script prints the string "Enabled" to stdout to indicate
> > > > > +#	that DHCP is enabled on the interface.
> > > >
> > > > What happens if DHCP is not enabled on the interface?  Shouldn't that
> > > > also return something other than "success"?
> > >
> > > The script is expected to write "Enabled" to stdout to indicate if DHCP is
> > enabled; if this
> > > is not the case, implicitly we assume DHCP is not enabled (since this is a binary
> > state).
> >
> > It's not really "binary" given that you are expecting "Enabled" or
> > nothing, right?  "Disabled" would make a bit more sense perhaps?
> 
> In the KVP daemon code, I currently only check for "Enabled" to see if
> DHCP is enabled.
> 
> >
> > Having a script return "nothing at all" to show the failure of a state
> > doesn't seem the wisest thing, does it?
> 
> I can certainly resubmit this patch with the change you are suggesting. This patch
> adds a new file
> and does not impact any of the other patches that follows this. Would you want
> me to resubmit all of the
> remaining patches starting with this one.

I have resent this patch with the change you have suggested. Since this patch is a 
standalone patch I have not resent the other patches that follow this patch. Let me
know if you want me to resend the remaining patches.

Regards,

K. Y 
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
> 
> 




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp state
  2012-09-05  2:25             ` KY Srinivasan
@ 2012-09-05 19:12               ` Greg KH
  0 siblings, 0 replies; 17+ messages in thread
From: Greg KH @ 2012-09-05 19:12 UTC (permalink / raw)
  To: KY Srinivasan; +Cc: olaf, thozza, linux-kernel, dcbw, apw, devel, ben

On Wed, Sep 05, 2012 at 02:25:28AM +0000, KY Srinivasan wrote:
> > I can certainly resubmit this patch with the change you are suggesting. This patch
> > adds a new file
> > and does not impact any of the other patches that follows this. Would you want
> > me to resubmit all of the
> > remaining patches starting with this one.
> 
> I have resent this patch with the change you have suggested. Since this patch is a 
> standalone patch I have not resent the other patches that follow this patch. Let me
> know if you want me to resend the remaining patches.

Please resend all of them, I no longer have them in my to-apply queue.

greg k-h

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2012-09-05 19:14 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-04 21:45 [PATCH V4 00/10] Tools: hv: kvp K. Y. Srinivasan
2012-09-04 21:46 ` [PATCH V4 01/10] Tools: hv: Get rid of some unused variables K. Y. Srinivasan
2012-09-04 21:46   ` [PATCH V4 02/10] Tools: hv: Correctly type string variables K. Y. Srinivasan
2012-09-04 21:46   ` [PATCH V4 03/10] Tools: hv: Add an example script to retrieve DNS entries K. Y. Srinivasan
2012-09-04 21:46   ` [PATCH V4 04/10] Tools: hv: Gather DNS information K. Y. Srinivasan
2012-09-04 21:46   ` [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp state K. Y. Srinivasan
2012-09-04 22:59     ` Greg KH
2012-09-04 23:39       ` KY Srinivasan
2012-09-05  0:04         ` Greg KH
2012-09-05  0:29           ` KY Srinivasan
2012-09-05  2:25             ` KY Srinivasan
2012-09-05 19:12               ` Greg KH
2012-09-04 21:46   ` [PATCH V4 06/10] Tools: hv: Gather DHCP information K. Y. Srinivasan
2012-09-04 21:46   ` [PATCH V4 07/10] Tools: hv: Add an example script to configure an interface K. Y. Srinivasan
2012-09-04 21:46   ` [PATCH V4 08/10] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO K. Y. Srinivasan
2012-09-04 21:46   ` [PATCH V4 09/10] Tools: hv: Rename the function kvp_get_ip_address() K. Y. Srinivasan
2012-09-04 21:46   ` [PATCH V4 10/10] Tools: hv: Implement the KVP verb - KVP_OP_GET_IP_INFO K. Y. Srinivasan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).