[libvirt] [PATCH v3 20/48] remote: introduce virtproxyd daemon to handle IP connectivity

Christophe de Dinechin dinechin at redhat.com
Tue Jul 30 11:04:27 UTC 2019


Daniel P. Berrangé writes:

> The libvirtd daemon provides the traditional libvirt experience where
> all the drivers are in a single daemon, and is accessible over both
> local UNIX sockets and remote IP sockets.
>
> In the new world we're having a set of per-driver daemons which will
> primarily be accessed locally via their own UNIX sockets.
>
> We still, however, need to allow for case of applications which will
> connect to libvirt remotely. These remote connections can be done as
> TCP/TLS sockets, or by SSH tunnelling to the UNIX socket.
>
> In the later case, the old libvirt.so clients will only know about
> the path to the old libvirtd socket /var/run/libvirt/libvirt-sock,
> and not the new driver sockets /var/run/libvirt/virtqemud-sock.
>
> It is also not desirable to expose the main driver specific daemons
> over IP directly to minimize their attack service.
>
> Thus the virtproxyd daemon steps into place, to provide TCP/TLS sockets,
> and back compat for the old libvirtd UNIX socket path(s). It will then
> forward all RPC calls made to the appropriate driver specific daemon.
>
> Essentially it is equivalent to the old libvirtd with absolutely no
> drivers registered except for the remote driver (and other stateless
> drivers in libvirt.so).
>
> We could have modified libvirtd so none of the drivers are registed
> to get the same end result. We could even add a libvirtd.conf parameter
> to control whether the drivers are loaded to enable users to switch back
> to the old world if we discover bugs in the split-daemon model. Using a
> new daemon though has some advantages
>
>  - We can make virtproxyd and the virtXXXd per-driver daemons all
>    have "Conflicts: libvirtd.service" in their systemd unit files.
>    This will guarantee that libvirtd is never started at the same
>    time, as this would result in two daemons running the same driver.
>    Fortunately drivers use locking to protect themselves, but it is
>    better to avoid starting a daemon we know will conflict.
>
>  - It allows us to break CLI compat to remove the --listen parameter.
>    Both listen_tcp and listen_tls parameters in /etc/libvirtd/virtd.conf
>    will default to zero. Either TLS or TCP can be enabled exclusively
>    though virtd.conf without requiring the extra step of adding --listen.
>
>  - It allows us to set a strict SELinux policy over virtproxyd. For
>    back compat the libvirtd policy must continue to allow all drivers
>    to run. We can't easily give a second policy to libvirtd which
>    locks it down. By introducing a new virtproxyd we can set a strict
>    policy for that daemon only.
>
>  - It gets rid of the wierd naming of having a daemon with "lib" in

"weird"

>    its name. Now all normal daemons libvirt ships will have "virt"
>    as their prefix not "libvirt".
>
>  - Distros can more easily choose their upgrade path. They can
>    ship both sets of daemons in their packages, and choose to
>    either enable libvirtd, or enable the per-driver daemons and
>    virtproxyd out of the box. Users can easily override this if
>    desired by just tweaking which systemd units are active.
>
> After some time we can deprecate use of libvirtd and after some more
> time delete it entirely, leaving us in a pretty world filled with
> prancing unicorns.

Had to learn a new word, "prancing" :-)

Note that prancing unicorns are surrounded by a swarm of daemons. Not
sure they will like it. Just saying.

