[PATCH] systemd: add support initial systemd service files

[PATCH] systemd: add support initial systemd service files

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" <mcgrof@suse.com>

I've tested these on an OpenSUSE Tumbleweed source install.
They are based on the ones from OpenSUSE but slightly modified.
In particular the xencommons systemd service file was changed
to not require the capability file to exist prior to
initialization as the xencommons service is the one that
provides it by mounting xenfs for you.

If you don't specify a prefix you will end up with the services
files under /usr/local/lib/systemd/system/ by default, which
systemd does look for. Distributions are expected to provide their
/usr/ prefix to end up in the more generic location upon distribution
install at /usr/lib/systemd/system/.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 Config.mk                                |  3 ++-
 Makefile                                 |  5 ++++-
 config/StdGNU.mk                         |  1 +
 tools/hotplug/Linux/Makefile             | 16 +++++++++++++++-
 tools/hotplug/Linux/xen-watchdog.service | 12 ++++++++++++
 tools/hotplug/Linux/xencommons.service   | 12 ++++++++++++
 tools/hotplug/Linux/xend.service         | 14 ++++++++++++++
 tools/hotplug/Linux/xendomains.service   | 14 ++++++++++++++
 8 files changed, 74 insertions(+), 3 deletions(-)
 create mode 100644 tools/hotplug/Linux/xen-watchdog.service
 create mode 100644 tools/hotplug/Linux/xencommons.service
 create mode 100644 tools/hotplug/Linux/xend.service
 create mode 100644 tools/hotplug/Linux/xendomains.service

diff --git a/Config.mk b/Config.mk
index 84c558b..1affbde 100644
--- a/Config.mk
+++ b/Config.mk
@@ -153,6 +153,7 @@ endef
 define buildmakevars2shellvars
     export PREFIX="$(PREFIX)";                                            \
     export XEN_SCRIPT_DIR="$(XEN_SCRIPT_DIR)";                            \
+    export XEN_SYSTEMD_DIR="$(XEN_SYSTEMD_DIR)";                          \
     export XEN_ROOT="$(XEN_ROOT)"
 endef
 
@@ -174,7 +175,7 @@ define buildmakevars2file-closure
 	$(foreach var,                                                      \
 	          SBINDIR BINDIR LIBEXEC LIBDIR SHAREDIR PRIVATE_BINDIR     \
 	          XENFIRMWAREDIR XEN_CONFIG_DIR XEN_SCRIPT_DIR XEN_LOCK_DIR \
-	          XEN_RUN_DIR XEN_PAGING_DIR,                               \
+	          XEN_SYSTEMD_DIR XEN_RUN_DIR XEN_PAGING_DIR,               \
 	          echo "$(var)=\"$($(var))\"" >>$(1).tmp;)        \
 	$(call move-if-changed,$(1).tmp,$(1))
 endef
diff --git a/Makefile b/Makefile
index 4e48457..9b5568a 100644
--- a/Makefile
+++ b/Makefile
@@ -243,7 +243,10 @@ uninstall:
 	rm -f  $(D)$(CONFIG_DIR)/udev/rules.d/xen-backend.rules
 	rm -f  $(D)$(CONFIG_DIR)/udev/rules.d/xend.rules
 	rm -f  $(D)$(SYSCONFIG_DIR)/xendomains
-	rm -f  $(D)$(SYSCONFIG_DIR)/xencommons
+	rm -f  $(D)$(XEN_SYSTEMD_DIR)/xencommons.service
+	rm -f  $(D)$(XEN_SYSTEMD_DIR)/xend.service
+	rm -f  $(D)$(XEN_SYSTEMD_DIR)/xendomains.service
+	rm -f  $(D)$(XEN_SYSTEMD_DIR)/xen-watchdog.service
 	rm -rf $(D)/var/run/xen* $(D)/var/lib/xen*
 	make -C tools uninstall
 	rm -rf $(D)/boot/tboot*
diff --git a/config/StdGNU.mk b/config/StdGNU.mk
index 25fc594..14b432d 100644
--- a/config/StdGNU.mk
+++ b/config/StdGNU.mk
@@ -50,6 +50,7 @@ SYSCONFIG_DIR = $(CONFIG_DIR)/$(CONFIG_LEAF_DIR)
 
 XEN_CONFIG_DIR = $(CONFIG_DIR)/xen
 XEN_SCRIPT_DIR = $(XEN_CONFIG_DIR)/scripts
+XEN_SYSTEMD_DIR = $(PREFIX)/lib/systemd/system/
 
 SOCKET_LIBS =
 UTIL_LIBS = -lutil
diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile
index 47655f6..b2fcc2e 100644
--- a/tools/hotplug/Linux/Makefile
+++ b/tools/hotplug/Linux/Makefile
@@ -25,6 +25,11 @@ XEN_SCRIPTS += vscsi
 XEN_SCRIPTS += block-iscsi
 XEN_SCRIPTS += $(XEN_SCRIPTS-y)
 
