[ovs-dev,V2,2/2] rhel: Add post installation check for kernel modules

Message ID 1515775064-18494-2-git-send-email-gvrose8192@gmail.com
State Not Applicable
Headers show
Series
  • [ovs-dev,V2,1/2] rhel: Add depmod file for openvswitch module search
Related show

Commit Message

Gregory Rose Jan. 12, 2018, 4:37 p.m.
A bug in RHEL 7.2 has been found in which a customer who installed
a RHEL 7.2 openvswitch kernel module rpm with a slightly different
minor build number than the running kernel found that the kernel
modules were installed to the wrong directory.

After the installation the new openvswitch kernel modules were
installed to:
/lib/modules/3.10.0-327.22.2.el7.x86_64/extra/openvswitch

But the running kernel was 3.10.0-327.el7.x86_64 and after the
installation was complete the kernel modules in the installed
directory were not linked to the "weak-updates" directory in
the running kernel.  So a critical bug was encountered in
which the in-tree openvswitch kernel module was loaded instead
of the one the customer explicitly installed with the rpm.

This patch replicates ./extra/openvswitch directory with kernel
modules, if for the currently running kernel there is neither
a ./extra/openvswitch nor ./weak-update/openvswitch directory.

Signed-off-by: Greg Rose <gvrose8192@gmail.com>

---

V2 - Incorporate feedback from V1
---
 rhel/openvswitch.spec.in | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

Comments

Darrell Ball Jan. 12, 2018, 5:32 p.m. | #1
Thanks for working on this Greg
I just have a few comments/questions.

On 1/12/18, 8:38 AM, "ovs-dev-bounces@openvswitch.org on behalf of Greg Rose" <ovs-dev-bounces@openvswitch.org on behalf of gvrose8192@gmail.com> wrote:

    A bug in RHEL 7.2 has been found in which a customer who installed
    a RHEL 7.2 openvswitch kernel module rpm with a slightly different
    minor build number than the running kernel found that the kernel
    modules were installed to the wrong directory.
    
    After the installation the new openvswitch kernel modules were
    installed to:
    /lib/modules/3.10.0-327.22.2.el7.x86_64/extra/openvswitch
    
    But the running kernel was 3.10.0-327.el7.x86_64 and after the
    installation was complete the kernel modules in the installed
    directory were not linked to the "weak-updates" directory in
    the running kernel. 


    So a critical bug was encountered in
    which the in-tree openvswitch kernel module was loaded instead
    of the one the customer explicitly installed with the rpm.


[Darrell] Is it necessary to say “critical” here ?
    
    This patch replicates ./extra/openvswitch directory with kernel
    modules, if for the currently running kernel there is neither
    a ./extra/openvswitch nor ./weak-update/openvswitch directory.
    
    Signed-off-by: Greg Rose <gvrose8192@gmail.com>

    
    ---
    
    V2 - Incorporate feedback from V1
    ---
     rhel/openvswitch.spec.in | 26 ++++++++++++++++++++++++++
     1 file changed, 26 insertions(+)
    
    diff --git a/rhel/openvswitch.spec.in b/rhel/openvswitch.spec.in
    index e510d35..b5b5122 100644
    --- a/rhel/openvswitch.spec.in
    +++ b/rhel/openvswitch.spec.in
    @@ -169,6 +169,32 @@ fi
     /sbin/chkconfig --add openvswitch
     /sbin/chkconfig openvswitch on
     
    +# In some cases a kernel module rpm will have a different minor build
    +# version than the currently running kernel.  In this case the kernel
    +# modules will be installed but not to the kernel modules directory
    +# of the currently running kernel. Check and copy modules if
    +# necessary.
    +# This is a bug that has only been found to occur on RHEL 7.2.
    +if [[ ! -d /lib/modules/$(uname -r)/extra/openvswitch && \
    +      ! -d /lib/modules/$(uname -r)/weak-updates/openvswitch ]]; then

[Darrell] This implies that if extra/openvswitch or weak-updates/openvswitch already exists, we won’t
               have a problem. I guess it is less likely, but I am not sure definitively. What do you think ?

               Why does this happen only on RHEL 7.2 ?
               Or was it only observed so far on 7.2 and there is some other common denominator ? 



    +    found="false"
    +    for i in `ls -t /lib/modules`
    +    do
    +        if [ -d /lib/modules/$i/extra/openvswitch ]; then
    +            mkdir -p /lib/modules/$(uname -r)/extra
    +            cp -r --preserve "/lib/modules/$i/extra/openvswitch" \
    +            /lib/modules/$(uname -r)/extra
    +            found="true"

