virsh nodeinfo doesn't report CPU frequency on s390x

[Frank Heimes]

With virsh it's possible to look-up certain CPU attributes, including CPU frequency:

on x86:
$ virsh nodeinfo
CPU model: x86_64
CPU(s): 4
CPU frequency: 600 MHz
CPU socket(s): 1
Core(s) per socket: 2
Thread(s) per core: 2
NUMA cell(s): 1
Memory size: 32543448 KiB

but on s390x CPU frequency is missing:

$ virsh nodeinfo
CPU model: s390x
CPU(s): 4
CPU socket(s): 1
Core(s) per socket: 4
Thread(s) per core: 1
NUMA cell(s): 1
Memory size: 10088212 KiB
$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 16.04.3 LTS
Release: 16.04
Codename: xenial
$ apt-cache policy libvirt-bin
libvirt-bin:
  Installed: 3.6.0-1ubuntu5~cloud0
  Candidate: 3.6.0-1ubuntu5~cloud0
  Version table:
 *** 3.6.0-1ubuntu5~cloud0 500
        500 http://ubuntu-cloud.archive.canonical.com/ubuntu xenial-updates/pike/main s390x Packages
        100 /var/lib/dpkg/status
     1.3.1-1ubuntu10.14 500
        500 http://us.ports.ubuntu.com/ubuntu-ports xenial-updates/main s390x Packages
     1.3.1-1ubuntu10 500
        500 http://us.ports.ubuntu.com/ubuntu-ports xenial/main s390x Packages

Other s390x example:

$ virsh nodeinfo
CPU model: s390x
CPU(s): 20
CPU socket(s): 1
Core(s) per socket: 20
Thread(s) per core: 1
NUMA cell(s): 1
Memory size: 20402368 KiB
$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 17.10
Release: 17.10
Codename: artful
$ apt-cache policy libvirt-bin
libvirt-bin:
  Installed: 3.6.0-1ubuntu6
  Candidate: 3.6.0-1ubuntu6
  Version table:
 *** 3.6.0-1ubuntu6 500
        500 http://us.ports.ubuntu.com/ubuntu-ports artful-updates/main s390x Packages
        100 /var/lib/dpkg/status
     3.6.0-1ubuntu5 500
        500 http://us.ports.ubuntu.com/ubuntu-ports artful/main s390x Packages

MAAS (pods) look up this value - hence would be good to have it implemented for s390x, too.

[Christian Ehrhardt  (paelzer)]

Hi Frank,
I see you already mirrored which is right as it just isn't implmented on s390 afaik.
But IMHO frequency these days is just as good as /dev/urandom - so I'd not be offended if somebody says "Won't fix"
A patch to drop it from all architectures would be nice thou ...

@Frank - did you have any particular use case in midn this might be useful?

[Frank Heimes (fheimes)]

It's used in MAAS - a workaround can be added for sure - but also thought
it's not to bad to close that little gap (on low prio ...)

Frank Heimes | Tech. Lead Ubuntu Server on Z | Canonical Ltd.
mail: <email address hidden>
irc: jfh
www.canonical.com | www.ubuntu.com | ubuntu-on-big-iron.blogspot.com
<http://ubuntu-on-big-iron.blogspot.com/?view=sidebar> |
http://fridge.ubuntu.com

On Wed, Nov 22, 2017 at 1:58 PM, ChristianEhrhardt <
<email address hidden>> wrote:

