[Spice-devel] [PATCH spice-vdagent] service: simplification and accessability - reversion

[Hans de Goede]

Hi,

On 19-10-15 16:55, poma wrote:
>
> While testing, I came to the same conclusion as Marc-André - commit 1587063.
>
> Hans, why the complexity of the "rules & target", isn't "ConditionPath*" sufficient?
> Besides, the lack of "WantedBy=multi-user.target" - back and forth with 'graphical.target' - 'spice-vdagentd' is "lost".
>
> Obviously X session (e.g. w/ 'startxfce4') can also start within 'multi-user.target',
> so why start 'spice-vdagentd' manually, unnecessarily, when sufficient is to append "multi-user.target", i.e.
> "WantedBy=spice-vdagentd.target multi-user.target"
>
>
> Simplifies start-up scheme and makes 'spice-vdagentd' more accessible as service.
>
> Tested-by: poma <pomidorabelisima at gmail.com>

Nack, the spice-vdagent is a hardware service (even if it is virtual hardware),
systemd has a well-documented standard mechanism for hardware activation, and that is
what is being used. Using ConditionPathIsSymbolicLink is just a hack to achieve more
or less the same (mostly less, e.g. if the port ever becomes a hotplugable device
in the future the hack will not work).

Regards,

Hans







> ---
>   Makefile.am                  | 8 +-------
>   data/70-spice-vdagentd.rules | 1 -
>   data/spice-vdagentd.service  | 3 ++-
>   data/spice-vdagentd.target   | 2 --
>   4 files changed, 3 insertions(+), 11 deletions(-)
>   delete mode 100644 data/70-spice-vdagentd.rules
>   delete mode 100644 data/spice-vdagentd.target
>
> diff --git a/Makefile.am b/Makefile.am
> index 8c55b43..de8159f 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -65,11 +65,7 @@ endif
>   if INIT_SCRIPT_SYSTEMD
>   systemdunitdir = $(SYSTEMDSYSTEMUNITDIR)
>   systemdunit_DATA = \
> -	$(top_srcdir)/data/spice-vdagentd.service \
> -	$(top_srcdir)/data/spice-vdagentd.target
> -
> -udevrulesdir = /lib/udev/rules.d
> -udevrules_DATA = $(top_srcdir)/data/70-spice-vdagentd.rules
> +	$(top_srcdir)/data/spice-vdagentd.service
>
>   tmpfilesdir = $(prefix)/lib/tmpfiles.d
>   tmpfiles_DATA = $(top_srcdir)/data/tmpfiles.d/spice-vdagentd.conf
> @@ -82,11 +78,9 @@ manpage_DATA = data/spice-vdagent.1	\
>   EXTRA_DIST =					\
>   	NEWS					\
>   	README.RHEL-5				\
> -	data/70-spice-vdagentd.rules		\
>   	data/spice-vdagent.desktop		\
>   	data/spice-vdagentd			\
>   	data/spice-vdagentd.service		\
> -	data/spice-vdagentd.target		\
>   	data/tmpfiles.d/spice-vdagentd.conf	\
>   	data/xorg.conf.RHEL-5			\
>   	$(NULL)
> diff --git a/data/70-spice-vdagentd.rules b/data/70-spice-vdagentd.rules
> deleted file mode 100644
> index a1785ba..0000000
> --- a/data/70-spice-vdagentd.rules
> +++ /dev/null
> @@ -1 +0,0 @@
> -ACTION=="add", SUBSYSTEM=="virtio-ports", ENV{DEVLINKS}=="/dev/virtio-ports/com.redhat.spice.0", ENV{SYSTEMD_WANTS}="spice-vdagentd.target"
> diff --git a/data/spice-vdagentd.service b/data/spice-vdagentd.service
> index 8c5effe..90eb1f3 100644
> --- a/data/spice-vdagentd.service
> +++ b/data/spice-vdagentd.service
> @@ -1,6 +1,7 @@
>   [Unit]
>   Description=Agent daemon for Spice guests
>   After=dbus.target
> +ConditionPathIsSymbolicLink=/dev/virtio-ports/com.redhat.spice.0
>
>   # TODO we should use:
>   #Requires=spice-vdagentd.socket
> @@ -14,4 +15,4 @@ PIDFile=/var/run/spice-vdagentd/spice-vdagentd.pid
>   PrivateTmp=true
>
>   [Install]
> -WantedBy=spice-vdagentd.target
> +WantedBy=multi-user.target
> diff --git a/data/spice-vdagentd.target b/data/spice-vdagentd.target
> deleted file mode 100644
> index 1f74931..0000000
> --- a/data/spice-vdagentd.target
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -[Unit]
> -Description=Agent daemon for Spice guests
>