[Darrell] I have seen a symbolic link used in some similar cases.


    +            break
    +        fi
    +    done
    +    if [ "$found" != "true" ]; then
    +        echo "Error in openvswitch kernel modules installation"
    +    else
    +        /usr/sbin/depmod -a
    +    fi
    +fi
    +
     %post selinux-policy
     /usr/sbin/semodule -i %{_datadir}/selinux/packages/%{name}/openvswitch-custom.pp &> /dev/null || :
     
    -- 
    1.8.3.1
    
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=6fJmIsIJeh610bYPTqJ81HEAabcWsimFnNMA901QScs&s=hmxDEn-HhsJNI4kS0xgSV1nBnPCX-n9Z8chesGnW6sg&e=
Guru Shetty Jan. 12, 2018, 5:37 p.m. | #2
On 12 January 2018 at 08:37, Greg Rose <gvrose8192@gmail.com> wrote:

> A bug in RHEL 7.2 has been found in which a customer who installed
> a RHEL 7.2 openvswitch kernel module rpm with a slightly different
> minor build number than the running kernel found that the kernel
> modules were installed to the wrong directory.
>
> After the installation the new openvswitch kernel modules were
> installed to:
> /lib/modules/3.10.0-327.22.2.el7.x86_64/extra/openvswitch
>
> But the running kernel was 3.10.0-327.el7.x86_64 and after the
> installation was complete the kernel modules in the installed
> directory were not linked to the "weak-updates" directory in
> the running kernel.  So a critical bug was encountered in
> which the in-tree openvswitch kernel module was loaded instead
> of the one the customer explicitly installed with the rpm.
>
> This patch replicates ./extra/openvswitch directory with kernel
> modules, if for the currently running kernel there is neither
> a ./extra/openvswitch nor ./weak-update/openvswitch directory.
>
> Signed-off-by: Greg Rose <gvrose8192@gmail.com>
>
> ---
>
> V2 - Incorporate feedback from V1
> ---
>  rhel/openvswitch.spec.in | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/rhel/openvswitch.spec.in b/rhel/openvswitch.spec.in
> index e510d35..b5b5122 100644
> --- a/rhel/openvswitch.spec.in
> +++ b/rhel/openvswitch.spec.in
> @@ -169,6 +169,32 @@ fi
>  /sbin/chkconfig --add openvswitch
>  /sbin/chkconfig openvswitch on
>
> +# In some cases a kernel module rpm will have a different minor build
> +# version than the currently running kernel.  In this case the kernel
> +# modules will be installed but not to the kernel modules directory
> +# of the currently running kernel. Check and copy modules if
> +# necessary.
> +# This is a bug that has only been found to occur on RHEL 7.2.
> +if [[ ! -d /lib/modules/$(uname -r)/extra/openvswitch && \
> +      ! -d /lib/modules/$(uname -r)/weak-updates/openvswitch ]]; then
>

This check may not be good enough. If we are doing a upgrade of OVS on the
system (say from 2.7 to 2.9) and previously we had something in
/lib/modules/$(uname -r)/weak-updates/openvswitch then we are no longer
going to add the newer version there.


> +    found="false"
> +    for i in `ls -t /lib/modules`
> +    do
> +        if [ -d /lib/modules/$i/extra/openvswitch ]; then
>
This will not help either. There is a possibility of multiple kernels
installed and it looks like we will choose one randomly. We should only
look at the path where the current rpm actually installed our files.


> +            mkdir -p /lib/modules/$(uname -r)/extra
> +            cp -r --preserve "/lib/modules/$i/extra/openvswitch" \
> +            /lib/modules/$(uname -r)/extra
> +            found="true"
> +            break
> +        fi
> +    done
> +    if [ "$found" != "true" ]; then
> +        echo "Error in openvswitch kernel modules installation"
> +    else
> +        /usr/sbin/depmod -a
> +    fi
> +fi
> +
>  %post selinux-policy
>  /usr/sbin/semodule -i %{_datadir}/selinux/packages/%{name}/openvswitch-custom.pp
> &> /dev/null || :
>
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Gregory Rose Jan. 12, 2018, 6:09 p.m. | #3
On 1/12/2018 9:32 AM, Darrell Ball wrote:
> Thanks for working on this Greg
> I just have a few comments/questions.
>
> On 1/12/18, 8:38 AM, "ovs-dev-bounces@openvswitch.org on behalf of Greg Rose" <ovs-dev-bounces@openvswitch.org on behalf of gvrose8192@gmail.com> wrote:
>
>      A bug in RHEL 7.2 has been found in which a customer who installed
>      a RHEL 7.2 openvswitch kernel module rpm with a slightly different
>      minor build number than the running kernel found that the kernel
>      modules were installed to the wrong directory.
>      
>      After the installation the new openvswitch kernel modules were
>      installed to:
>      /lib/modules/3.10.0-327.22.2.el7.x86_64/extra/openvswitch
>      
>      But the running kernel was 3.10.0-327.el7.x86_64 and after the
>      installation was complete the kernel modules in the installed
>      directory were not linked to the "weak-updates" directory in
>      the running kernel.
>
>
>      So a critical bug was encountered in
>      which the in-tree openvswitch kernel module was loaded instead
>      of the one the customer explicitly installed with the rpm.
>
>
> [Darrell] Is it necessary to say “critical” here ?

I suppose not - I can remove the word if you prefer.

>      
>      This patch replicates ./extra/openvswitch directory with kernel
>      modules, if for the currently running kernel there is neither
>      a ./extra/openvswitch nor ./weak-update/openvswitch directory.
>      
>      Signed-off-by: Greg Rose <gvrose8192@gmail.com>
>      
>      ---
>      
>      V2 - Incorporate feedback from V1
>      ---
>       rhel/openvswitch.spec.in | 26 ++++++++++++++++++++++++++
>       1 file changed, 26 insertions(+)
>      
>      diff --git a/rhel/openvswitch.spec.in b/rhel/openvswitch.spec.in
>      index e510d35..b5b5122 100644
>      --- a/rhel/openvswitch.spec.in
>      +++ b/rhel/openvswitch.spec.in
>      @@ -169,6 +169,32 @@ fi
>       /sbin/chkconfig --add openvswitch
>       /sbin/chkconfig openvswitch on
>       
>      +# In some cases a kernel module rpm will have a different minor build
>      +# version than the currently running kernel.  In this case the kernel
>      +# modules will be installed but not to the kernel modules directory
>      +# of the currently running kernel. Check and copy modules if
>      +# necessary.
>      +# This is a bug that has only been found to occur on RHEL 7.2.
>      +if [[ ! -d /lib/modules/$(uname -r)/extra/openvswitch && \
>      +      ! -d /lib/modules/$(uname -r)/weak-updates/openvswitch ]]; then
>
> [Darrell] This implies that if extra/openvswitch or weak-updates/openvswitch already exists, we won’t
>                 have a problem. I guess it is less likely, but I am not sure definitively. What do you think ?

Yes, if those directories exist in the running kernel then that shows 
that the process of installing the openvswitch kernel module has worked 
in the past.  If it's worked in the past it will work now.  I'm 
addressing a specific situation in which an openvswitch kernel install 
rpm did not install to either extra or weak-updates at all and is 
instead off in a different directory with just the 5 openvswitch kernel 
modules in it and nothing else.

>
>                 Why does this happen only on RHEL 7.2 ?

If I only knew...

>                 Or was it only observed so far on 7.2 and there is some other common denominator ?

Only reproducible on RHEL 7.2 so far as I have found with my own testing.

>
>
>
>      +    found="false"
>      +    for i in `ls -t /lib/modules`
>      +    do
>      +        if [ -d /lib/modules/$i/extra/openvswitch ]; then
>      +            mkdir -p /lib/modules/$(uname -r)/extra
>      +            cp -r --preserve "/lib/modules/$i/extra/openvswitch" \
>      +            /lib/modules/$(uname -r)/extra
>      +            found="true"
>
> [Darrell] I have seen a symbolic link used in some similar cases.

Now that you mention it that's a better idea.  I'll make that change in 
the next round.

