Linux virtualization list
 help / color / mirror / Atom feed
* [PATCH 13/14] Tools: hv: Get rid of some unused variables
From: K. Y. Srinivasan @ 2012-08-17  0:45 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization, olaf, apw, ben
In-Reply-To: <1345164325-5198-1-git-send-email-kys@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 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

^ permalink raw reply related

* [PATCH 14/14] Tools: hv: Correctly type string variables
From: K. Y. Srinivasan @ 2012-08-17  0:45 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization, olaf, apw, ben
  Cc: K. Y. Srinivasan
In-Reply-To: <1345164325-5198-1-git-send-email-kys@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 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

^ permalink raw reply related

* RE: [PATCH V3 00/14] drivers: hv: kvp
From: KY Srinivasan @ 2012-08-17  1:12 UTC (permalink / raw)
  To: KY Srinivasan, gregkh@linuxfoundation.org,
	linux-kernel@vger.kernel.org, devel@linuxdriverproject.org,
	virtualization@lists.osdl.org, olaf@aepfle.de, apw@canonical.com,
	ben@decadent.org.uk
In-Reply-To: <1345164300-5146-1-git-send-email-kys@microsoft.com>



> -----Original Message-----
> From: K. Y. Srinivasan [mailto:kys@microsoft.com]
> Sent: Thursday, August 16, 2012 8:45 PM
> To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; olaf@aepfle.de;
> apw@canonical.com; ben@decadent.org.uk
> Cc: KY Srinivasan
> 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.

Greg,

Please drop this set. In my haste to send out the patches, I have sent out the wrong patches. I am going to send out the correct set in a minute.

Regards,

K. Y


> 
> 
> 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
> 
> 
> 

^ permalink raw reply

* [PATCH V3 00/14] drivers: hv: kvp
From: K. Y. Srinivasan @ 2012-08-17  1:31 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization, olaf, apw, ben
  Cc: K. Y. Srinivasan

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

^ permalink raw reply

* [PATCH V3 01/14] Drivers: hv: kvp: Support the new IP injection messages
From: K. Y. Srinivasan @ 2012-08-17  1:32 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization, olaf, apw, ben
In-Reply-To: <1345167068-5461-1-git-send-email-kys@microsoft.com>

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

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 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

^ permalink raw reply related

* [PATCH V3 02/14] Tools: hv: Prepare to expand  kvp_get_ip_address() functionality
From: K. Y. Srinivasan @ 2012-08-17  1:32 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization, olaf, apw, ben
  Cc: K. Y. Srinivasan
In-Reply-To: <1345167145-5513-1-git-send-email-kys@microsoft.com>

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 <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 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

^ permalink raw reply related

* [PATCH V3 03/14] Tools: hv: Further refactor kvp_get_ip_address()
From: K. Y. Srinivasan @ 2012-08-17  1:32 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization, olaf, apw, ben
  Cc: K. Y. Srinivasan
In-Reply-To: <1345167145-5513-1-git-send-email-kys@microsoft.com>

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

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 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

^ permalink raw reply related

* [PATCH V3 04/14] Tools: hv: Gather address family information
From: K. Y. Srinivasan @ 2012-08-17  1:32 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization, olaf, apw, ben
In-Reply-To: <1345167145-5513-1-git-send-email-kys@microsoft.com>

Now, gather address family 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 |   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

^ permalink raw reply related

* [PATCH V3 05/14] Tools: hv: Gather subnet information
From: K. Y. Srinivasan @ 2012-08-17  1:32 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization, olaf, apw, ben
  Cc: K. Y. Srinivasan
In-Reply-To: <1345167145-5513-1-git-send-email-kys@microsoft.com>

Now, gather sub-net 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 |   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

^ permalink raw reply related

* [PATCH V3 06/14] Tools: hv: Represent the ipv6 mask using CIDR notation
From: K. Y. Srinivasan @ 2012-08-17  1:32 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization, olaf, apw, ben
In-Reply-To: <1345167145-5513-1-git-send-email-kys@microsoft.com>

