Re: [systemd-devel] [PATCH v1] core: mount xenfs, ignore proc-xen.mount (#6442, #6662)

[Lennart Poettering]

On Mi, 29.11.17 16:38, Olaf Hering (o...@aepfle.de) wrote:

> The detection of ConditionVirtualisation= relies on the presence of
> /proc/xen/capabilities. If the file exists and contains the string
> "control_d", the running system is a dom0 and VIRTUALIZATION_NONE should
> be set. In case /proc/xen exists, or some sysfs files indicate "xen",
> VIRTUALIZATION_XEN should be set to indicate the system is a domU.
> 
> With an (old) xenlinux based kernel, /proc/xen/capabilities is always
> available and the detection described above works always. But with a
> pvops based kernel, xenfs must be mounted on /proc/xen to get
> "capabilities". Up to now this was done by a proc-xen.mount unit, which
> is part of xen.git. Since the mounting happens "late", other units may
> be scheduled before "proc-xen.mount". If these other units make use of
> "ConditionVirtualisation=", the virtualization detection returns
> incorect results. detect_vm() will set VIRTUALIZATION_XEN because "xen"
> is found in sysfs. This value will be cached. Once xenfs is mounted,
> the next process that runs detect_vm() will get VIRTUALIZATION_NONE.
> 
> This misdetection can be fixed by turning xenfs into an "API
> filesystem", which is done by adding it to mount_table[]. That way xenfs
> will be mounted early, and detect_vm() returns correct results.
> 
> But since the "proc-xen.mount" unit still exists, the startup of that
> unit fails because it is rejected due to a conflict with an "API
> filesystem" mount. This is done in mount_verify(). The unit gets into
> state FAILED. Other Xen toolstack related .service files rely on the
> existance of the proc-xen.mount unit. If proc-xen.mount fails, the
> toolstack fails too.
> 
> To stay compatible with existing and upcoming releases of Xen, add
> "/proc/xen" to the list of ignored paths and adjust the check for API
> mounts.
> 
> Now "proc-xen.mount" is silently accepted, it gets into state "start
> condition failed" because "ConditionPathExists=!/proc/xen/capabilities
> was not met". But all units depending on "proc-xen.mount" start anyway.
> 
> Signed-off-by: Olaf Hering 

Could you please submit this through github? we much prefer doing
reviews through github these days.

Also S-o-b is a kernel thing, we don't do that on systemd.

> ---
> 
> I'm not 100% sure if the logic change in mount_verify() has undesired effects.
> 
>  src/core/mount-setup.c | 3 +++
>  src/core/mount.c   | 2 +-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c
> index d5c0fcddc..df23e71b5 100644
> --- a/src/core/mount-setup.c
> +++ b/src/core/mount-setup.c
> @@ -114,6 +114,8 @@ static const MountPoint mount_table[] = {
>cg_is_legacy_wanted, MNT_FATAL|MNT_IN_CONTAINER },
>  { "pstore",  "/sys/fs/pstore","pstore", NULL,
>   MS_NOSUID|MS_NOEXEC|MS_NODEV,
>NULL,  MNT_NONE   },
> +{ "xenfs",   "/proc/xen", "xenfs",  NULL,
>   MS_NOSUID|MS_NOEXEC,
> +  NULL,  MNT_NONE   },
>  #if ENABLE_EFI
>  { "efivarfs","/sys/firmware/efi/efivars", "efivarfs",   NULL,
>   MS_NOSUID|MS_NOEXEC|MS_NODEV,
>is_efi_boot,   MNT_NONE   },
> @@ -126,6 +128,7 @@ static const MountPoint mount_table[] = {
>  static const char ignore_paths[] =
>  /* SELinux file systems */
>  "/sys/fs/selinux\0"
> +"/proc/xen\0"
>  /* Container bind mounts */
>  "/proc/sys\0"
>  "/dev/console\0"
> diff --git a/src/core/mount.c b/src/core/mount.c
> index b25bb9cb4..5afe7fcd7 100644
> --- a/src/core/mount.c
> +++ b/src/core/mount.c
> @@ -526,7 +526,7 @@ static int mount_verify(Mount *m) {
>  return -EINVAL;
>  }
>  
> -if (mount_point_is_api(m->where) || mount_point_ignore(m->where)) {
> +if (mount_point_is_api(m->where) && !mount_point_ignore(m->where)) {
>  log_unit_error(UNIT(m), "Cannot create mount unit for API 
> file system %s. Refusing.", m->where);
>  return -EINVAL;
>  }
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/systemd-devel


Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH v1] core: mount xenfs, ignore proc-xen.mount (#6442, #6662)

[systemd github import bot]

Patchset imported to github.
To create a pull request, one of the main developers has to initiate one via:


--
Generated by https://github.com/haraldh/mail2git
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH v1] core: mount xenfs, ignore proc-xen.mount (#6442, #6662)

[Olaf Hering]

The detection of ConditionVirtualisation= relies on the presence of
/proc/xen/capabilities. If the file exists and contains the string
"control_d", the running system is a dom0 and VIRTUALIZATION_NONE should
be set. In case /proc/xen exists, or some sysfs files indicate "xen",
VIRTUALIZATION_XEN should be set to indicate the system is a domU.

With an (old) xenlinux based kernel, /proc/xen/capabilities is always
available and the detection described above works always. But with a
pvops based kernel, xenfs must be mounted on /proc/xen to get
"capabilities". Up to now this was done by a proc-xen.mount unit, which
is part of xen.git. Since the mounting happens "late", other units may
be scheduled before "proc-xen.mount". If these other units make use of
"ConditionVirtualisation=", the virtualization detection returns
incorect results. detect_vm() will set VIRTUALIZATION_XEN because "xen"
is found in sysfs. This value will be cached. Once xenfs is mounted,
the next process that runs detect_vm() will get VIRTUALIZATION_NONE.

This misdetection can be fixed by turning xenfs into an "API
filesystem", which is done by adding it to mount_table[]. That way xenfs
will be mounted early, and detect_vm() returns correct results.

But since the "proc-xen.mount" unit still exists, the startup of that
unit fails because it is rejected due to a conflict with an "API
filesystem" mount. This is done in mount_verify(). The unit gets into
state FAILED. Other Xen toolstack related .service files rely on the
existance of the proc-xen.mount unit. If proc-xen.mount fails, the
toolstack fails too.

To stay compatible with existing and upcoming releases of Xen, add
"/proc/xen" to the list of ignored paths and adjust the check for API
mounts.

Now "proc-xen.mount" is silently accepted, it gets into state "start
condition failed" because "ConditionPathExists=!/proc/xen/capabilities
was not met". But all units depending on "proc-xen.mount" start anyway.

Signed-off-by: Olaf Hering 
---

I'm not 100% sure if the logic change in mount_verify() has undesired effects.

 src/core/mount-setup.c | 3 +++
 src/core/mount.c   | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c
index d5c0fcddc..df23e71b5 100644
--- a/src/core/mount-setup.c
+++ b/src/core/mount-setup.c
@@ -114,6 +114,8 @@ static const MountPoint mount_table[] = {
   cg_is_legacy_wanted, MNT_FATAL|MNT_IN_CONTAINER },
 { "pstore",  "/sys/fs/pstore","pstore", NULL,  
MS_NOSUID|MS_NOEXEC|MS_NODEV,
   NULL,  MNT_NONE   },
+{ "xenfs",   "/proc/xen", "xenfs",  NULL,  
MS_NOSUID|MS_NOEXEC,
+  NULL,  MNT_NONE   },
 #if ENABLE_EFI
 { "efivarfs","/sys/firmware/efi/efivars", "efivarfs",   NULL,  
MS_NOSUID|MS_NOEXEC|MS_NODEV,
   is_efi_boot,   MNT_NONE   },
@@ -126,6 +128,7 @@ static const MountPoint mount_table[] = {
 static const char ignore_paths[] =
 /* SELinux file systems */
 "/sys/fs/selinux\0"
+"/proc/xen\0"
 /* Container bind mounts */
 "/proc/sys\0"
 "/dev/console\0"
diff --git a/src/core/mount.c b/src/core/mount.c
index b25bb9cb4..5afe7fcd7 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -526,7 +526,7 @@ static int mount_verify(Mount *m) {
 return -EINVAL;
 }
 
-if (mount_point_is_api(m->where) || mount_point_ignore(m->where)) {
+if (mount_point_is_api(m->where) && !mount_point_ignore(m->where)) {
 log_unit_error(UNIT(m), "Cannot create mount unit for API file 
system %s. Refusing.", m->where);
 return -EINVAL;
 }
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel