[ovs-dev] [PATCH] openvswitch init script modifications for kvm and lxc hosts

Patrick Mullaney pm.mullaney at gmail.com
Thu Dec 16 18:44:10 UTC 2010


Ben,

I reviewed the series. Looks great. Great way to break it up, I
should have really done it that way.

Thanks.
Patrick

On Wed, 2010-12-15 at 11:16 -0800, Ben Pfaff wrote:
> On Tue, Dec 14, 2010 at 04:25:33PM -0500, Patrick Mullaney wrote:
> > On Tue, 2010-12-14 at 11:42 -0800, Ben Pfaff wrote:
> > > On Mon, Dec 13, 2010 at 05:49:46PM -0500, Patrick Mullaney wrote:
> > > > Signed-off-by: Patrick Mullaney <pm.mullaney at gmail.com>
> > > 
> > > Thank you for the patch.  Most of this looks pretty good.  I have some
> > > questions and comments.
> > > 
> > > The /etc/sysconfig/openvswitch added by this patch conflicts with the
> > > one that the init script already expects to find, which is generated
> > > on XenServer from the template that is included as
> > > xenserver/usr_share_openvswitch_scripts_sysconfig.template. 
> > 
> > Yea, I thought about adding that NETWORK_MODE to your template and
> > installing that. Having that one line in the sea of comments made
> > me worry that it would get missed. I'm all for going back in that
> > direction if you would like.
> 
> Well, more than anything I'd rather not have /etc/sysconfig/openvswitch
> be completely different on XenServer and SuSE if we can avoid it,
> because that seems bound to cause confusion to anyone who happens to use
> both.  Is there another filename that might be suitable?
> 
> > > It would make sense to add $NETWORK_MODE to that template, if it is
> > > needed on SuSE.  Is it needed?  (It is used on XenServer because it has
> > > two different modes of operations, one that uses OVS, one that uses the
> > > Linux bridge.)  Would it make sense to name it something different,
> > > e.g. ENABLE_OVS?
> > 
> > I wanted to change the script as little as possible and maintain the
> > xenserver behavior. Right now, if xenserver is not installed
> > NETWORK_MODE should default to openvswitch(which is desired if you
> > enable it). If xenserver happens to get installed(on suse for example),
> > NETWORK_MODE should get set via the source of /etc/xensource-inventory.
> > I'm not sure how changing the variable name or adding a new one would
> > help but I am open to a suggestion(I'm probably missing something).
> > It seems if you install and enable openvswitch that should be enough
> > state to know you want to enable it.
> 
> There's some confusion here I think.  XenServer is a whole OS, just as
> SuSE is.  As far as I know, no one carves out bits and pieces of
> XenServer and installs them on top of other distributions.  (Maybe you
> know differently?  I am no expert on this.)
> 
> So, assuming I'm right about that, then there may be no need for it on
> SuSE, because admins want OVS to run if it is installed (as you say).
> 
> > > It doesn't make sense to install etc_init.d_openvswitch-xapi-update on
> > > SuSE at all, since it will at most print an error message and exit with
> > > status 1.  So it seems unnecessary to modify that file.
> > 
> > I can not package it, but on the chance that someone wanted to use suse
> > and xenserver, it might be easier if it was just there. My changes were
> > just about LSB compliance, other distros might care about this too.
> 
> Ah, OK.  Fine.
> 
> > Another approach might be to move basic startup into a separate common
> > script area and then have the xenserver scripts deal with starting
> > stuff specific to it. I don't have xenserver here and didn't watnt to
> > break it on a first attempt. The nice part is I think this should
> > work for both cases now.
> 
> That's a good idea too.
> 
> I haven't done that yet, but I've tweaked these patches to my liking and
> I'll send them back out to you and the list in a minute.  I've retained
> your authorship and sign-offs, and tried to note the changes that I made
> in the proper format, but please do review the changes that I made to
> ensure that they make sense.
> 
> Thanks,
> 
> Ben.






More information about the dev mailing list