One more thing. There is no BRCOMPAT in master. So that will have to go.
On Wed, Dec 4, 2013 at 1:04 PM, Gurucharan Shetty <shet...@nicira.com> wrote:
> On Wed, Dec 4, 2013 at 12:12 PM, Flavio Leitner <f...@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 <f...@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 <f...@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@openvswitch.org
>>> > http://openvswitch.org/mailman/listinfo/dev
>>>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
