| Message ID | 20190610135531.14010-1-jcaamano@suse.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] rhel: let *-ctl handle runtime directory | expand |
Bleep bloop. Greetings Jaime Caamaño Ruiz, I am a robot and I have tried out your patch.
Thanks for your contribution.
I encountered some error that I wasn't expecting. See the details below.
checkpatch:
WARNING: Line is 85 characters long (recommended limit is 79)
#33 FILE: rhel/usr_lib_systemd_system_ovsdb-server.service:14:
ExecStartPre=-/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch /var/log/openvswitch
Lines checked: 45, Warnings: 1, Errors: 0
Please check this out. If you feel there has been an error, please email aconole@bytheb.org
Thanks,
0-day Robot
On Mon, Jun 10, 2019 at 03:55:31PM +0200, Jaime Caamaño Ruiz wrote: > Recent versions of systemd restores RuntimeDirectory ownership to the > unit's User in between execution of *Exec directives (see [1]). Using > ExecStartPre to reset RuntimeDirectory ownership to OVS_USER no longer > works as expected. > > The ctl scripts already handle creation of the runtime directory with > correct ownership and permissions so we can basically remove > RuntimeDirectory from systemd unit file. There is still need to handle > ownsership to cover some upgrade scenarios, but success of that will be > optional as the directory itself wont exist at first time run. > > [1] https://github.com/systemd/systemd/issues/12713 > > Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com> Thanks, I applied this to master.
diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service b/rhel/usr_lib_systemd_system_ovsdb-server.service index 9bb37fd06..a6de4d3c1 100644 --- a/rhel/usr_lib_systemd_system_ovsdb-server.service +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service @@ -11,7 +11,7 @@ PIDFile=/var/run/openvswitch/ovsdb-server.pid Restart=on-failure EnvironmentFile=/etc/openvswitch/default.conf EnvironmentFile=-/etc/sysconfig/openvswitch -ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch /var/log/openvswitch +ExecStartPre=-/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch /var/log/openvswitch ExecStartPre=/bin/sh -c 'rm -f /run/openvswitch.useropts; /usr/bin/echo "OVS_USER_ID=${OVS_USER_ID}" > /run/openvswitch.useropts' ExecStartPre=/bin/sh -c 'if [ "$${OVS_USER_ID/:*/}" != "root" ]; then /usr/bin/echo "OVS_USER_OPT=--ovs-user=${OVS_USER_ID}" >> /run/openvswitch.useropts; fi' EnvironmentFile=/run/openvswitch.useropts @@ -23,5 +23,3 @@ ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd stop ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd \ ${OVS_USER_OPT} \ --no-monitor restart $OPTIONS -RuntimeDirectory=openvswitch -RuntimeDirectoryMode=0755
Recent versions of systemd restores RuntimeDirectory ownership to the unit's User in between execution of *Exec directives (see [1]). Using ExecStartPre to reset RuntimeDirectory ownership to OVS_USER no longer works as expected. The ctl scripts already handle creation of the runtime directory with correct ownership and permissions so we can basically remove RuntimeDirectory from systemd unit file. There is still need to handle ownsership to cover some upgrade scenarios, but success of that will be optional as the directory itself wont exist at first time run. [1] https://github.com/systemd/systemd/issues/12713 Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com> --- rhel/usr_lib_systemd_system_ovsdb-server.service | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)