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

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



It has been accidentally sent twice. Please ignore and review just one
version as both v4 are the same ;-)

Also, it has been `make check` and `make syntax-check` tested and passed :-)

Thanks,
Michal

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 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/docs/schemas/capability.rng b/docs/schemas/capability.rng
> index 106ca73..65c7c72 100644
> --- a/docs/schemas/capability.rng
> +++ b/docs/schemas/capability.rng
> @@ -290,6 +290,11 @@
>            <text/>
>          </attribute>
>        </optional>
> +      <optional>
> +        <attribute name='maxCpus'>
> +          <ref name='unsignedInt'/>
> +        </attribute>
> +      </optional>
>        <text/>
>      </element>
>    </define>
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index da92c78..5aeb2ab 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -853,6 +853,8 @@ virCapabilitiesFormatXML(virCapsPtr caps)
>              virBufferAddLit(&xml, "      <machine");
>              if (machine->canonical)
>                  virBufferAsprintf(&xml, " canonical='%s'", machine->canonical);
> +            if (machine->maxCpus > 0)
> +                virBufferAsprintf(&xml, " maxCpus='%d'", machine->maxCpus);
>              virBufferAsprintf(&xml, ">%s</machine>\n", machine->name);
>          }
>  
> @@ -871,6 +873,8 @@ virCapabilitiesFormatXML(virCapsPtr caps)
>                  virBufferAddLit(&xml, "        <machine");
>                  if (machine->canonical)
>                      virBufferAsprintf(&xml, " canonical='%s'", machine->canonical);
> +                if (machine->maxCpus > 0)
> +                    virBufferAsprintf(&xml, " maxCpus='%d'", machine->maxCpus);
>                  virBufferAsprintf(&xml, ">%s</machine>\n", machine->name);
>              }
>              virBufferAddLit(&xml, "      </domain>\n");
> diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
> index abcf6de..22b6fb6 100644
> --- a/src/conf/capabilities.h
> +++ b/src/conf/capabilities.h
> @@ -46,6 +46,7 @@ typedef virCapsGuestMachine *virCapsGuestMachinePtr;
>  struct _virCapsGuestMachine {
>      char *name;
>      char *canonical;
> +    int maxCpus;
>  };
>  
>  typedef struct _virCapsGuestDomainInfo virCapsGuestDomainInfo;
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index c4e076a..89f41b8 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -256,6 +256,7 @@ struct _virQEMUCaps {
>      size_t nmachineTypes;
>      char **machineTypes;
>      char **machineAliases;
> +    int *machineMaxCpus;
>  };
>  
>  struct _virQEMUCapsCache {
> @@ -335,6 +336,7 @@ virQEMUCapsSetDefaultMachine(virQEMUCapsPtr qemuCaps,
>  {
>      char *name = qemuCaps->machineTypes[defIdx];
>      char *alias = qemuCaps->machineAliases[defIdx];
> +    int cpu_max = qemuCaps->machineMaxCpus[defIdx];
>  
>      memmove(qemuCaps->machineTypes + 1,
>              qemuCaps->machineTypes,
> @@ -342,8 +344,12 @@ virQEMUCapsSetDefaultMachine(virQEMUCapsPtr qemuCaps,
>      memmove(qemuCaps->machineAliases + 1,
>              qemuCaps->machineAliases,
>              sizeof(qemuCaps->machineAliases[0]) * defIdx);
> +    memmove(qemuCaps->machineMaxCpus + 1,
> +            qemuCaps->machineMaxCpus,
> +            sizeof(qemuCaps->machineMaxCpus[0]) * defIdx);
>      qemuCaps->machineTypes[0] = name;
>      qemuCaps->machineAliases[0] = alias;
> +    qemuCaps->machineMaxCpus[0] = cpu_max;
>  }
>  
>  /* Format is:
> @@ -390,7 +396,8 @@ virQEMUCapsParseMachineTypesStr(const char *output,
>          }
>  
>          if (VIR_REALLOC_N(qemuCaps->machineTypes, qemuCaps->nmachineTypes + 1) < 0 ||
> -            VIR_REALLOC_N(qemuCaps->machineAliases, qemuCaps->nmachineTypes + 1) < 0) {
> +            VIR_REALLOC_N(qemuCaps->machineAliases, qemuCaps->nmachineTypes + 1) < 0 ||
> +            VIR_REALLOC_N(qemuCaps->machineMaxCpus, qemuCaps->nmachineTypes + 1) < 0) {
>              VIR_FREE(name);
>              VIR_FREE(canonical);
>              virReportOOMError();
> @@ -404,6 +411,8 @@ virQEMUCapsParseMachineTypesStr(const char *output,
>              qemuCaps->machineTypes[qemuCaps->nmachineTypes-1] = name;
>              qemuCaps->machineAliases[qemuCaps->nmachineTypes-1] = NULL;
>          }
> +        /* Value 0 means "unknown" as it's not exposed by QEMU binary */
> +        qemuCaps->machineMaxCpus[qemuCaps->nmachineTypes-1] = 0;
>      } while ((p = next));
>  
>  
> @@ -1764,11 +1773,14 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps)
>          goto no_memory;
>      if (VIR_ALLOC_N(ret->machineAliases, qemuCaps->nmachineTypes) < 0)
>          goto no_memory;
> +    if (VIR_ALLOC_N(ret->machineMaxCpus, qemuCaps->nmachineTypes) < 0)
> +        goto no_memory;
>      ret->nmachineTypes = qemuCaps->nmachineTypes;
>      for (i = 0; i < qemuCaps->nmachineTypes; i++) {
>          if (VIR_STRDUP(ret->machineTypes[i], qemuCaps->machineTypes[i]) < 0 ||
>              VIR_STRDUP(ret->machineAliases[i], qemuCaps->machineAliases[i]) < 0)
>              goto error;
> +        ret->machineMaxCpus[i] = qemuCaps->machineMaxCpus[i];
>      }
>  
>      return ret;
> @@ -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;
>      }
>      VIR_FREE(qemuCaps->machineTypes);
>      VIR_FREE(qemuCaps->machineAliases);
> +    VIR_FREE(qemuCaps->machineMaxCpus);
>  
>      for (i = 0; i < qemuCaps->ncpuDefinitions; i++) {
>          VIR_FREE(qemuCaps->cpuDefinitions[i]);
> @@ -1932,6 +1946,7 @@ int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps,
>              if (VIR_STRDUP(mach->name, qemuCaps->machineTypes[i]) < 0)
>                  goto error;
>          }
> +        mach->maxCpus = qemuCaps->machineMaxCpus[i];
>          (*machines)[i] = mach;
>      }
>  
> @@ -1966,6 +1981,25 @@ const char *virQEMUCapsGetCanonicalMachine(virQEMUCapsPtr qemuCaps,
>  }
>  
>  
> +int virQEMUCapsGetMachineMaxCpus(virQEMUCapsPtr qemuCaps,
> +                                 const char *name)
> +{
> +    size_t i;
> +
> +    if (!name)
> +        return 0;
> +
> +    for (i = 0; i < qemuCaps->nmachineTypes; i++) {
> +        if (!qemuCaps->machineMaxCpus[i])
> +            continue;
> +        if (STREQ(qemuCaps->machineTypes[i], name))
> +            return qemuCaps->machineMaxCpus[i];
> +    }
> +
> +    return 0;
> +}
> +
> +
>  static int
>  virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps,
>                              qemuMonitorPtr mon)
> @@ -2083,6 +2117,10 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps,
>          virReportOOMError();
>          goto cleanup;
>      }
> +    if (VIR_ALLOC_N(qemuCaps->machineMaxCpus, nmachines) < 0) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
>  
>      for (i = 0; i < nmachines; i++) {
>          if (VIR_STRDUP(qemuCaps->machineAliases[i], machines[i]->alias) < 0 ||
> @@ -2090,6 +2128,7 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps,
>              goto cleanup;
>          if (machines[i]->isDefault)
>              defIdx = i;
> +        qemuCaps->machineMaxCpus[i] = machines[i]->cpu_max;
>      }
>      qemuCaps->nmachineTypes = nmachines;
>  
> 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;
>  };
>  
>  int qemuMonitorGetMachines(qemuMonitorPtr mon,
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 88a0dc9..1362acf 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -4042,6 +4042,12 @@ int qemuMonitorJSONGetMachines(qemuMonitorPtr mon,
>              if (VIR_STRDUP(info->alias, tmp) < 0)
>                  goto cleanup;
>          }
> +        if (virJSONValueObjectHasKey(child, "cpu-max") &&
> +            virJSONValueObjectGetNumberInt(child, "cpu-max", &info->cpu_max) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("query-machines reply has malformed 'cpu-max' data"));
> +            goto cleanup;
> +        }
>      }
>  
>      ret = n;
> 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;
> +    }
> +
> +    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,
> @@ -3519,6 +3543,9 @@ int qemuProcessStart(virConnectPtr conn,
>                                                        vm->def->emulator)))
>          goto cleanup;
>  
> +    if (!qemuValidateCpuMax(vm->def, priv->qemuCaps))
> +        goto cleanup;
> +
>      if (qemuAssignDeviceAliases(vm->def, priv->qemuCaps) < 0)
>          goto cleanup;
>  
> diff --git a/tests/capabilityschemadata/caps-qemu-kvm.xml b/tests/capabilityschemadata/caps-qemu-kvm.xml
> index 36c4b49..1fbc22b 100644
> --- a/tests/capabilityschemadata/caps-qemu-kvm.xml
> +++ b/tests/capabilityschemadata/caps-qemu-kvm.xml
> @@ -33,18 +33,18 @@
>      <arch name='i686'>
>        <wordsize>32</wordsize>
>        <emulator>/usr/bin/qemu</emulator>
> -      <machine>pc-0.11</machine>
> -      <machine canonical='pc-0.11'>pc</machine>
> -      <machine>pc-0.10</machine>
> -      <machine>isapc</machine>
> +      <machine maxCpus='255'>pc-0.11</machine>
> +      <machine canonical='pc-0.11' maxCpus='255'>pc</machine>
> +      <machine maxCpus='255'>pc-0.10</machine>
> +      <machine maxCpus='1'>isapc</machine>
>        <domain type='qemu'>
>        </domain>
>        <domain type='kvm'>
>          <emulator>/usr/bin/qemu-kvm</emulator>
> -        <machine>pc-0.11</machine>
> -        <machine canonical='pc-0.11'>pc</machine>
> -        <machine>pc-0.10</machine>
> -        <machine>isapc</machine>
> +        <machine maxCpus='255'>pc-0.11</machine>
> +        <machine canonical='pc-0.11' maxCpus='255'>pc</machine>
> +        <machine maxCpus='255'>pc-0.10</machine>
> +        <machine maxCpus='1'>isapc</machine>
>        </domain>
>      </arch>
>      <features>

-- 
Michal Novotny <minovotn redhat com>, RHCE, Red Hat
Virtualization | libvirt-php bindings | php-virt-control.org


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