[libvirt] [PATCH v4] qemu: Implement CPUs check against machine type's cpu-max

[Martin Kletzander]

On 06/25/2013 05:44 PM, Michal Novotny wrote:
> Implement check whether (maximum) vCPUs doesn't exceed machine
> type's cpu-max settings.
> 
> Differences between v3 and v4 (this one):
>  - Rebased to latest libvirt version
>  - Capability XML output extended by maxCpus field
>  - Extended caps-qemu-kvm.xml test by maxCpus for one of test emulators
> 
> On older versions of QEMU the check is disabled.
> 
> Signed-off-by: Michal Novotny <minovotn at redhat.com>
> ---
>  docs/schemas/capability.rng                  |  5 ++++
>  src/conf/capabilities.c                      |  4 +++
>  src/conf/capabilities.h                      |  1 +
>  src/qemu/qemu_capabilities.c                 | 41 +++++++++++++++++++++++++++-
>  src/qemu/qemu_capabilities.h                 |  3 +-
>  src/qemu/qemu_monitor.h                      |  1 +
>  src/qemu/qemu_monitor_json.c                 |  6 ++++
>  src/qemu/qemu_process.c                      | 27 ++++++++++++++++++
>  tests/capabilityschemadata/caps-qemu-kvm.xml | 16 +++++------
>  9 files changed, 94 insertions(+), 10 deletions(-)
> 
[...]
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
[...]
> @@ -1789,9 +1801,11 @@ void virQEMUCapsDispose(void *obj)
>      for (i = 0; i < qemuCaps->nmachineTypes; i++) {
>          VIR_FREE(qemuCaps->machineTypes[i]);
>          VIR_FREE(qemuCaps->machineAliases[i]);
> +        qemuCaps->machineMaxCpus[i] = -1;

Pointless line.

>      }
>      VIR_FREE(qemuCaps->machineTypes);
>      VIR_FREE(qemuCaps->machineAliases);
> +    VIR_FREE(qemuCaps->machineMaxCpus);
>  
>      for (i = 0; i < qemuCaps->ncpuDefinitions; i++) {
>          VIR_FREE(qemuCaps->cpuDefinitions[i]);
[...]
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 64a4b1d..7088747 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -234,7 +234,8 @@ size_t virQEMUCapsGetMachineTypes(virQEMUCapsPtr qemuCaps,
>                                    char ***names);
>  const char *virQEMUCapsGetCanonicalMachine(virQEMUCapsPtr qemuCaps,
>                                             const char *name);
> -
> +int virQEMUCapsGetMachineMaxCpus(virQEMUCapsPtr qemuCaps,
> +                                 const char *name);
>  int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps,
>                                     size_t *nmachines,
>                                     virCapsGuestMachinePtr **machines);
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 3d9afa3..06ae4c5 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -654,6 +654,7 @@ struct _qemuMonitorMachineInfo {
>      char *name;
>      bool isDefault;
>      char *alias;
> +    int cpu_max;

This parameter should be unified to match the previous naming (maxCpus)
as cpu_max might be misunderstood as a maximum cpu number, not the
maximum number of cpus.

>  };
>  
>  int qemuMonitorGetMachines(qemuMonitorPtr mon,
[...]
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 5a0f18b..3146ce2 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3330,6 +3330,30 @@ error:
>  }
>  
>  
> +static bool
> +qemuValidateCpuMax(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
> +{
> +    int cpu_max;
> +
> +    cpu_max = virQEMUCapsGetMachineMaxCpus(qemuCaps, def->os.machine);
> +    if (!cpu_max)
> +        return true;
> +
> +    if (def->vcpus > cpu_max) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       "%s", _("CPUs greater than specified machine type limit"));
> +        return false;
> +    }
> +

This check is pointless since it's guaranteed that vcpus <= maxvcpus.

> +    if (def->maxvcpus > cpu_max) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       "%s", _("Maximum CPUs greater than specified machine type limit"));
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
>  int qemuProcessStart(virConnectPtr conn,
>                       virQEMUDriverPtr driver,
>                       virDomainObjPtr vm,

Other that that the patch looks fine, but I'd wait with the push after
release.  If nobody is against that (and against the changes I
proposed), I'll push this after the release.

Martin