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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Nov 21, 2018 at 17:01:44 +0300, Roman Bolshakov wrote:
> From: Roman Bolshakov <roolebo@xxxxxxxxx>
> 
> virQEMUCapsFormatCache/virQEMUCapsLoadCache adds/reads KVM CPUs to/from
> capabilities cache regardless of QEMU_CAPS_KVM. That can cause undesired
> side-effects when KVM CPUs are present in the cache on a platform that
> doesn't support it, e.g. macOS or Linux without KVM support.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> Signed-off-by: Roman Bolshakov <roolebo@xxxxxxxxx>

This doesn't look like a patch written by Daniel so why did you include
the Signed-off-by line? Or did I miss anything?

> ---
>  src/qemu/qemu_capabilities.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index fde27010e4..4ba8369e3a 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -3467,11 +3467,13 @@ virQEMUCapsLoadCache(virArch hostArch,
>      }
>      VIR_FREE(str);
>  
> -    if (virQEMUCapsLoadHostCPUModelInfo(qemuCaps, ctxt, VIR_DOMAIN_VIRT_KVM) < 0 ||
> +    if ((virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) &&
> +         virQEMUCapsLoadHostCPUModelInfo(qemuCaps, ctxt, VIR_DOMAIN_VIRT_KVM) < 0) ||
>          virQEMUCapsLoadHostCPUModelInfo(qemuCaps, ctxt, VIR_DOMAIN_VIRT_QEMU) < 0)
>          goto cleanup;

I don't think we should introduce these guards in all the places. All
the loading and formatting functions should return success if the
appropriate info is not available, so you should just make sure the
relevant info is NULL in qemuCaps.

>  
> -    if (virQEMUCapsLoadCPUModels(qemuCaps, ctxt, VIR_DOMAIN_VIRT_KVM) < 0 ||
> +    if ((virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) &&
> +         virQEMUCapsLoadCPUModels(qemuCaps, ctxt, VIR_DOMAIN_VIRT_KVM) < 0) ||
>          virQEMUCapsLoadCPUModels(qemuCaps, ctxt, VIR_DOMAIN_VIRT_QEMU) < 0)
>          goto cleanup;
>  
> @@ -3584,7 +3586,8 @@ virQEMUCapsLoadCache(virArch hostArch,
>      if (virQEMUCapsParseSEVInfo(qemuCaps, ctxt) < 0)
>          goto cleanup;
>  
> -    virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_KVM);
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
> +      virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_KVM);

Please, follow our coding style, i.e., indent by 4 spaces.

>      virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_QEMU);
>  
>      ret = 0;
...

Jirka

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux