Flavio Leitner <f...@sysclose.org> writes:
> On Fri, Jul 01, 2016 at 05:26:22PM -0400, Aaron Conole wrote:
>> Currently, openvswitch.service calls out to start
>> openvswitch-nonetwork.service. However, openvswitch-nonetwork.service
>> is better called ovsdb-server, since that is truly nonetwork. This
>> commit does make the file a bit of a misnomer - currently the
>> ovsdb-server service will start the ovs-vswitchd service as well. A
>> future commit will clean this up.
>
>
> For the record, the 'nonetwork' means that the systemd service
> doesn't depend on the 'network' target to support cases where
> OVS builds the network needed prior the 'network' target.
>
> However, it doesn't make sense if we move to 1:1 mapping
> between services and daemons which looks better nowadays.
Yeah, I know. This patch merely does the rename. So it's actually a
bit misleading because it says ovsdb-server, but it's really everything.
Thanks so much for the review, Flavio!
>>
>> Signed-off-by: Aaron Conole <acon...@redhat.com>
>> ---
>> rhel/automake.mk | 2 +-
>> rhel/etc_sysconfig_network-scripts_ifdown-ovs | 14
>> +++++++-------
>> rhel/etc_sysconfig_network-scripts_ifup-ovs | 14
>> +++++++-------
>> rhel/openvswitch-fedora.spec.in | 4 ++--
>> rhel/usr_lib_systemd_system_openvswitch-nonetwork.service | 15
>> ---------------
>> rhel/usr_lib_systemd_system_openvswitch.service | 4 ++--
>> rhel/usr_lib_systemd_system_ovsdb-server.service | 15
>> +++++++++++++++
>> 7 files changed, 34 insertions(+), 34 deletions(-)
>> delete mode 100644 rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
>> create mode 100644 rhel/usr_lib_systemd_system_ovsdb-server.service
>>
>> diff --git a/rhel/automake.mk b/rhel/automake.mk
>> index dc30715..7907a87 100644
>> --- a/rhel/automake.mk
>> +++ b/rhel/automake.mk
>> @@ -26,7 +26,7 @@ EXTRA_DIST += \
>> rhel/usr_share_openvswitch_scripts_sysconfig.template \
>> rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
>> rhel/usr_lib_systemd_system_openvswitch.service \
>> - rhel/usr_lib_systemd_system_openvswitch-nonetwork.service \
>> + rhel/usr_lib_systemd_system_ovsdb-server.service \
>> rhel/usr_lib_systemd_system_ovn-controller.service \
>> rhel/usr_lib_systemd_system_ovn-controller-vtep.service \
>> rhel/usr_lib_systemd_system_ovn-northd.service
>> diff --git a/rhel/etc_sysconfig_network-scripts_ifdown-ovs
>> b/rhel/etc_sysconfig_network-scripts_ifdown-ovs
>> index 46b6ca5..b57aebf 100755
>> --- a/rhel/etc_sysconfig_network-scripts_ifdown-ovs
>> +++ b/rhel/etc_sysconfig_network-scripts_ifdown-ovs
>> @@ -34,15 +34,15 @@ if [ ! -x ${OTHERSCRIPT} ]; then
>> OTHERSCRIPT="/etc/sysconfig/network-scripts/ifdown-eth"
>> fi
>>
>> -SERVICE_UNIT=/usr/lib/systemd/system/openvswitch-nonetwork.service
>> +SERVICE_UNIT=/usr/lib/systemd/system/ovsdb-server.service
>> if [ -f $SERVICE_UNIT ] && [ -x /usr/bin/systemctl ]; then
>> - if ! systemctl --quiet is-active openvswitch-nonetwork.service; then
>> - systemctl start openvswitch-nonetwork.service
>> - fi
>> + if ! systemctl --quiet is-active ovsdb-server.service; then
>> + systemctl start ovsdb-server.service
>> + fi
>
> When shutting down or bringing down an interface, it seems that
> having only ovsdb-server would be enough.
>
> Traditionally SysV scripts use TABs and not spaces. You can see
> some examples in /etc/rc.d/init.d/ yet today. But I have no
> strong opinion between TAB and spaces besides to keep consistency.
ACK. Will conserve tabs.
>> else
>> - if [ ! -f /var/lock/subsys/openvswitch ]; then
>> - /sbin/service openvswitch start
>> - fi
>> + if [ ! -f /var/lock/subsys/openvswitch ]; then
>> + /sbin/service openvswitch start
>> + fi
>> fi
>
> This seems unneeded or the intention was to fix the tabs.
Yeah, I switched the tabs to spaces, but I'll undo that.
>> case "$TYPE" in
>> diff --git a/rhel/etc_sysconfig_network-scripts_ifup-ovs
>> b/rhel/etc_sysconfig_network-scripts_ifup-ovs
>> index f3fc05e..9b2efbb 100755
>> --- a/rhel/etc_sysconfig_network-scripts_ifup-ovs
>> +++ b/rhel/etc_sysconfig_network-scripts_ifup-ovs
>> @@ -60,15 +60,15 @@ fi
>> fi
>> done
>>
>> -SERVICE_UNIT=/usr/lib/systemd/system/openvswitch-nonetwork.service
>> +SERVICE_UNIT=/usr/lib/systemd/system/ovsdb-server.service
>> if [ -f $SERVICE_UNIT ] && [ -x /usr/bin/systemctl ]; then
>> - if ! systemctl --quiet is-active openvswitch-nonetwork.service; then
>> - systemctl start openvswitch-nonetwork.service
>> - fi
>> + if ! systemctl --quiet is-active ovsdb-server.service; then
>> + systemctl start ovsdb-server.service
>> + fi
>
> However, I am not sure that is correct here because when we run
> 'ifup ovsbr0' and it has DHCP on it, the vswitchd is not running yet
> and dhcp would fail. It seems we need to start the whole service
> (openvswitch.service) at this point.
Well, ovsdb-server is the whole service but it has a weird name. This
is just a rename; but I agree it's confusing and can just squash the two
together if you want. Otherwise, you're right and 3/3 would need to
change this to openvswitch.service
>> else
>> - if [ ! -f /var/lock/subsys/openvswitch ]; then
>> - /sbin/service openvswitch start
>> - fi
>> + if [ ! -f /var/lock/subsys/openvswitch ]; then
>> + /sbin/service openvswitch start
>> + fi
>> fi
>>
>> case "$TYPE" in
>> diff --git a/rhel/openvswitch-fedora.spec.in
>> b/rhel/openvswitch-fedora.spec.in
>> index 16894b0..ed7b3c4 100644
>> --- a/rhel/openvswitch-fedora.spec.in
>> +++ b/rhel/openvswitch-fedora.spec.in
>> @@ -189,7 +189,7 @@ install -d -m 0755
>> $RPM_BUILD_ROOT%{_sysconfdir}/openvswitch
>> install -p -D -m 0644 \
>> rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
>> $RPM_BUILD_ROOT/%{_sysconfdir}/sysconfig/openvswitch
>> -for service in openvswitch openvswitch-nonetwork \
>> +for service in openvswitch ovsdb-server \
>> ovn-controller ovn-controller-vtep ovn-northd; do
>> install -p -D -m 0644 \
>> rhel/usr_lib_systemd_system_${service}.service \
>> @@ -416,7 +416,7 @@ fi
>> %config(noreplace) %{_sysconfdir}/sysconfig/openvswitch
>> %config(noreplace) %{_sysconfdir}/logrotate.d/openvswitch
>> %{_unitdir}/openvswitch.service
>> -%{_unitdir}/openvswitch-nonetwork.service
>> +%{_unitdir}/ovsdb-server.service
>> %{_datadir}/openvswitch/scripts/openvswitch.init
>> %{_sysconfdir}/sysconfig/network-scripts/ifup-ovs
>> %{_sysconfdir}/sysconfig/network-scripts/ifdown-ovs
>> diff --git a/rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
>> b/rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
>> deleted file mode 100644
>> index e4c2a66..0000000
>> --- a/rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
>> +++ /dev/null
>> @@ -1,15 +0,0 @@
>> -[Unit]
>> -Description=Open vSwitch Internal Unit
>> -After=syslog.target
>> -PartOf=openvswitch.service
>> -Wants=openvswitch.service
>> -
>> -[Service]
>> -Type=oneshot
>> -RemainAfterExit=yes
>> -EnvironmentFile=-/etc/sysconfig/openvswitch
>> -ExecStart=/usr/share/openvswitch/scripts/ovs-ctl start \
>> - --system-id=random $OPTIONS
>> -ExecStop=/usr/share/openvswitch/scripts/ovs-ctl stop
>> -RuntimeDirectory=openvswitch
>> -RuntimeDirectoryMode=0755
>> diff --git a/rhel/usr_lib_systemd_system_openvswitch.service
>> b/rhel/usr_lib_systemd_system_openvswitch.service
>> index f0bc16f..96c697b 100644
>> --- a/rhel/usr_lib_systemd_system_openvswitch.service
>> +++ b/rhel/usr_lib_systemd_system_openvswitch.service
>> @@ -1,7 +1,7 @@
>> [Unit]
>> Description=Open vSwitch
>> -After=syslog.target network.target openvswitch-nonetwork.service
>> -Requires=openvswitch-nonetwork.service
>> +After=syslog.target network.target ovsdb-server.service
>> +Requires=ovsdb-server.service
>
> If we change the above ifup-ovs to start the ovs service, we also
> need to change the 'After' line to not require network.target.
Okay. Why is that?
>> [Service]
>> Type=oneshot
>> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service
>> b/rhel/usr_lib_systemd_system_ovsdb-server.service
>> new file mode 100644
>> index 0000000..e4c2a66
>> --- /dev/null
>> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
>> @@ -0,0 +1,15 @@
>> +[Unit]
>> +Description=Open vSwitch Internal Unit
>> +After=syslog.target
>> +PartOf=openvswitch.service
>> +Wants=openvswitch.service
>> +
>> +[Service]
>> +Type=oneshot
>> +RemainAfterExit=yes
>> +EnvironmentFile=-/etc/sysconfig/openvswitch
>> +ExecStart=/usr/share/openvswitch/scripts/ovs-ctl start \
>> + --system-id=random $OPTIONS
>> +ExecStop=/usr/share/openvswitch/scripts/ovs-ctl stop
>> +RuntimeDirectory=openvswitch
>> +RuntimeDirectoryMode=0755
>> --
>> 2.5.5
>>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
