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

[Laszlo Ersek]

On 09/08/22 16:39, Daniel P. Berrangé wrote:
> On Thu, Sep 08, 2022 at 02:36:15PM +0100, 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?  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" :-)
> 
> Why not 'host_topo' since it is mirroring the host ?

The physical machine on which p2v runs need not even support
virtualization -- p2v runs natively there, from inside a Live CD image.
Only the conversion output is a guest (on some other computer that does
indeed need to act as a QEMU/KVM hypervisor). Three machiens in total
are involved: the phys machine being converted to a guest, a machine
running virt-v2v (the conversion server), and then the hypervisor /
management box accepting the output guest (can be libvirt, openstack,
RHV, etc the usual output modules of virt-v2v). IOW, "host" is not
really a good fit for the machine where p2v runs.

Plus I've just finished the (difficult) rebase with the dense_topo ->
phys_topo renaming... :)

Laszlo

> 
> With regards,
> Daniel
>