Re: [Libguestfs] [v2v PATCH] output/create_libvirt_xml: generate @check='none' CPU attribute

On 07/22/22 11:50, Richard W.M. Jones wrote: > On Fri, Jul 22, 2022 at 10:42:48AM +0100, Daniel P. Berrangé wrote: >> On Fri, Jul 22, 2022 at 10:34:44AM +0100, Richard W.M. Jones wrote: >>> Sorry for the delayed response to this. I see you've posted an >>> updated patch, so this is just a bit of FYI. >>> >>> I originally added CPU modelling in commit 11505e4b84 (March 2017): >>> >>> https://github.com/libguestfs/virt-v2v/commit/11505e4b84ce8d7eda4e2a275fd... >>> >>> What we were actually trying to achieve here was to preserve the CPU >>> topology. I believe the request came from Bill Helgerson who was >>> working on v2v in the proto-IMS product, and was working a lot with >>> customers. >>> >>> You can see in the code before the patch is applied we only modelled >>> the number of vCPUs. Afterwards we have: >>> >>> * number of vCPUs >>> * vendor (eg. AMD) >>> * model (eg. EPYC) >>> * sockets >>> * cores per socket >>> * threads per core >>> >>> I think here only the first 1 and last 3 (#vCPUS, topology) are really >>> important. I believe I added the vendor and model just because they >>> were there, without necessarily thinking too deeply about the >>> implications. >>> >>> As you covered in your email, what is the real meaning of converting a >>> source guest using eg AMD/EPYC with virt-v2v to some target? Does it >>> mean that the target must be able to emulate all EPYC feature (likely >>> impossible if the target is Intel)? I would say it's not that >>> important. This isn't live migration, and almost all guests can be >>> booted interchangably on different x86_64 hardware. >>> >>> Is topology important? I would say yes, or at least it's much more >>> important than vendor/model. Workloads may expect not just a number >>> of vCPUs, but a particular layout, especially the larger and more >>> complex ones. >> >> In terms of topology, if you have NOT set pCPU:vCPU 1:1 pinning, >> then NEVER set threads > 1. There's a choice of sockets vs cores >> for non-pinned scenario, and generally I'd recommends 'cores' >> always because high core counts are common in real world, and >> 'sockets' mostly maxes out at 2/4 in real world (ignoring wierd >> high end hardware), also some OS restrict you based on sockets, >> but not cores. So IMHO the only compelling reason to use >> sockets > 1 is you want to have virtual NUMA topology, but >> even that's dubious unless pinning. >> >> If you have set pCPU:vCPU 1:1 pinning, then set topology to >> try to match what you've pinned to. >> >>> So ... my question now is, should we simply remove the vendor and >>> model fields completely? >> >> Removing 'model' is not a good idea, as you'll get the default >> CPU model. >> >> If you don't have to pick a particular CPU, then IMHO either >> use host-model or host-passthrough depending on whether you >> think live migration is important or not. > > I mean remove them from virt-v2v's internal source model [confusing > terminology here - modelling the source != CPU model]. On targets > we'd choose something like cpu=host-model to get the best possible > migratable CPU. > > The point is we're not copying the Intel / Nehalem, AMD / EPYC etc of > the guest from the source to the destination hypervisor. I think producing host-passthrough indiscriminately on output (which we already do in the particular case only when the source does not specify a model and we know an OS does not run on qemu64) would be best. I don't think it would be a very difficult patch or patch set, but I dread the testing of it. :/ Let me go ahead and commit v2; and let's remember this discussion for the next time a CPU model related problem pops up. If switching to host-passthrough solves that problem then, we should implement it then. (And then ask QE to test it as comprehensively as they can...)