diff mbox series

[ovs-dev,v1] rhel: retain OVS_CTL_OPTS for systemd service files

Message ID 1536954563-103152-1-git-send-email-martinxu9.ovs@gmail.com
State Not Applicable
Headers show
Series [ovs-dev,v1] rhel: retain OVS_CTL_OPTS for systemd service files | expand

Commit Message

Martin Xu Sept. 14, 2018, 7:49 p.m. UTC
OVS init.d script calls ovs-ctl with $OVS_CTL_OPTS defined in the
config file. This variable is replaced by OPTIONS in systemd service
files. This patch addes $OVS_CTL_OPTS back to be passed along with $OPTIONS
for backward compatibility.

VMware-BZ: #2036847

Signed-off-by: Martin Xu <martinxu9.ovs@gmail.com>
CC: Aaron Conole <aconole@redhat.com>
---
 rhel/usr_lib_systemd_system_ovs-vswitchd.service.in           | 4 ++--
 rhel/usr_lib_systemd_system_ovsdb-server.service              | 4 ++--
 rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template | 5 +++++
 3 files changed, 9 insertions(+), 4 deletions(-)

Comments

Gregory Rose Jan. 22, 2019, 10:08 p.m. UTC | #1
Aaron,

This patch seems to have fallen through the cracks.  Would you mind 
giving this a review?  Martin is no
longer with VMware so I will be following up on this patch.

Thanks,

- Greg


On 9/14/2018 12:49 PM, Martin Xu wrote:
> OVS init.d script calls ovs-ctl with $OVS_CTL_OPTS defined in the
> config file. This variable is replaced by OPTIONS in systemd service
> files. This patch addes $OVS_CTL_OPTS back to be passed along with $OPTIONS
> for backward compatibility.
>
> VMware-BZ: #2036847
>
> Signed-off-by: Martin Xu <martinxu9.ovs@gmail.com>
> CC: Aaron Conole <aconole@redhat.com>
> ---
>   rhel/usr_lib_systemd_system_ovs-vswitchd.service.in           | 4 ++--
>   rhel/usr_lib_systemd_system_ovsdb-server.service              | 4 ++--
>   rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template | 5 +++++
>   3 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
> index 11b34c6..e4dbdf3 100644
> --- a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
> +++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
> @@ -21,10 +21,10 @@ ExecStartPre=-/usr/bin/chmod 0775 /dev/hugepages
>   ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
>             --no-ovsdb-server --no-monitor --system-id=random \
>             ${OVSUSER} \
> -          start $OPTIONS
> +          start $OPTIONS $OVS_CTL_OPTS
>   ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server stop
>   ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server \
>             --no-monitor --system-id=random \
>             ${OVSUSER} \
> -          restart $OPTIONS
> +          restart $OPTIONS $OVS_CTL_OPTS
>   TimeoutSec=300
> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service b/rhel/usr_lib_systemd_system_ovsdb-server.service
> index 70da1ec..09f946b 100644
> --- a/rhel/usr_lib_systemd_system_ovsdb-server.service
> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
> @@ -16,10 +16,10 @@ EnvironmentFile=-/run/openvswitch/useropts
>   ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
>             --no-ovs-vswitchd --no-monitor --system-id=random \
>             ${OVSUSER} \
> -          start $OPTIONS
> +          start $OPTIONS $OVS_CTL_OPTS
>   ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd stop
>   ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd \
>              ${OVSUSER} \
> -           --no-monitor restart $OPTIONS
> +           --no-monitor restart $OPTIONS $OVS_CTL_OPTS
>   RuntimeDirectory=openvswitch
>   RuntimeDirectoryMode=0755
> diff --git a/rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template b/rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template
> index 9364454..0ce5b6b 100644
> --- a/rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template
> +++ b/rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template
> @@ -23,5 +23,10 @@
>   #
>   OPTIONS=""
>   
> +# OVS_CTL_OPTS: Extra options to pass along with OPTIONS to ovs-ctl.
> +# This flag is retained for backward compatibility. For example,
> +# user can specify --ovs-vswitchd-wrapper=valgrind.
> +# OVS_CTL_OPTS=
> +
>   # Uncomment and set the OVS User/Group value
>   #OVS_USER_ID="openvswitch:openvswitch"
Gregory Rose Jan. 30, 2019, 7:06 p.m. UTC | #2
Pinging Aaron once again?

