On 9/4/26 9:00 AM, Zhang Hua wrote: > Hi All, > > Gentle ping for reviews of this patch: > > https://mail.openvswitch.org/pipermail/ovs-dev/2026-July/434368.html >
Hi Joshua, Sorry for the delay. I was really hoping someone with Debian/Ubuntu packaging experience would review at least the relevant parts of this patch. I can't have a very relevant opinion on those parts. > Since the v3 discussion, tmpfiles-based log ownership is now opt-in and > disabled by default. This preserves the existing ovn-ctl and ovn-lib > behavior for deployments that do not use systemd services or > systemd-tmpfiles. > > The Debian and Fedora packages enable the behavior explicitly. Debian > configures the log ownership as root:adm, while Fedora configures it as > openvswitch:openvswitch. I verified both packaging paths. > > Ilya, could you please take another look at whether the opt-in approach > addresses your compatibility concern? Dumitru, feedback on the > Debian/Ubuntu packaging side would also be appreciated. > > Thanks, > Joshua > > On Wed, Jul 22, 2026 at 4:30 PM Zhang Hua <[email protected]> > wrote: > >> Add --enable-tmpfiles-log-ownership to install tmpfiles.d and >> sysusers.d helpers for OVN log ownership. >> >> Keep this disabled by default so deployments that do not use >> systemd-tmpfiles retain the existing recursive log directory ownership >> handling in ovn-ctl and ovn-lib. Enable it explicitly in Debian and >> Fedora packaging. >> >> Configure Debian logs as root:adm. Configure Fedora logs as >> openvswitch:openvswitch, apply the tmpfiles configuration during RPM >> installation and upgrades, and leave user provisioning to the Open >> vSwitch package. >> >> Reported-at: https://github.com/ovn-org/ovn/issues/310 >> Signed-off-by: Zhang Hua <[email protected]> >> --- >> Submitted-at: https://github.com/ovn-org/ovn/pull/311 >> >> v2: >> - Added the trailing dot required by checkpatch to the subject. >> >> v3: >> - Added sysusers.d support alongside tmpfiles.d. >> - Added --with-sysusersdir configure option. >> - Generate sysusers.d entries from the configured log user/group. >> - Install the generated sysusers.d file in Debian packaging. >> - Do not package the generated sysusers.d file for Fedora/RHEL because >> the openvswitch user is managed by the Open vSwitch package there. >> >> v4: >> - Make tmpfiles-based log ownership opt-in with the new >> --enable-tmpfiles-log-ownership configure option. >> - Keep the option disabled by default so deployments without >> systemd-tmpfiles >> retain ovn-ctl and ovn-lib's existing recursive log-directory ownership >> handling. >> - Install the tmpfiles.d and sysusers.d helpers only when the option is >> enabled. >> - Explicitly enable tmpfiles log ownership in Debian and Fedora packaging. >> - Configure Debian log ownership as root:adm. >> - Configure Fedora log ownership as openvswitch:openvswitch and apply the >> tmpfiles configuration in the RPM %post script. >> - Do not package OVN's generated sysusers.d file on Fedora/RHEL, where the >> openvswitch user is managed by the Open vSwitch package. >> >> v5: >> - Use a consistent author and Signed-off-by identity. >> >> Testing: >> - Ran utilities/checkpatch.py -1. >> - Ran make check TESTSUITEFLAGS="-j$(nproc)". >> - Built and installed the tmpfiles-enabled configuration in an Ubuntu 24.04 >> LXD container. >> - Built and installed the Fedora RPM and verified its tmpfiles >> configuration >> creates the log directory and controller log as openvswitch:openvswitch. >> >> Makefile.am | 9 +++++ >> configure.ac | 5 +++ >> debian/ovn-common.install | 2 + >> debian/rules | 2 +- >> m4/ovn.m4 | 69 ++++++++++++++++++++++++++++++++++ >> rhel/ovn-fedora.spec.in | 9 +++++ >> utilities/automake.mk | 10 +++++ >> utilities/ovn-ctl | 9 +++-- >> utilities/ovn-lib.in | 5 ++- >> utilities/ovn-sysusers.conf.in | 2 + >> utilities/ovn-tmpfiles.conf.in | 2 + >> 11 files changed, 118 insertions(+), 6 deletions(-) >> create mode 100644 utilities/ovn-sysusers.conf.in >> create mode 100644 utilities/ovn-tmpfiles.conf.in >> >> diff --git a/Makefile.am b/Makefile.am >> index 0f2389b25..0bbe13054 100644 >> --- a/Makefile.am >> +++ b/Makefile.am >> @@ -131,6 +131,8 @@ noinst_PROGRAMS = >> noinst_SCRIPTS = >> OVSIDL_BUILT = >> pkgdata_DATA = >> +tmpfiles_DATA = >> +sysusers_DATA = >> sbin_SCRIPTS = >> scripts_SCRIPTS = >> completion_SCRIPTS = >> @@ -148,6 +150,8 @@ endif >> scriptsdir = $(pkgdatadir)/scripts >> completiondir = $(sysconfdir)/bash_completion.d >> pkgconfigdir = $(libdir)/pkgconfig >> +tmpfilesdir = @TMPFILESDIR@ >> +sysusersdir = @SYSUSERSDIR@ >> >> # 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 +170,11 @@ 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,[@]OVN_CHOWN_LOGDIR[@],$(OVN_CHOWN_LOGDIR),g' \ >> + -e 's,[@]SYSUSERS_GROUP_LINE[@],$(SYSUSERS_GROUP_LINE),g' \ >> + -e 's,[@]SYSUSERS_USER_LINE[@],$(SYSUSERS_USER_LINE),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..6094b3eaa 100644 >> --- a/configure.ac >> +++ b/configure.ac >> @@ -88,6 +88,11 @@ OVS_CHECK_NETLINK >> OVS_CHECK_LINUX_NETLINK >> OVS_CHECK_OPENSSL >> OVN_CHECK_LOGDIR >> +OVN_CHECK_LOGUSER >> +OVN_CHECK_LOGGROUP >> +OVN_CHECK_TMPFILES_LOG_OWNERSHIP >> +OVN_CHECK_TMPFILESDIR >> +OVN_CHECK_SYSUSERSDIR >> OVN_CHECK_PYTHON3 >> OVN_CHECK_FLAKE8 >> OVN_CHECK_SPHINX >> diff --git a/debian/ovn-common.install b/debian/ovn-common.install >> index fc48f07e4..7ae59e86a 100644 >> --- a/debian/ovn-common.install >> +++ b/debian/ovn-common.install >> @@ -12,4 +12,6 @@ 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/sysusers.d/ovn-sysusers.conf >> usr/lib/*/libovn*.so.* >> diff --git a/debian/rules b/debian/rules >> index b25a0b48e..abc3fe328 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 >> --enable-tmpfiles-log-ownership --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..2156ac831 100644 >> --- a/m4/ovn.m4 >> +++ b/m4/ovn.m4 >> @@ -127,6 +127,75 @@ 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 whether tmpfiles.d manages log ownership. >> +AC_DEFUN([OVN_CHECK_TMPFILES_LOG_OWNERSHIP], >> + [AC_ARG_ENABLE( >> + [tmpfiles-log-ownership], >> + [AS_HELP_STRING([--enable-tmpfiles-log-ownership], >> + [manage log ownership with tmpfiles.d])], >> + [case "${enableval}" in >> + (yes) tmpfiles_log_ownership=true ;; >> + (no) tmpfiles_log_ownership=false ;; >> + (*) AC_MSG_ERROR([bad value ${enableval} for >> --enable-tmpfiles-log-ownership]) ;; >> + esac], >> + [tmpfiles_log_ownership=false]) >> + AM_CONDITIONAL([TMPFILES_LOG_OWNERSHIP], >> + [test x$tmpfiles_log_ownership = xtrue]) >> + AS_IF([test x$tmpfiles_log_ownership = xtrue], >> + [OVN_CHOWN_LOGDIR=no], >> + [OVN_CHOWN_LOGDIR=yes]) >> + AC_SUBST([OVN_CHOWN_LOGDIR])]) >> + >> +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 install sysusers.d configuration. >> +AC_DEFUN([OVN_CHECK_SYSUSERSDIR], >> + [AC_ARG_WITH( >> + [sysusersdir], >> + AS_HELP_STRING([--with-sysusersdir=DIR], >> + [directory used for sysusers.d configuration >> + [[PREFIX/lib/sysusers.d]]]), >> + [SYSUSERSDIR=$withval], >> + [SYSUSERSDIR='${prefix}/lib/sysusers.d']) >> + AS_IF([test "x$LOGGROUP" = xroot], >> + [SYSUSERS_GROUP_LINE=], >> + [SYSUSERS_GROUP_LINE="g $LOGGROUP -"]) >> + AS_IF([test "x$LOGUSER" = xroot], >> + [SYSUSERS_USER_LINE=], >> + [SYSUSERS_USER_LINE="u $LOGUSER -:$LOGGROUP \"OVN log user\" -"]) >> + AC_SUBST([SYSUSERSDIR]) >> + AC_SUBST([SYSUSERS_GROUP_LINE]) >> + AC_SUBST([SYSUSERS_USER_LINE])]) >> + >> 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..bba4828db 100644 >> --- a/rhel/ovn-fedora.spec.in >> +++ b/rhel/ovn-fedora.spec.in Just an FYI, this is _not_ the spec Fedora actually uses, that's managed separately [0]. This in-tree spec is however used by our CI. [0] https://src.fedoraproject.org/rpms/ovn/blob/rawhide/f/ovn.spec >> @@ -163,6 +163,9 @@ cd - >> --disable-libcapng \ >> %endif >> --enable-ssl \ >> + --enable-tmpfiles-log-ownership \ >> + --with-log-user=openvswitch \ >> + --with-log-group=openvswitch \ >> --with-pkidir=%{_sharedstatedir}/openvswitch/pki \ >> --with-version-suffix=-%{release} \ >> PYTHON3=%{__python3} >> @@ -185,6 +188,10 @@ done >> >> rm -rf $RPM_BUILD_ROOT/%{_datadir}/openvswitch/python/ >> >> +# The openvswitch user is managed by the Open vSwitch package's >> +# sysusers.d configuration on Fedora and RHEL. >> +rm -f $RPM_BUILD_ROOT%{_sysusersdir}/ovn-sysusers.conf >> + I dislike this differentiation in behavior between Fedora/RHEL and Debian/Ubuntu. Why isn't the Debian/Ubuntu OVS package managing the sysusers.d configuration? >> install -d -m 0755 $RPM_BUILD_ROOT/%{_sharedstatedir}/ovn >> >> install -d $RPM_BUILD_ROOT%{_prefix}/lib/firewalld/services/ >> @@ -343,6 +350,7 @@ fi >> >> %post >> ln -sf ovn_detrace.py %{_bindir}/ovn-detrace >> +%tmpfiles_create ovn-tmpfiles.conf Right now this only creates ovn-controller.log with the correct ownership and permission. Should this be in a "%post host" section? See below my question for other OVN log files, should we add the relevant ones to each "%post .." section e.g., to "%post central"? >> >> %if %{with libcapng} >> if [ $1 -eq 1 ]; then >> @@ -531,6 +539,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..a6125f5b0 100644 >> --- a/utilities/automake.mk >> +++ b/utilities/automake.mk >> @@ -26,6 +26,8 @@ EXTRA_DIST += \ >> utilities/ovn-ctl \ >> utilities/ovn-lib.in \ >> utilities/ovn-ctl.8.xml \ >> + utilities/ovn-tmpfiles.conf.in \ >> + utilities/ovn-sysusers.conf.in \ >> utilities/ovn-docker-overlay-driver.in \ >> utilities/ovn-docker-underlay-driver.in \ >> utilities/ovn-nbctl.8.xml \ >> @@ -48,6 +50,8 @@ EXTRA_DIST += \ >> CLEANFILES += \ >> utilities/ovn-ctl.8 \ >> utilities/ovn-lib \ >> + utilities/ovn-tmpfiles.conf \ >> + utilities/ovn-sysusers.conf \ >> utilities/ovn-docker-overlay-driver \ >> utilities/ovn-docker-underlay-driver \ >> utilities/ovn-nbctl.8 \ >> @@ -66,7 +70,13 @@ CLEANFILES += \ >> EXTRA_DIST += utilities/ovn-sim.in >> noinst_SCRIPTS += utilities/ovn-sim >> >> +if TMPFILES_LOG_OWNERSHIP >> +tmpfiles_DATA += utilities/ovn-tmpfiles.conf >> +sysusers_DATA += utilities/ovn-sysusers.conf >> +endif >> utilities/ovn-lib: $(top_builddir)/config.status >> +utilities/ovn-tmpfiles.conf: $(top_builddir)/config.status >> +utilities/ovn-sysusers.conf: $(top_builddir)/config.status >> >> # ovn-nbctl >> bin_PROGRAMS += utilities/ovn-nbctl >> diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl >> index 3b62ca9b7..40794cedc 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,9 @@ $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 >> + if test "$ovn_chown_logdir" = yes; then >> + chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_logdir >> + fi >> 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..ce7d88b81 100644 >> --- a/utilities/ovn-lib.in >> +++ b/utilities/ovn-lib.in >> @@ -29,6 +29,7 @@ ovn_etcdir=$ovn_sysconfdir/ovn # >> /etc/ovn >> ovn_datadir=${OVN_PKGDATADIR-'@pkgdatadir@'} # /usr/share/ovn >> ovn_bindir=${OVN_BINDIR-'@bindir@'} # /usr/bin >> ovn_sbindir=${OVN_SBINDIR-'@sbindir@'} # /usr/sbin >> +ovn_chown_logdir='@OVN_CHOWN_LOGDIR@' >> >> # /etc/ovn or /var/lib/ovn >> if test X"$OVN_DBDIR" != X; then >> @@ -133,7 +134,9 @@ start_ovn_daemon () { >> set "$@" --detach >> test X"$MONITOR" = Xno || set "$@" --monitor >> >> - chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_logdir >> + if test "$ovn_chown_logdir" = yes; then >> + chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_logdir >> + fi >> chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_rundir >> >> start_wrapped_daemon "$wrapper" $daemon "$priority" "$@" >> diff --git a/utilities/ovn-sysusers.conf.in b/utilities/ >> ovn-sysusers.conf.in >> new file mode 100644 >> index 000000000..8745d0a44 >> --- /dev/null >> +++ b/utilities/ovn-sysusers.conf.in >> @@ -0,0 +1,2 @@ >> +@SYSUSERS_GROUP_LINE@ >> +@SYSUSERS_USER_LINE@ This yields a file with two empty lines if LOGGROUP and LOGUSER are not set. Kind of weird, but OK I guess. >> \ No newline at end of file No newline at end of file. >> 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@ - What about other log files, e.g. ovn-northd.log or all the DB log files? >> \ No newline at end of file No newline at end of file. >> -- >> 2.43.0 >> >> > Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
