[ovs-dev] [PATCH 2/2] datapath: omit _mod from module names

[Chris Wright]

* Jesse Gross (jesse at nicira.com) wrote:
> On Wed, Mar 7, 2012 at 10:28 AM, Chris Wright <chrisw at sous-sol.org> wrote:
> > This renames the datapath modules:
> >
> >  openvswitch_mod -> openvswitch
> >  brcompat_mod -> brcompat
> >
> > The first makes the module name consistent with upstream, and the latter
> > is just for internal consistency.  This makes tools, and documentation
> > refer to a common module name regardless if it's coming from upstream
> > linux or built from datapath/ as part of a local build.
> >
> > Signed-off-by: Chris Wright <chrisw at sous-sol.org>
> 
> I noticed that there were a few places that still use the _mod version
> of the name that probably need to be fixed up as well:
> 
> debian/dkms.conf.in:BUILT_MODULE_NAME[0]=openvswitch_mod
> utilities/ovs-ctl.8:openvswitch_mod kernel module. If the
> \fB\-\-brcompat\fR option is
> xenserver/openvswitch-xen.spec.in:Requires:
> openvswitch_mod.ko.%{module_abi_version}
> xenserver/openvswitch-xen.spec.in:Provides:
> %{name}-modules-%{kernel_flavor} = %{kernel_version},
> openvswitch_mod.ko.%{module_abi_version}
> xenserver/openvswitch-xen.spec.in:/lib/modules/%{xen_version}/extra/openvswitch/openvswitch_mod.ko
> 
> debian/dkms.conf.in:BUILT_MODULE_NAME[1]=brcompat_mod
> utilities/ovs-ctl.8:specified then the brcompat_mod kernel module is
> also loaded.
> vswitchd/ovs-brcompatd.c:    /* Parse the command received from brcompat_mod. */
> vswitchd/ovs-brcompatd.8.in:\fBovs\-brcompatd\fR requires the
> \fBbrcompat_mod.ko\fR kernel module to be
> xenserver/openvswitch-xen.spec.in:%exclude
> /lib/modules/%{xen_version}/extra/openvswitch/brcompat_mod.ko

Thanks, I'm not sure how I missed those.

> Would you also mind adding an entry to NEWS to give people a little
> bit of a heads-up?

Sure, I'll update and resend.

> > diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
> > index 9bdb3df..80e8f00 100755
> > --- a/utilities/ovs-ctl.in
> > +++ b/utilities/ovs-ctl.in
> >  insert_openvswitch_mod_if_required () {
> > -    # If openvswitch_mod is already loaded then we're done.
> > -    test -e /sys/module/openvswitch_mod && return 0
> > +    # If openvswitch is already loaded then we're done.
> > +    test -e /sys/module/openvswitch && return 0
> [...]
> > -    test -e /sys/module/brcompat_mod && return 0
> > -    action "Inserting brcompat module" modprobe brcompat_mod
> > +    test -e /sys/module/brcompat && return 0
> > +    action "Inserting brcompat module" modprobe brcompat
> 
> I think we might want to test for either name in these places.
> Previously, if an old version of the module was loaded then we would
> succeed, which should be OK now that we're maintaining stable
> interfaces.  However, if the names are different we'll try to load the
> new module when the old one is loaded, which will fail.

OK.  Were you thinking something simple like:

    test -e /sys/module/openvswitch_mod && rmmod openvswitch_mod

as a preamble, then allow the new module to get loaded?  Or something
more akin to force_reload_kmod (to save/restore, although restore may
not work)?

thanks,
-chris