[PATCH for-xen-4.5] tools/hotplug: fix systemd service files for dom0

[PATCH for-xen-4.5] tools/hotplug: fix systemd service files for dom0

[Olaf Hering]

ConditionVirtualization= checks if systemd runs in some sort of guest.
It is not supposed to detect host capabilities. The current
implementation happens to work because systemd-detect-virt from v208
returns also 'xen' in a dom0. In v210 and later 'none' is returned and
all service files are not started.

Adjust the checks to detect a dom0 vs. native boot. Mounting xenfs
depends on /proc/xen, but should only be done for pvops because xenfs
exists only there. All other service files should not be started in
domU. The file /proc/xen/capabilities exists in both dom0 and domU in a
pvops kernel, but only in dom0 it contains 'control_d'. The existing
ExecStartPre= check will prevent starting in a domU.

ConditionVirtualization=!xen is true in a dom0. But this check is broken
in systemd v208, so its not used.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---

I'm having a hard time to get a pvops dom0, so this is only tested with SUSE dom0.

 tools/hotplug/Linux/systemd/proc-xen.mount.in                     | 3 ++-
 tools/hotplug/Linux/systemd/var-lib-xenstored.mount.in            | 2 +-
 tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in | 2 +-
 tools/hotplug/Linux/systemd/xen-watchdog.service.in               | 2 +-
 tools/hotplug/Linux/systemd/xenconsoled.service.in                | 2 +-
 tools/hotplug/Linux/systemd/xendomains.service.in                 | 2 +-
 tools/hotplug/Linux/systemd/xenstored.service.in                  | 2 +-
 tools/hotplug/Linux/systemd/xenstored.socket.in                   | 2 +-
 tools/hotplug/Linux/systemd/xenstored_ro.socket.in                | 2 +-
 9 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/tools/hotplug/Linux/systemd/proc-xen.mount.in b/tools/hotplug/Linux/systemd/proc-xen.mount.in
index f0c4f3a..64ebe7f 100644
--- a/tools/hotplug/Linux/systemd/proc-xen.mount.in
+++ b/tools/hotplug/Linux/systemd/proc-xen.mount.in
@@ -1,6 +1,7 @@
 [Unit]
 Description=Mount /proc/xen files
-ConditionVirtualization=xen
+ConditionPathExists=/proc/xen
+ConditionPathExists=!/proc/xen/capabilities
 RefuseManualStop=true
 
 [Mount]
diff --git a/tools/hotplug/Linux/systemd/var-lib-xenstored.mount.in b/tools/hotplug/Linux/systemd/var-lib-xenstored.mount.in
index 1e930ed..64b560f 100644
--- a/tools/hotplug/Linux/systemd/var-lib-xenstored.mount.in
+++ b/tools/hotplug/Linux/systemd/var-lib-xenstored.mount.in
@@ -1,6 +1,6 @@
 [Unit]
 Description=mount xenstore file system
-ConditionVirtualization=xen
+ConditionPathExists=/proc/xen/capabilities
 RefuseManualStop=true
 
 [Mount]
diff --git a/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in b/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in
index 2282923..9125f43 100644
--- a/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in
+++ b/tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in
@@ -4,7 +4,7 @@ Requires=proc-xen.mount var-lib-xenstored.mount xenstored.socket
 After=xenstored.service xenconsoled.service
 Before=xendomains.service libvirtd.service libvirt-guests.service
 RefuseManualStop=true
-ConditionVirtualization=xen
+ConditionPathExists=/proc/xen/capabilities
 
 [Service]
 Type=simple
diff --git a/tools/hotplug/Linux/systemd/xen-watchdog.service.in b/tools/hotplug/Linux/systemd/xen-watchdog.service.in
index ee77bf9..1eecd2a 100644
--- a/tools/hotplug/Linux/systemd/xen-watchdog.service.in
+++ b/tools/hotplug/Linux/systemd/xen-watchdog.service.in
@@ -2,7 +2,7 @@
 Description=Xen-watchdog - run xen watchdog daemon
 Requires=proc-xen.mount
 After=proc-xen.mount xendomains.service
-ConditionVirtualization=xen
+ConditionPathExists=/proc/xen/capabilities
 
 [Service]
 Type=forking
diff --git a/tools/hotplug/Linux/systemd/xenconsoled.service.in b/tools/hotplug/Linux/systemd/xenconsoled.service.in
index 377f131..baaa411 100644
--- a/tools/hotplug/Linux/systemd/xenconsoled.service.in
+++ b/tools/hotplug/Linux/systemd/xenconsoled.service.in
@@ -2,7 +2,7 @@
 Description=Xenconsoled - handles logging from guest consoles and hypervisor
 Requires=xenstored.socket
 After=xenstored.service