+XEN_SYSTEMD_FILES = xend.service
+XEN_SYSTEMD_FILES += xencommons.service
+XEN_SYSTEMD_FILES += xendomains.service
+XEN_SYSTEMD_FILES += xen-watchdog.service
+
 XEN_SCRIPT_DATA = xen-script-common.sh locking.sh logging.sh
 XEN_SCRIPT_DATA += xen-hotplug-common.sh xen-network-common.sh vif-common.sh
 XEN_SCRIPT_DATA += block-common.sh
@@ -40,7 +45,7 @@ all:
 build:
 
 .PHONY: install
-install: all install-initd install-scripts install-udev
+install: all install-initd install-scripts install-udev install-systemd
 
 # See docs/misc/distro_mapping.txt for INITD_DIR location
 .PHONY: install-initd
@@ -69,6 +74,15 @@ install-scripts:
 	    $(INSTALL_DATA) $$i $(DESTDIR)$(XEN_SCRIPT_DIR); \
 	done
 
+.PHONY: install-systemd
+install-systemd:
+	[ -d $(DESTDIR)$(XEN_SYSTEMD_DIR) ] || \
+		$(INSTALL_DIR) $(DESTDIR)$(XEN_SYSTEMD_DIR)
+	set -e; for i in $(XEN_SYSTEMD_FILES); \
+	    do \
+	    $(INSTALL_PROG) $$i $(DESTDIR)$(XEN_SYSTEMD_DIR); \
+	done
+
 .PHONY: install-udev
 install-udev:
 	[ -d $(DESTDIR)$(UDEV_RULES_DIR) ] || \
diff --git a/tools/hotplug/Linux/xen-watchdog.service b/tools/hotplug/Linux/xen-watchdog.service
new file mode 100644
index 0000000..964a399
--- /dev/null
+++ b/tools/hotplug/Linux/xen-watchdog.service
@@ -0,0 +1,12 @@
+[Unit]
+Description=Xen-watchdog - run xen watchdog daemon
+After=xencommons.service
+ConditionPathIsDirectory=/proc/xen
+
+[Service]
+Type=forking
+ExecStart=/usr/local/sbin/xenwatchdogd 30 15
+KillSignal=USR1
+
+[Install]
+WantedBy=multi-user.target
diff --git a/tools/hotplug/Linux/xencommons.service b/tools/hotplug/Linux/xencommons.service
new file mode 100644
index 0000000..8042b24
--- /dev/null
+++ b/tools/hotplug/Linux/xencommons.service
@@ -0,0 +1,12 @@
+[Unit]
+Description=Xencommons - Script to start and stop xenstored and xenconsoled
+ConditionPathIsDirectory=/proc/xen
+
+[Service]
+Type=oneshot
+RemainAfterExit=true
+ExecStart=-/etc/init.d/xencommons start
+ExecStop=/etc/init.d/xencommons stop
+
+[Install]
+WantedBy=multi-user.target
diff --git a/tools/hotplug/Linux/xend.service b/tools/hotplug/Linux/xend.service
new file mode 100644
index 0000000..97cee20
--- /dev/null
+++ b/tools/hotplug/Linux/xend.service
@@ -0,0 +1,14 @@
+[Unit]
+Description=Xend - Starts and stops the Xen management daemon
+Before=libvirtd.service libvirt-guests.service
+ConditionPathIsDirectory=/proc/xen
+
+[Service]
+Type=forking
+PIDFile=/var/run/xend.pid
+Environment=HOME=/root
+ExecStartPre=/bin/grep -q control_d /proc/xen/capabilities
+ExecStart=/usr/sbin/xend start
+
+[Install]
+WantedBy=multi-user.target
diff --git a/tools/hotplug/Linux/xendomains.service b/tools/hotplug/Linux/xendomains.service
new file mode 100644
index 0000000..03158bd
--- /dev/null
+++ b/tools/hotplug/Linux/xendomains.service
@@ -0,0 +1,14 @@
+[Unit]
+Description=Xendomains - start and stop Xen VMs on boot and shutdown
+After=xencommons.service network.target
+ConditionPathIsDirectory=/proc/xen
+
+[Service]
+Type=oneshot
+RemainAfterExit=true
+ExecStartPre=/bin/grep -q control_d /proc/xen/capabilities
+ExecStart=-/etc/init.d/xendomains start
+ExecStop=/etc/init.d/xendomains stop
+
+[Install]
+WantedBy=multi-user.target
-- 
1.9.0

Re: [PATCH] systemd: add support initial systemd service files

[Jacek Konieczny]

On 03/12/14 01:03, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
> 
> I've tested these on an OpenSUSE Tumbleweed source install.
> They are based on the ones from OpenSUSE but slightly modified.

These seems quite wrong to me. Why create systemd services, while they
are only wrappers to the old-style LSB scripts? What do we gain?

Systemd provides great process control and monitoring, but it can only
do that if it starts the process by itself – it knows the main PID then,
knows if the service is running, etc.

systemd can do a better job handling old init.d scripts through its
compatibility functions than when it is told to run such a script
through ExecStart/ExecStop.

> diff --git a/tools/hotplug/Linux/xencommons.service b/tools/hotplug/Linux/xencommons.service
> new file mode 100644
> index 0000000..8042b24
> --- /dev/null
> +++ b/tools/hotplug/Linux/xencommons.service
> @@ -0,0 +1,12 @@
> +[Unit]
> +Description=Xencommons - Script to start and stop xenstored and xenconsoled
> +ConditionPathIsDirectory=/proc/xen
> +
> +[Service]xenstored
> +Type=oneshot
> +RemainAfterExit=true
> +ExecStart=-/etc/init.d/xencommons start
> +ExecStop=/etc/init.d/xencommons stop


This is especially bad: two processes started through one service, no
startup status verified.

The right thing to do is to provide separate systemd unit for xenstored
and xenconsoled. If 'xencommons.service' is needed, to have a compatible
service name with the LSB scripts, then it should be a dummy service with:

Wants=xenstored.service xenconsoled.service

And both xenstored.service and xenconsoled.service should have:

PartOf=xencommons.service

Greets,
     Jacek

Re: [PATCH] systemd: add support initial systemd service files

[Luis R. Rodriguez]

On Wed, Mar 12, 2014 at 1:16 AM, Jacek Konieczny <jajcus@jajcus.net> wrote:
> On 03/12/14 01:03, Luis R. Rodriguez wrote:
>> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>>
>> I've tested these on an OpenSUSE Tumbleweed source install.
>> They are based on the ones from OpenSUSE but slightly modified.
>
> These seems quite wrong to me. Why create systemd services, while they
> are only wrappers to the old-style LSB scripts? What do we gain?

Huh? LSB? Systemd is here to stay. I don't see what you are trying to
say with LSB stuff.

> Systemd provides great process control and monitoring, but it can only
> do that if it starts the process by itself – it knows the main PID then,
> knows if the service is running, etc.

Now that sounds like a reasonable constructive argument to NACK a
patch, can you provide pointers to one service file that does that
properly as you are suggesting?

> systemd can do a better job handling old init.d scripts through its
> compatibility functions than when it is told to run such a script
> through ExecStart/ExecStop.

Cool, like which ones?

>> diff --git a/tools/hotplug/Linux/xencommons.service b/tools/hotplug/Linux/xencommons.service
>> new file mode 100644
>> index 0000000..8042b24
>> --- /dev/null
>> +++ b/tools/hotplug/Linux/xencommons.service
>> @@ -0,0 +1,12 @@
>> +[Unit]
>> +Description=Xencommons - Script to start and stop xenstored and xenconsoled
>> +ConditionPathIsDirectory=/proc/xen
>> +
>> +[Service]xenstored
>> +Type=oneshot
>> +RemainAfterExit=true
>> +ExecStart=-/etc/init.d/xencommons start
>> +ExecStop=/etc/init.d/xencommons stop
>
>
> This is especially bad: two processes started through one service, no
> startup status verified.
>
> The right thing to do is to provide separate systemd unit for xenstored
> and xenconsoled. If 'xencommons.service' is needed, to have a compatible
> service name with the LSB scripts, then it should be a dummy service with:
>
> Wants=xenstored.service xenconsoled.service
>
> And both xenstored.service and xenconsoled.service should have:
>
> PartOf=xencommons.service

Seems like you can provide a much better set of suitable scripts, mind
just taking them and fixing them or would you like me to take your
advice steps by step?

  Luis

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] systemd: add support initial systemd service files

[Jacek Konieczny]

On 03/12/14 09:35, Luis R. Rodriguez wrote:
> On Wed, Mar 12, 2014 at 1:16 AM, Jacek Konieczny <jajcus@jajcus.net> wrote:
>> On 03/12/14 01:03, Luis R. Rodriguez wrote:
>>> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>>>
>>> I've tested these on an OpenSUSE Tumbleweed source install.
>>> They are based on the ones from OpenSUSE but slightly modified.
>>
>> These seems quite wrong to me. Why create systemd services, while they
>> are only wrappers to the old-style LSB scripts? What do we gain?
> 
> Huh? LSB? Systemd is here to stay. I don't see what you are trying to
> say with LSB stuff.

I just didn't want to abuse the 'System V init' term for these:
https://refspecs.linuxfoundation.org/LSB_2.1.0/LSB-generic/LSB-generic/iniscrptact.html

(System V init only uses /etc/inittab, the 'init.d' scripts are later
invention)

>> Systemd provides great process control and monitoring, but it can only
>> do that if it starts the process by itself – it knows the main PID then,
>> knows if the service is running, etc.
> 
> Now that sounds like a reasonable constructive argument to NACK a
> patch, can you provide pointers to one service file that does that
> properly as you are suggesting?

This is what we have inPLD Linux (currently for Xen 4.3.2), probably
based on work from some other distro (Fedora, I guess):

https://github.com/pld-linux/xen/blob/master/xen-watchdog.service
https://github.com/pld-linux/xen/blob/master/xenconsoled.service
https://github.com/pld-linux/xen/blob/master/xendomains.service
https://github.com/pld-linux/xen/blob/master/xenstored.service

