Re: [Libguestfs] [p2v PATCH 2/6] restrict vCPU topology to (a) fully populated physical, or (b) 1 * N * 1

On Mon, Sep 05, 2022 at 01:25:27PM +0200, Laszlo Ersek wrote: > Currently, the CPU topology for the converted domain is determined as > follows: > > (1) main() [main.c] > > (2) set_config_defaults() [main.c] > vcpus <-- sysconf (_SC_NPROCESSORS_ONLN) > get_cpu_config() [cpuid.c] > get_topology() [cpuid.c] > sockets <-- lscpu (physical) > cores <-- lscpu (physical) > threads <-- lscpu (physical) > > (3) update_config_from_kernel_cmdline() [kernel-config.c] > vcpus <-- kernel cmdline > sockets <-- kernel cmdline > cores <-- kernel cmdline > threads <-- kernel cmdline > > (4) opt#1: kernel_conversion() [kernel.c] > no change to topology info > opt#2: gui_conversion() [gui.c] > vcpus <-- GUI > > (5) ... > start_conversion() [conversion.c] > generate_physical_xml() [physical-xml.c] > format vcpus, sockets, cores, threads > > In (2), we initialize the topology information from the physical machine. > In (3), we override each property in isolation from the kernel command > line (this is done for both kernel conversion and GUI conversion; in the > former case, it is the only way for the user to influence each element of > the topology). In (4), in case we picked GUI conversion, the VCPU number > can be overridden yet another time on the GUI. In (5), we format the > topology information into the domain XML. > > The problem is that it's easy to create inconsistent topologies that > libvirt complains about. One example is that in step (4) on the GUI, if > the flat VCPU count is reduced by the user, it can result in sockets > partially populated with cores or threads, or in cores partially populated > with threads. Another example (even if the user does not touch the VCPU > count) is a partially onlined CPU topology on the physical machine: > _SC_NPROCESSORS_ONLN does not reflect any topology information, and if the > flat number it returns is less than the (sockets * cores * threads) > product from "lscpu", then the online logical processors will be > "compressed to the left" by libvirt just the same as after the manual VCPU > count reduction. > > An over-complicated way to fix this would be the following: > > - parse the output of "lscpu -p" (which is available since RHEL6 -- > "lscpu" is altogether missing in RHEL5), retrieving online-ness combined > with full topology information > > - expose complete topology info on the GUI > > - format the complete topology information for libvirt. > > A simpler way (which is what this patch series chooses) is to remove some > flexibility from virt-p2v's configuration interface. Namely, only allow > the user to choose betwen (a) copying the physical CPU topology, *fully > populated*, (b) specifying the core count N for a "1 socket * N > cores/socket * 1 thread/core" topology. > > Option (a) eliminates the "partially onlined physical CPU topology" > scenario; it produces a topology that exists in the physical world, and > does not require p2v to maintain topology information more expressively > than it currently does. > > Option (b) eliminates any conflicts between the physical (or any > user-configured) sockets:cores:threads hierarchy and a manually set > logical processor count. Option (b) produces the kind of flat CPU topology > that QEMU has defaulted to since version 6.2, with a simple "-smp" option. > Option (b) preserves the simple p2v user interface for entering only a > logical processor count. I would further say that option (b) is the *only* one that makes sense from a performance POV, if you are *not* doing 1:1 pCPU:vCPU pinning. The guest kernel scheduler will take different placement decisions based on the topology, and if guest CPUIs are floating across arbitrary host CPUs, those scheduler decisions are meaningless and potentially even harmful to performance. IOW, I'd say option (b) should be the default in the absence of any explicit override from the user.

When libvirt integrates support for core scheduling, then it becomes more reasonable to consider setting different topologies even when not pinning. With regards, Daniel