On 09/08/22 15:36, Richard W.M. Jones wrote:
(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?
Well it's a possibility.
Can that even happen when virt-p2v is running?)
I think so, yes; although it should be really rare (physically removed
CPU from multi-socket systems, or some cores offlined on the kernel
cmdline perhaps -- I think there could be reasons for that).
How about:
authentic_topo
physical_topo
accurate_topo
...?
The patch is totally fine, we're just quibbling about the
word "dense" :-)
I'll rename "dense_topo" to "phys_topo", and also adopt your
suggestion
for the manual.
Thanks!
Laszlo