It may be not perfect, nor ready for inclusion in the upstream Xen,
but I think this is the way to go. They may also need updates for
current Xen.

> Seems like you can provide a much better set of suitable scripts, mind
> just taking them and fixing them or would you like me to take your
> advice steps by step?

Feel free to take those scripts. Unfortunately, I won't be able to
prepare a ready patch for Xen by myself.

 Cc: Jan Rękorajski, who committed the first version of our systemd units.

Greets,
	Jacek

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] systemd: add support initial systemd service files

[Luis R. Rodriguez]

On Wed, Mar 12, 2014 at 2:07 AM, Jacek Konieczny <jajcus@jajcus.net> wrote:
>
> This is what we have inPLD Linux (currently for Xen 4.3.2), probably
> based on work from some other distro (Fedora, I guess):
>
> https://github.com/pld-linux/xen/blob/master/xen-watchdog.service
> https://github.com/pld-linux/xen/blob/master/xenconsoled.service
> https://github.com/pld-linux/xen/blob/master/xendomains.service
> https://github.com/pld-linux/xen/blob/master/xenstored.service
>
> It may be not perfect, nor ready for inclusion in the upstream Xen,
> but I think this is the way to go. They may also need updates for
> current Xen.

Thanks, I'll test and study these. If they are better then great. My
main issue is seeing evolution happening anywhere not upstream and Xen
doesn't need anymore of it.

>> Seems like you can provide a much better set of suitable scripts, mind
>> just taking them and fixing them or would you like me to take your
>> advice steps by step?
>
> Feel free to take those scripts. Unfortunately, I won't be able to
> prepare a ready patch for Xen by myself.
>
>  Cc: Jan Rękorajski, who committed the first version of our systemd units.

Jan, where did these come from?

  Luis

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] systemd: add support initial systemd service files

[Ian Jackson]

Jacek Konieczny writes ("Re: [Xen-devel] [PATCH] systemd: add support initial systemd service files"):
> These seems quite wrong to me. Why create systemd services, while they
> are only wrappers to the old-style LSB scripts? What do we gain?

I agree with this criticism.

> The right thing to do is to provide separate systemd unit for xenstored
> and xenconsoled.

If we do this then things will break if we add new functionality to
xencommons.  We would have to ask contributors who added new code to
xencommons to update the systemd unit files.  Those contributors, and
we, might not be able to test them (or alternatively wouldn't test the
sysvinit scripts).

Perhaps, until there is some way to generate all of this configuration
from a single input, it would be best for the upstream tree to
continue to provide just sysvinit scripts.

Thanks,
Ian.

Re: [PATCH] systemd: add support initial systemd service files

[Ian Campbell]

On Wed, 2014-03-12 at 10:07 +0100, Jacek Konieczny wrote:

> This is what we have inPLD Linux (currently for Xen 4.3.2), probably
> based on work from some other distro (Fedora, I guess):

FWIW I believe Michael has added stuff to Fedora, whether the inPLD
stuff is based on it I can't say. I've CCd him.

> 
> https://github.com/pld-linux/xen/blob/master/xen-watchdog.service
> https://github.com/pld-linux/xen/blob/master/xenconsoled.service
> https://github.com/pld-linux/xen/blob/master/xendomains.service
> https://github.com/pld-linux/xen/blob/master/xenstored.service
> 
> It may be not perfect, nor ready for inclusion in the upstream Xen,
> but I think this is the way to go. They may also need updates for
> current Xen.

Ian.

Re: [PATCH] systemd: add support initial systemd service files

[Jan Rękorajski]

On Wed, 12 Mar 2014, Luis R. Rodriguez wrote:

> On Wed, Mar 12, 2014 at 2:07 AM, Jacek Konieczny <jajcus@jajcus.net> wrote:
> >
> > This is what we have inPLD Linux (currently for Xen 4.3.2), probably
> > based on work from some other distro (Fedora, I guess):
> >
> > https://github.com/pld-linux/xen/blob/master/xen-watchdog.service
> > https://github.com/pld-linux/xen/blob/master/xenconsoled.service
> > https://github.com/pld-linux/xen/blob/master/xendomains.service
> > https://github.com/pld-linux/xen/blob/master/xenstored.service
> >
> > It may be not perfect, nor ready for inclusion in the upstream Xen,
> > but I think this is the way to go. They may also need updates for
> > current Xen.
> 
> Thanks, I'll test and study these. If they are better then great. My
> main issue is seeing evolution happening anywhere not upstream and Xen
> doesn't need anymore of it.
> 
> >> Seems like you can provide a much better set of suitable scripts, mind
> >> just taking them and fixing them or would you like me to take your
> >> advice steps by step?
> >
> > Feel free to take those scripts. Unfortunately, I won't be able to
> > prepare a ready patch for Xen by myself.
> >
> >  Cc: Jan Rękorajski, who committed the first version of our systemd units.
> 
> Jan, where did these come from?

The service files as such I took from Fedora, but the xendomains.sh
script used by the service is a slight modification of Debian's, because
I wanted our packages to support both xm and xl toolstacks.
I'm using them on my production servers without any problems.

