information returned by node-info from recent PR #1042 is incorrect

[memetb]

System Information

Linux distribution

any

Terraform version

terraform -v
Terraform v1.5.7
on linux_amd64

Provider and libvirt versions

terraform-provider-libvirt -version

---

Description of Issue/Question

Setup

I recently implemented a very similar feature in PR #1073 without realizing that #1042 had very recently been merged. I'm happy to close my PR however the output of the node info is incorrect and less expansive than it could.

Example:

// main.tf
terraform {
  required_version = ">= 0.13"
  required_providers {
    libvirt = {
      #      source  = "dmacvicar/libvirt"
      source = "terraform.local/local/libvirt"
      version = "1.0.0"
    }
  }
}

provider "libvirt" {
  uri = "qemu+ssh://${var.target}/system?sshauth=privkey&no_verify"
}

data "libvirt_node_info" "host" {
}


output node_info {
  value = data.libvirt_node_info.host
}

produces

Changes to Outputs:
  + node_info = {
      + cpu_cores_per_socket = 2
      + cpu_cores_total      = 2
      + cpu_model            = "aarch64"  # Incorrect
      + cpu_sockets          = 1
      + cpu_threads_per_core = 1
      + id                   = "1052227631"
      + memory_total_kb      = 2229728
      + numa_nodes           = 1
    }

By contrast, this is the output of the node info as implemented in PR #1073

Changes to Outputs:
  + node_info = {
      + arch                      = "aarch64"
      + cores                     = 2
      + dies                      = 1
      + features                  = [
          + "fp",
          + "asimd",
          + "evtstrm",
          + "aes",
          + "pmull",
          + "sha1",
          + "sha2",
          + "crc32",
          + "cpuid",
        ]
      + id                        = "d4e1bf70-6374-45b7-a970-fb584a1b4062" # this is the underlying libvirt native id
      + live_migration_support    = true
      + live_migration_transports = [
          + "tcp",
          + "rdma",
        ]
      + model                     = "cortex-a72"  # missing in PR 1042
      + sockets                   = 1
      + threads                   = 1
      + topology                  = [] # not yet implemented but can be expanded on
      + vendor                    = "ARM"  # missing in PR 1042
    }

The implementation for #1073 libvirt/data_source_libvirt_nodeinfo.go pulls and parses the direct XML dump from virt.

I believe it to be more expressive and open to upgrading, especially given that the actual topology can be decoded with a bit extra effort.

I am working on two other PR's right now (#1072 and #1059). I'd be happy to rework #1073 to fix the above. My only ask would be to name the parameters as I've done (i.e. cores not cpu_cores_per_socket) mainly because I've used the sames names as libvirt capabilities schema

@michaelbeaumont @rustydb @dmacvicar

[muresan]

Hi,

at the time I implemented node-info using https://libvirt.org/html/libvirt-libvirt-host.html#virNodeInfo which returns what you saw (with incorrect data) and I did not see that virConnectGetCapabilities exists. Since a release was not cut that includes this format, I'd say feel free to update it to the capabilities structure.

Alternatively, you can add a libvirt_node_capabilities data source, which would map to the virConnectGetCapabilities better, if someone is already using the current node_info (doubt it).

Edit: what I needed at the time was a way to determine total memory on the host and cpu cores, in order to make decisions about the number of cpu cores and memory allocated to VMs. On second thought I would recommend implementing an additional data source named libvirt_node_capabilities because memory doesn't "fit" in the capabilities object and to better map 1:1 what libvirt has with what the provider providess

[dmacvicar]

@memetb I think we can merge your approach, however I am a bit confused on:

• why would you name nodeinfo something that in libvirt is capabilities in the module host. Wouldn't that be libvirt_host_capabilities then? I don't understand either @muresan suggestion to call it libvirt_node_capabilities. The word node does not appear anywhere in the libvirt side.

If we get it with the right naming, I'd then suggest to deprecate the node info version:

[memetb]

@dmacvicar I'm open to name by consensus.

Reason for my chosing nodeinfo is because of the virsh command itself:

$ virsh help host
 Host and Hypervisor (help keyword 'host'):
    allocpages                     Manipulate pages pool size
    capabilities                   capabilities                            # <----------------
    cpu-baseline                   compute baseline CPU
    cpu-compare                    compare host CPU with a CPU described by an XML file
    cpu-models                     CPU models
    domcapabilities                domain capabilities
    freecell                       NUMA free memory
    freepages                      NUMA free pages
    hostname                       print the hypervisor hostname
    hypervisor-cpu-baseline        compute baseline CPU usable by a specific hypervisor
    hypervisor-cpu-compare         compare a CPU with the CPU created by a hypervisor on the host
    maxvcpus                       connection vcpu maximum
    node-memory-tune               Get or set node memory parameters
    nodecpumap                     node cpu map
    nodecpustats                   Prints cpu stats of the node.
    nodeinfo                       node information              # <----------------
    nodememstats                   Prints memory stats of the node.
    nodesevinfo                    node SEV information
    nodesuspend                    suspend the host node for a given time duration
    sysinfo                        print the hypervisor sysinfo
    uri                            print the hypervisor canonical URI
    version                        show version

