On 06/26/2013 04:29 PM, Martin Kletzander wrote:
> On 06/26/2013 04:20 PM, Michal Novotny wrote:
>> 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@xxxxxxxxxx>
>>>> ---
>>>> 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 ?
>>
> If you'll make it with the v5 before the release, the added value will
> be that everyone will be offered the look to how the final patch looks
> like, but if you won't I'll push this one with the changes I proposed.
>
> Martin
>
Feel free to push with changes you proposed, I need to take care of
other stuff ;-)
Thanks,
Michal
--
Michal Novotny <minovotn@xxxxxxxxxx>, RHCE, Red Hat
Virtualization | libvirt-php bindings | php-virt-control.org
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list