-ConditionVirtualization=xen
+ConditionPathExists=/proc/xen/capabilities
 
 [Service]
 Type=simple
diff --git a/tools/hotplug/Linux/systemd/xendomains.service.in b/tools/hotplug/Linux/systemd/xendomains.service.in
index f88c336..2684df0 100644
--- a/tools/hotplug/Linux/systemd/xendomains.service.in
+++ b/tools/hotplug/Linux/systemd/xendomains.service.in
@@ -2,7 +2,7 @@
 Description=Xendomains - start and stop guests on boot and shutdown
 Requires=xenstored.socket
 After=xenstored.service xenconsoled.service
-ConditionVirtualization=xen
+ConditionPathExists=/proc/xen/capabilities
 
 [Service]
 Type=oneshot
diff --git a/tools/hotplug/Linux/systemd/xenstored.service.in b/tools/hotplug/Linux/systemd/xenstored.service.in
index f85b37d..eebf7f8 100644
--- a/tools/hotplug/Linux/systemd/xenstored.service.in
+++ b/tools/hotplug/Linux/systemd/xenstored.service.in
@@ -4,7 +4,7 @@ Requires=xenstored_ro.socket xenstored.socket proc-xen.mount var-lib-xenstored.m
 After=proc-xen.mount var-lib-xenstored.mount
 Before=libvirtd.service libvirt-guests.service
 RefuseManualStop=true
-ConditionVirtualization=xen
+ConditionPathExists=/proc/xen/capabilities
 
 [Service]
 Type=notify
diff --git a/tools/hotplug/Linux/systemd/xenstored.socket.in b/tools/hotplug/Linux/systemd/xenstored.socket.in
index 461e4f4..9b28e74 100644
--- a/tools/hotplug/Linux/systemd/xenstored.socket.in
+++ b/tools/hotplug/Linux/systemd/xenstored.socket.in
@@ -1,6 +1,6 @@
 [Unit]
 Description=xenstore socket
-ConditionVirtualization=xen
+ConditionPathExists=/proc/xen/capabilities
 
 [Socket]
 ListenStream=/var/run/xenstored/socket
diff --git a/tools/hotplug/Linux/systemd/xenstored_ro.socket.in b/tools/hotplug/Linux/systemd/xenstored_ro.socket.in
index 6ab5c28..80402b3 100644
--- a/tools/hotplug/Linux/systemd/xenstored_ro.socket.in
+++ b/tools/hotplug/Linux/systemd/xenstored_ro.socket.in
@@ -1,6 +1,6 @@
 [Unit]
 Description=xenstore ro socket
-ConditionVirtualization=xen
+ConditionPathExists=/proc/xen/capabilities
 
 [Socket]
 ListenStream=/var/run/xenstored/socket_ro

Re: [PATCH for-xen-4.5] tools/hotplug: fix systemd service files for dom0

[Anthony PERARD]

On Wed, Oct 15, 2014 at 11:11:58AM +0200, Olaf Hering wrote:
> ConditionVirtualization= checks if systemd runs in some sort of guest.
> It is not supposed to detect host capabilities. The current
> implementation happens to work because systemd-detect-virt from v208
> returns also 'xen' in a dom0. In v210 and later 'none' is returned and
> all service files are not started.
> 
> Adjust the checks to detect a dom0 vs. native boot. Mounting xenfs
> depends on /proc/xen, but should only be done for pvops because xenfs
> exists only there. All other service files should not be started in
> domU. The file /proc/xen/capabilities exists in both dom0 and domU in a
> pvops kernel, but only in dom0 it contains 'control_d'. The existing
> ExecStartPre= check will prevent starting in a domU.
> 
> ConditionVirtualization=!xen is true in a dom0. But this check is broken
> in systemd v208, so its not used.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
> 
> I'm having a hard time to get a pvops dom0, so this is only tested with SUSE dom0.

I've tested this patch in both dom0 and bare metal (with Linux 3.17.1
and systemd v216) and it mostly works, only one unit does not work,
var-lib-xenstored.mount.

(for some reason, ConditionVirtualization=xen, is still true on dom0
with systemd v216, even so systemd-detect-virt is returning 'none' ...)

