[systemd-devel] [PATCH] systemd-detect-s390-virt: add virtualization detection on s390x

[Lennart Poettering]

On Fri, 04.07.14 15:07, Thomas Blume (Thomas.Blume at suse.com) wrote:

> +#if defined(__s390x__)
> +        /* First layer virtualization (PR/SM) is always present on s390x */
> +        _id = "PR/SM";
> +        r = 1;

Well, this is not useful, this is about detecting whether the OS we run
in is the closest to the hardware of the system or is is removed from it
via some virtualization layer.

This definition means that on Xen dom0 is not considered virtualization,
but domU is.

It appears to be that it's similar here, and hence the above makes
little sense.

> +
> +        /* Check for second layer virtualization */
> +        _cleanup_fclose_ FILE *f = NULL;
> +        char line[LINE_MAX];
> +        f = fopen("/proc/sysinfo", "re");

Please declare variables at the beginning of blocks, not in the
middle. WHile C99 allows that we do not make use of that in systemd, and
tried to keep variable declaration seperate from code.

> +        if (f) {
> +                FOREACH_LINE(line, f, return -errno) {
> +                        if (startswith(line, "VM00 Control Program:")) {
> +                                if (strstr(line,"z/VM"))
> +                                        _id = "z/VM";
> +                                else if (strstr(line,"KVM/Linux"))
> +                                        _id = "KVM";

We use lower-case ids only so far, without special characters. Also,
given that you detect kvm with this, you should use the existing kvm
identifier.

All new ids also need to be added to the various bits of documentation,
in particular systemd-detect-virt(1) and systemd.unit(5).

Lennart

-- 
Lennart Poettering, Red Hat