[PATCH v1 2/2] qemu: driver: perform CPU model expansion on single CPU during baseline

[Jiri Denemark]

On Wed, Sep 23, 2020 at 22:26:29 -0400, Collin Walling wrote:
> When executing the hypervisor-cpu-baseline command and if there is only
> a single CPU definition present in the XML file, then libvirt will
> print an unhelpful message:
> 
> "error: An error occurred, but the cause is unknown"
> 
> This is due to no CPU definition ever being "baselined", since the
> API expects at least two CPU models.
> 
> Let's fix this by performing a CPU model expansion on the single CPU
> definition and returning the result to the caller.

Ah, so when host-passthrough CPU is passed to the baseline API, we
should report an error. So this patch is actually trying to handle
single CPU definition with a non-empty <model> element specified. Good,
as the API is of course supposed to work in this case. It should
basically return the CPU definition passed to it with unsupported
features disabled.

Normally, expansion should only be done when requested by the
corresponding API flag. The simplest fix would be just returning the
original CPU definition if only one was passed to baseline. But the
result might not be the correct one as it could include unsupported
features. So is static expansion the thing that will return the expected
result? If so, this patch is mostly correct...

> Signed-off-by: Collin Walling <walling at linux.ibm.com>
> ---
>  src/qemu/qemu_driver.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 427d2419f3..97a960a769 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -12472,6 +12472,7 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps,
>      g_autoptr(qemuProcessQMP) proc = NULL;
>      g_autoptr(virCPUDef) baseline = NULL;
>      qemuMonitorCPUModelInfoPtr result = NULL;
> +    qemuMonitorCPUModelExpansionType expansion_type;
>      size_t i;
>  
>      if (!(proc = qemuProcessQMPNew(virQEMUCapsGetBinary(qemuCaps),
> @@ -12503,15 +12504,15 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps,
>              return NULL;
>      }
>  
> -    if (expand_features) {

While full expansion has to always be performed on the baseline result,
static expansion should only be called when ncpus == 1. In other words,
rather than removing this condition, you should change it to

       if (expand_features || ncpus == 1) {

> -        if (qemuMonitorGetCPUModelExpansion(proc->mon,
> -                                            QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL,
> -                                            baseline, true, false, &result) < 0)
> -            return NULL;
> +    expansion_type = expand_features ? QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL
> +                                     : QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC;
>  
> -        if (qemuConnectStealCPUModelFromInfo(baseline, &result) < 0)
> -            return NULL;
> -    }
> +    if (qemuMonitorGetCPUModelExpansion(proc->mon, expansion_type,
> +                                        baseline, true, false, &result) < 0)
> +        return NULL;
> +
> +    if (qemuConnectStealCPUModelFromInfo(baseline, &result) < 0)
> +        return NULL;
>  
>      return g_steal_pointer(&baseline);
>  }

Jirka