[libvirt] [PATCH 12/22] Introduce virConnectBaselineHypervisorCPU public API

[Collin Walling]

On 05/16/2018 04:39 AM, Jiri Denemark wrote:
> The new API computes the most feature-rich CPU which is compatible with
> all given CPUs and can be provided by the specified hypervisor. It is a
> more useful version of vitConnectBaselineCPU, which doesn't consider any

s/vit/vir

> hypervisor capabilities when computing the best CPU.
> 
> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> ---
>  include/libvirt/libvirt-host.h |  8 ++++
>  src/driver-hypervisor.h        | 10 +++++
>  src/libvirt-host.c             | 81 ++++++++++++++++++++++++++++++++++
>  src/libvirt_public.syms        |  1 +
>  4 files changed, 100 insertions(+)
> 
> diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h
> index e2054baebc..84f4858169 100644
> --- a/include/libvirt/libvirt-host.h
> +++ b/include/libvirt/libvirt-host.h
> @@ -667,6 +667,14 @@ char *virConnectBaselineCPU(virConnectPtr conn,
>                              const char **xmlCPUs,
>                              unsigned int ncpus,
>                              unsigned int flags);
> +char *virConnectBaselineHypervisorCPU(virConnectPtr conn,
> +                                      const char *emulator,
> +                                      const char *arch,
> +                                      const char *machine,
> +                                      const char *virttype,
> +                                      const char **xmlCPUs,
> +                                      unsigned int ncpus,
> +                                      unsigned int flags);
>  
>  
>  int virNodeGetFreePages(virConnectPtr conn,
> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
> index d64de2d54c..9ce0d7e5de 100644
> --- a/src/driver-hypervisor.h
> +++ b/src/driver-hypervisor.h
> @@ -687,6 +687,15 @@ typedef char *
>                              const char **xmlCPUs,
>                              unsigned int ncpus,
>                              unsigned int flags);
> +typedef char *
> +(*virDrvConnectBaselineHypervisorCPU)(virConnectPtr conn,
> +                                      const char *emulator,
> +                                      const char *arch,
> +                                      const char *machine,
> +                                      const char *virttype,
> +                                      const char **xmlCPUs,
> +                                      unsigned int ncpus,
> +                                      unsigned int flags);
>  
>  typedef int
>  (*virDrvConnectGetCPUModelNames)(virConnectPtr conn,
> @@ -1542,6 +1551,7 @@ struct _virHypervisorDriver {
>      virDrvDomainSetBlockThreshold domainSetBlockThreshold;
>      virDrvDomainSetLifecycleAction domainSetLifecycleAction;
>      virDrvConnectCompareHypervisorCPU connectCompareHypervisorCPU;
> +    virDrvConnectBaselineHypervisorCPU connectBaselineHypervisorCPU;
>  };
>  
>  
> diff --git a/src/libvirt-host.c b/src/libvirt-host.c
> index 17cf183499..cdbbe35e5a 100644
> --- a/src/libvirt-host.c
> +++ b/src/libvirt-host.c
> @@ -1127,6 +1127,9 @@ virConnectGetCPUModelNames(virConnectPtr conn, const char *arch, char ***models,
>   * Computes the most feature-rich CPU which is compatible with all given
>   * host CPUs.
>   *
> + * See vitConnectBaselineHypervisorCPU() to get a CPU which can be provided

same here

> + * by a specific hypervisor.

Maybe say "by the hypervisor" here? I don't think it's as important as the change to the 
documentation as discussed in the previous patch, but it might be good for consistency.

> + *
>   * If @flags includes VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES then libvirt
>   * will explicitly list all CPU features that are part of the host CPU,
>   * without this flag features that are part of the CPU model will not be
> @@ -1174,6 +1177,84 @@ virConnectBaselineCPU(virConnectPtr conn,
>  }
>  
>  
> +/**
> + * virConnectBaselineHypervisorCPU:
> + *
> + * @conn: pointer to the hypervisor connection
> + * @emulator: path to the emulator binary
> + * @arch: domain architecture

I'd say "CPU architecture" instead

> + * @machine: machine type
> + * @virttype: virtualization type
> + * @xmlCPUs: array of XML descriptions of CPUs
> + * @ncpus: number of CPUs in xmlCPUs
> + * @flags: bitwise-OR of virConnectBaselineCPUFlags
> + *
> + * Computes the most feature-rich CPU which is compatible with all given CPUs
> + * and can be provided by the specified hypervisor. For best results the
> + * host-model CPUs as advertised by virConnectGetDomainCapabilities() should be
> + * passed in @xmlCPUs. Any of @emulator, @arch, @machine, and @virttype
> + * parameters may be NULL; libvirt will choose sensible defaults tailored to
> + * the host and its current configuration.
> + *
> + * This is different from vitConnectBaselineCPU() which doesn't consider any

s/vit/vir

> + * hypervisor abilities when computing the best CPU.
> + *
> + * If @flags includes VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES then libvirt
> + * will explicitly list all CPU features that are part of the computed CPU,
> + * without this flag features that are part of the CPU model will not be
> + * listed.
> + *
> + * If @flags includes VIR_CONNECT_BASELINE_CPU_MIGRATABLE, the resulting
> + * CPU will not include features that block migration.
> + *
> + * Returns XML description of the computed CPU (caller frees) or NULL on error.
> + */
> +char *
> +virConnectBaselineHypervisorCPU(virConnectPtr conn,
> +                                const char *emulator,
> +                                const char *arch,
> +                                const char *machine,
> +                                const char *virttype,
> +                                const char **xmlCPUs,
> +                                unsigned int ncpus,
> +                                unsigned int flags)
> +{
> +    size_t i;
> +
> +    VIR_DEBUG("conn=%p, emulator=%s, arch=%s, machine=%s, "
> +              "virttype=%s, xmlCPUs=%p, ncpus=%u, flags=0x%x",
> +              conn, NULLSTR(emulator), NULLSTR(arch), NULLSTR(machine),
> +              NULLSTR(virttype), xmlCPUs, ncpus, flags);
> +    if (xmlCPUs) {
> +        for (i = 0; i < ncpus; i++)
> +            VIR_DEBUG("xmlCPUs[%zu]=%s", i, NULLSTR(xmlCPUs[i]));
> +    }
> +
> +    virResetLastError();
> +
> +    virCheckConnectReturn(conn, NULL);
> +    virCheckNonNullArgGoto(xmlCPUs, error);
> +
> +    if (conn->driver->connectBaselineHypervisorCPU) {
> +        char *cpu;
> +
> +        cpu = conn->driver->connectBaselineHypervisorCPU(conn, emulator, arch,
> +                                                         machine, virttype,
> +                                                         xmlCPUs, ncpus, flags);
> +        if (!cpu)
> +            goto error;
> +
> +        return cpu;
> +    }
> +
> +    virReportUnsupportedError();
> +
> + error:
> +    virDispatchError(conn);
> +    return NULL;
> +}
> +
> +
>  /**
>   * virConnectSetKeepAlive:
>   * @conn: pointer to a hypervisor connection
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 97597d7708..05ab077fb4 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -788,6 +788,7 @@ LIBVIRT_4.1.0 {
>  LIBVIRT_4.4.0 {
>      global:
>          virConnectCompareHypervisorCPU;
> +        virConnectBaselineHypervisorCPU;
>  } LIBVIRT_4.1.0;
>  
>  # .... define new API here using predicted next version number ....
> 

Other than the nits above:

Reviewed-by: Collin Walling <walling at linux.ibm.com>

-- 
Respectfully,
- Collin Walling