>
>
>      +            break
>      +        fi
>      +    done
>      +    if [ "$found" != "true" ]; then
>      +        echo "Error in openvswitch kernel modules installation"
>      +    else
>      +        /usr/sbin/depmod -a
>      +    fi
>      +fi
>      +
>       %post selinux-policy
>       /usr/sbin/semodule -i %{_datadir}/selinux/packages/%{name}/openvswitch-custom.pp &> /dev/null || :
>       
>      --
>      1.8.3.1
>      
>      _______________________________________________
>      dev mailing list
>      dev@openvswitch.org
>      https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=6fJmIsIJeh610bYPTqJ81HEAabcWsimFnNMA901QScs&s=hmxDEn-HhsJNI4kS0xgSV1nBnPCX-n9Z8chesGnW6sg&e=
>      
>
Gregory Rose Jan. 12, 2018, 6:16 p.m. | #4
On 1/12/2018 9:37 AM, Guru Shetty wrote:
>
>
> On 12 January 2018 at 08:37, Greg Rose <gvrose8192@gmail.com 
> <mailto:gvrose8192@gmail.com>> wrote:
>
>     A bug in RHEL 7.2 has been found in which a customer who installed
>     a RHEL 7.2 openvswitch kernel module rpm with a slightly different
>     minor build number than the running kernel found that the kernel
>     modules were installed to the wrong directory.
>
>     After the installation the new openvswitch kernel modules were
>     installed to:
>     /lib/modules/3.10.0-327.22.2.el7.x86_64/extra/openvswitch
>
>     But the running kernel was 3.10.0-327.el7.x86_64 and after the
>     installation was complete the kernel modules in the installed
>     directory were not linked to the "weak-updates" directory in
>     the running kernel.  So a critical bug was encountered in
>     which the in-tree openvswitch kernel module was loaded instead
>     of the one the customer explicitly installed with the rpm.
>
>     This patch replicates ./extra/openvswitch directory with kernel
>     modules, if for the currently running kernel there is neither
>     a ./extra/openvswitch nor ./weak-update/openvswitch directory.
>
>     Signed-off-by: Greg Rose <gvrose8192@gmail.com
>     <mailto:gvrose8192@gmail.com>>
>
>     ---
>
>     V2 - Incorporate feedback from V1
>     ---
>      rhel/openvswitch.spec.in <http://openvswitch.spec.in> | 26
>     ++++++++++++++++++++++++++
>      1 file changed, 26 insertions(+)
>
>     diff --git a/rhel/openvswitch.spec.in <http://openvswitch.spec.in>
>     b/rhel/openvswitch.spec.in <http://openvswitch.spec.in>
>     index e510d35..b5b5122 100644
>     --- a/rhel/openvswitch.spec.in <http://openvswitch.spec.in>
>     +++ b/rhel/openvswitch.spec.in <http://openvswitch.spec.in>
>     @@ -169,6 +169,32 @@ fi
>      /sbin/chkconfig --add openvswitch
>      /sbin/chkconfig openvswitch on
>
>     +# In some cases a kernel module rpm will have a different minor build
>     +# version than the currently running kernel.  In this case the kernel
>     +# modules will be installed but not to the kernel modules directory
>     +# of the currently running kernel. Check and copy modules if
>     +# necessary.
>     +# This is a bug that has only been found to occur on RHEL 7.2.
>     +if [[ ! -d /lib/modules/$(uname -r)/extra/openvswitch && \
>     +      ! -d /lib/modules/$(uname -r)/weak-updates/openvswitch ]]; then
>
>
> This check may not be good enough. If we are doing a upgrade of OVS on 
> the system (say from 2.7 to 2.9) and previously we had something in  
> /lib/modules/$(uname -r)/weak-updates/openvswitch then we are no 
> longer going to add the newer version there.

But that would assure that a previous attempt to install the openvswitch 
kernel modules rpm had succeeded, which means this system doesn't have 
the bug that is prompting this patch.  That means the newly installed 
kernel modules have been placed in the right kernel modules directory.  
So the test is correct.

>     +    found="false"
>     +    for i in `ls -t /lib/modules`
>     +    do
>     +        if [ -d /lib/modules/$i/extra/openvswitch ]; then
>
> This will not help either. There is a possibility of multiple kernels 
> installed and it looks like we will choose one randomly. We should 
> only look at the path where the current rpm actually installed our files.

I tried it on a system with multiple installed kernels and it does work 
because the sort by time will pick the most recently installed kernel 
modules directory which would be the one where the just installed kernel 
modules rpm had gone to (erroneously I should add).
>
>     +            mkdir -p /lib/modules/$(uname -r)/extra
>     +            cp -r --preserve "/lib/modules/$i/extra/openvswitch" \
>     +            /lib/modules/$(uname -r)/extra
>     +            found="true"
>     +            break
>     +        fi
>     +    done
>     +    if [ "$found" != "true" ]; then
>     +        echo "Error in openvswitch kernel modules installation"
>     +    else
>     +        /usr/sbin/depmod -a
>     +    fi
>     +fi
>     +
>      %post selinux-policy
>      /usr/sbin/semodule -i
>     %{_datadir}/selinux/packages/%{name}/openvswitch-custom.pp &>
>     /dev/null || :
>
>     --
>     1.8.3.1
>
>     _______________________________________________
>     dev mailing list
>     dev@openvswitch.org <mailto:dev@openvswitch.org>
>     https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>     <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
>
>
Guru Shetty Jan. 12, 2018, 7:06 p.m. | #5
On 12 January 2018 at 10:16, Gregory Rose <gvrose8192@gmail.com> wrote:

