Laszlo Ersek
2022-Sep-08 13:23 UTC
[Libguestfs] [p2v PATCH 2/6] restrict vCPU topology to (a) fully populated physical, or (b) 1 * N * 1
On 09/08/22 10:03, Richard W.M. Jones wrote:> On Mon, Sep 05, 2022 at 01:25:27PM +0200, Laszlo Ersek wrote: >> + "p2v.vcpu.dense_topo" => manual_entry->new( >> + shortopt => "", # ignored for booleans >> + description => " >> +Copy the physical machine's CPU topology, densely populated, to the >> +guest. Disabled by default. If disabled, the C<p2v.vcpu.cores> setting >> +takes effect.", > > > I just realised I'm not completely sure what "densely populated" > means here. I think we should have a bit more explanation. > > How about something like: > > "p2v.vcpu.dense_topo" => manual_entry->new( > shortopt => "", # ignored for booleans > description => " > Copy the physical machine's complete CPU topology (sockets, cores and > threads) to the guest. Disabled by default. If disabled, the > C<p2v.vcpu.cores> setting takes effect.", > > (Which might also imply that we rename this something like > "complete_topo" or "full_topo" but I'll leave that to you.)By "dense", I meant to express that there are no gaps in the onlining of the CPU topology. Assume we have 2 sockets, 2 cores/socket, 2 theads/core. Assume CPU#1 (in socket#1) is hot-pluggable, but isn't currently plugged, only CPU#0 (in socket#0) is present -- making for 1*2*2 = 4 logical processors in total. A physical machine may well boot like this. Then our topology is 2*2*2, but we only have 4 logical processors, so the topology is not densely populated. The language is supposed to express that in any such case, we'll ignore the online / plugged / etc count, and we'll just grab the static topology, and fully / densely populate it with logical processors. "Complete topology" does not express this. Sticking with the above example, the topology is already complete on the physical machine (we have full information about the levels of the hierarchy), but it's not densely populated. Another example would be 1 * 4 * 2 physical (a normal low-end machine by today's standards), where the sysadmin disables (say) cores #1 and #2 using /sys/devices/system/cpu/cpu{1,2}/online. (I think this may even be possible on the kernel command line, for whatever reason necessary.) In this case, during conversion, if "dense_topo" is set, we carry over not just the topology (= the 1 * 4 * 2 hierarchy), but we also densely populate it (producing 8 logical processors in the conversion output, disregarding the "gaps" on the source; i.e. that only 4 logical processors were available on the physical machine originally.) I considered "complete", and thought it didn't express my intent. "Full" is so-so -- my problem is it seems to have two meanings; one is in fact what I'm trying to say with "dense", but the other meaning is just "complete", which I don't find good. The choices p2v should offer are: - Just carry over a flat VCPU count N --> this maps to a 1 socket * N cores/socket * 1 thread / core topology, fully populated. - Otherwise (i.e., when the dense_topo knob is enabled), convert the original topology (S sockets * C/S cores/socket * T threads/core), *AND* fully populate that topology (disregarding the original "online count" on the physical machine, which may easily be less than the (S * C * T) product.) Thanks Laszlo
Richard W.M. Jones
2022-Sep-08 13:36 UTC
[Libguestfs] [p2v PATCH 2/6] restrict vCPU topology to (a) fully populated physical, or (b) 1 * N * 1
(Adding Dan for input) On Thu, Sep 08, 2022 at 03:23:41PM +0200, Laszlo Ersek wrote:> On 09/08/22 10:03, Richard W.M. Jones wrote: > > On Mon, Sep 05, 2022 at 01:25:27PM +0200, Laszlo Ersek wrote: > >> + "p2v.vcpu.dense_topo" => manual_entry->new( > >> + shortopt => "", # ignored for booleans > >> + description => " > >> +Copy the physical machine's CPU topology, densely populated, to the > >> +guest. Disabled by default. If disabled, the C<p2v.vcpu.cores> setting > >> +takes effect.", > > > > > > I just realised I'm not completely sure what "densely populated" > > means here. I think we should have a bit more explanation. > > > > How about something like: > > > > "p2v.vcpu.dense_topo" => manual_entry->new( > > shortopt => "", # ignored for booleans > > description => " > > Copy the physical machine's complete CPU topology (sockets, cores and > > threads) to the guest. Disabled by default. If disabled, the > > C<p2v.vcpu.cores> setting takes effect.", > > > > (Which might also imply that we rename this something like > > "complete_topo" or "full_topo" but I'll leave that to you.) > > By "dense", I meant to express that there are no gaps in the onlining of > the CPU topology. > > Assume we have 2 sockets, 2 cores/socket, 2 theads/core. Assume CPU#1 > (in socket#1) is hot-pluggable, but isn't currently plugged, only CPU#0 > (in socket#0) is present -- making for 1*2*2 = 4 logical processors in > total. A physical machine may well boot like this. Then our topology is > 2*2*2, but we only have 4 logical processors, so the topology is not > densely populated. The language is supposed to express that in any such > case, we'll ignore the online / plugged / etc count, and we'll just grab > the static topology, and fully / densely populate it with logical > processors. > > "Complete topology" does not express this. Sticking with the above > example, the topology is already complete on the physical machine (we > have full information about the levels of the hierarchy), but it's not > densely populated. > > Another example would be 1 * 4 * 2 physical (a normal low-end machine by > today's standards), where the sysadmin disables (say) cores #1 and #2 > using /sys/devices/system/cpu/cpu{1,2}/online. (I think this may even be > possible on the kernel command line, for whatever reason necessary.) In > this case, during conversion, if "dense_topo" is set, we carry over not > just the topology (= the 1 * 4 * 2 hierarchy), but we also densely > populate it (producing 8 logical processors in the conversion output, > disregarding the "gaps" on the source; i.e. that only 4 logical > processors were available on the physical machine originally.) > > I considered "complete", and thought it didn't express my intent. "Full" > is so-so -- my problem is it seems to have two meanings; one is in fact > what I'm trying to say with "dense", but the other meaning is just > "complete", which I don't find good. > > The choices p2v should offer are: > > - Just carry over a flat VCPU count N --> this maps to a 1 socket * N > cores/socket * 1 thread / core topology, fully populated. > > - Otherwise (i.e., when the dense_topo knob is enabled), convert the > original topology (S sockets * C/S cores/socket * T threads/core), *AND* > fully populate that topology (disregarding the original "online count" > on the physical machine, which may easily be less than the (S * C * T) > product.)I think the "mot juste" has to express that we're trying to model as closely as possible the real physical topology. (The denseness doesn't seem to be so important - are there many machines where CPUs are not online? Can that even happen when virt-p2v is running?) How about: authentic_topo physical_topo accurate_topo ...? The patch is totally fine, we're just quibbling about the word "dense" :-) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com nbdkit - Flexible, fast NBD server with plugins https://gitlab.com/nbdkit/nbdkit