Transform ipv6 subnet information to CIDR notation.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 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

^ permalink raw reply related

* [PATCH V3 07/14] Tools: hv: Gather ipv[4,6] gateway information
From: K. Y. Srinivasan @ 2012-08-17  1:32 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization, olaf, apw, ben
In-Reply-To: <1345167145-5513-1-git-send-email-kys@microsoft.com>

Gather information on the default gateways - ipv4/ipv6.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 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

^ permalink raw reply related

* [PATCH V3 08/14] Tools: hv: Gather DNS information
From: K. Y. Srinivasan @ 2012-08-17  1:32 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization, olaf, apw, ben
In-Reply-To: <1345167145-5513-1-git-send-email-kys@microsoft.com>

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 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

^ permalink raw reply related

* [PATCH V3 09/14] Tools: hv: Gather DHCP information
From: K. Y. Srinivasan @ 2012-08-17  1:32 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization, olaf, apw, ben
  Cc: K. Y. Srinivasan
In-Reply-To: <1345167145-5513-1-git-send-email-kys@microsoft.com>

Collect information on dhcp setting for the specified interface.
We invoke an external 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 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

^ permalink raw reply related

* [PATCH V3 10/14] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO
From: K. Y. Srinivasan @ 2012-08-17  1:32 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization, olaf, apw, ben
In-Reply-To: <1345167145-5513-1-git-send-email-kys@microsoft.com>

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 <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 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

^ permalink raw reply related

* [PATCH V3 11/14] Tools: hv: Rename the function kvp_get_ip_address()
From: K. Y. Srinivasan @ 2012-08-17  1:32 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization, olaf, apw, ben
In-Reply-To: <1345167145-5513-1-git-send-email-kys@microsoft.com>

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 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

^ permalink raw reply related

* [PATCH V3 12/14] Tools: hv: Implement the KVP verb - KVP_OP_GET_IP_INFO
From: K. Y. Srinivasan @ 2012-08-17  1:32 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization, olaf, apw, ben
In-Reply-To: <1345167145-5513-1-git-send-email-kys@microsoft.com>

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 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

^ permalink raw reply related

* [PATCH V3 13/14] Tools: hv: Get rid of some unused variables
From: K. Y. Srinivasan @ 2012-08-17  1:32 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization, olaf, apw, ben
In-Reply-To: <1345167145-5513-1-git-send-email-kys@microsoft.com>

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 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

^ permalink raw reply related

* [PATCH V3 14/14] Tools: hv: Correctly type string variables
From: K. Y. Srinivasan @ 2012-08-17  1:32 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization, olaf, apw, ben
In-Reply-To: <1345167145-5513-1-git-send-email-kys@microsoft.com>

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 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

^ permalink raw reply related

* Re: [PATCH V3 08/14] Tools: hv: Gather DNS information
From: Greg KH @ 2012-08-17 16:51 UTC (permalink / raw)
  To: K. Y. Srinivasan; +Cc: linux-kernel, devel, virtualization, olaf, apw, ben
In-Reply-To: <1345167145-5513-8-git-send-email-kys@microsoft.com>

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

^ permalink raw reply

* Re: [PATCH V3 08/14] Tools: hv: Gather DNS information
From: Greg KH @ 2012-08-17 16:55 UTC (permalink / raw)
  To: K. Y. Srinivasan; +Cc: olaf, linux-kernel, virtualization, apw, devel, ben
In-Reply-To: <20120817165109.GA11323@kroah.com>

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 <virtualization@lists.osdl.org> 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

^ permalink raw reply

* Re: [RFC-v2 6/6] virtio-scsi: Fix incorrect VirtIOSCSI->cmd_vqs[0] definition
From: Paolo Bonzini @ 2012-08-18 18:52 UTC (permalink / raw)
  To: Nicholas A. Bellinger
  Cc: Stefan Hajnoczi, kvm-devel, Michael S. Tsirkin, Jan Kiszka,
	qemu-devel, lf-virt, Anthony Liguori, target-devel, Zhi Yong Wu,
	Christoph Hellwig
