[netcf-devel] [PATCH 2/2] support systemd-based netcf-transaction

Eric Blake eblake at redhat.com
Tue Jan 21 19:14:13 UTC 2014


On 01/21/2014 05:55 AM, Laine Stump wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1044681
> 
> The netcf-transaction script is used in two ways:
> 
> 1) It is called by libnetcf.so to begin, commit, and manually rollback
> a set of network interface config changes.
> 
> 2) It is called as part of normal system startup to automatically
> rollback any config changes that are part of a netcf transaction that
> hasn't yet been committed. (in other words, if you reboot after
> calling ncf_change_begin() but before calling ncf_change_commit(), all
> the changes you made subsequent to ncf_change_begin() will be erased).
> 

> NB: the location of the netcf-transaction script, which src/drv_*.c
> find via the NETCF_TRANSACTION #define is currently hardcoded to look
> in /usr/libexec. Ideally, this should use $libexecdir, but that
> resolves to "${exec_prefix}/libexec" at configure time. To get the
> desired results, we need to update gnulib and use gnulib's configmake
> package, this way we'll end up with a NETCF_TRANSACTION that will be
> set at the time make is run rather than configure. This is left as a
> separate patch though, so that the initial support for systemd can be
> backported to older releases of netcf (a gnulib update shouldn't be
> backported to the maintenance branch of an already-cut release).

And I'm working on that followup :)


> +    *)
> +       AC_MSG_ERROR([Unknown system initscript flavour $with_sysinit])

Should I even care about US vs. UK spelling?

> +    ;;
> +esac
> +if test "x$with_sysinit" = "xcheck" ; then
> +  with_sysinit=none
> +fi
> +
> +AM_CONDITIONAL([NETCF_USE_INITSCRIPTS], test "$with_sysinit" = "initscripts")
> +AM_CONDITIONAL([NETCF_USE_SYSTEMD], test "$with_sysinit" = "systemd")

Libvirt allowed initscripts+systemd, for installing both methods
simultaneously; but I'm okay with not adding that complexity here in netcf.

> +AC_MSG_RESULT($with_sysinit)
> +if test "$with_sysinit" != "none" && test "$with_driver" != "redhat"; then
> +  AC_MSG_ERROR([netcf does not have support for $with_sysinit combined with the $with_driver driver])
> +fi

Maybe a bit over-restrictive, but it will force other distros to send
patches to get their flavor supported :)

> +++ b/src/Makefile.am
> @@ -16,7 +16,9 @@ include_HEADERS = netcf.h
>  lib_LTLIBRARIES = libnetcf.la
>  
>  bin_PROGRAMS = ncftool
> -
> +if NETCF_DRIVER_REDHAT
> +libexec_SCRIPTS = netcf-transaction.sh
> +endif

Rather, shouldn't this libexec_SCRIPTS be dependent on
NETCF_USE_INITSCRIPTS || NETCF_USE_SYSTEMD?  Just because
NETCF_DRIVER_REDHAT happens to be the only case where we need the helper
script, it seems like it is less fragile to make the installation of the
helper script depend on the init script mode chosen, rather than the
driver interaction that happens to use init scripts.  But again, it
matches your configure.ac, and if anyone else cares, they can fix this
at the same time as changing configure.ac to allow their distro.


> +netcf-transaction.init: netcf-transaction.init.in \
> +                        $(top_builddir)/config.status
> +	$(AM_V_GEN)sed \
> +		-e 's!\@localstatedir\@!$(localstatedir)!g' \
> +		-e 's|[@]libexecdir[@]|$(libexecdir)|g'     \

Odd mix of \@ vs. [@].  For consistency, it might be nice to pick one
form (and for that matter, \@ is non-portable sed) - but that's okay for
a followup patch particularly since it is probably copy-and-paste from
libvirt, meaning a similar patch is needed there too.

ACK.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <https://lists.fedorahosted.org/pipermail/netcf-devel/attachments/20140121/bf1e35d6/attachment.sig>


More information about the netcf-devel mailing list