- Greg

On 1/22/2019 2:08 PM, Gregory Rose wrote:
>
> Aaron,
>
> This patch seems to have fallen through the cracks.  Would you mind 
> giving this a review?  Martin is no
> longer with VMware so I will be following up on this patch.
>
> Thanks,
>
> - Greg
>
>
> On 9/14/2018 12:49 PM, Martin Xu wrote:
>> OVS init.d script calls ovs-ctl with $OVS_CTL_OPTS defined in the
>> config file. This variable is replaced by OPTIONS in systemd service
>> files. This patch addes $OVS_CTL_OPTS back to be passed along with 
>> $OPTIONS
>> for backward compatibility.
>>
>> VMware-BZ: #2036847
>>
>> Signed-off-by: Martin Xu <martinxu9.ovs@gmail.com>
>> CC: Aaron Conole <aconole@redhat.com>
>> ---
>>   rhel/usr_lib_systemd_system_ovs-vswitchd.service.in | 4 ++--
>>   rhel/usr_lib_systemd_system_ovsdb-server.service | 4 ++--
>>   rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template | 5 
>> +++++
>>   3 files changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in 
>> b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
>> index 11b34c6..e4dbdf3 100644
>> --- a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
>> +++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
>> @@ -21,10 +21,10 @@ ExecStartPre=-/usr/bin/chmod 0775 /dev/hugepages
>>   ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
>>             --no-ovsdb-server --no-monitor --system-id=random \
>>             ${OVSUSER} \
>> -          start $OPTIONS
>> +          start $OPTIONS $OVS_CTL_OPTS
>>   ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server stop
>>   ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server \
>>             --no-monitor --system-id=random \
>>             ${OVSUSER} \
>> -          restart $OPTIONS
>> +          restart $OPTIONS $OVS_CTL_OPTS
>>   TimeoutSec=300
>> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
>> b/rhel/usr_lib_systemd_system_ovsdb-server.service
>> index 70da1ec..09f946b 100644
>> --- a/rhel/usr_lib_systemd_system_ovsdb-server.service
>> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
>> @@ -16,10 +16,10 @@ EnvironmentFile=-/run/openvswitch/useropts
>>   ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
>>             --no-ovs-vswitchd --no-monitor --system-id=random \
>>             ${OVSUSER} \
>> -          start $OPTIONS
>> +          start $OPTIONS $OVS_CTL_OPTS
>>   ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd stop
>>   ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd \
>>              ${OVSUSER} \
>> -           --no-monitor restart $OPTIONS
>> +           --no-monitor restart $OPTIONS $OVS_CTL_OPTS
>>   RuntimeDirectory=openvswitch
>>   RuntimeDirectoryMode=0755
>> diff --git 
>> a/rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template 
>> b/rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template
>> index 9364454..0ce5b6b 100644
>> --- a/rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template
>> +++ b/rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template
>> @@ -23,5 +23,10 @@
>>   #
>>   OPTIONS=""
>>   +# OVS_CTL_OPTS: Extra options to pass along with OPTIONS to ovs-ctl.
>> +# This flag is retained for backward compatibility. For example,
>> +# user can specify --ovs-vswitchd-wrapper=valgrind.
>> +# OVS_CTL_OPTS=
>> +
>>   # Uncomment and set the OVS User/Group value
>>   #OVS_USER_ID="openvswitch:openvswitch"
>
Aaron Conole Jan. 30, 2019, 7:19 p.m. UTC | #3
Gregory Rose <gvrose8192@gmail.com> writes:

> Pinging Aaron once again?