> On 1/12/2018 9:37 AM, Guru Shetty wrote:
>
>
>
> On 12 January 2018 at 08:37, Greg Rose <gvrose8192@gmail.com> wrote:
>
>> A bug in RHEL 7.2 has been found in which a customer who installed
>> a RHEL 7.2 openvswitch kernel module rpm with a slightly different
>> minor build number than the running kernel found that the kernel
>> modules were installed to the wrong directory.
>>
>> After the installation the new openvswitch kernel modules were
>> installed to:
>> /lib/modules/3.10.0-327.22.2.el7.x86_64/extra/openvswitch
>>
>> But the running kernel was 3.10.0-327.el7.x86_64 and after the
>> installation was complete the kernel modules in the installed
>> directory were not linked to the "weak-updates" directory in
>> the running kernel.  So a critical bug was encountered in
>> which the in-tree openvswitch kernel module was loaded instead
>> of the one the customer explicitly installed with the rpm.
>>
>> This patch replicates ./extra/openvswitch directory with kernel
>> modules, if for the currently running kernel there is neither
>> a ./extra/openvswitch nor ./weak-update/openvswitch directory.
>>
>> Signed-off-by: Greg Rose <gvrose8192@gmail.com>
>>
>> ---
>>
>> V2 - Incorporate feedback from V1
>> ---
>>  rhel/openvswitch.spec.in | 26 ++++++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>>
>> diff --git a/rhel/openvswitch.spec.in b/rhel/openvswitch.spec.in
>> index e510d35..b5b5122 100644
>> --- a/rhel/openvswitch.spec.in
>> +++ b/rhel/openvswitch.spec.in
>> @@ -169,6 +169,32 @@ fi
>>  /sbin/chkconfig --add openvswitch
>>  /sbin/chkconfig openvswitch on
>>
>> +# In some cases a kernel module rpm will have a different minor build
>> +# version than the currently running kernel.  In this case the kernel
>> +# modules will be installed but not to the kernel modules directory
>> +# of the currently running kernel. Check and copy modules if
>> +# necessary.
>> +# This is a bug that has only been found to occur on RHEL 7.2.
>> +if [[ ! -d /lib/modules/$(uname -r)/extra/openvswitch && \
>> +      ! -d /lib/modules/$(uname -r)/weak-updates/openvswitch ]]; then
>>
>
> This check may not be good enough. If we are doing a upgrade of OVS on the
> system (say from 2.7 to 2.9) and previously we had something in
> /lib/modules/$(uname -r)/weak-updates/openvswitch then we are no longer
> going to add the newer version there.
>
>
> But that would assure that a previous attempt to install the openvswitch
> kernel modules rpm had succeeded, which means this system doesn't have the
> bug that is prompting this patch.  That means the newly installed kernel
> modules have been placed in the right kernel modules directory.  So the
> test is correct.
>

If this patch gets accepted and we install OVS 2.8 with this and next try
to upgrade to OVS 2.9, the above logic does not work anymore. Correct?


>
>
>
>
>> +    found="false"
>> +    for i in `ls -t /lib/modules`
>> +    do
>> +        if [ -d /lib/modules/$i/extra/openvswitch ]; then
>>
> This will not help either. There is a possibility of multiple kernels
> installed and it looks like we will choose one randomly. We should only
> look at the path where the current rpm actually installed our files.
>
>
> I tried it on a system with multiple installed kernels and it does work
> because the sort by time will pick the most recently installed kernel
> modules directory which would be the one where the just installed kernel
> modules rpm had gone to (erroneously I should add).
>


