[ovs-dev] [PATCH 2/2] fedora package: fix systemd order and dependencies
Gurucharan Shetty
shettyg at nicira.com
Wed Dec 4 21:04:38 UTC 2013
On Wed, Dec 4, 2013 at 12:12 PM, Flavio Leitner <fbl at redhat.com> wrote:
> On Wed, Dec 04, 2013 at 10:27:10AM -0800, Gurucharan Shetty wrote:
>> On Mon, Dec 2, 2013 at 5:13 PM, Flavio Leitner <fbl at redhat.com> wrote:
>> > There is a chicken and egg issue where common OVS
>> > configuration uses a controller which is only accessible
>> > via the network. So starting OVS before network.target
>> > would break those configurations.
>> >
>> > However, the network doesn't come up after boot because
>> > OVS isn't started until after the network scripts tries
>> > to configure the ovs.
>> >
>> > This is partially fixed by commits:
>> > commit: 602453000e28ec1076c0482ce13c284765a84409
>> > rhel: Automatically start openvswitch service before bringing an ovs interfa
>> >
>> > commit: 3214851c31538e8690e31f95702f8927a8c0838b
>> > rhel: Add OVSREQUIRES to automatically bring up OpenFlow interface dependencies
>> >
>> > But still there is the dependency issue between network.target
>> > and openvswitch which this patch fixes it. It provides two systemd
>> > service units. One to run at any time (openvswitch-nonetwork.service)
>> > which runs 'ovs-ctl start' and the other one (openvswith.service) to
>> > run after network.target which works as a frontend to the admin.
>> >
>> > The openvswitch-nonetwork.service is used internally by the
>> > 'ifup-ovs/ifdown-ovs' scripts when adding or removing ports to
>> > the bridge or when the openvswitch.service is enabled by the admin.
>> >
>> > Signed-off-by: Flavio Leitner <fbl at redhat.com>
>> > ---
>> > rhel/automake.mk | 3 ++-
>> > rhel/etc_sysconfig_network-scripts_ifdown-ovs | 10 +++++++++-
>> > rhel/etc_sysconfig_network-scripts_ifup-ovs | 12 ++++++++++--
>> > rhel/openvswitch-fedora.spec.in | 3 +++
>> > rhel/usr_lib_systemd_system_openvswitch-nonetwork.service | 14 ++++++++++++++
>> > rhel/usr_lib_systemd_system_openvswitch.service | 11 +++++++----
>> > 6 files changed, 45 insertions(+), 8 deletions(-)
>> > create mode 100644 rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
>> >
>> > diff --git a/rhel/automake.mk b/rhel/automake.mk
>> > index 2911e71..cd06823 100644
>> > --- a/rhel/automake.mk
>> > +++ b/rhel/automake.mk
>> > @@ -22,7 +22,8 @@ EXTRA_DIST += \
>> > rhel/openvswitch-fedora.spec \
>> > rhel/openvswitch-fedora.spec.in \
>> > rhel/usr_share_openvswitch_scripts_sysconfig.template \
>> > - rhel/usr_lib_systemd_system_openvswitch.service
>> > + rhel/usr_lib_systemd_system_openvswitch.service \
>> > + rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
>> >
>> > update_rhel_spec = \
>> > ($(ro_shell) && sed -e 's,[@]VERSION[@],$(VERSION),g') \
>> > diff --git a/rhel/etc_sysconfig_network-scripts_ifdown-ovs b/rhel/etc_sysconfig_network-scripts_ifdown-ovs
>> > index 8e768c8..2a3d8d2 100755
>> > --- a/rhel/etc_sysconfig_network-scripts_ifdown-ovs
>> > +++ b/rhel/etc_sysconfig_network-scripts_ifdown-ovs
>> > @@ -34,7 +34,15 @@ if [ ! -x ${OTHERSCRIPT} ]; then
>> > OTHERSCRIPT="/etc/sysconfig/network-scripts/ifdown-eth"
>> > fi
>> >
>> > -[ -f /var/lock/subsys/openvswitch ] || /sbin/service openvswitch start
>> > +if [ -x /usr/bin/systemctl ]; then
>> > + if ! systemctl --quiet is-active openvswitch-nonetwork.service; then
>> I think this is considered non-portable.
>> https://www.sourceware.org/autobook/autobook/autobook_216.html
>
> That construction form is used quite frequently in rpm distros. There
> are some examples in /etc/sysconfig/network-scripts/network-functions
> and this is specific to rhel/.
>
> What about using the return value? Something like:
As Ben mentioned, since this is only for RHEL distros, the current
code should be okay.
>
> /usr/bin/systemctl --quiet is-active openvswitch-nonetwork.service
> if [ $? -ne 0 ]; then
> /usr/bin/systemctl start openvswitch-nonetwork.service
> fi
Also,
> +if [ -x /usr/bin/systemctl ]; then
Should we also check whether the platform is fedora? Otherwise, it may
break centos openvwitch rpms in the future if it includes systemctl?
>
>
>> > + /usr/bin/systemctl start openvswitch-nonetwork.service
>> > + fi
>> > +else
>> > + if [ ! -f /var/lock/subsys/openvswitch ]; then
>> > + /sbin/service openvswitch start
>> > + fi
>> > +fi
>> >
>> > case "$TYPE" in
>> > OVSBridge)
>> > diff --git a/rhel/etc_sysconfig_network-scripts_ifup-ovs b/rhel/etc_sysconfig_network-scripts_ifup-ovs
>> > index 017346d..817f93f 100755
>> > --- a/rhel/etc_sysconfig_network-scripts_ifup-ovs
>> > +++ b/rhel/etc_sysconfig_network-scripts_ifup-ovs
>> > @@ -60,8 +60,16 @@ fi
>> > fi
>> > done
>> >
>> > -[ -f /var/lock/subsys/openvswitch ] || /sbin/service openvswitch start
>> > -
>> > +if [ -x /usr/bin/systemctl ]; then
>> > + if ! systemctl --quiet is-active openvswitch-nonetwork.service; then
>> > + /usr/bin/systemctl start openvswitch-nonetwork.service
>> > + fi
>> > +else
>> > + if [ ! -f /var/lock/subsys/openvswitch ]; then
>> > + /sbin/service openvswitch start
>> > + fi
>> > +fi
>> > +
>> > case "$TYPE" in
>> > OVSBridge)
>> > # If bridge already exists and is up, it has been configured through
>> > diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
>> > index 5384c32..e6b40a5 100644
>> > --- a/rhel/openvswitch-fedora.spec.in
>> > +++ b/rhel/openvswitch-fedora.spec.in
>> > @@ -45,6 +45,8 @@ install -d -m 755 $RPM_BUILD_ROOT/etc
>> > install -d -m 755 $RPM_BUILD_ROOT/etc/openvswitch
>> > install -p -D -m 0644 rhel/usr_lib_systemd_system_openvswitch.service \
>> > $RPM_BUILD_ROOT%{_unitdir}/openvswitch.service
>> > +install -p -D -m 0644 rhel/usr_lib_systemd_system_openvswitch-nonetwork.service \
>> > + $RPM_BUILD_ROOT%{_unitdir}/openvswitch-nonetwork.service
>> > install -m 755 rhel/etc_init.d_openvswitch \
>> > $RPM_BUILD_ROOT%{_datadir}/openvswitch/scripts/openvswitch.init
>> > install -d -m 755 $RPM_BUILD_ROOT/etc/sysconfig
>> > @@ -101,6 +103,7 @@ systemctl start openvswitch.service
>> > %config /etc/sysconfig/openvswitch
>> > %config /etc/logrotate.d/openvswitch
>> > %{_unitdir}/openvswitch.service
>> > +%{_unitdir}/openvswitch-nonetwork.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
>> > new file mode 100644
>> > index 0000000..f9fc83d
>> > --- /dev/null
>> > +++ b/rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
>> > @@ -0,0 +1,14 @@
>> > +[Unit]
>> > +Description=Open vSwitch Internal Unit
>> > +After=syslog.target
>> > +PartOf=openvswitch.service
>> > +Wants=openvswitch.service
>> > +
>> > +[Service]
>> > +Type=oneshot
>> > +EnvironmentFile=-/etc/sysconfig/openvswitch
>> > +RemainAfterExit=yes
>> > +ExecStart=/usr/share/openvswitch/scripts/ovs-ctl start --system-id=random \
>> > + $FORCE_COREFILES $OVSDB_SERVER_PRIORITY $VSWITCHD_PRIORITY \
>> > + $VSWITCHD_MLOCKALL $BRCOMPAT $OVS_CTL_OPTS
>> I think this will not work well.
>> If you see the rhel startup script, you will see code like this:
>> set ovs_ctl ${1-start}
>> if test X"$OVSDB_SERVER_PRIORITY" != X; then
>> set "$@" --ovsdb-server-priority="$OVSDB_SERVER_PRIORITY"
>> fi
>> "$@"
>
> I forgot to include the /etc/sysconfig/openvswitch in the patch.
> It looks like this:
Okay. Is there any reason not to call
"/usr/share/openvswitch/scripts/openvswitch.init start" instead? It
will ensure that we have the same type of template file for all
platforms.
I saw a "-" in "EnvironmentFile=-/etc/sysconfig/openvswitch". Does it
have any significance?
>
> ### Configuration options for openvswitch
>
> ## FORCE_COREFILES: If 'yes' then core files will be enabled.
> #FORCE_COREFILES="--force-corefiles=yes"
>
> ## OVSDB_SERVER_PRIORITY: "nice" priority at which to run ovsdb-server.
> #OVSDB_SERVER_PRIORITY="--ovsdb-server-priority=-10"
>
> ## VSWITCHD_PRIORITY: "nice" priority at which to run ovs-vswitchd.
> #VSWITCHD_PRIORITY="--ovs-vswitchd-priority=-10"
>
> ## VSWITCHD_MLOCKALL: Whether to pass ovs-vswitchd the --mlockall option.
> # This option should be set to "yes" or "no". The default is "yes".
> # Enabling this option can avoid networking interruptions due to
> # system memory pressure in extraordinary situations, such as
> # multiple
> # concurrent VM import operations.
> #VSWITCHD_MLOCKALL="--mlockall=yes"
>
> ## BRCOMPAT: If 'yes' compatibility mode will be enabled.
> #BRCOMPAT=--brcompat
>
> ## OVS_CTL_OPTS: Extra options to pass to ovs-ctl. This is, for example,
> # a suitable place to specify --ovs-vswitchd-wrapper=valgrind.
> #OVS_CTL_OPTS="--ovs-vswitchd-wrapper=valgrind"
>
>
> By the way, it is hard coded --systemd-id=random in
> rhel/etc_init.d_openvswitch. I think it should be in
> /etc/sysconfig/openvswitch too, right?
I don't have any strong feelings about it (Others may). Feel free to
submit a patch.
"random" has a special meaning (you can look at ovs-ctl man page) and
should be the default. So the startup scripts will have to change to
handle that.
>
> Thanks for reviewing it.
> fbl
>
>
>> > +ExecStop=/usr/share/openvswitch/scripts/ovs-ctl stop
>> > diff --git a/rhel/usr_lib_systemd_system_openvswitch.service b/rhel/usr_lib_systemd_system_openvswitch.service
>> > index f39d7e6..6e08a9a 100644
>> > --- a/rhel/usr_lib_systemd_system_openvswitch.service
>> > +++ b/rhel/usr_lib_systemd_system_openvswitch.service
>> > @@ -1,12 +1,15 @@
>> > [Unit]
>> > -Description=Open vSwitch
>> > -After=syslog.target network.target
>> > +Description=Open vSwitch Unit
>> > +After=syslog.target
>> > +After=network.target
>> > +After=openvswitch-nonetwork.service
>> > +Requires=openvswitch-nonetwork.service
>> >
>> > [Service]
>> > Type=oneshot
>> > -ExecStart=/usr/share/openvswitch/scripts/openvswitch.init start
>> > -ExecStop=/usr/share/openvswitch/scripts/openvswitch.init stop
>> > RemainAfterExit=yes
>> > +ExecStart=/bin/true
>> > +ExecStop=/bin/true
>>
>>
>> >
>> > [Install]
>> > WantedBy=multi-user.target
>> > --
>> > 1.8.3.1
>> >
>> > _______________________________________________
>> > dev mailing list
>> > dev at openvswitch.org
>> > http://openvswitch.org/mailman/listinfo/dev
>>
More information about the dev
mailing list