Whoops - completely missed this.  I'll take a look.

> - Greg
>
> On 1/22/2019 2:08 PM, Gregory Rose wrote:
>>
>> Aaron,
>>
>> This patch seems to have fallen through the cracks.  Would you mind
>> giving this a review?  Martin is no
>> longer with VMware so I will be following up on this patch.
>>
>> Thanks,
>>
>> - Greg
>>
>>
>> On 9/14/2018 12:49 PM, Martin Xu wrote:
>>> OVS init.d script calls ovs-ctl with $OVS_CTL_OPTS defined in the
>>> config file. This variable is replaced by OPTIONS in systemd service
>>> files. This patch addes $OVS_CTL_OPTS back to be passed along with
>>> $OPTIONS
>>> for backward compatibility.
>>>
>>> VMware-BZ: #2036847
>>>
>>> Signed-off-by: Martin Xu <martinxu9.ovs@gmail.com>
>>> CC: Aaron Conole <aconole@redhat.com>
>>> ---
>>>   rhel/usr_lib_systemd_system_ovs-vswitchd.service.in | 4 ++--
>>>   rhel/usr_lib_systemd_system_ovsdb-server.service | 4 ++--
>>>   rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template | 5
>>> +++++
>>>   3 files changed, 9 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
>>> b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
>>> index 11b34c6..e4dbdf3 100644
>>> --- a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
>>> +++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
>>> @@ -21,10 +21,10 @@ ExecStartPre=-/usr/bin/chmod 0775 /dev/hugepages
>>>   ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
>>>             --no-ovsdb-server --no-monitor --system-id=random \
>>>             ${OVSUSER} \
>>> -          start $OPTIONS
>>> +          start $OPTIONS $OVS_CTL_OPTS
>>>   ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server stop
>>>   ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server \
>>>             --no-monitor --system-id=random \
>>>             ${OVSUSER} \
>>> -          restart $OPTIONS
>>> +          restart $OPTIONS $OVS_CTL_OPTS
>>>   TimeoutSec=300
>>> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service
>>> b/rhel/usr_lib_systemd_system_ovsdb-server.service
>>> index 70da1ec..09f946b 100644
>>> --- a/rhel/usr_lib_systemd_system_ovsdb-server.service
>>> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
>>> @@ -16,10 +16,10 @@ EnvironmentFile=-/run/openvswitch/useropts
>>>   ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
>>>             --no-ovs-vswitchd --no-monitor --system-id=random \
>>>             ${OVSUSER} \
>>> -          start $OPTIONS
>>> +          start $OPTIONS $OVS_CTL_OPTS
>>>   ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd stop
>>>   ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd \
>>>              ${OVSUSER} \
>>> -           --no-monitor restart $OPTIONS
>>> +           --no-monitor restart $OPTIONS $OVS_CTL_OPTS
>>>   RuntimeDirectory=openvswitch
>>>   RuntimeDirectoryMode=0755
>>> diff --git
>>> a/rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template
>>> b/rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template
>>> index 9364454..0ce5b6b 100644
>>> --- a/rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template
>>> +++ b/rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template
>>> @@ -23,5 +23,10 @@
>>>   #
>>>   OPTIONS=""
>>>   +# OVS_CTL_OPTS: Extra options to pass along with OPTIONS to ovs-ctl.
>>> +# This flag is retained for backward compatibility. For example,
>>> +# user can specify --ovs-vswitchd-wrapper=valgrind.
>>> +# OVS_CTL_OPTS=
>>> +
>>>   # Uncomment and set the OVS User/Group value
>>>   #OVS_USER_ID="openvswitch:openvswitch"
>>
Aaron Conole Jan. 31, 2019, 6:38 p.m. UTC | #4
Martin Xu <martinxu9.ovs@gmail.com> writes:

