[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



On 06/26/2013 04:17 PM, Martin Kletzander wrote:
> 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/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

Ok Martin, would you like me to do the changes you proposed or will you
refresh the patch yourself and no need to submit v5 ?

Thanks,
Michal

-- 
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]