-- 
Jan Rękorajski                                 | PLD/Linux
SysAdm                                         | http://www.pld-linux.org/
baggins<at>mimuw.edu.pl
baggins<at>pld-linux.org

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] systemd: add support initial systemd service files

[Ian Campbell]

On Wed, 2014-03-12 at 11:14 +0000, Ian Jackson wrote:
> Jacek Konieczny writes ("Re: [Xen-devel] [PATCH] systemd: add support initial systemd service files"):
> > These seems quite wrong to me. Why create systemd services, while they
> > are only wrappers to the old-style LSB scripts? What do we gain?
> 
> I agree with this criticism.
> 
> > The right thing to do is to provide separate systemd unit for xenstored
> > and xenconsoled.
> 
> If we do this then things will break if we add new functionality to
> xencommons.  We would have to ask contributors who added new code to
> xencommons to update the systemd unit files.  Those contributors, and
> we, might not be able to test them (or alternatively wouldn't test the
> sysvinit scripts).

I hadn't thought of this aspect, it is a concern.

On the other hand if systemd using distros are carrying their own
service files then each of them will be rediscovering the issues
themselves, possibly after we've triaged it here for the Nth time. Even
if the set of upstream service files are not kept completely up to date
at least they are in a common location, with some >0 chance of getting
update when things change.

I'm not sure which way I am leaning right now.

> Perhaps, until there is some way to generate all of this configuration
> from a single input, it would be best for the upstream tree to
> continue to provide just sysvinit scripts.
> 
> Thanks,
> Ian.

Re: [PATCH] systemd: add support initial systemd service files

[Andrew Cooper]

On 12/03/14 11:25, Ian Campbell wrote:
> On Wed, 2014-03-12 at 11:14 +0000, Ian Jackson wrote:
>> Jacek Konieczny writes ("Re: [Xen-devel] [PATCH] systemd: add support initial systemd service files"):
>>> These seems quite wrong to me. Why create systemd services, while they
>>> are only wrappers to the old-style LSB scripts? What do we gain?
>> I agree with this criticism.
>>
>>> The right thing to do is to provide separate systemd unit for xenstored
>>> and xenconsoled.
>> If we do this then things will break if we add new functionality to
>> xencommons.  We would have to ask contributors who added new code to
>> xencommons to update the systemd unit files.  Those contributors, and
>> we, might not be able to test them (or alternatively wouldn't test the
>> sysvinit scripts).
> I hadn't thought of this aspect, it is a concern.
>
> On the other hand if systemd using distros are carrying their own
> service files then each of them will be rediscovering the issues
> themselves, possibly after we've triaged it here for the Nth time. Even
> if the set of upstream service files are not kept completely up to date
> at least they are in a common location, with some >0 chance of getting
> update when things change.
>
> I'm not sure which way I am leaning right now.

Speaking in my capacity as a downstream consumer, I would much prefer
upstream to carry scripts for each init system.

IMO, the benefits of not repeatedly solving the same bug different ways,
and a wider set of eyes checking changes outweigh the problems of things
getting out of sync.

Testing can be trivially gated by running OSS test on different base
distros and strictly not using sysvinit compatibility.

And speaking of upstart, XenServer has some experimental upstart
configuration files for use in a CentOS 6.x environment.  (They should
be on github, but it would appear github is out of date, which I need to
investigate)

~Andrew

Re: [PATCH] systemd: add support initial systemd service files

[Luis R. Rodriguez]

On Wed, Mar 12, 2014 at 4:42 AM, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
>> On the other hand if systemd using distros are carrying their own
>> service files then each of them will be rediscovering the issues
>> themselves, possibly after we've triaged it here for the Nth time. Even
>> if the set of upstream service files are not kept completely up to date
>> at least they are in a common location, with some >0 chance of getting
>> update when things change.
>>
>> I'm not sure which way I am leaning right now.
>
> Speaking in my capacity as a downstream consumer, I would much prefer
> upstream to carry scripts for each init system.
>
> IMO, the benefits of not repeatedly solving the same bug different ways,
> and a wider set of eyes checking changes outweigh the problems of things
> getting out of sync.

My hope was also that we could work together on fixing things in one
place, as terrible and imperfect as they may be right now. The other
thing to keep in mind is that systemd is simply optional and the files
will only be used on systems that would prefer to use them.

  Luis

Re: [PATCH] systemd: add support initial systemd service files

[M A Young]

On Wed, 12 Mar 2014, Ian Campbell wrote:

> On Wed, 2014-03-12 at 10:07 +0100, Jacek Konieczny wrote:
>
>> This is what we have inPLD Linux (currently for Xen 4.3.2), probably
>> based on work from some other distro (Fedora, I guess):
>
> FWIW I believe Michael has added stuff to Fedora, whether the inPLD
> stuff is based on it I can't say. I've CCd him.
>
>>
>> https://github.com/pld-linux/xen/blob/master/xen-watchdog.service
>> https://github.com/pld-linux/xen/blob/master/xenconsoled.service
>> https://github.com/pld-linux/xen/blob/master/xendomains.service
>> https://github.com/pld-linux/xen/blob/master/xenstored.service

