On Wed, Jan 24, 2018 at 07:23:06PM -0800, Greg Rose wrote:
> A previous patch added post install and post uninstall scripts which
> use the weak-modules utility to make sure that openvswitch kernel
> modules are copied to the correct kernel directory.  While this
> patch did fix some issues there are two remaining issues we have
> found.
> 
> 1) In the case where the OS is running kernel X and the openvswitch
>    kernel modules have been previously installed correctly and are
>    working without a problem and then updating to Kernel X.1 we find
>    that the correct openvswitch kernel module no longer loads.
> 2) In the case where a kernel module rpm has been built against an
>    older kernel but installed on a newer kernel then the correct
>    openvswitch kernel module will not load.
> 
> This patch changes the weak-modules parameter to --add-kernel instead
> of --add-modules. This fixes the problem in case 1.  In addition
> we modify the openvswitch depmod configuration file installed to the
> /etc/depmod.d directory and prepend a "01" to the name of the file.
> We then insert the first line of the file with a search path that
> will find weak-updates first. This change along with the change
> to the weak-modules utility parameters fixes the problem in case 2.
> 
> I also modified the kernel module configuration file for both the
> rhel6 and rhel7/fedora specs to use a common "01openvswitch.conf"
> file name format.

Well, we used kmods in the past to provide modules that were not
shipped with the kernel, so that might be the reason I didn't see
the depmod config tweaking before.  Although it seems we need to take
care of that in this case, messing with the default search order
doesn't sound like a good idea to me.

What about this instead?  I found that in a couple kmod specs.

    override openvswitch * weak-updates/openvswitch

This is documented in depmod.d(5)
[...]
       override modulename kernelversion modulesubdirectory
           This command allows you to override which version of a
specific module will be used when more than one module sharing the
same name is processed by the depmod command. It is possible
           to specify one kernel or all kernels using the * wildcard.
modulesubdirectory is the name of the subdirectory under /lib/modules
(or other module location) where the target module is
           installed.

           For example, it is possible to override the priority of an
updated test module called kmod by specifying the following command:
"override kmod * extra". This will ensure that any
           matching module name installed under the extra subdirectory
within /lib/modules (or other module location) will take priority over
any likenamed module already provided by the kernel.


That seems better because it doesn't depend on the depmod config
filename (no config processing order issue), it doesn't change
system's default, or impact possible other kmod or even custom
configurations other than the specific for this kmod.

Regarding to change --add-modules to --add-kernel, well, I don't see
how that is helping. Maybe I am missing something, but every time a
kernel is installed, it will run weak-modules --add-kernel which
should create the weak-updates links accordingly, unless the kernel
is not compatible.

You can verify what the kernel's post install scripts does with this
command:
# rpm -q kernel --scripts

You should find this snippet:
if [ -x /sbin/weak-modules ]
then
    /sbin/weak-modules --add-kernel 2.6.32-696.18.7.el6.x86_64 || exit $?
fi

fbl

