diff mbox

Revert "daemon: use socket activation with systemd"

Message ID 03725964ac422f27bb1600c1eeb91d7ee2e057e5.1460416087.git.crobinso@redhat.com
State Accepted
Commit 3b9100a4d255fdb5f731fcfa49326746d371894a
Headers show

Commit Message

Cole Robinson April 11, 2016, 11:08 p.m. UTC
This reverts commit 1e9808d3a1e00a7121bae8b163d9c42d441d2ca8.

We shouldn't advertise libvirtd.socket activation, since currently
it means VM/network/... autostart won't work as expected.

We tried to find a middle ground by installing the config file without
an [Install] section, since systemd won't allow .socket to be enabled
without one... or at least it did do that; presently on f24 it allows
activating the socket quite happily. This also caused user confusion[1]

Just remove the socket file. I've filed a new RFE to track coming up
with a solution to the autostart problem[2], we can point users at that
if there's more confusion:

[1]: https://bugzilla.redhat.com/show_bug.cgi?id=1279348
[2]: https://bugzilla.redhat.com/show_bug.cgi?id=1326136
---
 .gitignore                 |  1 -
 daemon/Makefile.am         | 14 ++------------
 daemon/libvirtd.conf       |  5 -----
 daemon/libvirtd.service.in |  5 +++++
 daemon/libvirtd.socket.in  | 11 -----------
 libvirt.spec.in            |  7 ++-----
 6 files changed, 9 insertions(+), 34 deletions(-)
 delete mode 100644 daemon/libvirtd.socket.in

-- 
2.7.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Comments

Cole Robinson April 19, 2016, 3:41 p.m. UTC | #1
ping. Martin you had suggested removing the socket file in one of the bugs,
are you cool with this?

Thanks,
Cole

On 04/11/2016 07:08 PM, Cole Robinson wrote:
> This reverts commit 1e9808d3a1e00a7121bae8b163d9c42d441d2ca8.

> 

> We shouldn't advertise libvirtd.socket activation, since currently

> it means VM/network/... autostart won't work as expected.

> 

> We tried to find a middle ground by installing the config file without

> an [Install] section, since systemd won't allow .socket to be enabled

> without one... or at least it did do that; presently on f24 it allows

> activating the socket quite happily. This also caused user confusion[1]

> 

> Just remove the socket file. I've filed a new RFE to track coming up

> with a solution to the autostart problem[2], we can point users at that

> if there's more confusion:

> 

> [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1279348

> [2]: https://bugzilla.redhat.com/show_bug.cgi?id=1326136

> ---

>  .gitignore                 |  1 -

>  daemon/Makefile.am         | 14 ++------------

>  daemon/libvirtd.conf       |  5 -----

>  daemon/libvirtd.service.in |  5 +++++

>  daemon/libvirtd.socket.in  | 11 -----------

>  libvirt.spec.in            |  7 ++-----

>  6 files changed, 9 insertions(+), 34 deletions(-)

>  delete mode 100644 daemon/libvirtd.socket.in

> 

> diff --git a/.gitignore b/.gitignore

> index 0d12c5c..381db69 100644

> --- a/.gitignore

> +++ b/.gitignore

> @@ -63,7 +63,6 @@

>  /daemon/libvirtd.pod

>  /daemon/libvirtd.policy

>  /daemon/libvirtd.service

> -/daemon/libvirtd.socket

>  /daemon/test_libvirtd.aug

>  /docs/aclperms.htmlinc

>  /docs/apibuild.py.stamp

> diff --git a/daemon/Makefile.am b/daemon/Makefile.am

> index 2dbe81b..fc6fd95 100644

> --- a/daemon/Makefile.am

> +++ b/daemon/Makefile.am

> @@ -59,7 +59,6 @@ EXTRA_DIST =						\

>  	libvirt.rules					\

>  	libvirtd.sasl					\

>  	libvirtd.service.in				\

> -	libvirtd.socket.in				\

>  	libvirtd.sysconf				\

>  	libvirtd.sysctl					\

>  	libvirtd.aug                                    \

> @@ -446,18 +445,15 @@ endif ! LIBVIRT_INIT_SCRIPT_UPSTART

>  if LIBVIRT_INIT_SCRIPT_SYSTEMD

>  

>  SYSTEMD_UNIT_DIR = $(prefix)/lib/systemd/system

> -BUILT_SOURCES += libvirtd.service libvirtd.socket

> +BUILT_SOURCES += libvirtd.service

>  

> -install-init-systemd: install-sysconfig libvirtd.service libvirtd.socket

> +install-init-systemd: install-sysconfig libvirtd.service

>  	$(MKDIR_P) $(DESTDIR)$(SYSTEMD_UNIT_DIR)

>  	$(INSTALL_DATA) libvirtd.service \

>  	  $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.service

> -	$(INSTALL_DATA) libvirtd.socket \

> -	  $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.socket

>  

>  uninstall-init-systemd: uninstall-sysconfig

>  	rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.service

> -	rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.socket

>  	rmdir $(DESTDIR)$(SYSTEMD_UNIT_DIR) || :

>  else ! LIBVIRT_INIT_SCRIPT_SYSTEMD

>  install-init-systemd:

> @@ -481,12 +477,6 @@ libvirtd.service: libvirtd.service.in $(top_builddir)/config.status

>  	    < $< > $@-t &&					\

>  	    mv $@-t $@

>  

> -libvirtd.socket: libvirtd.socket.in $(top_builddir)/config.status

> -	$(AM_V_GEN)sed						\

> -	    -e 's|[@]runstatedir[@]|$(runstatedir)|g'		\

> -	    < $< > $@-t &&					\

> -	    mv $@-t $@

> -

>  

>  check-local: check-augeas

>  

> diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf

> index 5485f98..d2c439c 100644

> --- a/daemon/libvirtd.conf

> +++ b/daemon/libvirtd.conf

> @@ -77,11 +77,6 @@

>  # UNIX socket access controls

>  #

>  

> -# Beware that if you are changing *any* of these options, and you use

> -# socket activation with systemd, you need to adjust the settings in

> -# the libvirtd.socket file as well since it could impose a security

> -# risk if you rely on file permission checking only.

> -

>  # Set the UNIX domain socket group ownership. This can be used to

>  # allow a 'trusted' set of users access to management capabilities

>  # without becoming root.

> diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in

> index 608221c..1616e7a 100644

> --- a/daemon/libvirtd.service.in

> +++ b/daemon/libvirtd.service.in

> @@ -1,3 +1,8 @@

> +# NB we don't use socket activation. When libvirtd starts it will

> +# spawn any virtual machines registered for autostart. We want this

> +# to occur on every boot, regardless of whether any client connects

> +# to a socket. Thus socket activation doesn't have any benefit

> +

>  [Unit]

>  Description=Virtualization daemon

>  Before=libvirt-guests.service

> diff --git a/daemon/libvirtd.socket.in b/daemon/libvirtd.socket.in

> deleted file mode 100644

> index 0915bb3..0000000

> --- a/daemon/libvirtd.socket.in

> +++ /dev/null

> @@ -1,11 +0,0 @@

> -[Socket]

> -ListenStream=@runstatedir@/libvirt/libvirt-sock

> -ListenStream=@runstatedir@/libvirt/libvirt-sock-ro

> -

> -; The following settings must match libvirtd.conf file in order to

> -; work as expected because libvirtd can't change them later.

> -; SocketMode=0777 is safe only if authentication on the socket is set

> -; up.  For further information, please see the libvirtd.conf file.

> -SocketMode=0777

> -SocketUser=root

> -SocketGroup=root

> diff --git a/libvirt.spec.in b/libvirt.spec.in

> index 8036fa3..c3bfea3 100644

> --- a/libvirt.spec.in

> +++ b/libvirt.spec.in

> @@ -1710,7 +1710,7 @@ exit 0

>  

>      %if %{with_systemd}

>          %if %{with_systemd_macros}

> -            %systemd_post virtlockd.socket virtlogd.socket libvirtd.service libvirtd.socket

> +            %systemd_post virtlockd.socket virtlogd.socket libvirtd.service

>          %else

>  if [ $1 -eq 1 ] ; then

>      # Initial installation

> @@ -1739,19 +1739,17 @@ fi

>  %preun daemon

>      %if %{with_systemd}

>          %if %{with_systemd_macros}

> -            %systemd_preun libvirtd.socket libvirtd.service virtlogd.socket virtlogd.service virtlockd.socket virtlockd.service

> +            %systemd_preun libvirtd.service virtlogd.socket virtlogd.service virtlockd.socket virtlockd.service

>          %else

>  if [ $1 -eq 0 ] ; then

>      # Package removal, not upgrade

>      /bin/systemctl --no-reload disable \

> -        libvirtd.socket \

>          libvirtd.service \

>          virtlogd.socket \

>          virtlogd.service \

>          virtlockd.socket \

>          virtlockd.service > /dev/null 2>&1 || :

>      /bin/systemctl stop \

> -        libvirtd.socket \

>          libvirtd.service \

>          virtlogd.socket \

>          virtlogd.service \

> @@ -1966,7 +1964,6 @@ exit 0

>  

>      %if %{with_systemd}

>  %{_unitdir}/libvirtd.service

> -%{_unitdir}/libvirtd.socket

>  %{_unitdir}/virtlogd.service

>  %{_unitdir}/virtlogd.socket

>  %{_unitdir}/virtlockd.service

> 


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
diff mbox

Patch

diff --git a/.gitignore b/.gitignore
index 0d12c5c..381db69 100644
--- a/.gitignore
+++ b/.gitignore
@@ -63,7 +63,6 @@ 
 /daemon/libvirtd.pod
 /daemon/libvirtd.policy
 /daemon/libvirtd.service
-/daemon/libvirtd.socket
 /daemon/test_libvirtd.aug
 /docs/aclperms.htmlinc
 /docs/apibuild.py.stamp
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 2dbe81b..fc6fd95 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -59,7 +59,6 @@  EXTRA_DIST =						\
 	libvirt.rules					\
 	libvirtd.sasl					\
 	libvirtd.service.in				\
-	libvirtd.socket.in				\
 	libvirtd.sysconf				\
 	libvirtd.sysctl					\
 	libvirtd.aug                                    \
@@ -446,18 +445,15 @@  endif ! LIBVIRT_INIT_SCRIPT_UPSTART
 if LIBVIRT_INIT_SCRIPT_SYSTEMD
 
 SYSTEMD_UNIT_DIR = $(prefix)/lib/systemd/system
-BUILT_SOURCES += libvirtd.service libvirtd.socket
+BUILT_SOURCES += libvirtd.service
 
-install-init-systemd: install-sysconfig libvirtd.service libvirtd.socket
+install-init-systemd: install-sysconfig libvirtd.service
 	$(MKDIR_P) $(DESTDIR)$(SYSTEMD_UNIT_DIR)
 	$(INSTALL_DATA) libvirtd.service \
 	  $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.service
-	$(INSTALL_DATA) libvirtd.socket \
-	  $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.socket
 
 uninstall-init-systemd: uninstall-sysconfig
 	rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.service
-	rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.socket
 	rmdir $(DESTDIR)$(SYSTEMD_UNIT_DIR) || :
 else ! LIBVIRT_INIT_SCRIPT_SYSTEMD
 install-init-systemd:
@@ -481,12 +477,6 @@  libvirtd.service: libvirtd.service.in $(top_builddir)/config.status
 	    < $< > $@-t &&					\
 	    mv $@-t $@
 
-libvirtd.socket: libvirtd.socket.in $(top_builddir)/config.status
-	$(AM_V_GEN)sed						\
-	    -e 's|[@]runstatedir[@]|$(runstatedir)|g'		\
-	    < $< > $@-t &&					\
-	    mv $@-t $@
-
 
 check-local: check-augeas
 
diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf
index 5485f98..d2c439c 100644
--- a/daemon/libvirtd.conf
+++ b/daemon/libvirtd.conf
@@ -77,11 +77,6 @@ 
 # UNIX socket access controls
 #
 
-# Beware that if you are changing *any* of these options, and you use
-# socket activation with systemd, you need to adjust the settings in
-# the libvirtd.socket file as well since it could impose a security
-# risk if you rely on file permission checking only.
-
 # Set the UNIX domain socket group ownership. This can be used to
 # allow a 'trusted' set of users access to management capabilities
 # without becoming root.
diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in
index 608221c..1616e7a 100644
--- a/daemon/libvirtd.service.in
+++ b/daemon/libvirtd.service.in
@@ -1,3 +1,8 @@ 
+# NB we don't use socket activation. When libvirtd starts it will
+# spawn any virtual machines registered for autostart. We want this
+# to occur on every boot, regardless of whether any client connects
+# to a socket. Thus socket activation doesn't have any benefit
+
 [Unit]
 Description=Virtualization daemon
 Before=libvirt-guests.service
diff --git a/daemon/libvirtd.socket.in b/daemon/libvirtd.socket.in
deleted file mode 100644
index 0915bb3..0000000
--- a/daemon/libvirtd.socket.in
+++ /dev/null
@@ -1,11 +0,0 @@ 
-[Socket]
-ListenStream=@runstatedir@/libvirt/libvirt-sock
-ListenStream=@runstatedir@/libvirt/libvirt-sock-ro
-
-; The following settings must match libvirtd.conf file in order to
-; work as expected because libvirtd can't change them later.
-; SocketMode=0777 is safe only if authentication on the socket is set
-; up.  For further information, please see the libvirtd.conf file.
-SocketMode=0777
-SocketUser=root
-SocketGroup=root
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 8036fa3..c3bfea3 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1710,7 +1710,7 @@  exit 0
 
     %if %{with_systemd}
         %if %{with_systemd_macros}
-            %systemd_post virtlockd.socket virtlogd.socket libvirtd.service libvirtd.socket
+            %systemd_post virtlockd.socket virtlogd.socket libvirtd.service
         %else
 if [ $1 -eq 1 ] ; then
     # Initial installation
@@ -1739,19 +1739,17 @@  fi
 %preun daemon
     %if %{with_systemd}
         %if %{with_systemd_macros}
-            %systemd_preun libvirtd.socket libvirtd.service virtlogd.socket virtlogd.service virtlockd.socket virtlockd.service
+            %systemd_preun libvirtd.service virtlogd.socket virtlogd.service virtlockd.socket virtlockd.service
         %else
 if [ $1 -eq 0 ] ; then
     # Package removal, not upgrade
     /bin/systemctl --no-reload disable \
-        libvirtd.socket \
         libvirtd.service \
         virtlogd.socket \
         virtlogd.service \
         virtlockd.socket \
         virtlockd.service > /dev/null 2>&1 || :
     /bin/systemctl stop \
-        libvirtd.socket \
         libvirtd.service \
         virtlogd.socket \
         virtlogd.service \
@@ -1966,7 +1964,6 @@  exit 0
 
     %if %{with_systemd}
 %{_unitdir}/libvirtd.service
-%{_unitdir}/libvirtd.socket
 %{_unitdir}/virtlogd.service
 %{_unitdir}/virtlogd.socket
 %{_unitdir}/virtlockd.service