They look to be based on the Fedora ones which I wrote.

Fedora has the following systemd files (in xen-4.3)

proc-xen.mount - to mount /proc/xen

var-lib-xenstored.mount - to create a tmpfs mount at /var/lib/xenstored

xenstored.service - to start xenstored (if /proc/xen/capabilities exists)
and to set /local/domain/0/name in xenstore

oxenstored.service - the same as xenstored.service but to start oxenstored.
It conflicts with xenstored.service which means it should get started 
instead of xenstored if both are installed

blktapctrl.service - to start blktapctrl though I don't think it has ever 
been useful in Fedora and I intend to drop it when I build a xen 4.4.0 
package.

xenconsoled.service - to start xenconsoled

xend.service - to start xend

xen-watchdog.service - to start xenwatchdogd

xendomains.service - to run what is essentially a copy of the xendomains 
script with minor changes to take out the Fedora success/failure actions
which caused problems when I first wrote the systemd file. It might be 
possible to modify the xendomains init.d script so it works in both init.d 
and systemd contexts.

These don't try to start a qemu-xen instance as the current xencommons 
init.d script does, and we would probably need a xencommons.service to 
stop a xencommons init.d script being run.

 	Michael Young

Re: [PATCH] systemd: add support initial systemd service files

[Luis R. Rodriguez]

On Wed, Mar 12, 2014 at 12:44 PM, M A Young <m.a.young@durham.ac.uk> wrote:
> Fedora has the following systemd files (in xen-4.3)
>
> proc-xen.mount - to mount /proc/xen

OK

> var-lib-xenstored.mount - to create a tmpfs mount at /var/lib/xenstored

/var/lib/xenstored is the default path but why is tmpfs used? Are we
supposed to shred the TBD upon reboot? I can see how using memory
would yield better results than a filesystem, and it seems my tbd gets
rid of anything other than dom0 data after reboot. Without starting
xenstored I get upon boot:

mcgrof@ergon ~ $ sudo tdbtool /var/lib/xenstored/tdb dump| grep "/local/"
/local/domain/0/device-model
/local/domain
/local/domain/0/memory/static-max
/local/domain/0/memory/target
/local/domain/0/name
/local/domain/0/device-model/0
/local/domain/0/domid
/local/domain/0/memory/freemem-slack
/local/domain/0/libxl
/local/domain/0/memory
/local/domain/0/libxl/disable_udev
/local/domain/0
/local/domain/0/device-model/0/state

Are there no gains in keeping that?

I will note that upon starting xenstored I then get:

mcgrof@ergon ~ $ sudo tdbtool /var/lib/xenstored/tdb dump| grep "/local/"
/local/domain/0/device-model
/local/domain
/local/domain/0/name
/local/domain/0/device-model/0
/local/domain/0/domid
/local/domain/0
/local/domain/0/device-model/0/state

I am not sure so I guess that the initial tdb is cleared then? I see
that the pld linux systemd scripts clear the file anyway, so maybe
there is no point to make it stateful across reboots.

What if we save / restore a guest state?

> xenstored.service - to start xenstored (if /proc/xen/capabilities exists)
> and to set /local/domain/0/name in xenstore
>
> oxenstored.service - the same as xenstored.service but to start oxenstored.
> It conflicts with xenstored.service which means it should get started
> instead of xenstored if both are installed

Neat!

> blktapctrl.service - to start blktapctrl though I don't think it has ever
> been useful in Fedora and I intend to drop it when I build a xen 4.4.0
> package.

OK so lets ignore that.

> xenconsoled.service - to start xenconsoled

OK

> xend.service - to start xend

OK

> xen-watchdog.service - to start xenwatchdogd

OK

> xendomains.service - to run what is essentially a copy of the xendomains
> script with minor changes to take out the Fedora success/failure actions
> which caused problems when I first wrote the systemd file. It might be
> possible to modify the xendomains init.d script so it works in both init.d
> and systemd contexts.

That'd be great.

I see the pld systemd scripts have static paths for binary runs, it'd
be nice if we could use the same strategy by the upstream xencommons
init script of including the variables from

/etc/xen/scripts/hotplugpath.sh

so that the --prefix is respected.

> These don't try to start a qemu-xen instance as the current xencommons
> init.d script does,

Any particular reason for this? I see it only eats up about

> and we would probably need a xencommons.service to stop
> a xencommons init.d script being run.

What would be stopped exactly? If you've split out consoled to its own
control domains and if you don't run the new initial qemu instance
that the upstream xencommons script does start then what's left to
stop? xensored cannot be currently stopped as it seems threads in the
kernel are not kicked to stop using it. This could likely be fixed
though.

  Luis

Re: [PATCH] systemd: add support initial systemd service files

[Luis R. Rodriguez]

On Wed, Mar 12, 2014 at 12:44 PM, M A Young <m.a.young@durham.ac.uk> wrote:
>>>
>>> https://github.com/pld-linux/xen/blob/master/xen-watchdog.service
>>> https://github.com/pld-linux/xen/blob/master/xenconsoled.service
>>> https://github.com/pld-linux/xen/blob/master/xendomains.service
>>> https://github.com/pld-linux/xen/blob/master/xenstored.service


