RE: [PATCH 13/17] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO

KY Srinivasan
2012-07-31T10:34:56Z

> -----Original Message-----
> From: Ben Hutchings [mailto:ben@decadent.org.uk]
> Sent: Monday, July 30, 2012 3:19 PM
> To: KY Srinivasan
> Cc: Olaf Hering; gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; apw@canonical.com; netdev@vger.kernel.org
> Subject: Re: [PATCH 13/17] Tools: hv: Implement the KVP verb -
> KVP_OP_SET_IP_INFO
>
> On Mon, Jul 30, 2012 at 06:32:15PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Olaf Hering [mailto:olaf@aepfle.de]
> > > Sent: Monday, July 30, 2012 2:03 PM
> > > To: KY Srinivasan
> > > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > > devel@linuxdriverproject.org; apw@canonical.com; netdev@vger.kernel.org;
> > > ben@decadent.org.uk
> > > Subject: Re: [PATCH 13/17] Tools: hv: Implement the KVP verb -
> > > KVP_OP_SET_IP_INFO
> > >
> > > On Tue, Jul 24, K. Y. Srinivasan wrote:
> > >
> > > > + /*
> > > > + * 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.
> > >
> > > Maybe this should be written as 'A info file that has the requested
> > > network configuration' or something like that.
> >
> > That is the idea. This configuration file simply reflects all the
> > information we have perhaps with some additional constant
> > information. The script is free to ignore what it does not need.
> [...]
>
> This does not strike me as a sensible interface. If scripts are
> 'free to ignore' information then the KVP interface becomes unreliable
> as a means for managing networking on Linux guests. I would suggest
> that at the least the script should be able to report that it did not
> recognise some parts of the configuration. This would be logged
> and/or reported back to the hypervisor.
>
> (This is separate from the issue of constant configuration lines;
> for some distributions the script might recognise but ignore them
> because they have no use on that distribution. I don't see the
> point in constant lines, but they don't seem to result in any
> unreliability.)

Ben,

I see your point. I have cleaned up the contents of the KVP produced
configuration file to not include constant information that can be
auto generated by the distro specific script if it needs to. Also, I have
tried to make the documentation of the contents of the file a little
better. I will send out these new patches soon. Still, there is a possibility that
some of the content of this file may be redundant on a specific distro and I think
that should be fine. For instance, per Olaf's suggestion, I have included a line that
specifies the interface name in the file (in addition to the mac address). Given the
current format of the name of the config file (where the interface name is embedded
in the config file name, this additional name entry may be redundant on some distros.

Once again, thank you for taking the time to review this code.

Regards,

K. Y