>
>
>> +            mkdir -p /lib/modules/$(uname -r)/extra
>> +            cp -r --preserve "/lib/modules/$i/extra/openvswitch" \
>> +            /lib/modules/$(uname -r)/extra
>> +            found="true"
>> +            break
>> +        fi
>> +    done
>> +    if [ "$found" != "true" ]; then
>> +        echo "Error in openvswitch kernel modules installation"
>> +    else
>> +        /usr/sbin/depmod -a
>> +    fi
>> +fi
>> +
>>  %post selinux-policy
>>  /usr/sbin/semodule -i %{_datadir}/selinux/packages/%{name}/openvswitch-custom.pp
>> &> /dev/null || :
>>
>> --
>> 1.8.3.1
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>
>
>
Gregory Rose Jan. 12, 2018, 7:16 p.m. | #6
On 1/12/2018 11:06 AM, Guru Shetty wrote:
>
>
> On 12 January 2018 at 10:16, Gregory Rose <gvrose8192@gmail.com 
> <mailto:gvrose8192@gmail.com>> wrote:
>
>     On 1/12/2018 9:37 AM, Guru Shetty wrote:
>>
>>
>>     On 12 January 2018 at 08:37, Greg Rose <gvrose8192@gmail.com
>>     <mailto:gvrose8192@gmail.com>> wrote:
>>
>>         A bug in RHEL 7.2 has been found in which a customer who
>>         installed
>>         a RHEL 7.2 openvswitch kernel module rpm with a slightly
>>         different
>>         minor build number than the running kernel found that the kernel
>>         modules were installed to the wrong directory.
>>
>>         After the installation the new openvswitch kernel modules were
>>         installed to:
>>         /lib/modules/3.10.0-327.22.2.el7.x86_64/extra/openvswitch
>>
>>         But the running kernel was 3.10.0-327.el7.x86_64 and after the
>>         installation was complete the kernel modules in the installed
>>         directory were not linked to the "weak-updates" directory in
>>         the running kernel.  So a critical bug was encountered in
>>         which the in-tree openvswitch kernel module was loaded instead
>>         of the one the customer explicitly installed with the rpm.
>>
>>         This patch replicates ./extra/openvswitch directory with kernel
>>         modules, if for the currently running kernel there is neither
>>         a ./extra/openvswitch nor ./weak-update/openvswitch directory.
>>
>>         Signed-off-by: Greg Rose <gvrose8192@gmail.com
>>         <mailto:gvrose8192@gmail.com>>
>>
>>         ---
>>
>>         V2 - Incorporate feedback from V1
>>         ---
>>          rhel/openvswitch.spec.in <http://openvswitch.spec.in> | 26
>>         ++++++++++++++++++++++++++
>>          1 file changed, 26 insertions(+)
>>
>>         diff --git a/rhel/openvswitch.spec.in
>>         <http://openvswitch.spec.in> b/rhel/openvswitch.spec.in
>>         <http://openvswitch.spec.in>
>>         index e510d35..b5b5122 100644
>>         --- a/rhel/openvswitch.spec.in <http://openvswitch.spec.in>
>>         +++ b/rhel/openvswitch.spec.in <http://openvswitch.spec.in>
>>         @@ -169,6 +169,32 @@ fi
>>          /sbin/chkconfig --add openvswitch
>>          /sbin/chkconfig openvswitch on
>>
>>         +# In some cases a kernel module rpm will have a different
>>         minor build
>>         +# version than the currently running kernel.  In this case
>>         the kernel
>>         +# modules will be installed but not to the kernel modules
>>         directory
>>         +# of the currently running kernel. Check and copy modules if
>>         +# necessary.
>>         +# This is a bug that has only been found to occur on RHEL 7.2.
>>         +if [[ ! -d /lib/modules/$(uname -r)/extra/openvswitch && \
>>         +      ! -d /lib/modules/$(uname -r)/weak-updates/openvswitch
>>         ]]; then
>>
>>
>>     This check may not be good enough. If we are doing a upgrade of
>>     OVS on the system (say from 2.7 to 2.9) and previously we had
>>     something in /lib/modules/$(uname -r)/weak-updates/openvswitch
>>     then we are no longer going to add the newer version there.
>
>     But that would assure that a previous attempt to install the
>     openvswitch kernel modules rpm had succeeded, which means this
>     system doesn't have the bug that is prompting this patch.  That
>     means the newly installed kernel modules have been placed in the
>     right kernel modules directory.  So the test is correct.
>
>
> If this patch gets accepted and we install OVS 2.8 with this and next 
> try to upgrade to OVS 2.9, the above logic does not work anymore. Correct?
>
>

I haven't tested the upgrade scenario that is true.  I've been in a bit 
of a rush and not able to test every corner case.  I'm not sure what 
happens.

The RHEL 7.2 machine I was testing on is no longer available.  I can 
download  RHEL 7.2 and create a local VM to test it and see.

In the meantime - do you have an alternative suggestion?  Right now it's 
this or let the installation fail silently and the customer will file a 
bug report.

- Greg

>
>>         + found="false"
>>         +    for i in `ls -t /lib/modules`
>>         + do
>>         +        if [ -d /lib/modules/$i/extra/openvswitch ]; then
>>
>>     This will not help either. There is a possibility of multiple
>>     kernels installed and it looks like we will choose one randomly.
>>     We should only look at the path where the current rpm actually
>>     installed our files.
>
>     I tried it on a system with multiple installed kernels and it does
>     work because the sort by time will pick the most recently
>     installed kernel modules directory which would be the one where
>     the just installed kernel modules rpm had gone to (erroneously I
>     should add).
>
>
>
>>         +           mkdir -p /lib/modules/$(uname -r)/extra
>>         +            cp -r --preserve
>>         "/lib/modules/$i/extra/openvswitch" \
>>         +            /lib/modules/$(uname -r)/extra
>>         +            found="true"
>>         +            break
>>         +       fi
>>         +    done
>>         +    if [ "$found" != "true" ]; then
>>         +   echo "Error in openvswitch kernel modules installation"
>>         +    else
>>         +        /usr/sbin/depmod -a
>>         +   fi
>>         +fi
>>         +
>>          %post selinux-policy
>>          /usr/sbin/semodule -i
>>         %{_datadir}/selinux/packages/%{name}/openvswitch-custom.pp &>
>>         /dev/null || :
>>
>>         --
>>         1.8.3.1
>>
>>         _______________________________________________
>>         dev mailing list
>>         dev@openvswitch.org <mailto:dev@openvswitch.org>
>>         https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>         <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
>>
>>
>
>
Flavio Leitner Jan. 12, 2018, 7:39 p.m. | #7
On Fri, Jan 12, 2018 at 08:37:44AM -0800, Greg Rose wrote:
> A bug in RHEL 7.2 has been found in which a customer who installed
> a RHEL 7.2 openvswitch kernel module rpm with a slightly different
> minor build number than the running kernel found that the kernel
> modules were installed to the wrong directory.