I have reviewed these and they seem to be a good step forward but I
think there's two things needed:

  * Modify upstream xen to stash things into environment variable
files for systmed depending on the system configuration upon
./configure. xen already does this and stashes stuff into
/etc/xen/scripts/hotplugpath.sh. I tried to modify these with what I
got on my configuration with defaults and I get for example:

SBINDIR=/usr/local/sbin
BINDIR=/usr/local/bin
LIBEXEC=/usr/local/lib/xen/bin
LIBDIR="/usr/local/lib64
SHAREDIR=/usr/local/share
PRIVATE_BINDIR=/usr/local/lib64/xen/bin
XENFIRMWAREDIR=/usr/local/lib/xen/boot
XEN_CONFIG_DIR=/etc/xen
XEN_SCRIPT_DIR=/etc/xen/scripts
XEN_LOCK_DIR=/var/lock
XEN_SYSTEMD_DIR=/usr/local/lib/systemd/system/
XEN_RUN_DIR=/var/run/xen
XEN_PAGING_DIR=/var/lib/xen/xenpaging

In this case I used the scripts's hints and stashed these into
/etc/sysconfig/xenstored. I then relied on the
EnvironmentFile=-/etc/sysconfig/xenstored you use and modified the
Exec parts as follows:

ExecStartPre=/bin/grep -q control_d /proc/xen/capabilities
ExecStartPre=-/bin/rm -f "$XENSTORED_ROOTDIR"/tdb*
ExecStart=$SBINDIR/xenstored --pid-file "$XEN_RUN_DIR"/xenstored.pid
$XENSTORED_ARGS
ExecStartPost=$BINDIR/xenstore-write "/local/domain/0/name" "Domain-0"

But this didn't seem to work, I suppose I'm not using the variables correctly.

  * You seem to have split out xendomains functionality into
/usr/lib/xen/bin/xendomains.sh, this would need an equivalent path to
help configurations that differ to be respected and for the changes
needed to split this to go upstream into xen

  Luis

Re: [PATCH] systemd: add support initial systemd service files

[M A Young]

On Wed, 12 Mar 2014, Luis R. Rodriguez wrote:

> On Wed, Mar 12, 2014 at 12:44 PM, M A Young <m.a.young@durham.ac.uk> wrote:
>>>>
>>>> https://github.com/pld-linux/xen/blob/master/xen-watchdog.service
>>>> https://github.com/pld-linux/xen/blob/master/xenconsoled.service
>>>> https://github.com/pld-linux/xen/blob/master/xendomains.service
>>>> https://github.com/pld-linux/xen/blob/master/xenstored.service
>
>
> I have reviewed these and they seem to be a good step forward but I
> think there's two things needed:
>
>  * Modify upstream xen to stash things into environment variable
> files for systmed depending on the system configuration upon
> ./configure. xen already does this and stashes stuff into
> /etc/xen/scripts/hotplugpath.sh. I tried to modify these with what I
> got on my configuration with defaults and I get for example:
>
> SBINDIR=/usr/local/sbin
> BINDIR=/usr/local/bin
> LIBEXEC=/usr/local/lib/xen/bin
> LIBDIR="/usr/local/lib64
> SHAREDIR=/usr/local/share
> PRIVATE_BINDIR=/usr/local/lib64/xen/bin
> XENFIRMWAREDIR=/usr/local/lib/xen/boot
> XEN_CONFIG_DIR=/etc/xen
> XEN_SCRIPT_DIR=/etc/xen/scripts
> XEN_LOCK_DIR=/var/lock
> XEN_SYSTEMD_DIR=/usr/local/lib/systemd/system/
> XEN_RUN_DIR=/var/run/xen
> XEN_PAGING_DIR=/var/lib/xen/xenpaging
>
> In this case I used the scripts's hints and stashed these into
> /etc/sysconfig/xenstored. I then relied on the
> EnvironmentFile=-/etc/sysconfig/xenstored you use and modified the
> Exec parts as follows:
>
> ExecStartPre=/bin/grep -q control_d /proc/xen/capabilities
> ExecStartPre=-/bin/rm -f "$XENSTORED_ROOTDIR"/tdb*
> ExecStart=$SBINDIR/xenstored --pid-file "$XEN_RUN_DIR"/xenstored.pid
> $XENSTORED_ARGS
> ExecStartPost=$BINDIR/xenstore-write "/local/domain/0/name" "Domain-0"
>
> But this didn't seem to work, I suppose I'm not using the variables correctly.

Systemd only provides limited variable support - you can use them for 
arguments but not the executables.

 	Michael Young

Re: [PATCH] systemd: add support initial systemd service files

[Luis R. Rodriguez]

On Wed, Mar 12, 2014 at 5:01 PM, M A Young <m.a.young@durham.ac.uk> wrote:
>> But this didn't seem to work, I suppose I'm not using the variables
>> correctly.
>
>
> Systemd only provides limited variable support - you can use them for
> arguments but not the executables.

Not even the path to the executables ? ie $FOO/bar

 Luis

Re: [PATCH] systemd: add support initial systemd service files

[M A Young]

On Wed, 12 Mar 2014, Luis R. Rodriguez wrote:

> On Wed, Mar 12, 2014 at 12:44 PM, M A Young <m.a.young@durham.ac.uk> wrote:
>> Fedora has the following systemd files (in xen-4.3)
>>
>> proc-xen.mount - to mount /proc/xen
>
> OK
>
>> var-lib-xenstored.mount - to create a tmpfs mount at /var/lib/xenstored
>
> /var/lib/xenstored is the default path but why is tmpfs used? Are we
> supposed to shred the TBD upon reboot? I can see how using memory
> would yield better results than a filesystem, and it seems my tbd gets
> rid of anything other than dom0 data after reboot. Without starting
> xenstored I get upon boot:

I followed what the Fedora package was already doing when I took over 
managing it. However I believe the xencommons script clears out 
/var/lib/xenstored so this is equivalent behaviour.

> ...
>> These don't try to start a qemu-xen instance as the current xencommons
>> init.d script does,
>
> Any particular reason for this? I see it only eats up about

Fedora just hasn't done it so far. Perhaps it should, but I imagine 
offical systemd files would be expected to do this.

> ...
>> and we would probably need a xencommons.service to stop
>> a xencommons init.d script being run.
>
> What would be stopped exactly? If you've split out consoled to its own
> control domains and if you don't run the new initial qemu instance
> that the upstream xencommons script does start then what's left to
> stop? xensored cannot be currently stopped as it seems threads in the
> kernel are not kicked to stop using it. This could likely be fixed
> though.

Systemd will use init.d scripts if no corresponding .service file 
exists. The idea of adding a xencommons.service file would be to stop the 
xencommons init.d script being run and trying to do what the other systemd 
files have already done if both systemd and init.d scripts are installed.

 	Michael Young

Re: [PATCH] systemd: add support initial systemd service files

[Jacek Konieczny]

On 03/13/14 01:06, Luis R. Rodriguez wrote:
> On Wed, Mar 12, 2014 at 5:01 PM, M A Young <m.a.young@durham.ac.uk> wrote:
>>> But this didn't seem to work, I suppose I'm not using the variables
>>> correctly.
>>
>>
>> Systemd only provides limited variable support - you can use them for
>> arguments but not the executables.
> 
> Not even the path to the executables ? ie $FOO/bar

No, and there are some good reasons for that.
And if anybody really needs to change that after installation, he may
copy the unit files from /lib/systemd/system to /etc/systemd/system and
edit them.

Other way would be to use generators to dynamically build the units, but
I really don't thing there is a good reason for having the paths
dynamic.

Greets,
	Jacek

Re: [PATCH] systemd: add support initial systemd service files

[Ian Campbell]

On Thu, 2014-03-13 at 09:03 +0100, Jacek Konieczny wrote:
> On 03/13/14 01:06, Luis R. Rodriguez wrote:
> > On Wed, Mar 12, 2014 at 5:01 PM, M A Young <m.a.young@durham.ac.uk> wrote:
> >>> But this didn't seem to work, I suppose I'm not using the variables
> >>> correctly.
> >>
> >>
> >> Systemd only provides limited variable support - you can use them for
> >> arguments but not the executables.
> > 
> > Not even the path to the executables ? ie $FOO/bar
> 
> No, and there are some good reasons for that.
> And if anybody really needs to change that after installation, he may
> copy the unit files from /lib/systemd/system to /etc/systemd/system and
> edit them.
> 
> Other way would be to use generators to dynamically build the units, but
> I really don't thing there is a good reason for having the paths
> dynamic.

Would it be preferable to have the Xen build system produce foo.service
from foo.service.in doing the variable substitution at build time?

Ian.

Re: [PATCH] systemd: add support initial systemd service files

[Jacek Konieczny]

On 03/13/14 10:52, Ian Campbell wrote:
> On Thu, 2014-03-13 at 09:03 +0100, Jacek Konieczny wrote:
>> On 03/13/14 01:06, Luis R. Rodriguez wrote:
>>> On Wed, Mar 12, 2014 at 5:01 PM, M A Young <m.a.young@durham.ac.uk> wrote:
>>>>> But this didn't seem to work, I suppose I'm not using the variables
>>>>> correctly.
>>>>
>>>>
>>>> Systemd only provides limited variable support - you can use them for
>>>> arguments but not the executables.
>>>
>>> Not even the path to the executables ? ie $FOO/bar
>>
>> No, and there are some good reasons for that.
>> And if anybody really needs to change that after installation, he may
>> copy the unit files from /lib/systemd/system to /etc/systemd/system and
>> edit them.
>>
>> Other way would be to use generators to dynamically build the units, but
>> I really don't thing there is a good reason for having the paths
>> dynamic.
> 
> Would it be preferable to have the Xen build system produce foo.service
> from foo.service.in doing the variable substitution at build time?

Yes, IMHO, that is the right way to do this.

Greets,
Jacek