[ovs-dev] [PATCH branch-1.4] ovs-ctl: Add support for newer name for Open vSwitch kernel module.

[Justin Pettit]

Looks good to me.

--Justin


On Aug 6, 2012, at 1:22 PM, Ben Pfaff <blp at nicira.com> wrote:

> Good idea. Like this?
> 
> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
> index 03150a8..41edb93 100755
> --- a/utilities/ovs-ctl.in
> +++ b/utilities/ovs-ctl.in
> @@ -49,6 +49,11 @@ insert_openvswitch_mod_if_required () {
>         return 0
>     fi
> 
> +    if (modprobe --dry-run openvswitch_mod && \
> +        modprobe --dry-run openvswitch) 2>/dev/null; then
> +        log_warning_msg "openvswitch_mod.ko and openvswitch.ko modules are both available.  openvswitch_mod.ko will be preferred."
> +    fi
> +
>     # Load openvswitch_mod.  If that's successful then we're done.
>     action "Inserting openvswitch module" do_modprobe openvswitch && return 0
> 
> On Mon, Aug 06, 2012 at 11:21:22AM -0700, Justin Pettit wrote:
>> Looks reasonable to me.  I worry a bit about preferring "_mod" over
>> the newer convention, but I can't think of a better way to infer the
>> user's intention.  If they've gone through the trouble of building
>> OVS from scratch, it's probably reasonable to think they should be
>> able to diagnose this themselves.  I suppose one thing we could do
>> is check for the presence of both with "modprobe --dryrun" and print
>> a warning if both exist.
>> 
>> --Justin
>> 
>> 
>> On Aug 6, 2012, at 10:16 AM, Ben Pfaff <blp at nicira.com> wrote:
>> 
>>> Open vSwitch 1.4 and later is compatible with the upstream Linux kernel
>>> module but the init scripts hadn't been adapted to work with the upstream
>>> module name.
>>> 
>>> Debian bug #684057.
>>> Signed-off-by: Ben Pfaff <blp at nicira.com>
>>> ---
>>> utilities/ovs-ctl.in |   36 ++++++++++++++++++++++++++----------
>>> 1 files changed, 26 insertions(+), 10 deletions(-)
>>> 
>>> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
>>> index 527bd60..03150a8 100755
>>> --- a/utilities/ovs-ctl.in
>>> +++ b/utilities/ovs-ctl.in
>>> @@ -30,12 +30,27 @@ done
>>> ## start ##
>>> ## ----- ##
>>> 
>>> +do_modprobe () {
>>> +    # First try the name suffixed with _mod, because this is the
>>> +    # original name used for the out-of-tree kernel module.  If the
>>> +    # user has a module by that name, then it means that he went to
>>> +    # some trouble to build it, so presumably he wants to use it.
>>> +    #
>>> +    # Then try the name without the _mod suffix, the upstream Linux
>>> +    # kernel module name and the name used by later versions of Open
>>> +    # vSwitch.
>>> +    modprobe ${1}_mod || modprobe $1
>>> +} 2>/dev/null
>>> +
>>> insert_openvswitch_mod_if_required () {
>>>    # If openvswitch_mod is already loaded then we're done.
>>> -    test -e /sys/module/openvswitch_mod && return 0
>>> +    if test -e /sys/module/openvswitch_mod || test -e /sys/module/openvswitch
>>> +    then
>>> +        return 0
>>> +    fi
>>> 
>>>    # Load openvswitch_mod.  If that's successful then we're done.
>>> -    action "Inserting openvswitch module" modprobe openvswitch_mod && return 0
>>> +    action "Inserting openvswitch module" do_modprobe openvswitch && return 0
>>> 
>>>    # If the bridge module is loaded, then that might be blocking
>>>    # openvswitch_mod.  Try to unload it, if there are no bridges.
>>> @@ -48,12 +63,12 @@ insert_openvswitch_mod_if_required () {
>>>    action "removing bridge module" rmmod bridge || return 1
>>> 
>>>    # Try loading openvswitch_mod again.
>>> -    action "Inserting openvswitch module" modprobe openvswitch_mod
>>> +    action "Inserting openvswitch module" do_modprobe openvswitch
>>> }
>>> 
>>> insert_brcompat_mod_if_required () {
>>>    test -e /sys/module/brcompat_mod && return 0
>>> -    action "Inserting brcompat module" modprobe brcompat_mod
>>> +    action "Inserting brcompat module" do_modprobe brcompat
>>> }
>>> 
>>> insert_mod_if_required () {
>>> @@ -287,12 +302,13 @@ force_reload_kmod () {
>>>        action "Removing datapath: $dp" ovs-dpctl del-dp "$dp"
>>>    done
>>> 
>>> -    if test -e /sys/module/brcompat_mod; then
>>> -        action "Removing brcompat module" rmmod brcompat_mod
>>> -    fi
>>> -    if test -e /sys/module/openvswitch_mod; then
>>> -        action "Removing openvswitch module" rmmod openvswitch_mod
>>> -    fi
>>> +    for module in brcompat openvswitch; do
>>> +        for fullname in ${module}_mod $module; do
>>> +            if test -e /sys/module/$fullname; then
>>> +                action "Removing $module module" rmmod $fullname
>>> +            fi
>>> +        done
>>> +    done
>>> 
>>>    start
>>> 
>>> -- 
>>> 1.7.2.5
>>> 
>>> _______________________________________________
>>> dev mailing list
>>> dev at openvswitch.org
>>> http://openvswitch.org/mailman/listinfo/dev
>>