I think this needs to be reworded because I thought something
has changed in 7.2 to introduce this problem when it seems
a bug in the way kmod is being handled in kmod package reproduces
in RHEL 7.2.

> 
> After the installation the new openvswitch kernel modules were
> installed to:
> /lib/modules/3.10.0-327.22.2.el7.x86_64/extra/openvswitch
> 
> But the running kernel was 3.10.0-327.el7.x86_64 and after the
> installation was complete the kernel modules in the installed
> directory were not linked to the "weak-updates" directory in
> the running kernel.  So a critical bug was encountered in
> which the in-tree openvswitch kernel module was loaded instead
> of the one the customer explicitly installed with the rpm.
> 
> This patch replicates ./extra/openvswitch directory with kernel
> modules, if for the currently running kernel there is neither
> a ./extra/openvswitch nor ./weak-update/openvswitch directory.

I think the same comment I did for RHEL-6 works for RHEL-7,
all you need is to run /sbin/weak-modules --add-module
in the %post section. 

Don't forget to run again in %postun to remove the symlinks
as the module will not exist.

fbl

> 
> Signed-off-by: Greg Rose <gvrose8192@gmail.com>
> 
> ---
> 
> V2 - Incorporate feedback from V1
> ---
>  rhel/openvswitch.spec.in | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/rhel/openvswitch.spec.in b/rhel/openvswitch.spec.in
> index e510d35..b5b5122 100644
> --- a/rhel/openvswitch.spec.in
> +++ b/rhel/openvswitch.spec.in
> @@ -169,6 +169,32 @@ fi
>  /sbin/chkconfig --add openvswitch
>  /sbin/chkconfig openvswitch on
>  
> +# In some cases a kernel module rpm will have a different minor build
> +# version than the currently running kernel.  In this case the kernel
> +# modules will be installed but not to the kernel modules directory
> +# of the currently running kernel. Check and copy modules if
> +# necessary.
> +# This is a bug that has only been found to occur on RHEL 7.2.
> +if [[ ! -d /lib/modules/$(uname -r)/extra/openvswitch && \
> +      ! -d /lib/modules/$(uname -r)/weak-updates/openvswitch ]]; then
> +    found="false"
> +    for i in `ls -t /lib/modules`
> +    do
> +        if [ -d /lib/modules/$i/extra/openvswitch ]; then
> +            mkdir -p /lib/modules/$(uname -r)/extra
> +            cp -r --preserve "/lib/modules/$i/extra/openvswitch" \
> +            /lib/modules/$(uname -r)/extra
> +            found="true"
> +            break
> +        fi
> +    done
> +    if [ "$found" != "true" ]; then
> +        echo "Error in openvswitch kernel modules installation"
> +    else
> +        /usr/sbin/depmod -a
> +    fi
> +fi
> +
>  %post selinux-policy
>  /usr/sbin/semodule -i %{_datadir}/selinux/packages/%{name}/openvswitch-custom.pp &> /dev/null || :
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Gregory Rose Jan. 12, 2018, 7:43 p.m. | #8
On 1/12/2018 11:39 AM, Flavio Leitner wrote:
> On Fri, Jan 12, 2018 at 08:37:44AM -0800, Greg Rose wrote:
>> A bug in RHEL 7.2 has been found in which a customer who installed
>> a RHEL 7.2 openvswitch kernel module rpm with a slightly different
>> minor build number than the running kernel found that the kernel
>> modules were installed to the wrong directory.
>
> I think this needs to be reworded because I thought something
> has changed in 7.2 to introduce this problem when it seems
> a bug in the way kmod is being handled in kmod package reproduces
> in RHEL 7.2.

OK, I'll fix that up.