> 
> Cc: Flavio Leitner <f...@sysclose.org>
> Co-authored-by: Gurucharan Shetty <g...@ovn.org>
> Signed-off-by: Greg Rose <gvrose8...@gmail.com>
> ---
>  rhel/openvswitch-kmod-fedora.spec.in | 12 +++++++-----
>  rhel/openvswitch-kmod-rhel6.spec.in  | 14 ++++++++------
>  rhel/openvswitch-kmod.files          |  2 +-
>  3 files changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/rhel/openvswitch-kmod-fedora.spec.in 
> b/rhel/openvswitch-kmod-fedora.spec.in
> index c0cd298..ae3a4d8 100644
> --- a/rhel/openvswitch-kmod-fedora.spec.in
> +++ b/rhel/openvswitch-kmod-fedora.spec.in
> @@ -43,13 +43,15 @@ make %{_smp_mflags} -C datapath/linux
>  rm -rf $RPM_BUILD_ROOT
>  make INSTALL_MOD_PATH=$RPM_BUILD_ROOT -C datapath/linux modules_install
>  mkdir -p $RPM_BUILD_ROOT/etc/depmod.d
> +
> +echo "search weak-updates updates extra built-in" >> 
> $RPM_BUILD_ROOT/etc/depmod.d/01openvswitch.conf
>  for module in $RPM_BUILD_ROOT/lib/modules/%{kernel}/extra/*.ko
>  do
>      modname="$(basename ${module})"
>      echo "override ${modname%.ko} * extra" >> \
> -        $RPM_BUILD_ROOT/etc/depmod.d/kmod-openvswitch.conf
> +        $RPM_BUILD_ROOT/etc/depmod.d/01openvswitch.conf
>      echo "override ${modname%.ko} * weak-updates" >> \
> -        $RPM_BUILD_ROOT/etc/depmod.d/kmod-openvswitch.conf
> +        $RPM_BUILD_ROOT/etc/depmod.d/01openvswitch.conf
>  done
>  
>  %clean
> @@ -64,7 +66,7 @@ if [ -x "/sbin/weak-modules" ]; then
>      for m in openvswitch vport-gre vport-stt vport-geneve \
>               vport-lisp vport-vxlan; do
>          echo "/lib/modules/%{kernel}/extra/$m.ko"
> -    done | /sbin/weak-modules --add-modules
> +    done | /sbin/weak-modules --add-kernel
>  fi
>  
>  %postun
> @@ -76,14 +78,14 @@ if [ "$1" = 0 ]; then  # Erase, not upgrade
>          for m in openvswitch vport-gre vport-stt vport-geneve \
>                   vport-lisp vport-vxlan; do
>          echo "/lib/modules/%{kernel}/extra/$m.ko"
> -    done | /sbin/weak-modules --remove-modules
> +    done | /sbin/weak-modules --remove-kernel
>      fi
>  fi
>  
>  %files
>  %defattr(0644,root,root)
>  /lib/modules/%{kernel}/extra/*.ko
> -/etc/depmod.d/kmod-openvswitch.conf
> +/etc/depmod.d/01openvswitch.conf
>  %exclude /lib/modules/%{kernel}/modules.*
>  
>  %changelog
> diff --git a/rhel/openvswitch-kmod-rhel6.spec.in 
> b/rhel/openvswitch-kmod-rhel6.spec.in
> index c6e6db5..28c7ce1 100644
> --- a/rhel/openvswitch-kmod-rhel6.spec.in
> +++ b/rhel/openvswitch-kmod-rhel6.spec.in
> @@ -62,13 +62,15 @@ for flavor in %flavors_to_build ; do
>           find $INSTALL_MOD_PATH/lib/modules -iname 'modules.*' -exec rm {} \;
>  done
>  install -d %{buildroot}%{_sysconfdir}/depmod.d/
> +
> +echo "search extra/openvswitch weak-updates/openvswitch updates extra 
> built-in weak-updates" >> 01%{oname}.conf
>  for module in 
> %{buildroot}/lib/modules/%{kernel_version}/$INSTALL_MOD_DIR/*.ko;
>  do
>      modname="$(basename ${module})"
> -    echo "override ${modname%.ko} * extra/%{oname}" >> %{oname}.conf
> -    echo "override ${modname%.ko} * weak-updates/%{oname}" >> %{oname}.conf
> +    echo "override ${modname%.ko} * extra/%{oname}" >> 01%{oname}.conf
> +    echo "override ${modname%.ko} * weak-updates/%{oname}" >> 01%{oname}.conf
>  done
> -install -m 644 %{oname}.conf %{buildroot}%{_sysconfdir}/depmod.d/
> +install -m 644 01%{oname}.conf %{buildroot}%{_sysconfdir}/depmod.d/
>  
>  %post
>  # Ensure that modprobe will find our modules.
> @@ -79,7 +81,7 @@ if [ -x "/sbin/weak-modules" ]; then
>      for m in openvswitch vport-gre vport-stt vport-geneve \
>               vport-lisp vport-vxlan; do
>          echo "/lib/modules/%{kernel}/extra/$m.ko"
> -    done | /sbin/weak-modules --add-modules
> +    done | /sbin/weak-modules --add-kernel
>  fi
>  
>  %postun
> @@ -91,13 +93,13 @@ if [ "$1" = 0 ]; then  # Erase, not upgrade
>          for m in openvswitch vport-gre vport-stt vport-geneve \
>                   vport-lisp vport-vxlan; do
>          echo "/lib/modules/%{kernel}/extra/$m.ko"
> -    done | /sbin/weak-modules --remove-modules
> +    done | /sbin/weak-modules --remove-kernel
>      fi
>  fi
>  
>  %files
>  %defattr(644,root,root)
> -/etc/depmod.d/%{oname}.conf
> +/etc/depmod.d/01%{oname}.conf
>  
>  %clean
>  rm -rf $RPM_BUILD_ROOT
> diff --git a/rhel/openvswitch-kmod.files b/rhel/openvswitch-kmod.files
> index 357c2e8..49262f3 100644
> --- a/rhel/openvswitch-kmod.files
> +++ b/rhel/openvswitch-kmod.files
> @@ -1,3 +1,3 @@
>  %defattr(644,root,root,755)
>  /lib/modules/%2-%1
> -/etc/depmod.d/openvswitch.conf
> +/etc/depmod.d/01openvswitch.conf
> -- 
> 1.8.3.1
> 
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

-- 
Flavio

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to