[PATCH 1/2] tools: hv: Fix how ifcfg-* file is created

[PATCH 1/2] tools: hv: Fix how ifcfg-* file is created

[Tomas Hozza]

Fix for the daemon code and for hv_set_ifconfig.sh script, so
that the created ifcfg-* file is consistent with initscripts
documentation.

Signed-off-by: Tomas Hozza <thozza@redhat.com>
---
 tools/hv/hv_kvp_daemon.c    | 73 ++++++++++++++++++++++-----------------------
 tools/hv/hv_set_ifconfig.sh | 22 ++++++--------
 2 files changed, 44 insertions(+), 51 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index d25a469..6b56b75 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -1162,16 +1162,13 @@ static int process_ip_string(FILE *f, char *ip_string, int type)
 				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) {
+			
+			if (type == DNS) {
 				snprintf(sub_str, sizeof(sub_str), "%d", ++i);
+			} else if (type == GATEWAY && i == 0) {
+				++i;
+			} else {
+				snprintf(sub_str, sizeof(sub_str), "%d", i++);
 			}
 
 
@@ -1191,17 +1188,13 @@ static int process_ip_string(FILE *f, char *ip_string, int type)
 				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);
+			
+			if (type == DNS) {
+				snprintf(sub_str, sizeof(sub_str), "%d", ++i);
+			} else if (j == 0) {
+				++j;
+			} else {
+				snprintf(sub_str, sizeof(sub_str), "_%d", j++);
 			}
 		} else {
 			return  HV_INVALIDARG;
@@ -1244,18 +1237,19 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
 	 * 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)
+	 * DEVICE=interface name
+	 * BOOTPROTO=<protocol> (where <protocol> is "dhcp" if DHCP is configured
+	 *                       or "none" if no boot-time protocol should be used)
 	 *
-	 * IPADDR=ipaddr1
-	 * IPADDR_1=ipaddr2
-	 * IPADDR_x=ipaddry (where y = x + 1)
+	 * IPADDR0=ipaddr1
+	 * IPADDR1=ipaddr2
+	 * IPADDRx=ipaddry (where y = x + 1)
 	 *
-	 * NETMASK=netmask1
-	 * NETMASK_x=netmasky (where y = x + 1)
+	 * NETMASK0=netmask1
+	 * NETMASKx=netmasky (where y = x + 1)
 	 *
 	 * GATEWAY=ipaddr1
-	 * GATEWAY_x=ipaddry (where y = x + 1)
+	 * GATEWAYx=ipaddry (where y = x + 1)
 	 *
 	 * DNSx=ipaddrx (where first DNS address is tagged as DNS1 etc)
 	 *
@@ -1294,20 +1288,23 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
 	if (error)
 		goto setval_error;
 
-	error = kvp_write_file(file, "IF_NAME", "", if_name);
+	error = kvp_write_file(file, "DEVICE", "", if_name);
 	if (error)
 		goto setval_error;
 
-	if (new_val->dhcp_enabled) {
-		error = kvp_write_file(file, "DHCP", "", "yes");
-		if (error)
-			goto setval_error;
+	if (new_val->dhcp_enabled)
+		error = kvp_write_file(file, "BOOTPROTO", "", "dhcp");
+	else
+		error = kvp_write_file(file, "BOOTPROTO", "", "none");
+        
+	if (error)
+		goto setval_error;
+
+	/*
+	 * We are done!.
+	 */
+	goto setval_done;
 
-		/*
-		 * We are done!.
-		 */
-		goto setval_done;
-	}
 
 	/*
 	 * Write the configuration for ipaddress, netmask, gateway and
diff --git a/tools/hv/hv_set_ifconfig.sh b/tools/hv/hv_set_ifconfig.sh
index 3e9427e..00b66be 100755
--- a/tools/hv/hv_set_ifconfig.sh
+++ b/tools/hv/hv_set_ifconfig.sh
@@ -20,18 +20,19 @@
 # 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)
+# DEVICE=interface name
+# BOOTPROTO=<protocol> (where <protocol> is "dhcp" if DHCP is configured
+#                       or "none" if no boot-time protocol should be used)
 #
-# IPADDR=ipaddr1
-# IPADDR_1=ipaddr2
-# IPADDR_x=ipaddry (where y = x + 1)
+# IPADDR0=ipaddr1
+# IPADDR1=ipaddr2
+# IPADDRx=ipaddry (where y = x + 1)
 #
-# NETMASK=netmask1
-# NETMASK_x=netmasky (where y = x + 1)
+# NETMASK0=netmask1
+# NETMASKx=netmasky (where y = x + 1)
 #
 # GATEWAY=ipaddr1
-# GATEWAY_x=ipaddry (where y = x + 1)
+# GATEWAYx=ipaddry (where y = x + 1)
 #
 # DNSx=ipaddrx (where first DNS address is tagged as DNS1 etc)
 #
@@ -53,11 +54,6 @@ 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/
 
-- 
1.7.11.7

[PATCH 2/2] tools: hv: Use CLOEXEC when opening kvp_pool files

[Tomas Hozza]

Use CLOEXEC flag when opening kvp_pool_x files to prevent file
descriptor leakage. Not using it was causing a problem when
SELinux was enabled.

Signed-off-by: Tomas Hozza <thozza@redhat.com>
---
 tools/hv/hv_kvp_daemon.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 6b56b75..31f839cc 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -151,7 +151,7 @@ static void kvp_update_file(int pool)
 	 */
 	kvp_acquire_lock(pool);
 
-	filep = fopen(kvp_file_info[pool].fname, "w");
+	filep = fopen(kvp_file_info[pool].fname, "we");
 	if (!filep) {
 		kvp_release_lock(pool);
 		syslog(LOG_ERR, "Failed to open file, pool: %d", pool);
@@ -182,7 +182,7 @@ static void kvp_update_mem_state(int pool)
 
 	kvp_acquire_lock(pool);
 
-	filep = fopen(kvp_file_info[pool].fname, "r");
+	filep = fopen(kvp_file_info[pool].fname, "re");
 	if (!filep) {
 		kvp_release_lock(pool);
 		syslog(LOG_ERR, "Failed to open file, pool: %d", pool);
@@ -246,13 +246,13 @@ static int kvp_file_init(void)
 		records_read = 0;
 		num_blocks = 1;
 		sprintf(fname, "/var/opt/hyperv/.kvp_pool_%d", i);
-		fd = open(fname, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR | S_IROTH);
+		fd = open(fname, O_RDWR | O_CREAT | O_CLOEXEC, S_IRUSR | S_IWUSR | S_IROTH);
 
 		if (fd == -1)
 			return 1;
 
 
-		filep = fopen(fname, "r");
+		filep = fopen(fname, "re");
 		if (!filep)
 			return 1;
 
-- 
1.7.11.7

RE: [PATCH 2/2] tools: hv: Use CLOEXEC when opening kvp_pool files

[KY Srinivasan]

> -----Original Message-----
> From: Tomas Hozza [mailto:thozza@redhat.com]
> Sent: Tuesday, January 08, 2013 6:27 AM
> To: gregkh@linuxfoundation.org
> Cc: KY Srinivasan; jasowang@redhat.com; Haiyang Zhang; linux-
> kernel@vger.kernel.org; Hashir Abdi; Tomas Hozza
> Subject: [PATCH 2/2] tools: hv: Use CLOEXEC when opening kvp_pool files
> 
> Use CLOEXEC flag when opening kvp_pool_x files to prevent file
> descriptor leakage. Not using it was causing a problem when
> SELinux was enabled.
> 
> Signed-off-by: Tomas Hozza <thozza@redhat.com>
Acked-by: K. Y. Srinivasan <kys@microsoft.com

> ---
>  tools/hv/hv_kvp_daemon.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index 6b56b75..31f839cc 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -151,7 +151,7 @@ static void kvp_update_file(int pool)
>  	 */
>  	kvp_acquire_lock(pool);
> 
> -	filep = fopen(kvp_file_info[pool].fname, "w");
> +	filep = fopen(kvp_file_info[pool].fname, "we");
>  	if (!filep) {
>  		kvp_release_lock(pool);
>  		syslog(LOG_ERR, "Failed to open file, pool: %d", pool);
> @@ -182,7 +182,7 @@ static void kvp_update_mem_state(int pool)
> 
>  	kvp_acquire_lock(pool);
> 
> -	filep = fopen(kvp_file_info[pool].fname, "r");
> +	filep = fopen(kvp_file_info[pool].fname, "re");
>  	if (!filep) {
>  		kvp_release_lock(pool);
>  		syslog(LOG_ERR, "Failed to open file, pool: %d", pool);
> @@ -246,13 +246,13 @@ static int kvp_file_init(void)
>  		records_read = 0;
>  		num_blocks = 1;
>  		sprintf(fname, "/var/opt/hyperv/.kvp_pool_%d", i);
> -		fd = open(fname, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR |
> S_IROTH);
> +		fd = open(fname, O_RDWR | O_CREAT | O_CLOEXEC, S_IRUSR |
> S_IWUSR | S_IROTH);
> 
>  		if (fd == -1)
>  			return 1;
> 
> 
> -		filep = fopen(fname, "r");
> +		filep = fopen(fname, "re");
>  		if (!filep)
>  			return 1;
> 
> --
> 1.7.11.7
> 
> 

Re: [PATCH 2/2] tools: hv: Use CLOEXEC when opening kvp_pool files

[Greg KH]

On Tue, Jan 08, 2013 at 12:26:43PM +0100, Tomas Hozza wrote:
> Use CLOEXEC flag when opening kvp_pool_x files to prevent file
> descriptor leakage. Not using it was causing a problem when
> SELinux was enabled.
> 
> Signed-off-by: Tomas Hozza <thozza@redhat.com>
> Acked-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  tools/hv/hv_kvp_daemon.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

This patch no longer applies, care to redo it and resend?

thanks,

greg k-h

[PATCH] tools: hv: Use CLOEXEC when opening kvp_pool files

[Tomas Hozza]

Use CLOEXEC flag when opening kvp_pool_x files to prevent file
descriptor leakage. Not using it was causing a problem when
SELinux was enabled.

Signed-off-by: Tomas Hozza <thozza@redhat.com>
---
 tools/hv/hv_kvp_daemon.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 3840517..c800ea4 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -151,7 +151,7 @@ static void kvp_update_file(int pool)
 	 */
 	kvp_acquire_lock(pool);
 
-	filep = fopen(kvp_file_info[pool].fname, "w");
+	filep = fopen(kvp_file_info[pool].fname, "we");
 	if (!filep) {
 		kvp_release_lock(pool);
 		syslog(LOG_ERR, "Failed to open file, pool: %d", pool);
@@ -182,7 +182,7 @@ static void kvp_update_mem_state(int pool)
 
 	kvp_acquire_lock(pool);
 
-	filep = fopen(kvp_file_info[pool].fname, "r");
+	filep = fopen(kvp_file_info[pool].fname, "re");
 	if (!filep) {
 		kvp_release_lock(pool);
 		syslog(LOG_ERR, "Failed to open file, pool: %d", pool);
@@ -246,13 +246,13 @@ static int kvp_file_init(void)
 		records_read = 0;
 		num_blocks = 1;
 		sprintf(fname, "%s/.kvp_pool_%d", KVP_CONFIG_LOC, i);
-		fd = open(fname, O_RDWR | O_CREAT, 0644 /* rw-r--r-- */);
+		fd = open(fname, O_RDWR | O_CREAT | O_CLOEXEC, 0644 /* rw-r--r-- */);
 
 		if (fd == -1)
 			return 1;
 
 
-		filep = fopen(fname, "r");
+		filep = fopen(fname, "re");
 		if (!filep)
 			return 1;
 
-- 
1.7.11.7

RE: [PATCH] tools: hv: Use CLOEXEC when opening kvp_pool files

[KY Srinivasan]

> -----Original Message-----
> From: Tomas Hozza [mailto:thozza@redhat.com]
> Sent: Friday, January 18, 2013 9:24 AM
> To: gregkh@linuxfoundation.org
> Cc: KY Srinivasan; jasowang@redhat.com; Haiyang Zhang; linux-
> kernel@vger.kernel.org; Hashir Abdi; Tomas Hozza
> Subject: [PATCH] tools: hv: Use CLOEXEC when opening kvp_pool files
> 
> Use CLOEXEC flag when opening kvp_pool_x files to prevent file
> descriptor leakage. Not using it was causing a problem when
> SELinux was enabled.
> 
> Signed-off-by: Tomas Hozza <thozza@redhat.com>
Acked-by:  K. Y. Srinivasan <kys@microsoft.com>

> ---
>  tools/hv/hv_kvp_daemon.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index 3840517..c800ea4 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -151,7 +151,7 @@ static void kvp_update_file(int pool)
>  	 */
>  	kvp_acquire_lock(pool);
> 
> -	filep = fopen(kvp_file_info[pool].fname, "w");
> +	filep = fopen(kvp_file_info[pool].fname, "we");
>  	if (!filep) {
>  		kvp_release_lock(pool);
>  		syslog(LOG_ERR, "Failed to open file, pool: %d", pool);
> @@ -182,7 +182,7 @@ static void kvp_update_mem_state(int pool)
> 
>  	kvp_acquire_lock(pool);
> 
> -	filep = fopen(kvp_file_info[pool].fname, "r");
> +	filep = fopen(kvp_file_info[pool].fname, "re");
>  	if (!filep) {
>  		kvp_release_lock(pool);
>  		syslog(LOG_ERR, "Failed to open file, pool: %d", pool);
> @@ -246,13 +246,13 @@ static int kvp_file_init(void)
>  		records_read = 0;
>  		num_blocks = 1;
>  		sprintf(fname, "%s/.kvp_pool_%d", KVP_CONFIG_LOC, i);
> -		fd = open(fname, O_RDWR | O_CREAT, 0644 /* rw-r--r-- */);
> +		fd = open(fname, O_RDWR | O_CREAT | O_CLOEXEC, 0644 /* rw-
> r--r-- */);
> 
>  		if (fd == -1)
>  			return 1;
> 
> 
> -		filep = fopen(fname, "r");
> +		filep = fopen(fname, "re");
>  		if (!filep)
>  			return 1;
> 
> --
> 1.7.11.7
> 
> 

RE: [PATCH 1/2] tools: hv: Fix how ifcfg-* file is created

[KY Srinivasan]

> -----Original Message-----
> From: Tomas Hozza [mailto:thozza@redhat.com]
> Sent: Tuesday, January 08, 2013 6:27 AM
> To: gregkh@linuxfoundation.org
> Cc: KY Srinivasan; jasowang@redhat.com; Haiyang Zhang; linux-
> kernel@vger.kernel.org; Hashir Abdi; Tomas Hozza
> Subject: [PATCH 1/2] tools: hv: Fix how ifcfg-* file is created
> 
> Fix for the daemon code and for hv_set_ifconfig.sh script, so
> that the created ifcfg-* file is consistent with initscripts
> documentation.
> 
> Signed-off-by: Tomas Hozza <thozza@redhat.com>
Acked-by: K. Y. Srinivasan <kys@microsoft.com>

> ---
>  tools/hv/hv_kvp_daemon.c    | 73 ++++++++++++++++++++++---------------------
> --
>  tools/hv/hv_set_ifconfig.sh | 22 ++++++--------
>  2 files changed, 44 insertions(+), 51 deletions(-)
> 
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index d25a469..6b56b75 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -1162,16 +1162,13 @@ static int process_ip_string(FILE *f, char *ip_string, int
> type)
>  				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) {
> +
> +			if (type == DNS) {
>  				snprintf(sub_str, sizeof(sub_str), "%d", ++i);
> +			} else if (type == GATEWAY && i == 0) {
> +				++i;
> +			} else {
> +				snprintf(sub_str, sizeof(sub_str), "%d", i++);
>  			}
> 
> 
> @@ -1191,17 +1188,13 @@ static int process_ip_string(FILE *f, char *ip_string, int
> type)
>  				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);
> +
> +			if (type == DNS) {
> +				snprintf(sub_str, sizeof(sub_str), "%d", ++i);
> +			} else if (j == 0) {
> +				++j;
> +			} else {
> +				snprintf(sub_str, sizeof(sub_str), "_%d", j++);
>  			}
>  		} else {
>  			return  HV_INVALIDARG;
> @@ -1244,18 +1237,19 @@ static int kvp_set_ip_info(char *if_name, struct
> hv_kvp_ipaddr_value *new_val)
>  	 * 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)
> +	 * DEVICE=interface name
> +	 * BOOTPROTO=<protocol> (where <protocol> is "dhcp" if DHCP is
> configured
> +	 *                       or "none" if no boot-time protocol should be used)
>  	 *
> -	 * IPADDR=ipaddr1
> -	 * IPADDR_1=ipaddr2
> -	 * IPADDR_x=ipaddry (where y = x + 1)
> +	 * IPADDR0=ipaddr1
> +	 * IPADDR1=ipaddr2
> +	 * IPADDRx=ipaddry (where y = x + 1)
>  	 *
> -	 * NETMASK=netmask1
> -	 * NETMASK_x=netmasky (where y = x + 1)
> +	 * NETMASK0=netmask1
> +	 * NETMASKx=netmasky (where y = x + 1)
>  	 *
>  	 * GATEWAY=ipaddr1
> -	 * GATEWAY_x=ipaddry (where y = x + 1)
> +	 * GATEWAYx=ipaddry (where y = x + 1)
>  	 *
>  	 * DNSx=ipaddrx (where first DNS address is tagged as DNS1 etc)
>  	 *
> @@ -1294,20 +1288,23 @@ static int kvp_set_ip_info(char *if_name, struct
> hv_kvp_ipaddr_value *new_val)
>  	if (error)
>  		goto setval_error;
> 
> -	error = kvp_write_file(file, "IF_NAME", "", if_name);
> +	error = kvp_write_file(file, "DEVICE", "", if_name);
>  	if (error)
>  		goto setval_error;
> 
> -	if (new_val->dhcp_enabled) {
> -		error = kvp_write_file(file, "DHCP", "", "yes");
> -		if (error)
> -			goto setval_error;
> +	if (new_val->dhcp_enabled)
> +		error = kvp_write_file(file, "BOOTPROTO", "", "dhcp");
> +	else
> +		error = kvp_write_file(file, "BOOTPROTO", "", "none");
> +
> +	if (error)
> +		goto setval_error;
> +
> +	/*
> +	 * We are done!.
> +	 */
> +	goto setval_done;
> 
> -		/*
> -		 * We are done!.
> -		 */
> -		goto setval_done;
> -	}
> 
>  	/*
>  	 * Write the configuration for ipaddress, netmask, gateway and
> diff --git a/tools/hv/hv_set_ifconfig.sh b/tools/hv/hv_set_ifconfig.sh
> index 3e9427e..00b66be 100755
> --- a/tools/hv/hv_set_ifconfig.sh
> +++ b/tools/hv/hv_set_ifconfig.sh
> @@ -20,18 +20,19 @@
>  # 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)
> +# DEVICE=interface name
> +# BOOTPROTO=<protocol> (where <protocol> is "dhcp" if DHCP is configured
> +#                       or "none" if no boot-time protocol should be used)
>  #
> -# IPADDR=ipaddr1
> -# IPADDR_1=ipaddr2
> -# IPADDR_x=ipaddry (where y = x + 1)
> +# IPADDR0=ipaddr1
> +# IPADDR1=ipaddr2
> +# IPADDRx=ipaddry (where y = x + 1)
>  #
> -# NETMASK=netmask1
> -# NETMASK_x=netmasky (where y = x + 1)
> +# NETMASK0=netmask1
> +# NETMASKx=netmasky (where y = x + 1)
>  #
>  # GATEWAY=ipaddr1
> -# GATEWAY_x=ipaddry (where y = x + 1)
> +# GATEWAYx=ipaddry (where y = x + 1)
>  #
>  # DNSx=ipaddrx (where first DNS address is tagged as DNS1 etc)
>  #
> @@ -53,11 +54,6 @@ 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/
> 
> --
> 1.7.11.7
> 
> 

Re: [PATCH 1/2] tools: hv: Fix how ifcfg-* file is created

[Tomas Hozza]

There is a mistake in this Patch. Sorry for this.
I'll send corrected one ASAP after testing it!

----- Original Message -----
> 
> 
> > -----Original Message-----
> > From: Tomas Hozza [mailto:thozza@redhat.com]
> > Sent: Tuesday, January 08, 2013 6:27 AM
> > To: gregkh@linuxfoundation.org
> > Cc: KY Srinivasan; jasowang@redhat.com; Haiyang Zhang; linux-
> > kernel@vger.kernel.org; Hashir Abdi; Tomas Hozza
> > Subject: [PATCH 1/2] tools: hv: Fix how ifcfg-* file is created
> > 
> > Fix for the daemon code and for hv_set_ifconfig.sh script, so
> > that the created ifcfg-* file is consistent with initscripts
> > documentation.
> > 
> > Signed-off-by: Tomas Hozza <thozza@redhat.com>
> Acked-by: K. Y. Srinivasan <kys@microsoft.com>
> 
> > ---
> >  tools/hv/hv_kvp_daemon.c    | 73
> >  ++++++++++++++++++++++---------------------
> > --
> >  tools/hv/hv_set_ifconfig.sh | 22 ++++++--------
> >  2 files changed, 44 insertions(+), 51 deletions(-)
> > 
> > diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> > index d25a469..6b56b75 100644
> > --- a/tools/hv/hv_kvp_daemon.c
> > +++ b/tools/hv/hv_kvp_daemon.c
> > @@ -1162,16 +1162,13 @@ static int process_ip_string(FILE *f, char
> > *ip_string, int
> > type)
> >  				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) {
> > +
> > +			if (type == DNS) {
> >  				snprintf(sub_str, sizeof(sub_str), "%d", ++i);
> > +			} else if (type == GATEWAY && i == 0) {
> > +				++i;
> > +			} else {
> > +				snprintf(sub_str, sizeof(sub_str), "%d", i++);
> >  			}
> > 
> > 
> > @@ -1191,17 +1188,13 @@ static int process_ip_string(FILE *f, char
> > *ip_string, int
> > type)
> >  				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);
> > +
> > +			if (type == DNS) {
> > +				snprintf(sub_str, sizeof(sub_str), "%d", ++i);
> > +			} else if (j == 0) {
> > +				++j;
> > +			} else {
> > +				snprintf(sub_str, sizeof(sub_str), "_%d", j++);
> >  			}
> >  		} else {
> >  			return  HV_INVALIDARG;
> > @@ -1244,18 +1237,19 @@ static int kvp_set_ip_info(char *if_name,
> > struct
> > hv_kvp_ipaddr_value *new_val)
> >  	 * 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)
> > +	 * DEVICE=interface name
> > +	 * BOOTPROTO=<protocol> (where <protocol> is "dhcp" if DHCP is
> > configured
> > +	 *                       or "none" if no boot-time protocol
> > should be used)
> >  	 *
> > -	 * IPADDR=ipaddr1
> > -	 * IPADDR_1=ipaddr2
> > -	 * IPADDR_x=ipaddry (where y = x + 1)
> > +	 * IPADDR0=ipaddr1
> > +	 * IPADDR1=ipaddr2
> > +	 * IPADDRx=ipaddry (where y = x + 1)
> >  	 *
> > -	 * NETMASK=netmask1
> > -	 * NETMASK_x=netmasky (where y = x + 1)
> > +	 * NETMASK0=netmask1
> > +	 * NETMASKx=netmasky (where y = x + 1)
> >  	 *
> >  	 * GATEWAY=ipaddr1
> > -	 * GATEWAY_x=ipaddry (where y = x + 1)
> > +	 * GATEWAYx=ipaddry (where y = x + 1)
> >  	 *
> >  	 * DNSx=ipaddrx (where first DNS address is tagged as DNS1 etc)
> >  	 *
> > @@ -1294,20 +1288,23 @@ static int kvp_set_ip_info(char *if_name,
> > struct
> > hv_kvp_ipaddr_value *new_val)
> >  	if (error)
> >  		goto setval_error;
> > 
> > -	error = kvp_write_file(file, "IF_NAME", "", if_name);
> > +	error = kvp_write_file(file, "DEVICE", "", if_name);
> >  	if (error)
> >  		goto setval_error;
> > 
> > -	if (new_val->dhcp_enabled) {
> > -		error = kvp_write_file(file, "DHCP", "", "yes");
> > -		if (error)
> > -			goto setval_error;
> > +	if (new_val->dhcp_enabled)
> > +		error = kvp_write_file(file, "BOOTPROTO", "", "dhcp");
> > +	else
> > +		error = kvp_write_file(file, "BOOTPROTO", "", "none");
> > +
> > +	if (error)
> > +		goto setval_error;
> > +
> > +	/*
> > +	 * We are done!.
> > +	 */
> > +	goto setval_done;

This part is the problem. We should goto setval_done only
if we are using DHCP. Therefore now the IPADDR, NETMASK and
so on will not be in the generated ifcfg-* if not using DHCP!

> > 
> > -		/*
> > -		 * We are done!.
> > -		 */
> > -		goto setval_done;
> > -	}
> > 
> >  	/*
> >  	 * Write the configuration for ipaddress, netmask, gateway and
> > diff --git a/tools/hv/hv_set_ifconfig.sh
> > b/tools/hv/hv_set_ifconfig.sh
> > index 3e9427e..00b66be 100755
> > --- a/tools/hv/hv_set_ifconfig.sh
> > +++ b/tools/hv/hv_set_ifconfig.sh
> > @@ -20,18 +20,19 @@
> >  # 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)
> > +# DEVICE=interface name
> > +# BOOTPROTO=<protocol> (where <protocol> is "dhcp" if DHCP is
> > configured
> > +#                       or "none" if no boot-time protocol should
> > be used)
> >  #
> > -# IPADDR=ipaddr1
> > -# IPADDR_1=ipaddr2
> > -# IPADDR_x=ipaddry (where y = x + 1)
> > +# IPADDR0=ipaddr1
> > +# IPADDR1=ipaddr2
> > +# IPADDRx=ipaddry (where y = x + 1)
> >  #
> > -# NETMASK=netmask1
> > -# NETMASK_x=netmasky (where y = x + 1)
> > +# NETMASK0=netmask1
> > +# NETMASKx=netmasky (where y = x + 1)
> >  #
> >  # GATEWAY=ipaddr1
> > -# GATEWAY_x=ipaddry (where y = x + 1)
> > +# GATEWAYx=ipaddry (where y = x + 1)
> >  #
> >  # DNSx=ipaddrx (where first DNS address is tagged as DNS1 etc)
> >  #
> > @@ -53,11 +54,6 @@ 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/
> > 
> > --
> > 1.7.11.7
> > 
> > 
> 
> 
> 

[PATCH] tools: hv: Fix how ifcfg-* file is created

[Tomas Hozza]

Fix for the daemon code and for hv_set_ifconfig.sh script, so
that the created ifcfg-* file is consistent with initscripts
documentation.

Signed-off-by: Tomas Hozza <thozza@redhat.com>
---
 tools/hv/hv_kvp_daemon.c    | 59 ++++++++++++++++++++++-----------------------
 tools/hv/hv_set_ifconfig.sh | 22 +++++++----------
 2 files changed, 38 insertions(+), 43 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index d25a469..e6fe8d5 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -1162,16 +1162,13 @@ static int process_ip_string(FILE *f, char *ip_string, int type)
 				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) {
+
+			if (type == DNS) {
 				snprintf(sub_str, sizeof(sub_str), "%d", ++i);
+			} else if (type == GATEWAY && i == 0) {
+				++i;
+			} else {
+				snprintf(sub_str, sizeof(sub_str), "%d", i++);
 			}
 
 
@@ -1191,17 +1188,13 @@ static int process_ip_string(FILE *f, char *ip_string, int type)
 				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);
+
+			if (type == DNS) {
+				snprintf(sub_str, sizeof(sub_str), "%d", ++i);
+			} else if (j == 0) {
+				++j;
+			} else {
+				snprintf(sub_str, sizeof(sub_str), "_%d", j++);
 			}
 		} else {
 			return  HV_INVALIDARG;
@@ -1244,18 +1237,19 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
 	 * 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)
+	 * DEVICE=interface name
+	 * BOOTPROTO=<protocol> (where <protocol> is "dhcp" if DHCP is configured
+	 *                       or "none" if no boot-time protocol should be used)
 	 *
-	 * IPADDR=ipaddr1
-	 * IPADDR_1=ipaddr2
-	 * IPADDR_x=ipaddry (where y = x + 1)
+	 * IPADDR0=ipaddr1
+	 * IPADDR1=ipaddr2
+	 * IPADDRx=ipaddry (where y = x + 1)
 	 *
-	 * NETMASK=netmask1
-	 * NETMASK_x=netmasky (where y = x + 1)
+	 * NETMASK0=netmask1
+	 * NETMASKx=netmasky (where y = x + 1)
 	 *
 	 * GATEWAY=ipaddr1
-	 * GATEWAY_x=ipaddry (where y = x + 1)
+	 * GATEWAYx=ipaddry (where y = x + 1)
 	 *
 	 * DNSx=ipaddrx (where first DNS address is tagged as DNS1 etc)
 	 *
@@ -1294,12 +1288,12 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
 	if (error)
 		goto setval_error;
 
-	error = kvp_write_file(file, "IF_NAME", "", if_name);
+	error = kvp_write_file(file, "DEVICE", "", if_name);
 	if (error)
 		goto setval_error;
 
 	if (new_val->dhcp_enabled) {
-		error = kvp_write_file(file, "DHCP", "", "yes");
+		error = kvp_write_file(file, "BOOTPROTO", "", "dhcp");
 		if (error)
 			goto setval_error;
 
@@ -1307,6 +1301,11 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
 		 * We are done!.
 		 */
 		goto setval_done;
+
+	} else {
+		error = kvp_write_file(file, "BOOTPROTO", "", "none");
+		if (error)
+			goto setval_error;
 	}
 
 	/*
diff --git a/tools/hv/hv_set_ifconfig.sh b/tools/hv/hv_set_ifconfig.sh
index 3e9427e..00b66be 100755
--- a/tools/hv/hv_set_ifconfig.sh
+++ b/tools/hv/hv_set_ifconfig.sh
@@ -20,18 +20,19 @@
 # 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)
+# DEVICE=interface name
+# BOOTPROTO=<protocol> (where <protocol> is "dhcp" if DHCP is configured
+#                       or "none" if no boot-time protocol should be used)
 #
-# IPADDR=ipaddr1
-# IPADDR_1=ipaddr2
-# IPADDR_x=ipaddry (where y = x + 1)
+# IPADDR0=ipaddr1
+# IPADDR1=ipaddr2
+# IPADDRx=ipaddry (where y = x + 1)
 #
-# NETMASK=netmask1
-# NETMASK_x=netmasky (where y = x + 1)
+# NETMASK0=netmask1
+# NETMASKx=netmasky (where y = x + 1)
 #
 # GATEWAY=ipaddr1
-# GATEWAY_x=ipaddry (where y = x + 1)
+# GATEWAYx=ipaddry (where y = x + 1)
 #
 # DNSx=ipaddrx (where first DNS address is tagged as DNS1 etc)
 #
@@ -53,11 +54,6 @@ 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/
 
-- 
1.7.11.7

Re: [PATCH] tools: hv: Fix how ifcfg-* file is created

[Olaf Hering]

On Sun, Jan 13, Tomas Hozza wrote:

> -# IPADDR=ipaddr1
> -# IPADDR_1=ipaddr2
> -# IPADDR_x=ipaddry (where y = x + 1)
> +# IPADDR0=ipaddr1
> +# IPADDR1=ipaddr2
> +# IPADDRx=ipaddry (where y = x + 1)

Before this change it was IPADDR=, now its IPADDR0=.
Furthermore, IPADDR_n was changed to IPADDRn.
Does that match what the tools consuming the ifcfg-* files expect?
Why did it work before this change?


Olaf

Re: [PATCH] tools: hv: Fix how ifcfg-* file is created

[Tomas Hozza]

----- Original Message -----
> On Sun, Jan 13, Tomas Hozza wrote:
> 
> > -# IPADDR=ipaddr1
> > -# IPADDR_1=ipaddr2
> > -# IPADDR_x=ipaddry (where y = x + 1)
> > +# IPADDR0=ipaddr1
> > +# IPADDR1=ipaddr2
> > +# IPADDRx=ipaddry (where y = x + 1)
> 
> Before this change it was IPADDR=, now its IPADDR0=.
> Furthermore, IPADDR_n was changed to IPADDRn.

>From initscripts (ifcfg-* part) documentation:

Base items:
    NAME=
      Most important for PPP.  Only used in front ends.
    DEVICE=PPP
      devices where it is the "logical name")>
    IPADDRn=
    PREFIXn=
      Network prefix.  It is used for all configurations except aliases
      and ippp devices.  It takes precedence over NETMASK when both
      PREFIX and NETMASK are set.
    NETMASKn=
      Subnet mask; just useful for aliases and ippp devices.  For all other
      configurations, use PREFIX instead.

    The "n" is expected to be consecutive positive integers starting from 0.
    It can be omitted if there is only one address being configured.


So I think this explains a lot. In hyperv KVP daemon source there is no logic
to determine if we are going to set more than SINGLE IP address to the interface.
Therefore we have to set the first one as IPADDR0. This is completely OK and
in compliance with the documentation.

> Does that match what the tools consuming the ifcfg-* files expect?

Since the current format looks the same as described in documentation
I assume tools consuming ifcfg-* files expect exactly this. I also checked
scripts handling those ifcfg-* files and they did not expect it to be
IPADDR_n for some "n". We also tested the daemon and it worked just fine.

> Why did it work before this change?

For single IPADDR this should work just fine and it is expected to. But did you
try also to set more than just a single IP address to the interface?

Regards,
Tomas Hozza