>> After the installation the new openvswitch kernel modules were
>> installed to:
>> /lib/modules/3.10.0-327.22.2.el7.x86_64/extra/openvswitch
>>
>> But the running kernel was 3.10.0-327.el7.x86_64 and after the
>> installation was complete the kernel modules in the installed
>> directory were not linked to the "weak-updates" directory in
>> the running kernel.  So a critical bug was encountered in
>> which the in-tree openvswitch kernel module was loaded instead
>> of the one the customer explicitly installed with the rpm.
>>
>> This patch replicates ./extra/openvswitch directory with kernel
>> modules, if for the currently running kernel there is neither
>> a ./extra/openvswitch nor ./weak-update/openvswitch directory.
> I think the same comment I did for RHEL-6 works for RHEL-7,
> all you need is to run /sbin/weak-modules --add-module
> in the %post section.
>
> Don't forget to run again in %postun to remove the symlinks
> as the module will not exist.

Yes, now that I *know* about /sbin/weak-updates many of the problems that
we've been talking about with this patch are resolved in a much more elegant
and system friendly manner.

Thank you so much for the pointer!

- Greg

>
> fbl
>
>> Signed-off-by: Greg Rose <gvrose8192@gmail.com>
>>
>> ---
>>
>> V2 - Incorporate feedback from V1
>> ---
>>   rhel/openvswitch.spec.in | 26 ++++++++++++++++++++++++++
>>   1 file changed, 26 insertions(+)
>>
>> diff --git a/rhel/openvswitch.spec.in b/rhel/openvswitch.spec.in
>> index e510d35..b5b5122 100644
>> --- a/rhel/openvswitch.spec.in
>> +++ b/rhel/openvswitch.spec.in
>> @@ -169,6 +169,32 @@ fi
>>   /sbin/chkconfig --add openvswitch
>>   /sbin/chkconfig openvswitch on
>>   
>> +# In some cases a kernel module rpm will have a different minor build
>> +# version than the currently running kernel.  In this case the kernel
>> +# modules will be installed but not to the kernel modules directory
>> +# of the currently running kernel. Check and copy modules if
>> +# necessary.
>> +# This is a bug that has only been found to occur on RHEL 7.2.
>> +if [[ ! -d /lib/modules/$(uname -r)/extra/openvswitch && \
>> +      ! -d /lib/modules/$(uname -r)/weak-updates/openvswitch ]]; then
>> +    found="false"
>> +    for i in `ls -t /lib/modules`
>> +    do
>> +        if [ -d /lib/modules/$i/extra/openvswitch ]; then
>> +            mkdir -p /lib/modules/$(uname -r)/extra
>> +            cp -r --preserve "/lib/modules/$i/extra/openvswitch" \
>> +            /lib/modules/$(uname -r)/extra
>> +            found="true"
>> +            break
>> +        fi
>> +    done
>> +    if [ "$found" != "true" ]; then
>> +        echo "Error in openvswitch kernel modules installation"
>> +    else
>> +        /usr/sbin/depmod -a
>> +    fi
>> +fi
>> +
>>   %post selinux-policy
>>   /usr/sbin/semodule -i %{_datadir}/selinux/packages/%{name}/openvswitch-custom.pp &> /dev/null || :
>>   
>> -- 
>> 1.8.3.1
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Patch

diff --git a/rhel/openvswitch.spec.in b/rhel/openvswitch.spec.in
index e510d35..b5b5122 100644
--- a/rhel/openvswitch.spec.in
+++ b/rhel/openvswitch.spec.in
@@ -169,6 +169,32 @@  fi
 /sbin/chkconfig --add openvswitch
 /sbin/chkconfig openvswitch on
 
+# In some cases a kernel module rpm will have a different minor build
+# version than the currently running kernel.  In this case the kernel
+# modules will be installed but not to the kernel modules directory
+# of the currently running kernel. Check and copy modules if
+# necessary.
+# This is a bug that has only been found to occur on RHEL 7.2.
+if [[ ! -d /lib/modules/$(uname -r)/extra/openvswitch && \
+      ! -d /lib/modules/$(uname -r)/weak-updates/openvswitch ]]; then
+    found="false"
+    for i in `ls -t /lib/modules`
+    do
+        if [ -d /lib/modules/$i/extra/openvswitch ]; then
+            mkdir -p /lib/modules/$(uname -r)/extra
+            cp -r --preserve "/lib/modules/$i/extra/openvswitch" \
+            /lib/modules/$(uname -r)/extra
+            found="true"
+            break
+        fi
+    done
+    if [ "$found" != "true" ]; then
+        echo "Error in openvswitch kernel modules installation"
+    else
+        /usr/sbin/depmod -a
+    fi
+fi
+
 %post selinux-policy
 /usr/sbin/semodule -i %{_datadir}/selinux/packages/%{name}/openvswitch-custom.pp &> /dev/null || :