[PATCHv2] tools: hv: hv_set_ifconfig.sh double check before setting ip

[PATCHv2] tools: hv: hv_set_ifconfig.sh double check before setting ip

[Eduardo Otubo]

This patch fixes the behavior of the hv_set_ifconfig script when setting
the interface ip. Sometimes the interface has already been configured by
network daemon, in this case hv_set_ifconfig causes "RTNETLINK: file
exists error"; in order to avoid this error this patch makes sure double
checks the interface before trying anything.

Signed-off-by: Eduardo Otubo <otubo@redhat.com>
---
v2: wrap the interface configuration inside a safe wAY to avoid
interaction with network script.
 tools/hv/hv_set_ifconfig.sh | 45 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/tools/hv/hv_set_ifconfig.sh b/tools/hv/hv_set_ifconfig.sh
index 7ed9f85ef908..b6429703cc98 100755
--- a/tools/hv/hv_set_ifconfig.sh
+++ b/tools/hv/hv_set_ifconfig.sh
@@ -61,5 +61,46 @@ cp $1 /etc/sysconfig/network-scripts/
 
 interface=$(echo $1 | awk -F - '{ print $2 }')
 
-/sbin/ifdown $interface 2>/dev/null
-/sbin/ifup $interface 2>/dev/null
+current_ip=$(ip addr show $interface|sed -En 's/.*inet (addr:)?(([0-9*\.){3}[0-9]*).*/\2/p');
+config_file_ip=$(grep IPADDR /etc/sysconfig/network-scripts/ifcfg-$interface|cut -d"=" -f2);
+
+current_ipv6=$(ip addr show $interface|sed -E 's/^*.inet6\ (.*)\ scope\ global/\1/');
+config_file_ipv6=$(grep IPV6ADDR /etc/sysconfig/network-scripts/ifcfg-eth-$interface|cut -d"=" -f2);
+config_file_ipv6_netmask=$(grep IPV6NETMASK /etc/sysconfig/network-scripts/ifcfg-eth-$interface|cut -d"=" -f2);
+config_file_ipv6=${config_file_ipv6}/${config_file_ipv6_netmask};
+
+configure_interface(){
+    # only set the IP if the network service has not done yet
+    if [[ ${current_ip} != *${config_file_ip}* || ${current_ipv6} != ${config_file_ipv6} ]]; then
+        /sbin/ifdown $interface 2>/dev/null
+        /sbin/ifup $interface 2>/dev/null
+    fi
+}
+
+error_exit(){
+    message=$1
+    logger "KVP daemon: $message"
+    exit 1;
+}
+
+if [ -e /var/run/subsys/network ]; then
+    # network script is already up
+    # it is safe to configure the interface
+    configure_interface;
+else
+    i=0;
+    while [ ! -e /var/run/subsys/network ]; do
+        # network script might be still starting.
+        # let's wait for 3 minutes
+        sleep 1m;
+        ((i++));
+
+        # if network service doens't come up in 3 minutes
+        # perhaps there's no network service at all
+        [[ $i == 3 ]] && break;
+    done
+
+    # at this point it doesn't matter if network service is up or down
+    # we're safe either way
+    configure_interface;
+fi
-- 
2.13.6

Re: [PATCHv2] tools: hv: hv_set_ifconfig.sh double check before setting ip

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 712 bytes --]

On Fri, Dec 08, Eduardo Otubo wrote:

>  tools/hv/hv_set_ifconfig.sh | 45 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 43 insertions(+), 2 deletions(-)

> +        # let's wait for 3 minutes

Was this codepath runtime tested?

Last time this came up, the conclusion was that Windows terminates the
"KVP connection" after 60 seconds. After that, the VM must be rebooted
because the kernel does not deal with the long-running script. It is
also not clear what the kernel is supposed to do: either silently report
success and hope the scripts reports success one day, or report error
independent from what the script actually returns. New KVP requests
would need to be queued either way.


Olaf

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCHv2] tools: hv: hv_set_ifconfig.sh double check before setting ip

[Eduardo Otubo]

On Fri, Dec 08, 2017 at 12:33:38PM +0100, Olaf Hering wrote:
> On Fri, Dec 08, Eduardo Otubo wrote:
> 
> >  tools/hv/hv_set_ifconfig.sh | 45 +++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 43 insertions(+), 2 deletions(-)
> 
> > +        # let's wait for 3 minutes
> 
> Was this codepath runtime tested?
> 
> Last time this came up, the conclusion was that Windows terminates the
> "KVP connection" after 60 seconds. After that, the VM must be rebooted
> because the kernel does not deal with the long-running script. It is
> also not clear what the kernel is supposed to do: either silently report
> success and hope the scripts reports success one day, or report error
> independent from what the script actually returns. New KVP requests
> would need to be queued either way.
> 
Now that you mentioned, I guess my tests never touched this part. But I guess
we're going to fix this downstream-only setting a rule on systemd to start
hypervkvpd only after NetworkManager started. Please drop the reviews on this.

Regards,

RE: [PATCHv2] tools: hv: hv_set_ifconfig.sh double check before setting ip

[KY Srinivasan]

> -----Original Message-----
> From: Olaf Hering [mailto:olaf@aepfle.de]
> Sent: Friday, December 8, 2017 3:34 AM
> To: Eduardo Otubo <otubo@redhat.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org; Stephen
> Hemminger <sthemmin@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; KY Srinivasan <kys@microsoft.com>;
> vkuznets@redhat.com; mgamal@redhat.com; cavery@redhat.com
> Subject: Re: [PATCHv2] tools: hv: hv_set_ifconfig.sh double check before
> setting ip
> 
> On Fri, Dec 08, Eduardo Otubo wrote:
> 
> >  tools/hv/hv_set_ifconfig.sh | 45
> +++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 43 insertions(+), 2 deletions(-)
> 
> > +        # let's wait for 3 minutes
> 
> Was this codepath runtime tested?
> 
> Last time this came up, the conclusion was that Windows terminates the
> "KVP connection" after 60 seconds. After that, the VM must be rebooted
> because the kernel does not deal with the long-running script. It is
> also not clear what the kernel is supposed to do: either silently report
> success and hope the scripts reports success one day, or report error
> independent from what the script actually returns. New KVP requests
> would need to be queued either way.

Agreed; 180 seconds is more than what the host is willing to wait.

K. Y
> 
> 
> Olaf