> Hi Frank,
> I see you already mirrored which is right as it just isn't implmented on
> s390 afaik.
> But IMHO frequency these days is just as good as /dev/urandom - so I'd not
> be offended if somebody says "Won't fix"
> A patch to drop it from all architectures would be nice thou ...
>
> @Frank - did you have any particular use case in midn this might be
> useful?
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1733688
>
> Title:
> virsh nodeinfo doesn't report CPU frequency on s390x
>
> Status in Ubuntu on IBM z Systems:
> New
> Status in libvirt package in Ubuntu:
> New
>
> Bug description:
> With virsh it's possible to look-up certain CPU attributes, including
> CPU frequency:
>
> on x86:
> $ virsh nodeinfo
> CPU model: x86_64
> CPU(s): 4
> CPU frequency: 600 MHz
> CPU socket(s): 1
> Core(s) per socket: 2
> Thread(s) per core: 2
> NUMA cell(s): 1
> Memory size: 32543448 KiB
>
> but on s390x CPU frequency is missing:
>
> $ virsh nodeinfo
> CPU model: s390x
> CPU(s): 4
> CPU socket(s): 1
> Core(s) per socket: 4
> Thread(s) per core: 1
> NUMA cell(s): 1
> Memory size: 10088212 KiB
> $ lsb_release -a
> No LSB modules are available.
> Distributor ID: Ubuntu
> Description: Ubuntu 16.04.3 LTS
> Release: 16.04
> Codename: xenial
> $ apt-cache policy libvirt-bin
> libvirt-bin:
> Installed: 3.6.0-1ubuntu5~cloud0
> Candidate: 3.6.0-1ubuntu5~cloud0
> Version table:
> *** 3.6.0-1ubuntu5~cloud0 500
> 500 http://ubuntu-cloud.archive.canonical.com/ubuntu
> xenial-updates/pike/main s390x Packages
> 100 /var/lib/dpkg/status
> 1.3.1-1ubuntu10.14 500
> 500 http://us.ports.ubuntu.com/ubuntu-ports xenial-updates/main
> s390x Packages
> 1.3.1-1ubuntu10 500
> 500 http://us.ports.ubuntu.com/ubuntu-ports xenial/main s390x
> Packages
>
>
> Other s390x example:
>
> $ virsh nodeinfo
> CPU model: s390x
> CPU(s): 20
> CPU socket(s): 1
> Core(s) per socket: 20
> Thread(s) per core: 1
> NUMA cell(s): 1
> Memory size: 20402368 KiB
> $ lsb_release -a
> No LSB modules are available.
> Distributor ID: Ubuntu
> Description: Ubuntu 17.10
> Release: 17.10
> Codename: artful
> $ apt-cache policy libvirt-bin
> libvirt-bin:
> Installed: 3.6.0-1ubuntu6
> Candidate: 3.6.0-1ubuntu6
> Version table:
> *** 3.6.0-1ubuntu6 500
> 500 http://us.ports.ubuntu.com/ubuntu-ports artful...

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2017-11-22 09:33 EDT-------
I think this is really not important (and almost always wrong on ARM and x86 anyway) but if we want to have it we could parse /proc/cpuinfo on newer host kernels in the libvirt function
virHostCPUGetInfoPopulateLinux
newer s390 kernels now have
something like this
cpu number : 0
cpu MHz dynamic : 5208
cpu MHz static : 5208

in /proc/cpuinfo

Given the non-reliability on x86 and ARM I still think that MASS should not use that field.

[Christian Ehrhardt  (paelzer)]

Nice: comment #1 + comment #3 = Christians are in sync :-)

Frank I think you should actually open a bug task against MAAS on this.

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2017-12-08 04:23 EDT-------
I have created a patch to include this information in libvirt and will send it upstream soon. Hopefully, it is available in version 4.0 then.

But I agree with Christian that this information is highly arbitrary and should not be given to much value.

[Frank Heimes (fheimes)]

I can understand both views:
1) the fact that the cpu frequency data is probably not very useful
2) in case a component is ported to Z, the features shouldn't be reduced until it fits

But I'll ask the MAAS team for a comment here ...

[Frank Heimes (fheimes)]

answer from MAAS team:

'MAAS has an option were it allows you to create machines from pods using the frequency. I created a work around for the issue. I really have no strong feelings on having it, but for consistency, it is probably required.'

[Christian Ehrhardt  (paelzer)]

My honest opinion, this sounds like the worst feature ever.
Due to that being affected by cpu freq scaling this is rather close to "if $RANDOM > 50% then do".
I expect/guess/hope there is much more detail behind it that makes this more meaningful as it sounds.

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2017-12-20 01:18 EDT-------
I have sent a patch upstream. It depends on another upstream change, so integration could take a little longer, still, feature should be available in 4.0.

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2018-01-12 02:47 EDT-------
Patches for virsh nodeinfo have been accepted by upstream and will be available in 4.0. Patches for virsh sysinfo are still being discussed.

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2018-01-24 04:51 EDT-------
Libvirt v4.0.0 has been released on January 19th, 2018.
It includes all required patches.

These are the patches:
util: virhostcpu: parse frequency information on S390 - https://libvirt.org/git/?p=libvirt.git;a=commit;h=ae68dbffe9ad78e58b63b4367b96b41475dc3268
tests: virhostcputest: testcase for S390 system - https://libvirt.org/git/?p=libvirt.git;a=commit;h=65dace5a96d7c50ef700c7f68a4a8bc3681c703a
util: virsysinfo: parse frequency information on S390 - https://libvirt.org/git/?p=libvirt.git;a=commit;h=4be9959b41834f8fefd2427f3cd3b98c6288e8c8
tests: sysinfotest: add new test case for S390 - https://libvirt.org/git/?p=libvirt.git;a=commit;h=44601840ddb233d9ce9ec72d67bdab74d208f32d

[Christian Ehrhardt  (paelzer)]

Hi Boris, I already saw that and prepped the 4.0 upload that way - thanks for the ping still, it is not always that clear.

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2018-01-24 05:33 EDT-------
Updated target distro to Ubuntu 18.04.
Libvirt v4.0.0 has been released on January 19th, 2018.

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2018-01-30 11:52 EDT-------
Since libvirt 4.0 is available within proposed. Canonical please verify if the problem is solved.

[Christian Ehrhardt  (paelzer)]

It is fixed:
ubuntu@s1lp5:~$ virsh nodeinfo
CPU model: s390x
CPU(s): 12
CPU frequency: 5000 MHz
CPU socket(s): 2
Core(s) per socket: 6
Thread(s) per core: 1
NUMA cell(s): 1
Memory size: 10080436 KiB

Still personally don't find the information useful, but yeah parity is worth a thing as well.

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2018-01-31 02:26 EDT-------
IBM bugzilla status -> closed, Fixed and verfied by Canonical

[Launchpad Janitor (janitor)]

This bug was fixed in the package libvirt - 4.0.0-1ubuntu1

---------------
libvirt (4.0.0-1ubuntu1) bionic; urgency=medium

  * Merged with Debian unstable (4.0)
    This closes several bugs:
    - Error generating apparmor profile when hostname contains spaces
      (LP: #799997)
    - qemu 2.10 locks files, libvirt shared now sets share-rw=on (LP: #1716028)
    - libvirt usb passthrough throws apparmor denials related to
      /run/udev/data/+usb (LP: #1727311)
    - AppArmor denies access to /sys/block/*/queue/max_segments (LP: #1729626)
    - iohelper improvements to let bypass-cache work without opening up the
      apparmor isolation (LP: #1719579)
    - nodeinfo on s390x to contain more CPU info (LP: #1733688)
    - Upgrade libvirt >= 4.0 (LP: #1745934)
  * Remaining changes:
    - Disable libssh2 support (universe dependency)
    - Disable firewalld support (universe dependency)
    - Disable selinux
    - Set qemu-group to kvm (for compat with older ubuntu)
    - Additional apport package-hook
    - Modifications to adapt for our delayed switch away from libvirt-bin (can
      be dropped >18.04).
      + d/p/ubuntu/libvirtd-service-add-bin-alias.patch: systemd: define alias
        to old service name so that old references work
      + d/p/ubuntu/libvirtd-init-add-bin-alias.patch: sysv init: define alias
        to old service name so that old references work
      + d/control: transitional package with the old name and maintainer
        scripts to handle the transition
    - Backwards compatible handling of group rename (can be dropped >18.04).
    - config details and autostart of default bridged network. Creating that is
      now the default in general, yet our solution provides the following on
      top as of today:
      + autostart the default network by default
      + do not autostart if subnet is already taken (e.g. in guests).
    - d/p/ubuntu/Allow-libvirt-group-to-access-the-socket.patch: This is
      the group based access to libvirt functions as it was used in Ubuntu
      for quite long.
      + d/p/ubuntu/daemon-augeas-fix-expected.patch fix some related tests
        due to the group access change.
    - ubuntu/parallel-shutdown.patch: set parallel shutdown by default.
    - d/p/ubuntu/enable-kvm-spice.patch: compat with older Ubuntu qemu/kvm
      which provided a separate kvm-spice.
    - d/p/ubuntu/ubuntu-libxl-qemu-path.patch: this change was split. The
      section that adapts the path of the emulator to the Debian/Ubuntu
      packaging is kept.
    - d/p/ubuntu/ubuntu-libxl-Fix-up-VRAM-to-minimum-requirements.patch: auto
      set VRAM to minimum requirements
    - d/p/ubuntu/xen-default-uri.patch: set default URI on xen hosts
    - Add libxl log directory
    - libvirt-uri.sh: Automatically switch default libvirt URI for users on
      Xen dom0 via user profile (was missing on changelogs before)
    - d/p/ubuntu/apibuild-skip-libvirt-common.h: drop libvirt-common.h from
      included_files to avoid build failures due to duplicate definitions.
    - Update README.Debian with Ubuntu changes
    - Convert libvirt0, libnss_libvirt and libvirt-dev to multi-arch.
    - Enable some additional features on ppc...