Hi Dumitru, Frode, Thanks for taking a look.
For some background, this work started from an ovn-controller logging issue reported against charm-ovn-chassis: https://bugs.launchpad.net/charm-ovn-chassis/+bug/2143318 I initially proposed handling it in the charm. Following discussion with Frode, I moved the proposed fix to the Debian OVN packaging, which led to the following Salsa merge request: https://salsa.debian.org/openstack-team/third-party/ovn/-/merge_requests/69 During the review of that merge request, Luca pointed out that OVN's startup scripts recursively change the ownership of the log directory, which would override the ownership configured by the package. He therefore suggested addressing that behavior upstream first and managing the log ownership with tmpfiles.d. The sysusers.d support was added based on further review there. The current upstream patch is therefore the result of moving the fix from the charm to Debian packaging and then upstream, following the feedback at each stage. Thanks, Zhang Hua On Mon, Jul 20, 2026 at 5:09 PM Dumitru Ceara <[email protected]> wrote: > On 7/20/26 11:02 AM, Dumitru Ceara wrote: > > On 7/2/26 4:32 AM, Zhang Hua via dev wrote: > >> ovn-ctl and ovn-lib currently chown the OVN log directory > >> recursively when services start. That can undo ownership set by > >> distribution tooling such as tmpfiles.d and logrotate. In particular, > >> Debian and Ubuntu need ovn-controller.log to remain root:adm so rsyslog > >> can read it, but a later OVN service restart may change it back to the > >> OVN daemon user/group. > >> > >> Add an OVN tmpfiles.d template for the log directory and > >> ovn-controller.log, with configurable log user, log group and tmpfiles.d > >> installation directory. Debian configures the log owner as root:adm, > >> while Fedora/RHEL keeps openvswitch:openvswitch. > >> > >> Stop recursively changing the OVN log directory ownership from ovn-ctl > >> and ovn-lib at service startup. Runtime, database and configuration > >> paths are still chowned according to --ovn-user, preserving the existing > >> privilege-drop behavior for non-log state. > >> > >> Reported-at: https://github.com/ovn-org/ovn/issues/310 > >> Signed-off-by: Zhang Hua <[email protected]> > >> --- > > > > Hi Zhang Hua, > > > > Thanks for the patch! > > > > Hi Frode, > > > > Would you have some time to look at this patch from an Ubuntu/Debian > > perspective? > > > > Sorry, I should've replied to v3 instead: > > > https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/ > > > Thanks, > > Dumitru > > > >> Submitted-at: https://github.com/ovn-org/ovn/pull/311 > >> > >> v2: > >> - Added the trailing dot required by checkpatch to the subject. > >> > >> Testing: > >> - Ran utilities/checkpatch.py -1. > >> - Ran make check TESTSUITEFLAGS="-j$(nproc)". > >> > >> Makefile.am | 4 ++++ > >> configure.ac | 3 +++ > >> debian/ovn-common.install | 1 + > >> debian/rules | 2 +- > >> m4/ovn.m4 | 31 +++++++++++++++++++++++++++++++ > >> rhel/ovn-fedora.spec.in | 3 +++ > >> utilities/automake.mk | 4 ++++ > >> utilities/ovn-ctl | 6 ++---- > >> utilities/ovn-lib.in | 1 - > >> utilities/ovn-tmpfiles.conf.in | 2 ++ > >> 10 files changed, 51 insertions(+), 6 deletions(-) > >> create mode 100644 utilities/ovn-tmpfiles.conf.in > >> > >> diff --git a/Makefile.am b/Makefile.am > >> index 0f2389b25..79929ee6c 100644 > >> --- a/Makefile.am > >> +++ b/Makefile.am > >> @@ -131,6 +131,7 @@ noinst_PROGRAMS = > >> noinst_SCRIPTS = > >> OVSIDL_BUILT = > >> pkgdata_DATA = > >> +tmpfiles_DATA = > >> sbin_SCRIPTS = > >> scripts_SCRIPTS = > >> completion_SCRIPTS = > >> @@ -148,6 +149,7 @@ endif > >> scriptsdir = $(pkgdatadir)/scripts > >> completiondir = $(sysconfdir)/bash_completion.d > >> pkgconfigdir = $(libdir)/pkgconfig > >> +tmpfilesdir = @TMPFILESDIR@ > >> > >> # This ensures that files added to EXTRA_DIST are always distributed, > >> # even if they are inside an Automake if...endif conditional block > that is > >> @@ -166,6 +168,8 @@ SUFFIXES += .in > >> sed \ > >> -e 's,[@]PKIDIR[@],$(PKIDIR),g' \ > >> -e 's,[@]LOGDIR[@],$(LOGDIR),g' \ > >> + -e 's,[@]LOGUSER[@],$(LOGUSER),g' \ > >> + -e 's,[@]LOGGROUP[@],$(LOGGROUP),g' \ > >> -e 's,[@]DBDIR[@],$(DBDIR),g' \ > >> -e 's,[@]PYTHON3[@],$(PYTHON3),g' \ > >> -e 's,[@]OVN_RUNDIR[@],$(OVN_RUNDIR),g' \ > >> diff --git a/configure.ac b/configure.ac > >> index cfa4cc386..d19e26612 100644 > >> --- a/configure.ac > >> +++ b/configure.ac > >> @@ -88,6 +88,9 @@ OVS_CHECK_NETLINK > >> OVS_CHECK_LINUX_NETLINK > >> OVS_CHECK_OPENSSL > >> OVN_CHECK_LOGDIR > >> +OVN_CHECK_LOGUSER > >> +OVN_CHECK_LOGGROUP > >> +OVN_CHECK_TMPFILESDIR > >> OVN_CHECK_PYTHON3 > >> OVN_CHECK_FLAKE8 > >> OVN_CHECK_SPHINX > >> diff --git a/debian/ovn-common.install b/debian/ovn-common.install > >> index fc48f07e4..b890c6797 100644 > >> --- a/debian/ovn-common.install > >> +++ b/debian/ovn-common.install > >> @@ -12,4 +12,5 @@ usr/share/ovn/scripts/ovn-lib > >> usr/share/ovn/scripts/ovn-bugtool-nbctl-show > >> usr/share/ovn/scripts/ovn-bugtool-sbctl-lflow-list > >> usr/share/ovn/scripts/ovn-bugtool-sbctl-show > >> +usr/lib/tmpfiles.d/ovn-tmpfiles.conf > >> usr/lib/*/libovn*.so.* > >> diff --git a/debian/rules b/debian/rules > >> index b25a0b48e..0d5da5b26 100755 > >> --- a/debian/rules > >> +++ b/debian/rules > >> @@ -30,7 +30,7 @@ override_dh_autoreconf: > >> dh_autoreconf $(DH_AS_NEEDED) > >> > >> override_dh_auto_configure: > >> - dh_auto_configure -- --enable-ssl --enable-shared > --with-ovs-source=${OVSDIR} $(EXTRA_CONFIGURE_OPTS) > >> + dh_auto_configure -- --enable-ssl --enable-shared > --with-ovs-source=${OVSDIR} --with-log-user=root --with-log-group=adm > $(EXTRA_CONFIGURE_OPTS) > >> > >> override_dh_auto_test: > >> ifeq (,$(filter nocheck,$(DEB_BUILD_OPTIONS))) > >> diff --git a/m4/ovn.m4 b/m4/ovn.m4 > >> index 22ad1a27f..f022a0152 100644 > >> --- a/m4/ovn.m4 > >> +++ b/m4/ovn.m4 > >> @@ -127,6 +127,37 @@ AC_DEFUN([OVN_CHECK_LOGDIR], > >> [LOGDIR='${localstatedir}/log/${PACKAGE}']) > >> AC_SUBST([LOGDIR])]) > >> > >> +dnl Checks for the user that should own log files. > >> +AC_DEFUN([OVN_CHECK_LOGUSER], > >> + [AC_ARG_WITH( > >> + [log-user], > >> + AS_HELP_STRING([--with-log-user=USER], > >> + [user used for log files [[root]]]), > >> + [LOGUSER=$withval], > >> + [LOGUSER=root]) > >> + AC_SUBST([LOGUSER])]) > >> + > >> +dnl Checks for the group that should own log files. > >> +AC_DEFUN([OVN_CHECK_LOGGROUP], > >> + [AC_ARG_WITH( > >> + [log-group], > >> + AS_HELP_STRING([--with-log-group=GROUP], > >> + [group used for log files [[root]]]), > >> + [LOGGROUP=$withval], > >> + [LOGGROUP=root]) > >> + AC_SUBST([LOGGROUP])]) > >> + > >> +dnl Checks for the directory in which to install tmpfiles.d > configuration. > >> +AC_DEFUN([OVN_CHECK_TMPFILESDIR], > >> + [AC_ARG_WITH( > >> + [tmpfilesdir], > >> + AS_HELP_STRING([--with-tmpfilesdir=DIR], > >> + [directory used for tmpfiles.d configuration > >> + [[PREFIX/lib/tmpfiles.d]]]), > >> + [TMPFILESDIR=$withval], > >> + [TMPFILESDIR='${prefix}/lib/tmpfiles.d']) > >> + AC_SUBST([TMPFILESDIR])]) > >> + > >> dnl Checks for the directory in which to store the OVN database. > >> AC_DEFUN([OVN_CHECK_DBDIR], > >> [AC_ARG_WITH( > >> diff --git a/rhel/ovn-fedora.spec.in b/rhel/ovn-fedora.spec.in > >> index 131b3eaab..1fdaad706 100644 > >> --- a/rhel/ovn-fedora.spec.in > >> +++ b/rhel/ovn-fedora.spec.in > >> @@ -159,6 +159,8 @@ cd - > >> --with-ovs-source=$PWD/openvswitch-%{ovsver} \ > >> %if %{with libcapng} > >> --enable-libcapng \ > >> + --with-log-user=openvswitch \ > >> + --with-log-group=openvswitch \ > >> %else > >> --disable-libcapng \ > >> %endif > >> @@ -531,6 +533,7 @@ fi > >> %{_mandir}/man8/ovn-debug.8* > >> %{_prefix}/lib/ocf/resource.d/ovn/ovndb-servers > >> %config(noreplace) %{_sysconfdir}/logrotate.d/ovn > >> +%{_tmpfilesdir}/ovn-tmpfiles.conf > >> %{_unitdir}/[email protected] > >> > >> %files docker > >> diff --git a/utilities/automake.mk b/utilities/automake.mk > >> index b620038d0..c22b5d8c3 100644 > >> --- a/utilities/automake.mk > >> +++ b/utilities/automake.mk > >> @@ -26,6 +26,7 @@ EXTRA_DIST += \ > >> utilities/ovn-ctl \ > >> utilities/ovn-lib.in \ > >> utilities/ovn-ctl.8.xml \ > >> + utilities/ovn-tmpfiles.conf.in \ > >> utilities/ovn-docker-overlay-driver.in \ > >> utilities/ovn-docker-underlay-driver.in \ > >> utilities/ovn-nbctl.8.xml \ > >> @@ -48,6 +49,7 @@ EXTRA_DIST += \ > >> CLEANFILES += \ > >> utilities/ovn-ctl.8 \ > >> utilities/ovn-lib \ > >> + utilities/ovn-tmpfiles.conf \ > >> utilities/ovn-docker-overlay-driver \ > >> utilities/ovn-docker-underlay-driver \ > >> utilities/ovn-nbctl.8 \ > >> @@ -66,7 +68,9 @@ CLEANFILES += \ > >> EXTRA_DIST += utilities/ovn-sim.in > >> noinst_SCRIPTS += utilities/ovn-sim > >> > >> +tmpfiles_DATA += utilities/ovn-tmpfiles.conf > >> utilities/ovn-lib: $(top_builddir)/config.status > >> +utilities/ovn-tmpfiles.conf: $(top_builddir)/config.status > >> > >> # ovn-nbctl > >> bin_PROGRAMS += utilities/ovn-nbctl > >> diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl > >> index 3b62ca9b7..58effd5e5 100755 > >> --- a/utilities/ovn-ctl > >> +++ b/utilities/ovn-ctl > >> @@ -276,9 +276,8 @@ $cluster_remote_port > >> upgrade_db "$file" "$schema" > >> fi > >> > >> - # Set the owner of the ovn_dbdir (with -R option) to OVN_USER if > set. > >> - # This is required because the ovndbs are created with root > permission > >> - # if not present when create_cluster/upgrade_db is called. > >> + # Database files may be created as root before ovsdb-server drops > >> + # privileges, so keep ownership aligned with OVN_USER when > configured. > >> INSTALL_USER="$(id -un)" > >> INSTALL_GROUP="$(id -gn)" > >> [ "$OVN_USER" != "" ] && INSTALL_USER="${OVN_USER%:*}" > >> @@ -286,7 +285,6 @@ $cluster_remote_port > >> > >> chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_dbdir > >> chown -R $INSTALL_USER:$INSTALL_GROUP $OVN_RUNDIR > >> - chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_logdir > >> chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_etcdir > >> > >> set ovsdb-server > >> diff --git a/utilities/ovn-lib.in b/utilities/ovn-lib.in > >> index 5a0766816..0b19149da 100644 > >> --- a/utilities/ovn-lib.in > >> +++ b/utilities/ovn-lib.in > >> @@ -133,7 +133,6 @@ start_ovn_daemon () { > >> set "$@" --detach > >> test X"$MONITOR" = Xno || set "$@" --monitor > >> > >> - chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_logdir > >> chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_rundir > >> > >> start_wrapped_daemon "$wrapper" $daemon "$priority" "$@" > >> diff --git a/utilities/ovn-tmpfiles.conf.in b/utilities/ > ovn-tmpfiles.conf.in > >> new file mode 100644 > >> index 000000000..d37391f88 > >> --- /dev/null > >> +++ b/utilities/ovn-tmpfiles.conf.in > >> @@ -0,0 +1,2 @@ > >> +d @LOGDIR@ 0750 @LOGUSER@ @LOGGROUP@ - > >> +f @LOGDIR@/ovn-controller.log 0640 @LOGUSER@ @LOGGROUP@ - > >> \ No newline at end of file > > -- Best Regards Zhang Hua(张华） Software Engineer | Canonical IRC: zhhuabj We are hiring <https://canonical.com/careers?utm_source=signature&utm_medium=email> | Life at Canonical <https://www.linkedin.com/company/canonical/life/> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
