[ovs-dev] [PATCH 2/2] fedora package: fix systemd order and dependencies
Flavio Leitner
fbl at redhat.com
Thu Dec 5 15:46:54 UTC 2013
On Wed, Dec 04, 2013 at 01:06:35PM -0800, Gurucharan Shetty wrote:
> One more thing. There is no BRCOMPAT in master. So that will have to go.
Ok, I will remove it.
> On Wed, Dec 4, 2013 at 1:04 PM, Gurucharan Shetty <shettyg at nicira.com> wrote:
> > 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.
Ok
> >> /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?
systemctl is part of systemd. If you have systemctl installed, then the
system should be ready to use systemd service units just fine.
[...]
> >>> > 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.
Yes, systemd is a bit smart, so it monitors the services for us. So
taking out any wrappers is better. Since that openvswitch.init script
is just a wrapper and uses a deprecated /var/lock, I considered that
it would be better to just leave it alone not use it with systemd.
> > I saw a "-" in "EnvironmentFile=-/etc/sysconfig/openvswitch". Does it
> > have any significance?
Yes. You can check in systemd.exec(1):
EnvironmentFile=
[...]
The argument passed should be an absolute file name or wildcard
expression, optionally prefixed with "-", which indicates that if
the file does not exist it won't be read and no error or warning
message is logged.
> >> 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.
Ok.
Thanks,
fbl
More information about the dev
mailing list