> OVS init.d script calls ovs-ctl with $OVS_CTL_OPTS defined in the
> config file. This variable is replaced by OPTIONS in systemd service
> files. This patch addes $OVS_CTL_OPTS back to be passed along with $OPTIONS
> for backward compatibility.
>
> VMware-BZ: #2036847
>
> Signed-off-by: Martin Xu <martinxu9.ovs@gmail.com>
> CC: Aaron Conole <aconole@redhat.com>
> ---

I'm not sure why there should be two variables in the sysconfig file for
this.  The following would preserve the old and not introduce a new
variable (I think.. it's completely untested).  I guess this is because
the debian-distro openvswitch-switch.template file doesn't match the
rhel-distro template file, and we want to make a common set of systemd
scripts? Otherwise I don't see what the purpose is - what is the
migration path that this is addressing?

---
diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service b/rhel/usr_lib_systemd_system_ovsdb-server.service
index 09f946bb1..660ec75ef 100644
--- a/rhel/usr_lib_systemd_system_ovsdb-server.service
+++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
@@ -12,6 +12,7 @@ EnvironmentFile=/etc/openvswitch/default.conf
 EnvironmentFile=-/etc/sysconfig/openvswitch
 ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch /var/log/openvswitch
 ExecStartPre=/bin/sh -c 'rm -f /run/openvswitch/useropts; if [ "$${OVS_USER_ID/:*/}" != "root" ]; then /usr/bin/echo "OVSUSER=--ovs-user=${OVS_USER_ID}" > /run/openvswitch/useropts; fi'
+ExecStartPre=/bin/sh -c 'if [ "${OVS_CTL_OPTS}" != "" -a "${OPTIONS}" == "" ]; then /usr/bin/echo "OPTIONS=\"${OPTIONS} ${OVS_CTL_OPTS}\"" >> /run/openvswitch/useropts; fi'
 EnvironmentFile=-/run/openvswitch/useropts
 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
           --no-ovs-vswitchd --no-monitor --system-id=random \
---
Gregory Rose Feb. 5, 2019, 9:19 p.m. UTC | #5
On 1/31/2019 10:38 AM, Aaron Conole wrote:
> Martin Xu <martinxu9.ovs@gmail.com> writes:
>
>> OVS init.d script calls ovs-ctl with $OVS_CTL_OPTS defined in the
>> config file. This variable is replaced by OPTIONS in systemd service
>> files. This patch addes $OVS_CTL_OPTS back to be passed along with $OPTIONS
>> for backward compatibility.
>>
>> VMware-BZ: #2036847
>>
>> Signed-off-by: Martin Xu <martinxu9.ovs@gmail.com>
>> CC: Aaron Conole <aconole@redhat.com>
>> ---
> I'm not sure why there should be two variables in the sysconfig file for
> this.  The following would preserve the old and not introduce a new
> variable (I think.. it's completely untested).  I guess this is because
> the debian-distro openvswitch-switch.template file doesn't match the
> rhel-distro template file, and we want to make a common set of systemd
> scripts? Otherwise I don't see what the purpose is - what is the
> migration path that this is addressing?

Aaron,

I owe you a response on this and will get to it but some fires need 
putting out at the moment.

Thanks,

- Greg

>
> ---
> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service b/rhel/usr_lib_systemd_system_ovsdb-server.service
> index 09f946bb1..660ec75ef 100644
> --- a/rhel/usr_lib_systemd_system_ovsdb-server.service
> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
> @@ -12,6 +12,7 @@ EnvironmentFile=/etc/openvswitch/default.conf
>   EnvironmentFile=-/etc/sysconfig/openvswitch
>   ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch /var/log/openvswitch
>   ExecStartPre=/bin/sh -c 'rm -f /run/openvswitch/useropts; if [ "$${OVS_USER_ID/:*/}" != "root" ]; then /usr/bin/echo "OVSUSER=--ovs-user=${OVS_USER_ID}" > /run/openvswitch/useropts; fi'
> +ExecStartPre=/bin/sh -c 'if [ "${OVS_CTL_OPTS}" != "" -a "${OPTIONS}" == "" ]; then /usr/bin/echo "OPTIONS=\"${OPTIONS} ${OVS_CTL_OPTS}\"" >> /run/openvswitch/useropts; fi'
>   EnvironmentFile=-/run/openvswitch/useropts
>   ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
>             --no-ovs-vswitchd --no-monitor --system-id=random \
> ---
Ilya Maximets Sept. 9, 2020, 12:51 p.m. UTC | #6
> On 1/31/2019 10:38 AM, Aaron Conole wrote:
>> Martin Xu <martinxu9.ovs at gmail.com> writes:
>>
>>> OVS init.d script calls ovs-ctl with $OVS_CTL_OPTS defined in the
>>> config file. This variable is replaced by OPTIONS in systemd service
>>> files. This patch addes $OVS_CTL_OPTS back to be passed along with $OPTIONS
>>> for backward compatibility.
>>>
>>> VMware-BZ: #2036847
>>>
>>> Signed-off-by: Martin Xu <martinxu9.ovs at gmail.com>
>>> CC: Aaron Conole <aconole at redhat.com>
>>> ---
>> I'm not sure why there should be two variables in the sysconfig file for
>> this.  The following would preserve the old and not introduce a new
>> variable (I think.. it's completely untested).  I guess this is because
>> the debian-distro openvswitch-switch.template file doesn't match the
>> rhel-distro template file, and we want to make a common set of systemd
>> scripts? Otherwise I don't see what the purpose is - what is the
>> migration path that this is addressing?
> 
> Aaron,
> 
> I owe you a response on this and will get to it but some fires need 
> putting out at the moment.

Hi, Greg, Martin.

I'm looking through old patches after the patchwork cleanup and this
one seems to be never applied.  I'm assuming that it's not needed
anymore, however, I'd like to have some comment on it if possible.

For now marking it as 'Not Applicable'.  Please, resubmit in case it's
still needed.

Best regards, Ilya Maximets.

> 
> Thanks,
> 
> - Greg
> 
>>
>> ---
>> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service b/rhel/usr_lib_systemd_system_ovsdb-server.service
>> index 09f946bb1..660ec75ef 100644
>> --- a/rhel/usr_lib_systemd_system_ovsdb-server.service
>> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
>> @@ -12,6 +12,7 @@ EnvironmentFile=/etc/openvswitch/default.conf
>>   EnvironmentFile=-/etc/sysconfig/openvswitch
>>   ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch /var/log/openvswitch
>>   ExecStartPre=/bin/sh -c 'rm -f /run/openvswitch/useropts; if [ "$${OVS_USER_ID/:*/}" != "root" ]; then /usr/bin/echo "OVSUSER=--ovs-user=${OVS_USER_ID}" > /run/openvswitch/useropts; fi'
>> +ExecStartPre=/bin/sh -c 'if [ "${OVS_CTL_OPTS}" != "" -a "${OPTIONS}" == "" ]; then /usr/bin/echo "OPTIONS=\"${OPTIONS} ${OVS_CTL_OPTS}\"" >> /run/openvswitch/useropts; fi'
>>   EnvironmentFile=-/run/openvswitch/useropts
>>   ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
>>             --no-ovs-vswitchd --no-monitor --system-id=random \
>> ---
Gregory Rose Sept. 9, 2020, 3:39 p.m. UTC | #7
On 9/9/2020 5:51 AM, Ilya Maximets wrote:
>> On 1/31/2019 10:38 AM, Aaron Conole wrote:
>>> Martin Xu <martinxu9.ovs at gmail.com> writes:
>>>
>>>> OVS init.d script calls ovs-ctl with $OVS_CTL_OPTS defined in the
>>>> config file. This variable is replaced by OPTIONS in systemd service
>>>> files. This patch addes $OVS_CTL_OPTS back to be passed along with $OPTIONS
>>>> for backward compatibility.
>>>>
>>>> VMware-BZ: #2036847
>>>>
>>>> Signed-off-by: Martin Xu <martinxu9.ovs at gmail.com>
>>>> CC: Aaron Conole <aconole at redhat.com>
>>>> ---
>>> I'm not sure why there should be two variables in the sysconfig file for
>>> this.  The following would preserve the old and not introduce a new
>>> variable (I think.. it's completely untested).  I guess this is because
>>> the debian-distro openvswitch-switch.template file doesn't match the
>>> rhel-distro template file, and we want to make a common set of systemd
>>> scripts? Otherwise I don't see what the purpose is - what is the
>>> migration path that this is addressing?
>>
>> Aaron,
>>
>> I owe you a response on this and will get to it but some fires need
>> putting out at the moment.
> 
> Hi, Greg, Martin.
> 
> I'm looking through old patches after the patchwork cleanup and this
> one seems to be never applied.  I'm assuming that it's not needed
> anymore, however, I'd like to have some comment on it if possible.
> 
> For now marking it as 'Not Applicable'.  Please, resubmit in case it's
> still needed.

It appears to be no longer required so this is the right thing to do.

Thanks,

- Greg


> 
> Best regards, Ilya Maximets.
> 
>>
>> Thanks,
>>
>> - Greg
>>
>>>
>>> ---
>>> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service b/rhel/usr_lib_systemd_system_ovsdb-server.service
>>> index 09f946bb1..660ec75ef 100644
>>> --- a/rhel/usr_lib_systemd_system_ovsdb-server.service
>>> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
>>> @@ -12,6 +12,7 @@ EnvironmentFile=/etc/openvswitch/default.conf
>>>    EnvironmentFile=-/etc/sysconfig/openvswitch
>>>    ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch /var/log/openvswitch
>>>    ExecStartPre=/bin/sh -c 'rm -f /run/openvswitch/useropts; if [ "$${OVS_USER_ID/:*/}" != "root" ]; then /usr/bin/echo "OVSUSER=--ovs-user=${OVS_USER_ID}" > /run/openvswitch/useropts; fi'
>>> +ExecStartPre=/bin/sh -c 'if [ "${OVS_CTL_OPTS}" != "" -a "${OPTIONS}" == "" ]; then /usr/bin/echo "OPTIONS=\"${OPTIONS} ${OVS_CTL_OPTS}\"" >> /run/openvswitch/useropts; fi'
>>>    EnvironmentFile=-/run/openvswitch/useropts
>>>    ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
>>>              --no-ovs-vswitchd --no-monitor --system-id=random \
>>> ---
Ilya Maximets Sept. 9, 2020, 5:53 p.m. UTC | #8
On 9/9/20 5:39 PM, Gregory Rose wrote:
> 
> 
> On 9/9/2020 5:51 AM, Ilya Maximets wrote:
>>> On 1/31/2019 10:38 AM, Aaron Conole wrote:
>>>> Martin Xu <martinxu9.ovs at gmail.com> writes:
>>>>
>>>>> OVS init.d script calls ovs-ctl with $OVS_CTL_OPTS defined in the
>>>>> config file. This variable is replaced by OPTIONS in systemd service
>>>>> files. This patch addes $OVS_CTL_OPTS back to be passed along with $OPTIONS
>>>>> for backward compatibility.
>>>>>
>>>>> VMware-BZ: #2036847
>>>>>
>>>>> Signed-off-by: Martin Xu <martinxu9.ovs at gmail.com>
>>>>> CC: Aaron Conole <aconole at redhat.com>
>>>>> ---
>>>> I'm not sure why there should be two variables in the sysconfig file for
>>>> this.  The following would preserve the old and not introduce a new
>>>> variable (I think.. it's completely untested).  I guess this is because
>>>> the debian-distro openvswitch-switch.template file doesn't match the
>>>> rhel-distro template file, and we want to make a common set of systemd
>>>> scripts? Otherwise I don't see what the purpose is - what is the
>>>> migration path that this is addressing?
>>>
>>> Aaron,
>>>
>>> I owe you a response on this and will get to it but some fires need
>>> putting out at the moment.
>>
>> Hi, Greg, Martin.
>>
>> I'm looking through old patches after the patchwork cleanup and this
>> one seems to be never applied.  I'm assuming that it's not needed
>> anymore, however, I'd like to have some comment on it if possible.
>>
>> For now marking it as 'Not Applicable'.  Please, resubmit in case it's
>> still needed.
> 
> It appears to be no longer required so this is the right thing to do.

Great!  Thanks for the information.

> 
> Thanks,
> 
> - Greg
> 
> 
>>
>> Best regards, Ilya Maximets.
>>
>>>
>>> Thanks,
>>>
>>> - Greg
>>>
>>>>
>>>> ---
>>>> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service b/rhel/usr_lib_systemd_system_ovsdb-server.service
>>>> index 09f946bb1..660ec75ef 100644
>>>> --- a/rhel/usr_lib_systemd_system_ovsdb-server.service
>>>> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
>>>> @@ -12,6 +12,7 @@ EnvironmentFile=/etc/openvswitch/default.conf
>>>>    EnvironmentFile=-/etc/sysconfig/openvswitch
>>>>    ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch /var/log/openvswitch
>>>>    ExecStartPre=/bin/sh -c 'rm -f /run/openvswitch/useropts; if [ "$${OVS_USER_ID/:*/}" != "root" ]; then /usr/bin/echo "OVSUSER=--ovs-user=${OVS_USER_ID}" > /run/openvswitch/useropts; fi'
>>>> +ExecStartPre=/bin/sh -c 'if [ "${OVS_CTL_OPTS}" != "" -a "${OPTIONS}" == "" ]; then /usr/bin/echo "OPTIONS=\"${OPTIONS} ${OVS_CTL_OPTS}\"" >> /run/openvswitch/useropts; fi'
>>>>    EnvironmentFile=-/run/openvswitch/useropts
>>>>    ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
>>>>              --no-ovs-vswitchd --no-monitor --system-id=random \
>>>> ---
diff mbox series

Patch

diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
index 11b34c6..e4dbdf3 100644
--- a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
+++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
@@ -21,10 +21,10 @@  ExecStartPre=-/usr/bin/chmod 0775 /dev/hugepages
 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
           --no-ovsdb-server --no-monitor --system-id=random \
           ${OVSUSER} \
-          start $OPTIONS
+          start $OPTIONS $OVS_CTL_OPTS
 ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server stop
 ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server \
           --no-monitor --system-id=random \
           ${OVSUSER} \
-          restart $OPTIONS
+          restart $OPTIONS $OVS_CTL_OPTS
 TimeoutSec=300
diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service b/rhel/usr_lib_systemd_system_ovsdb-server.service
index 70da1ec..09f946b 100644
--- a/rhel/usr_lib_systemd_system_ovsdb-server.service
+++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
@@ -16,10 +16,10 @@  EnvironmentFile=-/run/openvswitch/useropts
 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
           --no-ovs-vswitchd --no-monitor --system-id=random \
           ${OVSUSER} \
-          start $OPTIONS
+          start $OPTIONS $OVS_CTL_OPTS
 ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd stop
 ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd \
            ${OVSUSER} \
-           --no-monitor restart $OPTIONS
+           --no-monitor restart $OPTIONS $OVS_CTL_OPTS
 RuntimeDirectory=openvswitch
 RuntimeDirectoryMode=0755
diff --git a/rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template b/rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template
index 9364454..0ce5b6b 100644
--- a/rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template
+++ b/rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template
@@ -23,5 +23,10 @@ 
 #
 OPTIONS=""
 
+# OVS_CTL_OPTS: Extra options to pass along with OPTIONS to ovs-ctl.
+# This flag is retained for backward compatibility. For example,
+# user can specify --ovs-vswitchd-wrapper=valgrind.
+# OVS_CTL_OPTS=
+
 # Uncomment and set the OVS User/Group value
 #OVS_USER_ID="openvswitch:openvswitch"