From: KY Srinivasan <kys@microsoft.com>
To: Eduardo Otubo <otubo@redhat.com>
CC: Haiyang Zhang <haiyangz@microsoft.com>,
        Stephen Hemminger <stephen@networkplumber.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "devel@linuxdriverproject.org" <devel@linuxdriverproject.org>,
        Stephen Hemminger <sthemmin@microsoft.com>,
        David Miller <davem@davemloft.net>
Subject: RE: [PATCHv2] hv_set_ifconfig.sh double check before setting ip
Thread-Topic: [PATCHv2] hv_set_ifconfig.sh double check before setting ip
Thread-Index: AQHTH+Sg2sqCExznaUONoTHjUfe0jaKZ4W8AgAALRQCAAA2u0IAEKMqAgADHvEA=
Date: Thu, 31 Aug 2017 20:21:45 +0000
Message-ID: <DM5PR21MB0476D3B9A9FF0DAA06256905A09D0@DM5PR21MB0476.namprd21.prod.outlook.com>
References: <20170828100121.17321-1-otubo@redhat.com>
 <20170828081624.16a1e4de@xeon-e3>
 <MWHPR21MB0479B4F79FB796D079B65DFCCA9E0@MWHPR21MB0479.namprd21.prod.outlook.com>
 <DM5PR21MB0476B5475402DD155AE2FE20A09E0@DM5PR21MB0476.namprd21.prod.outlook.com>
 <20170831081643.GA2491@vader>
In-Reply-To: <20170831081643.GA2491@vader>
Accept-Language: en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Aug 2017 20:21:45.9077
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR21MB0699
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from base64 to 8bit by nfs id v7VKLt3n022218
Content-Length: 3679
Lines: 90


> -----Original Message-----
> From: Eduardo Otubo [mailto:otubo@redhat.com]
> Sent: Thursday, August 31, 2017 1:17 AM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: Haiyang Zhang <haiyangz@microsoft.com>; Stephen Hemminger
> <stephen@networkplumber.org>; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; Stephen Hemminger
> <sthemmin@microsoft.com>; David Miller <davem@davemloft.net>
> Subject: Re: [PATCHv2] hv_set_ifconfig.sh double check before setting ip
> 
> On Mon, Aug 28, 2017 at 04:48:32PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Haiyang Zhang
> > > Sent: Monday, August 28, 2017 8:57 AM
> > > To: Stephen Hemminger <stephen@networkplumber.org>; Eduardo
> Otubo
> > > <otubo@redhat.com>; KY Srinivasan <kys@microsoft.com>
> > > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org; Stephen
> > > Hemminger <sthemmin@microsoft.com>; David Miller
> > > <davem@davemloft.net>
> > > Subject: RE: [PATCHv2] hv_set_ifconfig.sh double check before setting ip
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > > > Sent: Monday, August 28, 2017 11:16 AM
> > > > To: Eduardo Otubo <otubo@redhat.com>
> > > > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> Haiyang
> > > > Zhang <haiyangz@microsoft.com>; Stephen Hemminger
> > > > <sthemmin@microsoft.com>; David Miller <davem@davemloft.net>
> > > > Subject: Re: [PATCHv2] hv_set_ifconfig.sh double check before setting
> ip
> > > >
> > > > On Mon, 28 Aug 2017 12:01:21 +0200
> > > > Eduardo Otubo <otubo@redhat.com> wrote:
> > > >
> > > > > v2: The script is now a little bit safer so it doesn't conflicts with
> > > > > network daemon trying to set configurations at the same time.
> > > > >
> > > > > 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>
> > > >
> > > > Adding new dependency on systemd is not going to make this script
> > > > even less useful.  I wonder why the script still exists at all? Most of
> > > > the
> > > > Linux distro's can already setup HV networking without it.
> > > >
> > >
> > > This script is used by a host to inject IP into guests. KY knows more
> > > details about it.
> >
> > I wrote this script initially to provide an example script for Distros to base
> their solution on.
> > KVP supports IP injection to enable VM migration. For this scenario, I think
> we recommend that NM be
> > disabled.
> >
> 
> So, what you're saying is that this should be fixed downstream,
> instead? This solution seems pretty safe for me and long term we can
> think about something else that could get rid of this script. So NM or
> whatever is in use can actually do the configuration.
> 
> Any chance to have this patch ACK'd as a form of a short term
> solution?

Given that this is just an example script to be customized on a per Distro basis, do
you still see value in making this change upstream. In any case, as Olaf noted the delay will
cause problems. One option would be to simply fail if network scripts are running. As you say, if you
are making sure that the KVP daemon is only run after the network script have executed, this restriction
(failing if the scripts are active) should not be an issue.

Regards,

K. Y