>
> The main downside with introducing a new daemon, and with the
> per-driver daemons in general, is figuring out the correct upgrade
> path.
>
> The conservative option is to leave libvirtd running if it was
> an existing installation. Only use the new daemons & virtproxyd
> on completely new installs.
>
> The aggressive option is to disable libvirtd if already running
> and activate all the new daemons.
>
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>  .gitignore                        |   4 ++
>  libvirt.spec.in                   |  10 +++
>  src/remote/Makefile.inc.am        | 111 +++++++++++++++++++++++++++---
>  src/remote/remote_daemon.c        |  28 +++++---
>  src/remote/remote_daemon_config.c |   6 +-
>  src/remote/virtproxyd.service.in  |  24 +++++++
>  6 files changed, 163 insertions(+), 20 deletions(-)
>  create mode 100644 src/remote/virtproxyd.service.in
>
> diff --git a/.gitignore b/.gitignore
> index 4463660c85..05bc166860 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -161,6 +161,9 @@
>  /src/remote/libvirtd.aug
>  /src/remote/libvirtd.conf
>  /src/remote/test_libvirtd.aug
> +/src/remote/test_virtproxyd.aug
> +/src/remote/virtproxyd.aug
> +/src/remote/virtproxyd.conf
>  /src/rpc/virkeepaliveprotocol.[ch]
>  /src/rpc/virnetprotocol.[ch]
>  /src/util/virkeycodetable*.h
> @@ -168,6 +171,7 @@
>  /src/virt-aa-helper
>  /src/virtlockd
>  /src/virtlogd
> +/src/virtproxyd
>  /src/virt-guest-shutdown.target
>  /tests/*.log
>  /tests/*.pid
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 045c0fed1a..c7f276b2bc 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -1527,6 +1527,12 @@ exit 0
>  %{_unitdir}/libvirtd-admin.socket
>  %{_unitdir}/libvirtd-tcp.socket
>  %{_unitdir}/libvirtd-tls.socket
> +%{_unitdir}/virtproxyd.service
> +%{_unitdir}/virtproxyd.socket
> +%{_unitdir}/virtproxyd-ro.socket
> +%{_unitdir}/virtproxyd-admin.socket
> +%{_unitdir}/virtproxyd-tcp.socket
> +%{_unitdir}/virtproxyd-tls.socket
>  %{_unitdir}/virt-guest-shutdown.target
>  %{_unitdir}/virtlogd.service
>  %{_unitdir}/virtlogd.socket
> @@ -1538,6 +1544,7 @@ exit 0
>  %config(noreplace) %{_sysconfdir}/sysconfig/virtlogd
>  %config(noreplace) %{_sysconfdir}/sysconfig/virtlockd
>  %config(noreplace) %{_sysconfdir}/libvirt/libvirtd.conf
> +%config(noreplace) %{_sysconfdir}/libvirt/virtproxyd.conf
>  %config(noreplace) %{_sysconfdir}/libvirt/virtlogd.conf
>  %config(noreplace) %{_sysconfdir}/libvirt/virtlockd.conf
>  %config(noreplace) %{_sysconfdir}/sasl2/libvirt.conf
> @@ -1565,6 +1572,8 @@ exit 0
>  %{_datadir}/augeas/lenses/tests/test_virtlogd.aug
>  %{_datadir}/augeas/lenses/virtlockd.aug
>  %{_datadir}/augeas/lenses/tests/test_virtlockd.aug
> +%{_datadir}/augeas/lenses/virtproxyd.aug
> +%{_datadir}/augeas/lenses/tests/test_virtproxyd.aug
>  %{_datadir}/augeas/lenses/libvirt_lockd.aug
>  %if %{with_qemu}
>  %{_datadir}/augeas/lenses/tests/test_libvirt_lockd.aug
> @@ -1579,6 +1588,7 @@ exit 0
>  %attr(0755, root, root) %{_libexecdir}/libvirt_iohelper
>
>  %attr(0755, root, root) %{_sbindir}/libvirtd
> +%attr(0755, root, root) %{_sbindir}/virtproxyd
>  %attr(0755, root, root) %{_sbindir}/virtlogd
>  %attr(0755, root, root) %{_sbindir}/virtlockd
>
> diff --git a/src/remote/Makefile.inc.am b/src/remote/Makefile.inc.am
> index c9c3c7203a..344f19311a 100644
> --- a/src/remote/Makefile.inc.am
> +++ b/src/remote/Makefile.inc.am
> @@ -89,18 +89,40 @@ SYSCONF_FILES += remote/libvirtd.sysconf
>  PODFILES += remote/libvirtd.pod
>  MANINFILES += libvirtd.8.in
>
> -LIBVIRTD_UNIT_FILES_IN = \
> -	remote/libvirtd.service.in \
> +LIBVIRTD_SOCKET_UNIT_FILES_IN = \
>  	remote/libvirtd.socket.in \
>  	remote/libvirtd-ro.socket.in \
>  	remote/libvirtd-admin.socket.in \
>  	remote/libvirtd-tcp.socket.in \
>  	remote/libvirtd-tls.socket.in \
> +	$(NULL)
> +
> +LIBVIRTD_SOCKET_UNIT_FILES = $(notdir $(LIBVIRTD_SOCKET_UNIT_FILES_IN:%.in=%))
> +
> +LIBVIRTD_UNIT_FILES_IN = \
> +	remote/libvirtd.service.in \
> +	$(LIBVIRTD_SOCKET_UNIT_FILES_IN) \
> +	$(NULL)
> +
> +VIRTPROXYD_UNIT_FILES_IN = \
> +	remote/virtproxyd.service.in \
> +	$(NULL)
> +
> +GUEST_UNIT_FILES_IN = \
>  	remote/virt-guest-shutdown.target.in \
>  	$(NULL)
>
> -SYSTEMD_UNIT_FILES += $(notdir $(LIBVIRTD_UNIT_FILES_IN:%.in=%))
> -SYSTEMD_UNIT_FILES_IN += $(LIBVIRTD_UNIT_FILES_IN)
> +
> +SYSTEMD_UNIT_FILES += \
> +	$(notdir $(LIBVIRTD_UNIT_FILES_IN:%.in=%)) \
> +	$(notdir $(LIBVIRTD_UNIT_FILES_IN:remote/libvirtd%.in=remote/virtproxyd%)) \
> +	$(notdir $(GUEST_UNIT_FILES_IN:%.in=%)) \
> +	$(NULL)
> +SYSTEMD_UNIT_FILES_IN += \
> +	$(LIBVIRTD_UNIT_FILES_IN) \
> +	$(VIRTPROXYD_UNIT_FILES_IN) \
> +	$(GUEST_UNIT_FILES_IN) \
> +	$(NULL)
>
>  REMOTE_PROTOCOL = $(srcdir)/remote/remote_protocol.x
>  LXC_PROTOCOL = $(srcdir)/remote/lxc_protocol.x
> @@ -138,6 +160,7 @@ MAINTAINERCLEANFILES += \
>  	$(NULL)
>  CLEANFILES += \
>  	remote/libvirtd.conf \
> +	remote/virtproxyd.conf \
>  	$(NULL)
>
>  if WITH_REMOTE
> @@ -168,15 +191,27 @@ endif ! WITH_REMOTE
>
>  if WITH_LIBVIRTD
>
> -sbin_PROGRAMS += libvirtd
> +sbin_PROGRAMS += libvirtd virtproxyd
>
> -augeas_DATA += remote/libvirtd.aug
> +augeas_DATA += \
> +	remote/libvirtd.aug \
> +	remote/virtproxyd.aug \
> +	$(NULL)
>
> -augeastest_DATA += remote/test_libvirtd.aug
> +augeastest_DATA += \
> +	remote/test_libvirtd.aug \
> +	remote/test_virtproxyd.aug \
> +	$(NULL)
>
> -nodist_conf_DATA += remote/libvirtd.conf
> +nodist_conf_DATA += \
> +	remote/libvirtd.conf \
> +	remote/virtproxyd.conf \
> +	$(NULL)
>
> -CLEANFILES += remote/libvirtd.aug
> +CLEANFILES += \
> +	remote/libvirtd.aug \
> +	remote/virtproxyd.aug \
> +	$(NULL)
>
>  man8_MANS += libvirtd.8
>
> @@ -187,12 +222,23 @@ libvirtd_CFLAGS = \
>  	-DSOCK_PREFIX="\"libvirt\"" \
>  	-DDAEMON_NAME="\"libvirtd\"" \
>  	-DENABLE_IP \
> +	-DLIBVIRTD \
>  	$(NULL)
>
>  libvirtd_LDFLAGS = $(REMOTE_DAEMON_LD_FLAGS)
>
>  libvirtd_LDADD = $(REMOTE_DAEMON_LD_ADD)
>
> +virtproxyd_SOURCES = $(REMOTE_DAEMON_SOURCES)
> +virtproxyd_CFLAGS = \
> +	$(REMOTE_DAEMON_CFLAGS) \
> +	-DSOCK_PREFIX="\"libvirt\"" \
> +	-DDAEMON_NAME="\"virtproxyd\"" \
> +	-DENABLE_IP \
> +	$(NULL)
> +virtproxyd_LDFLAGS = $(REMOTE_DAEMON_LD_FLAGS)
> +virtproxyd_LDADD = $(REMOTE_DAEMON_LD_ADD)
> +
>  remote/libvirtd.conf: remote/libvirtd.conf.in
>  	$(AM_V_GEN)$(SED) \
>  		-e '/[@]CUT_ENABLE_IP[@]/d' \
> @@ -201,6 +247,13 @@ remote/libvirtd.conf: remote/libvirtd.conf.in
>  		-e 's|[@]DAEMON_NAME[@]|libvirtd|' \
>  		< $< > $@
>
> +remote/virtproxyd.conf: remote/libvirtd.conf.in
> +	$(AM_V_GEN)sed \
> +		-e '/[@]CUT_ENABLE_IP[@]/d' \
> +		-e '/[@]END[@]/d' \
> +		-e 's/[@]DAEMON_NAME[@]/virtproxyd/' \
> +		< $^ > $@
> +
>  INSTALL_DATA_DIRS += remote
>
>  install-data-remote:
> @@ -218,6 +271,14 @@ remote/libvirtd.aug: remote/libvirtd.aug.in
>  		-e 's|[@]DAEMON_NAME_UC[@]|Libvirtd|' \
>  		$< > $@
>
> +remote/virtproxyd.aug: remote/libvirtd.aug.in
> +	$(AM_V_GEN)$(SED) \
> +		-e '/[@]CUT_ENABLE_IP[@]/d' \
> +		-e '/[@]END[@]/d' \
> +		-e 's/[@]DAEMON_NAME[@]/virtproxyd/' \
> +		-e 's/[@]DAEMON_NAME_UC[@]/Virtproxyd/' \
> +		$< > $@
> +
>  remote/test_libvirtd.aug: remote/test_libvirtd.aug.in \
>  		remote/libvirtd.conf $(AUG_GENTEST)
>  	$(AM_V_GEN)$(AUG_GENTEST) remote/libvirtd.conf \
> @@ -229,6 +290,16 @@ remote/test_libvirtd.aug: remote/test_libvirtd.aug.in \
>  		-e 's|[@]DAEMON_NAME_UC[@]|Libvirtd|' \
>  		> $@ || rm -f $@
>
> +remote/test_virtproxyd.aug: remote/test_libvirtd.aug.in \
> +		remote/virtproxyd.conf $(AUG_GENTEST)
> +	$(AM_V_GEN)$(AUG_GENTEST) remote/virtproxyd.conf \
> +		$(srcdir)/remote/test_libvirtd.aug.in | \
> +		$(SED) -e '/[@]CUT_ENABLE_IP[@]/d' \
> +		-e '/[@]END[@]/d' \
> +		-e 's/[@]DAEMON_NAME[@]/virtproxyd/' \
> +		-e 's/[@]DAEMON_NAME_UC[@]/Virtproxyd/' \
> +		> $@ || rm -f $@
> +
>  if WITH_SYSCTL
>  # Use $(prefix)/lib rather than $(libdir), since man sysctl.d insists on
>  # /usr/lib/sysctl.d/ even when libdir is /usr/lib64
> @@ -303,11 +374,31 @@ LIBVIRTD_UNIT_VARS = \
>  	-e 's|[@]deps[@]||g' \
>  	$(NULL)
>
> +VIRTD_UNIT_VARS = \
> +	$(COMMON_UNIT_VARS) \
> +	-e 's|[@]deps[@]|Conflicts=$(LIBVIRTD_SOCKET_UNIT_FILES)|g' \
> +	$(NULL)
> +
> +VIRTPROXYD_UNIT_VARS = \
> +	$(VIRTD_UNIT_VARS) \
> +	-e 's|[@]name[@]|Libvirt proxy|g' \
> +	-e 's|[@]service[@]|virtproxyd|g' \
> +	-e 's|[@]sockprefix[@]|libvirt|g' \
> +	$(NULL)
> +
>  libvirtd.service: remote/libvirtd.service.in $(top_builddir)/config.status
>  	$(AM_V_GEN)sed $(LIBVIRTD_UNIT_VARS) < $< > $@-t && mv $@-t $@
>
>  libvirt%.socket: remote/libvirt%.socket.in $(top_builddir)/config.status
> -	$(AM_V_GEN)sed $(LIBVIRTD_UNIT_VARS) < $< > $@-t && mv $@-t $@
> +	$(AM_V_GEN)sed $(LIBVIRTD_UNIT_VARS) \
> +		< $< > $@-t && mv $@-t $@
> +
> +virtproxyd.service: remote/virtproxyd.service.in $(top_builddir)/config.status
> +	$(AM_V_GEN)sed $(VIRTPROXYD_UNIT_VARS) < $< > $@-t && mv $@-t $@
> +
> +virtproxy%.socket: remote/libvirt%.socket.in $(top_builddir)/config.status
> +	$(AM_V_GEN)sed $(VIRTPROXYD_UNIT_VARS) \
> +		< $< > $@-t && mv $@-t $@
>
>  virt-guest-shutdown.target: remote/virt-guest-shutdown.target.in \
>  			$(top_builddir)/config.status
> diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
> index 42c51c1329..02a33d6754 100644
> --- a/src/remote/remote_daemon.c
> +++ b/src/remote/remote_daemon.c
> @@ -311,10 +311,16 @@ static int daemonErrorLogFilter(virErrorPtr err, int priority)
>
>  static int daemonInitialize(void)
>  {
> -#ifdef MODULE_NAME
> +#ifndef LIBVIRTD
> +# ifdef MODULE_NAME
>      /* This a dedicated per-driver daemon build */
>      if (virDriverLoadModule(MODULE_NAME, MODULE_NAME "Register", true) < 0)
>          return -1;
> +# else
> +    /* This is virtproxyd which merely proxies to the per-driver
> +     * daemons for back compat, and also allows IP connectivity.
> +     */
> +# endif
>  #else
>      /* This is the legacy monolithic libvirtd built with all drivers
>       *
> @@ -906,9 +912,9 @@ daemonUsage(const char *argv0, bool privileged)
>          { "-h | --help", N_("Display program help") },
>          { "-v | --verbose", N_("Verbose messages") },
>          { "-d | --daemon", N_("Run as a daemon & write PID file") },
> -#ifdef ENABLE_IP
> +#if defined(ENABLE_IP) && defined(LIBVIRTD)
>          { "-l | --listen", N_("Listen for TCP/IP connections") },
> -#endif /* ENABLE_IP */
> +#endif /* ENABLE_IP && LIBVIRTD */
>          { "-t | --timeout <secs>", N_("Exit after timeout period") },
>          { "-f | --config <file>", N_("Configuration file") },
>          { "-V | --version", N_("Display version information") },
> @@ -985,7 +991,11 @@ int main(int argc, char **argv) {
>      int verbose = 0;
>      int godaemon = 0;
>  #ifdef ENABLE_IP
> +# ifdef LIBVIRTD
>      int ipsock = 0;
> +# else
> +    int ipsock = 1; /* listen_tcp/listen_tls default to 0 */
> +# endif
>  #endif /* ! ENABLE_IP */
>      struct daemonConfig *config;
>      bool privileged = geteuid() == 0 ? true : false;
> @@ -996,9 +1006,9 @@ int main(int argc, char **argv) {
>      struct option opts[] = {
>          { "verbose", no_argument, &verbose, 'v'},
>          { "daemon", no_argument, &godaemon, 'd'},
> -#ifdef ENABLE_IP
> +#if defined(ENABLE_IP) && defined(LIBVIRTD)
>          { "listen", no_argument, &ipsock, 'l'},
> -#endif /* ! ENABLE_IP */
> +#endif /* ENABLE_IP && LIBVIRTD */
>          { "config", required_argument, NULL, 'f'},
>          { "timeout", required_argument, NULL, 't'},
>          { "pid-file", required_argument, NULL, 'p'},
> @@ -1021,11 +1031,11 @@ int main(int argc, char **argv) {
>          int optidx = 0;
>          int c;
>          char *tmp;
> -#ifdef ENABLE_IP
> +#if defined(ENABLE_IP) && defined(LIBVIRTD)
>          const char *optstr = "ldf:p:t:vVh";
> -#else /* ! ENABLE_IP */
> +#else /* ! ENABLE_IP && ! LIBVIRTD */
>          const char *optstr = "df:p:t:vVh";
> -#endif /* ! ENABLE_IP */
> +#endif /* ! ENABLE_IP && ! LIBVIRTD */
>
>          c = getopt_long(argc, argv, optstr, opts, &optidx);
>
> @@ -1043,7 +1053,7 @@ int main(int argc, char **argv) {
>              godaemon = 1;
>              break;
>
> -#ifdef ENABLE_IP
> +#if defined(ENABLE_IP) && defined(LIBVIRTD)
>          case 'l':
>              ipsock = 1;
>              break;
> diff --git a/src/remote/remote_daemon_config.c b/src/remote/remote_daemon_config.c
> index 3c5ccd5ba8..f583442dc7 100644
> --- a/src/remote/remote_daemon_config.c
> +++ b/src/remote/remote_daemon_config.c
> @@ -108,7 +108,11 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED)
>          return NULL;
>
>  #ifdef ENABLE_IP
> -    data->listen_tls = 1;
> +# ifdef LIBVIRTD
> +    data->listen_tls = 1; /* Only honoured it --listen is set */
> +# else /* ! LIBVIRTD */
> +    data->listen_tls = 0; /* Always honoured, --listen doesn't exist. */
> +# endif /* ! LIBVIRTD */
>      data->listen_tcp = 0;
>
>      if (VIR_STRDUP(data->tls_port, LIBVIRTD_TLS_PORT) < 0 ||
> diff --git a/src/remote/virtproxyd.service.in b/src/remote/virtproxyd.service.in
> new file mode 100644
> index 0000000000..e99e2af19c
> --- /dev/null
> +++ b/src/remote/virtproxyd.service.in
> @@ -0,0 +1,24 @@
> +[Unit]
> +Description=Virtualization daemon
> +Conflicts=libvirtd.service
> +Requires=virtproxyd.socket
> +Requires=virtproxyd-ro.socket
> +Requires=virtproxyd-admin.socket
> +After=network.target
> +After=dbus.service
> +After=apparmor.service
> +After=local-fs.target
> +Documentation=man:libvirtd(8)
> +Documentation=https://libvirt.org
> +
> +[Service]
> +Type=notify
> +ExecStart=@sbindir@/virtproxyd --timeout 120
> +ExecReload=/bin/kill -HUP $MAINPID
> +Restart=on-failure
> +
> +[Install]
> +WantedBy=multi-user.target
> +Also=virtproxyd.socket
> +Also=virtproxyd-ro.socket
> +Also=virtproxyd-admin.socket
> --
> 2.21.0

Reviewed-by: Christophe de Dinechin <dinechin at redhat.com>

--
Cheers,
Christophe de Dinechin (IRC c3d)




More information about the libvir-list mailing list