In-Reply-To: <1344975625.22433.88.camel@haakon2.linux-iscsi.org>

Il 14/08/2012 22:20, Nicholas A. Bellinger ha scritto:
>>> > > Since virtio_scsi currently assumes a single vqs for data, this patch
>>> > > simply changes ->cmd_vqs[1] to handle the single VirtQueue.

Wrong, multiqueue works just fine. :)  It's just the kernel driver that
doesn't support it yet.

>>> > > Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> > > Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>>> > > Cc: Michael S. Tsirkin <mst@redhat.com>
>>> > > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
>> > 
>> > This is a bugfix we need even without vhost, right?
>> > 
> I believe so, as it appears to be stomping past the end of memory for
> every virtio-scsi initialization regardless of vhost usage.. 

You just did a wrong merge.  When commit d2ad7dd (virtio-scsi: add
multiqueue capability, 2012-04-06) changed cmd_vq from pointer to array
of pointers, you should have moved the following fields to the middle of
the struct, just like that commit did.

Paolo

^ permalink raw reply

* Re: [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost
From: Michael S. Tsirkin @ 2012-08-18 19:10 UTC (permalink / raw)
  To: Nicholas A. Bellinger
  Cc: Anthony Liguori, Stefan Hajnoczi, kvm-devel, Jan Kiszka,
	qemu-devel, Zhi Yong Wu, Anthony Liguori, target-devel,
	Paolo Bonzini, lf-virt, Christoph Hellwig
In-Reply-To: <1344978749.22433.116.camel@haakon2.linux-iscsi.org>

On Tue, Aug 14, 2012 at 02:12:29PM -0700, Nicholas A. Bellinger wrote:
> On Mon, 2012-08-13 at 11:59 +0300, Michael S. Tsirkin wrote:
> > On Mon, Aug 13, 2012 at 08:35:14AM +0000, Nicholas A. Bellinger wrote:
> > > From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> > > 
> > > This patch adds a new type of host device that drives the vhost_scsi
> > > device.  The syntax to add vhost-scsi is:
> > > 
> > >   qemu -vhost-scsi id=vhost-scsi0,wwpn=...,tpgt=123
> > > 
> > > The virtio-scsi emulated device will make use of vhost-scsi to process
> > > virtio-scsi requests inside the kernel and hand them to the in-kernel
> > > SCSI target stack using the tcm_vhost fabric driver.
> > > 
> > > The tcm_vhost driver was merged into the upstream linux kernel for 3.6-rc2,
> > > and the commit can be found here:
> > > 
> > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=057cbf49a1f08297
> > > 
> > > Changelog v1 -> v2:
> > > 
> > > - Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as
> > >   starting point for v3.6-rc code (Stefan + ALiguori + nab)
> > > - Fix upstream qemu conflict in hw/qdev-properties.c
> > > - Make GET_ABI_VERSION use int (nab + mst)
> > > - Fix vhost-scsi case lables in configure (reported by paolo)
> > > - Convert qdev_prop_vhost_scsi to use ->get() + ->set() following
> > >   qdev_prop_netdev (reported by paolo)
> > > - Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo)
> > > 
> > > Changelog v0 -> v1:
> > > 
> > > - Add VHOST_SCSI_SET_ENDPOINT call (stefan)
> > > - Enable vhost notifiers for multiple queues (Zhi)
> > > - clear vhost-scsi endpoint on stopped (Zhi)
> > > - Add CONFIG_VHOST_SCSI for QEMU build configure (nab)
> > > - Rename vhost_vring_target -> vhost_scsi_target (mst + nab)
> > > - Add support for VHOST_SCSI_GET_ABI_VERSION ioctl (aliguori + nab)
> > > 
> > > Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> > > Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> > > Cc: Anthony Liguori <aliguori@us.ibm.com>
> > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> > 
> > 
> > Sent mail too fast, sorry. More comments below.
> > 
> > > ---
> > >  configure            |   10 +++
> > >  hw/Makefile.objs     |    1 +
> > >  hw/qdev-properties.c |   40 ++++++++++++
> > >  hw/qdev.h            |    3 +
> > >  hw/vhost-scsi.c      |  170 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  hw/vhost-scsi.h      |   50 +++++++++++++++
> > >  qemu-common.h        |    1 +
> > >  qemu-config.c        |   16 +++++
> > >  qemu-options.hx      |    4 +
> > >  vl.c                 |   18 +++++
> > >  10 files changed, 313 insertions(+), 0 deletions(-)
> > >  create mode 100644 hw/vhost-scsi.c
> > >  create mode 100644 hw/vhost-scsi.h
> > > 
> > > diff --git a/configure b/configure
> > > index f0dbc03..1f03202 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -168,6 +168,7 @@ libattr=""
> > >  xfs=""
> > >  
> > >  vhost_net="no"
> > > +vhost_scsi="no"
> > >  kvm="no"
> > >  gprof="no"
> > >  debug_tcg="no"
> > > @@ -513,6 +514,7 @@ Haiku)
> > >    usb="linux"
> > >    kvm="yes"
> > >    vhost_net="yes"
> > > +  vhost_scsi="yes"
> > >    if [ "$cpu" = "i386" -o "$cpu" = "x86_64" ] ; then
> > >      audio_possible_drivers="$audio_possible_drivers fmod"
> > >    fi
> > > @@ -818,6 +820,10 @@ for opt do
> > >    ;;
> > >    --enable-vhost-net) vhost_net="yes"
> > >    ;;
> > > +  --disable-vhost-scsi) vhost_scsi="no"
> > > +  ;;
> > > +  --enable-vhost-scsi) vhost_scsi="yes"
> > > +  ;;
> > >    --disable-opengl) opengl="no"
> > >    ;;
> > >    --enable-opengl) opengl="yes"
> > > @@ -3116,6 +3122,7 @@ echo "posix_madvise     $posix_madvise"
> > >  echo "uuid support      $uuid"
> > >  echo "libcap-ng support $cap_ng"
> > >  echo "vhost-net support $vhost_net"
> > > +echo "vhost-scsi support $vhost_scsi"
> > >  echo "Trace backend     $trace_backend"
> > >  echo "Trace output file $trace_file-<pid>"
> > >  echo "spice support     $spice"
> > > @@ -3828,6 +3835,9 @@ case "$target_arch2" in
> > >        if test "$vhost_net" = "yes" ; then
> > >          echo "CONFIG_VHOST_NET=y" >> $config_target_mak
> > >        fi
> > > +      if test "$vhost_scsi" = "yes" ; then
> > > +        echo "CONFIG_VHOST_SCSI=y" >> $config_target_mak
> > > +      fi
> > >      fi
> > >  esac
> > >  case "$target_arch2" in
> > > diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> > > index 3ba5dd0..6ab75ec 100644
> > > --- a/hw/Makefile.objs
> > > +++ b/hw/Makefile.objs
> > > @@ -169,6 +169,7 @@ obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o
> > >  obj-$(CONFIG_VIRTIO) += virtio-serial-bus.o virtio-scsi.o
> > >  obj-$(CONFIG_SOFTMMU) += vhost_net.o
> > >  obj-$(CONFIG_VHOST_NET) += vhost.o
> > > +obj-$(CONFIG_VHOST_SCSI) += vhost-scsi.o
> > >  obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/
> > >  obj-$(CONFIG_NO_PCI) += pci-stub.o
> > >  obj-$(CONFIG_VGA) += vga.o
> > > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> > > index 8aca0d4..0266266 100644
> > > --- a/hw/qdev-properties.c
> > > +++ b/hw/qdev-properties.c
> > > @@ -4,6 +4,7 @@
> > >  #include "blockdev.h"
> > >  #include "hw/block-common.h"
> > >  #include "net/hub.h"
> > > +#include "vhost-scsi.h"
> > >  
> > >  void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
> > >  {
> > > @@ -696,6 +697,45 @@ PropertyInfo qdev_prop_vlan = {
> > >      .set   = set_vlan,
> > >  };
> > >  
> > > +/* --- vhost-scsi --- */
> > > +
> > > +static int parse_vhost_scsi_dev(DeviceState *dev, const char *str, void **ptr)
> > > +{
> > > +   VHostSCSI *p;
> > > +
> > > +   p = find_vhost_scsi(str);
> > > +   if (p == NULL)
> > > +       return -ENOENT;
> > > +
> > > +   *ptr = p;
> > > +   return 0;
> > > +}
> > > +
> > > +static const char *print_vhost_scsi_dev(void *ptr)
> > > +{
> > > +    VHostSCSI *p = ptr;
> > > +
> > > +    return (p) ? vhost_scsi_get_id(p) : "<null>";
> > > +}
> > > +
> > > +static void get_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque,
> > > +                       const char *name, Error **errp)
> > > +{
> > > +    get_pointer(obj, v, opaque, print_vhost_scsi_dev, name, errp);
> > > +}
> > > +
> > > +static void set_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque,
> > > +                               const char *name, Error **errp)
> > > +{
> > > +    set_pointer(obj, v, opaque, parse_vhost_scsi_dev, name, errp);
> > > +}
> > > +
> > > +PropertyInfo qdev_prop_vhost_scsi = {
> > > +     .name = "vhost-scsi",
> > > +     .get  = get_vhost_scsi_dev,
> > > +     .set  = set_vhost_scsi_dev,
> > > +};
> > > +
> > >  /* --- pointer --- */
> > >  
> > >  /* Not a proper property, just for dirty hacks.  TODO Remove it!  */
> > 
> > Why does this make sense in the generic qdev-properties?
> > There's exactly one device that can use this, no?
> > 
> 
> Mmmm, not sure on this one either..  Stefan..?
> 
> > > diff --git a/hw/qdev.h b/hw/qdev.h
> > > index d699194..d5873bb 100644
> > > --- a/hw/qdev.h
> > > +++ b/hw/qdev.h
> > > @@ -238,6 +238,7 @@ extern PropertyInfo qdev_prop_vlan;
> > >  extern PropertyInfo qdev_prop_pci_devfn;
> > >  extern PropertyInfo qdev_prop_blocksize;
> > >  extern PropertyInfo qdev_prop_pci_host_devaddr;
> > > +extern PropertyInfo qdev_prop_vhost_scsi;
> > >  
> > >  #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
> > >          .name      = (_name),                                    \
> > > @@ -305,6 +306,8 @@ extern PropertyInfo qdev_prop_pci_host_devaddr;
> > >      DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t)
> > >  #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
> > >      DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
> > > +#define DEFINE_PROP_VHOST_SCSI(_n, _s, _f)       \
> > > +    DEFINE_PROP(_n, _s, _f, qdev_prop_vhost_scsi, VHostSCSI*)
> > >
> > 
> > Can this move to vhost-scsi.c?
> >   
> 
> Done
> 
> > >  #define DEFINE_PROP_END_OF_LIST()               \
> > >      {}
> > > diff --git a/hw/vhost-scsi.c b/hw/vhost-scsi.c
> > > new file mode 100644
> > > index 0000000..7145b2d
> > > --- /dev/null
> > > +++ b/hw/vhost-scsi.c
> > > @@ -0,0 +1,170 @@
> > > +/*
> > > + * vhost_scsi host device
> > > + *
> > > + * Copyright IBM, Corp. 2011
> > > + *
> > > + * Authors:
> > > + *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
> > > + *
> > > + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> > > + * See the COPYING.LIB file in the top-level directory.
> > > + *
> > > + */
> > > +
> > > +#include <sys/ioctl.h>
> > > +#include "config.h"
> > > +#include "qemu-queue.h"
> > > +#include "vhost-scsi.h"
> > > +#include "vhost.h"
> > > +
> > > +struct VHostSCSI {
> > > +    const char *id;
> > > +    const char *wwpn;
> > > +    uint16_t tpgt;
> > > +    struct vhost_dev dev;
> > > +    struct vhost_virtqueue vqs[3];
> > 
> > Could you add enum for vq numbers pls?
> > 
> 
> Done
> 
> > > +    QLIST_ENTRY(VHostSCSI) list;
> > > +};
> > > +
> > > +static QLIST_HEAD(, VHostSCSI) vhost_scsi_list =
> > > +    QLIST_HEAD_INITIALIZER(vhost_scsi_list);
> > > +
> > > +VHostSCSI *find_vhost_scsi(const char *id)
> > > +{
> > > +    VHostSCSI *vs;
> > > +
> > > +    QLIST_FOREACH(vs, &vhost_scsi_list, list) {
> > > +        if (strcmp(id, vs->id) == 0) {
> > 
> > !strcmp
> > 
> 
> Done
> 
> > > +            return vs;
> > > +        }
> > > +    }
> > > +    return NULL;
> > > +}
> > > +
> > > +const char *vhost_scsi_get_id(VHostSCSI *vs)
> > > +{
> > > +    return vs->id;
> > > +}
> > > +
> > > +int vhost_scsi_start(VHostSCSI *vs, VirtIODevice *vdev)
> > > +{
> > > +    int ret, abi_version;
> > > +    struct vhost_scsi_target backend;
> > > +
> > > +    if (!vhost_dev_query(&vs->dev, vdev)) {
> > > +        return -ENOTSUP;
> > > +    }
> > > +
> > > +    vs->dev.nvqs = 3;
> > > +    vs->dev.vqs = vs->vqs;
> > > +
> > > +    ret = vhost_dev_enable_notifiers(&vs->dev, vdev);
> > > +    if (ret < 0) {
> > > +        return ret;
> > > +    }
> > > +
> > > +    ret = vhost_dev_start(&vs->dev, vdev);
> > > +    if (ret < 0) {
> > > +        return ret;
> > > +    }
> > > +
> > > +    memset(&backend, 0, sizeof(backend));
> > > +    ret = ioctl(vs->dev.control, VHOST_SCSI_GET_ABI_VERSION, &abi_version);
> > > +    if (ret < 0) {
> > > +        ret = -errno;
> > > +        vhost_dev_stop(&vs->dev, vdev);
> > > +        return ret;
> > > +    }
> > > +    if (abi_version > VHOST_SCSI_ABI_VERSION) {
> > > +        fprintf(stderr, "The running tcm_vhost kernel abi_version: %d is greater"
> > > +		" than vhost_scsi userspace supports: %d\n", abi_version,
> > > +		VHOST_SCSI_ABI_VERSION);
> > > +        ret = -ENOSYS;
> > > +        vhost_dev_stop(&vs->dev, vdev);
> > > +        return ret;
> > > +    }
> > > +    fprintf(stdout, "TCM_vHost ABI version: %d\n", abi_version);
> > > +
> > > +    pstrcpy((char *)backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->wwpn);
> > > +    backend.vhost_tpgt = vs->tpgt;
> > > +    ret = ioctl(vs->dev.control, VHOST_SCSI_SET_ENDPOINT, &backend);
> > > +    if (ret < 0) {
> > > +        ret = -errno;
> > > +        vhost_dev_stop(&vs->dev, vdev);
> > > +        return ret;
> > > +    }
> > > +
> > > +    return 0;
> > > +}
> > > +
> > > +void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev)
> > > +{
> > > +    int ret;
> > > +    struct vhost_scsi_target backend;
> > > +
> > > +    pstrcpy((char *)backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->wwpn);
> > > +    backend.vhost_tpgt = vs->tpgt;
> > > +    ret = ioctl(vs->dev.control, VHOST_SCSI_CLEAR_ENDPOINT, &backend);
> > > +    if (ret < 0) {
> > > +        fprintf(stderr, "Failed to clear endpoint\n");
> > > +    }
> > > +
> > > +    vhost_dev_stop(&vs->dev, vdev);
> > > +}
> > > +
> > > +static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
> > > +                                 uint16_t tpgt)
> > > +{
> > > +    VHostSCSI *vs = g_malloc0(sizeof(*vs));
> > > +    int ret;
> > > +
> > > +    /* TODO set up vhost-scsi device and bind to tcm_vhost/$wwpm/tpgt_$tpgt */
> > > +    fprintf(stderr, "wwpn = \"%s\" tpgt = \"%u\"\n", id, tpgt);
> > > +
> > 
> > Please do not keep debugging fprintfs around.
> > 
> 
> Dropped
> 
> > > +    ret = vhost_dev_init(&vs->dev, -1, "/dev/vhost-scsi", false);
> > 
> > commented on this separately
> > 
> 
> ...
> 
> > > +    if (ret < 0) {
> > > +        fprintf(stderr, "vhost-scsi: vhost initialization failed: %s\n",
> > > +                strerror(-ret));
> > 
> > errors should go to monitor, here and elsewhere.
> > 
> 
> I think this means using monitor_printf() right..?
> 
> Looking at that now..


error_report is handier.

^ permalink raw reply

* Re: [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost
From: Michael S. Tsirkin @ 2012-08-18 19:12 UTC (permalink / raw)
  To: Nicholas A. Bellinger
  Cc: Anthony Liguori, Stefan Hajnoczi, kvm-devel, Jan Kiszka,
	qemu-devel, Zhi Yong Wu, Anthony Liguori, target-devel,
	Paolo Bonzini, lf-virt, Christoph Hellwig
In-Reply-To: <1344976274.22433.93.camel@haakon2.linux-iscsi.org>

On Tue, Aug 14, 2012 at 01:31:14PM -0700, Nicholas A. Bellinger wrote:
> On Mon, 2012-08-13 at 11:53 +0300, Michael S. Tsirkin wrote:
> > On Mon, Aug 13, 2012 at 08:35:14AM +0000, Nicholas A. Bellinger wrote:
> > > From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> > > 
> > > This patch adds a new type of host device that drives the vhost_scsi
> > > device.  The syntax to add vhost-scsi is:
> > > 
> > >   qemu -vhost-scsi id=vhost-scsi0,wwpn=...,tpgt=123
> > > 
> > > The virtio-scsi emulated device will make use of vhost-scsi to process
> > > virtio-scsi requests inside the kernel and hand them to the in-kernel
> > > SCSI target stack using the tcm_vhost fabric driver.
> 
> <SNIP>
> 
> > > +static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
> > > +                                 uint16_t tpgt)
> > > +{
> > > +    VHostSCSI *vs = g_malloc0(sizeof(*vs));
> > > +    int ret;
> > > +
> > > +    /* TODO set up vhost-scsi device and bind to tcm_vhost/$wwpm/tpgt_$tpgt */
> > > +    fprintf(stderr, "wwpn = \"%s\" tpgt = \"%u\"\n", id, tpgt);
> > > +
> > > +    ret = vhost_dev_init(&vs->dev, -1, "/dev/vhost-scsi", false);
> > 
> > This -1 is a hack. You need to support passing in fd from
> > the monitor, and pass it here.
> > 
> 
> Mmm, looking at how vhost_net_init + tap.c does this, but am not quite
> what fd needs to be propagated up for virtio-scsi -> vhost-scsi..
> 
> Can you please elaborate on this one a bit more..?
> 
> --nab
> 


The idea is to allow running as a user without access to
/dev/vhost-scsi.
For this, allow passing in the fd of /dev/vhost-scsi through unix domain sockets.

-- 
MST

^ permalink raw reply

* Re: [GIT PULL] tcm_vhost: Initial merge of vhost level target fabric driver
From: Michael S. Tsirkin @ 2012-08-18 20:04 UTC (permalink / raw)
  To: Nicholas A. Bellinger
  Cc: Jens Axboe, kvm-devel, linux-scsi, qemu-devel, Zhi Yong Wu,
	target-devel, Anthony Liguori, Paolo Bonzini, lf-virt,
	Christoph Hellwig, LKML
In-Reply-To: <1343697577.22538.661.camel@haakon2.linux-iscsi.org>

Hi Nicholas,
I just noticed this problem in the interface:

+#include <linux/vhost.h>
+
+/*
+ * Used by QEMU userspace to ensure a consistent vhost-scsi ABI.
+ *
+ * ABI Rev 0: July 2012 version starting point for v3.6-rc merge
candidate +
+ *            RFC-v2 vhost-scsi userspace.  Add GET_ABI_VERSION ioctl
usage
+ */
+
+#define VHOST_SCSI_ABI_VERSION 0
+
+struct vhost_scsi_target {
+       int abi_version;
+       unsigned char vhost_wwpn[TRANSPORT_IQN_LEN];
+       unsigned short vhost_tpgt;
+};
+

Here TRANSPORT_IQN_LEN is 224, which is a multiple of 4.
Since vhost_tpgt is 2 bytes and abi_version is 4, the total size would
be 230.  But gcc needs struct size be aligned to first field size, which
is 4 bytes, so it pads the structure by extra 2 bytes to the total of
232.

This padding is very undesirable in an ABI:
- it can not be initialized easily
- it can not be checked easily
- it can leak information between kernel and userspace

Simplest solution is probably just to make the padding
explicit:

+struct vhost_scsi_target {
+       int abi_version;
+       unsigned char vhost_wwpn[TRANSPORT_IQN_LEN];
+       unsigned short vhost_tpgt;
+       unsigned short reserved;
+};
+

I think we should fix this buglet before it goes out to users.

-- 
MST

^ permalink raw reply

* Re: [RFC-v2 6/6] virtio-scsi: Fix incorrect VirtIOSCSI->cmd_vqs[0] definition
From: Nicholas A. Bellinger @ 2012-08-18 21:47 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Stefan Hajnoczi, kvm-devel, Michael S. Tsirkin, Jan Kiszka,
	qemu-devel, lf-virt, Anthony Liguori, target-devel, Zhi Yong Wu,
	Christoph Hellwig
In-Reply-To: <502FE488.3000303@redhat.com>

On Sat, 2012-08-18 at 20:52 +0200, Paolo Bonzini wrote:
> Il 14/08/2012 22:20, Nicholas A. Bellinger ha scritto:
> >>> > > Since virtio_scsi currently assumes a single vqs for data, this patch
> >>> > > simply changes ->cmd_vqs[1] to handle the single VirtQueue.
> 
> Wrong, multiqueue works just fine. :)  It's just the kernel driver that
> doesn't support it yet.
> 

<nod>

> >>> > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> >>> > > Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> >>> > > Cc: Michael S. Tsirkin <mst@redhat.com>
> >>> > > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> >> > 
> >> > This is a bugfix we need even without vhost, right?
> >> > 
> > I believe so, as it appears to be stomping past the end of memory for
> > every virtio-scsi initialization regardless of vhost usage.. 
> 
> You just did a wrong merge.  When commit d2ad7dd (virtio-scsi: add
> multiqueue capability, 2012-04-06) changed cmd_vq from pointer to array
> of pointers, you should have moved the following fields to the middle of
> the struct, just like that commit did.

Ahh, I see how virtio_scsi_init() -> virtio_common_init() are setting up
the memory now..  Apologies, my mistake.

So moving the vhost-scsi related structure members ahead of the
VirtQueue releated definitions for RFC-v3, and dropping this patch.

Thanks Paolo!

--nab

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox