Hi Dumitru, Thanks for the review. I've addressed your comments [1] below in v6.
1, this is _not_ the spec Fedora actually uses, that's managed separately. This in-tree spec is however used by our CI. Thanks for the clarification. The in-tree spec change is only for the in-tree RPM/CI build and does not claim to update Fedora's separately maintained spec. 2, 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? Both Debian/Ubuntu and Fedora/RHEL already provide the required identities: Debian uses root:adm, while Fedora/RHEL uses the openvswitch identity provided by Open vSwitch. Therefore both disable OVN sysusers by default. v6 decouples sysusers from tmpfiles. The new --enable-sysusers-log-ownership option controls sysusers independently. The in-tree Fedora spec has an optional ovn_sysusers condition controlling configure, package contents, and %sysusers_create consistently. 3, Right now this only creates ovn-controller.log with the correct ownership and permission. Should this be in a "%post host" section? ... What about other log files, e.g. ovn-northd.log or all the DB log files? v6 does not enumerate log files or add per-package %post handling. tmpfiles creates only the log directory with mode 2750. The setgid bit makes files created by OVN daemons inherit the configured log group, so this also covers northd and database log files without a static list. 4, This yields a file with two empty lines if LOGGROUP and LOGUSER are not set. Kind of weird, but OK I guess. v6 installs the sysusers file only when sysusers ownership is enabled and at least one configured identity is non-root. It is not installed for root:root. 5, No newline at end of file. Added final newlines to both templates. [1] https://mail.openvswitch.org/pipermail/ovs-dev/2026-September/436171.html On Tue, Sep 22, 2026 at 3:56 PM Dumitru Ceara <[email protected]> wrote: > 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 > > -- 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
