On Wed, Nov 23, 2022 at 13:40:21 +0530, manish.mishra wrote: > Hi Jiri, Thanks, We discussed internally, yes something like that > works for us. We wanted to dicusss how it will look like roughly > before sending patch. Does something like below works. <qemu:cpu> will > be a sub-property within <cpu> and it will contains details only about > cpu-model, cpu-features and check everything else can be as it is. So > for CPU feature and model either user can define those in <cpu> itself > as in example 1 or in <qemu:cpu> as in example 2. Also either example > 1 or example 2 are valid, if user mixes up both, i mean define cpu > feature or models in both <qemu:cpu> and <cpu> it will be a parsing > error to avoid conflicts. In feature policy we as of now need only > require/disable for <qemu:cpu> case and no need of options like match, > mode if <qemu:cpu> is defined. Please let us know if something like > that works. It may vary a bit while working on patch, so probaly it > will be more clear when patch is out, I just wanted to put our > requirements roughly before starting working on patch :). > > <cpu> > <vendor>Intel</vendor> > <topology sockets='240' dies='1' cores='1' threads='1'/> > <cache level='3' mode='emulate'/> > <qemu:cpu check='enfoce'> > <model>Icelake-Server-V4</model> > <feature policy='require' name='smap'/> > <feature policy='require' name='avx'/> > <feature policy='require' name='xsaveopt'/> > <feature policy='disable' name='tsc_adjust'/> > <qemu:cpu /> > <numa> > <cell id='0' cpus='0-239' memory='8388608' unit='KiB' memAccess='shared'/> > </numa> > </cpu> I think this would be quite hard to notice in the XML as it looks almost like a normal CPU definition and it would also complicate the <cpu> parser. We should keep this completely separated similarly to other elements in qemu namespace (as described in https://libvirt.org/drvqemu.html#pass-through-of-arbitrary-qemu-commands So for example, <domain type='qemu' xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'> ... <cpu> <topology sockets='240' dies='1' cores='1' threads='1'/> <cache level='3' mode='emulate'/> <numa>...</numa> ... </cpu> <qemu:cpu> <qemu:model name='Icelake-Server-V4'/> <qemu:option name='enforce' value='on'/> <qemu:option name='smap' value='on'/> <qemu:option name='avx' value='on'/> <qemu:option name='tsc-adjust' value='off'/> <qemu:option name='strange-option' value='1234'/> </qemu:cpu> <devices> ... </devices> </domain> and this would directly translate to -cpu Icelake-Server-V4,enforce=on,smap=on,avx=on,tsc-adjust=off,strange-option=1234 In other words, make it a complete passthrough of the values to QEMU without any extra parsing or translating on libvirt side. This way you can express anything QEMU supports for the -cpu command line option and libvirt has not implemented yet. Defining a CPU model both in <cpu> and <qemu:cpu> would be an error indeed. But eware, even usage of hypervisor features or other XML configuration outside of <cpu> that are translated into -cpu options could be unsupported and overridden by the content of <qemu:cpu>. We have several options for dealing with this: 1. check all of this and report an error 2. document the new <qemu:cpu> element to completely override anything libvirt would pass to -cpu and it's up to the user to include anything they need there (such as hypervisor features) 3. use the content of <qemu:cpu> as a base for -cpu and add other staff there as usual with a proper documentation that mentioning a specific option both in <qemu:cpu> and elsewhere results in an undefined behavior Personally, I would vote for 2 or 3 and perhaps slightly more for 3 as it is a bit more user friendly in case the goal is to override the CPU model but keep using normal configuration for hypervisor features. And of course, using <qemu:cpu> element would result in the domain being tainted with a new VIR_DOMAIN_TAINT_CUSTOM_CPU. Jirka
