Re: [libvirt] [PATCH v3 20/48] remote: introduce virtproxyd daemon to handle IP connectivity
On Mon, 2019-07-29 at 18:11 +0100, Daniel P. Berrangé wrote: [...] > +++ b/src/remote/Makefile.inc.am > +remote/virtproxyd.conf: remote/libvirtd.conf.in > + $(AM_V_GEN)sed \ > + -e '/[@]CUT_ENABLE_IP[@]/d' \ > + -e '/[@]END[@]/d' \ > + -e 's/[@]DAEMON_NAME[@]/virtproxyd/' \ > + < $^ > $@ Unnecessary input redirection. [...] > +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/' \ > + $< > $@ Same here. [...] > +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 $@ Wrong formatting for first argument (should be on its own line). [...] > 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 $@ Unnecessary input redirection and questionable formatting: $(LIBVIRTD_UNIT_VARS) should be on a separate line. Also, and I'm sorry I've just noticed this, you're using 'sed' instead of $(SED). All three issues trace back to previous patches and should be fixed there. > +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 $@ These rules also suffer from all the issues listed above, but they're introduced by this patch. [...] > +++ b/src/remote/remote_daemon_config.c > @@ -108,7 +108,11 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED) > +# ifdef LIBVIRTD > +data->listen_tls = 1; /* Only honoured it --listen is set */ s/it --listen/if --listen/ -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 20/48] remote: introduce virtproxyd daemon to handle IP connectivity
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é > --- > .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 045c0fed1
[libvirt] [PATCH v3 20/48] remote: introduce virtproxyd daemon to handle IP connectivity
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
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.
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é
---
.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-tl