Note that capabilities and nodeinfo are not the same thing: the former lists what the host is able to do, whereas the latter gives information about what the host is. The original motivation for this feature for me was to determine what type of hardware I was running on (more than what type of hardware I could emulate).

With regards to your highlighted warning in the libvirt documentation, the implementation does end up using virtConn.ConnectGetCapabilities(), however the way I read it, that documentation is saying "use virtConn.ConnectGetCapabilities() to obtain nodeinfo".

However, all that said (as to why I did it that way), there is clearly some dirty abstraction leaks going on above, which is unfortunate.

Here are the outputs of the two calls on my system:

memet@arachnid:~$ virsh nodeinfo
CPU model:           x86_64
CPU(s):              32
CPU frequency:       800 MHz
CPU socket(s):       1
Core(s) per socket:  32
Thread(s) per core:  1
NUMA cell(s):        1
Memory size:         98606452 KiB

memet@arachnid:~$ virsh capabilities
<capabilities>

  <host>
    <uuid>00000000-0000-0000-0000-00000000000</uuid>
    <cpu>
      <arch>x86_64</arch>
      <model>Broadwell-noTSX-IBRS</model>
      <vendor>Intel</vendor>
      <microcode version='287'/>
      <signature family='6' model='183' stepping='1'/>
      <topology sockets='1' dies='1' cores='32' threads='1'/>
      <maxphysaddr mode='emulate' bits='46'/>
      <feature name='vme'/>
      <feature name='ds'/>
<!-- ellided for brevity -->
      <feature name='invtsc'/>
      <pages unit='KiB' size='4'/>
      <pages unit='KiB' size='2048'/>
      <pages unit='KiB' size='1048576'/>
    </cpu>
    <power_management>
      <suspend_mem/>
      <suspend_disk/>
      <suspend_hybrid/>
    </power_management>
    <iommu support='yes'/>
    <migration_features>
      <live/>
      <uri_transports>
        <uri_transport>tcp</uri_transport>
        <uri_transport>rdma</uri_transport>
      </uri_transports>
    </migration_features>
    <topology>
      <cells num='1'>
        <cell id='0'>
          <memory unit='KiB'>98606452</memory>
          <pages unit='KiB' size='4'>24651613</pages>
          <pages unit='KiB' size='2048'>0</pages>
          <pages unit='KiB' size='1048576'>0</pages>
          <distances>
            <sibling id='0' value='10'/>
          </distances>
          <cpus num='32'>
            <cpu id='0' socket_id='0' die_id='0' core_id='0' siblings='0-1'/>
<!-- ellided for brevity -->
            <cpu id='31' socket_id='0' die_id='0' core_id='47' siblings='31'/>
          </cpus>
        </cell>
      </cells>
    </topology>
    <cache>
      <bank id='0' level='3' type='both' size='36' unit='MiB' cpus='0-31'/>
    </cache>
    <secmodel>
      <model>none</model>
      <doi>0</doi>
    </secmodel>
  </host>

  <guest>
    <os_type>hvm</os_type>
    <arch name='armv6l'>
      <wordsize>32</wordsize>
      <emulator>/usr/bin/qemu-system-arm</emulator>
      <machine maxCpus='1'>integratorcp</machine>
<!-- ellided for brevity -->
      <machine maxCpus='1'>collie</machine>
      <machine maxCpus='1'>raspi0</machine>
      <machine maxCpus='1'>fp5280g2-bmc</machine>
      <domain type='qemu'/>
    </arch>
    <features>
      <cpuselection/>
      <deviceboot/>
      <disksnapshot default='on' toggle='no'/>
    </features>
  </guest>

  <guest>
    <os_type>hvm</os_type>
    <arch name='armv7l'>
<!-- ellided for brevity -->
    </arch>
  </guest>

  <guest>
    <os_type>hvm</os_type>
    <arch name='aarch64'>
<!-- ellided for brevity -->
    </arch>
  </guest>

  <guest>
    <os_type>hvm</os_type>
    <arch name='i686'>
<!-- ellided for brevity -->
    </arch>
  </guest>

  <guest>
    <os_type>hvm</os_type>
    <arch name='x86_64'>
<!-- ellided for brevity -->
    </arch>
  </guest>

</capabilities>

The original goal of this PR was to quickly (and without any hacks like running a command remotely) determine the host's native CPU arch. I ended up using the capabilities path because the output was incorrect as @muresan noted. The implementation as it is right now only unmarshalls nodeinfo related information, and lacks the full unmarshalling of available capabilities (in particular, topology isn't unmarshalled yet), however it is more correctly the capabilities.

In conclusion, I think your request is sensible: we can name it libvirt_host_capabilities. Question about whether to use node or host is up to consensus. After writing this comment up, I'm leaning towards host, because that's actually the heading section name on the virsh help host command itself.

[dmacvicar]

Looking at the virsh nodeinfo implementation, it is using virNodeGetInfo.

cmdCapabilities is implemented with virConnectGetCapabilities.

So, I think it make sense to separate them. The prefix could be host, or just nothing (I am leaning towards host). It also does not matter that the topology is not fully implemnted, as long it is documented.

[memetb]

That sounds good. I will get these updates done sometime this week. We should at least move these conclusions over to #1073 comment thread for posterity.