>  tools/hotplug/Linux/systemd/proc-xen.mount.in                     | 3 ++-
>  tools/hotplug/Linux/systemd/var-lib-xenstored.mount.in            | 2 +-
>  tools/hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service.in | 2 +-
>  tools/hotplug/Linux/systemd/xen-watchdog.service.in               | 2 +-
>  tools/hotplug/Linux/systemd/xenconsoled.service.in                | 2 +-
>  tools/hotplug/Linux/systemd/xendomains.service.in                 | 2 +-
>  tools/hotplug/Linux/systemd/xenstored.service.in                  | 2 +-
>  tools/hotplug/Linux/systemd/xenstored.socket.in                   | 2 +-
>  tools/hotplug/Linux/systemd/xenstored_ro.socket.in                | 2 +-
>  9 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/hotplug/Linux/systemd/proc-xen.mount.in b/tools/hotplug/Linux/systemd/proc-xen.mount.in
> index f0c4f3a..64ebe7f 100644
> --- a/tools/hotplug/Linux/systemd/proc-xen.mount.in
> +++ b/tools/hotplug/Linux/systemd/proc-xen.mount.in
> @@ -1,6 +1,7 @@
>  [Unit]
>  Description=Mount /proc/xen files
> -ConditionVirtualization=xen
> +ConditionPathExists=/proc/xen
> +ConditionPathExists=!/proc/xen/capabilities
>  RefuseManualStop=true
>  
>  [Mount]
> diff --git a/tools/hotplug/Linux/systemd/var-lib-xenstored.mount.in b/tools/hotplug/Linux/systemd/var-lib-xenstored.mount.in
> index 1e930ed..64b560f 100644
> --- a/tools/hotplug/Linux/systemd/var-lib-xenstored.mount.in
> +++ b/tools/hotplug/Linux/systemd/var-lib-xenstored.mount.in
> @@ -1,6 +1,6 @@
>  [Unit]
>  Description=mount xenstore file system
> -ConditionVirtualization=xen
> +ConditionPathExists=/proc/xen/capabilities

On my systemd, var/lib/xenstored is not mounted because
ConditionPathExists=/proc/xen/capabilities was not met
even so proc-xen.mount is mounted.

So there is probably a dependency missing on proc-xen.mount.

>  RefuseManualStop=true
>  
>  [Mount]

-- 
Anthony PERARD

Re: [PATCH for-xen-4.5] tools/hotplug: fix systemd service files for dom0

[Olaf Hering]

On Thu, Oct 23, Anthony PERARD wrote:

> On Wed, Oct 15, 2014 at 11:11:58AM +0200, Olaf Hering wrote:
> > +++ b/tools/hotplug/Linux/systemd/var-lib-xenstored.mount.in
> > @@ -1,6 +1,6 @@
> >  [Unit]
> >  Description=mount xenstore file system
> > -ConditionVirtualization=xen
> > +ConditionPathExists=/proc/xen/capabilities
> 
> On my systemd, var/lib/xenstored is not mounted because
> ConditionPathExists=/proc/xen/capabilities was not met
> even so proc-xen.mount is mounted.
> 
> So there is probably a dependency missing on proc-xen.mount.

Yes, that is fixed with the latest version.

Olaf

Re: [PATCH for-xen-4.5] tools/hotplug: fix systemd service files for dom0

[Ian Campbell]

On Thu, 2014-10-23 at 18:17 +0100, Anthony PERARD wrote:
> On Wed, Oct 15, 2014 at 11:11:58AM +0200, Olaf Hering wrote:
> > ConditionVirtualization= checks if systemd runs in some sort of guest.
> > It is not supposed to detect host capabilities. The current
> > implementation happens to work because systemd-detect-virt from v208
> > returns also 'xen' in a dom0. In v210 and later 'none' is returned and
> > all service files are not started.
> > 
> > Adjust the checks to detect a dom0 vs. native boot. Mounting xenfs
> > depends on /proc/xen, but should only be done for pvops because xenfs
> > exists only there. All other service files should not be started in
> > domU. The file /proc/xen/capabilities exists in both dom0 and domU in a
> > pvops kernel, but only in dom0 it contains 'control_d'. The existing
> > ExecStartPre= check will prevent starting in a domU.
> > 
> > ConditionVirtualization=!xen is true in a dom0. But this check is broken
> > in systemd v208, so its not used.
> > 
> > Signed-off-by: Olaf Hering <olaf@aepfle.de>
> > Cc: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > ---
> > 
> > I'm having a hard time to get a pvops dom0, so this is only tested with SUSE dom0.
> 
> I've tested this patch in both dom0 and bare metal (with Linux 3.17.1
> and systemd v216) and it mostly works, only one unit does not work,
> var-lib-xenstored.mount.
> 
> (for some reason, ConditionVirtualization=xen, is still true on dom0
> with systemd v216, even so systemd-detect-virt is returning 'none' ...)

Based on a comment I saw on either an earlier version of this series or
one of Olaf's fixes this is expected, the behaviour changed (to exclude
dom0) in a more recent version.