Re: [libvirt] [PATCHv2 01/16] qemu: Add KVM CPUs into cache only if KVM is present

On Fri, Nov 23, 2018 at 17:16:12 +0300, Roman Bolshakov wrote: > On Wed, Nov 21, 2018 at 07:43:43PM +0100, Jiri Denemark wrote: > > virQEMUCapsInitHostCPUModel always fills in something and your check > > should probably remain in place for it > > > > virQEMUCapsFormatHostCPUModelInfo does > > virQEMUCapsHostCPUDataPtr cpuData = &qemuCaps->kvmCPU; > > qemuMonitorCPUModelInfoPtr model = cpuData->info; > > if (!model) > > return; > > > > virQEMUCapsFormatCPUModels > > cpus = qemuCaps->kvmCPUModels; > > if (!cpus) > > return; > > > > So to me it looks like all functions are ready to see NULL pointers and > > just do nothing if that's the case. Thus the only thing this patch > > should need to do is to make sure virQEMUCapsInitHostCPUModel does not > > set something non-NULL there. > > Unfortunately, that won't work for the patch series. kvmCPUModels is renamed to > accelCPUModels and kvmCPU is renamed to accelCPU in PATCH 6. And how does different name change the behavior? > So, virQEMUCapsFormatHostCPUModelInfo looks like: > if (virQEMUCapsTypeIsAccelerated(type)) > cpuData = qemuCaps->accelCPU; > else > cpuData = qemuCaps->tcgCPU; > > and virQEMUCapsFormatCPUModels looks like: > if (virQEMUCapsTypeIsAccelerated(type)) > cpus = qemuCaps->accelCPUModels; > else > cpus = qemuCaps->tcgCPUModels; > > Without the check we'd return CPUs for KVM domain on the platform that doesn't > support it. It won't return anything because the code will make sure accelCPUModels and accelCPU will be NULL when no accel method is supported.