9:27 a.m.
On 08/15/2018 05:35 AM, Marcos Paulo de Souza wrote:
Before this patch, esxDomainGetVcpusFlags was not checking the flags
argument. Current, get and set vcpus was failing in ESX since it was
checking for "maxSupportedVcpus", and this configuration can be ommited
by ESXi[1]. Now, if VIR_DOMAIN_VCPU_MAXIMUM is specified in flags argument
esxDomainGetVcpusFlags the maximum number of vcpus allowed for that VM.
Otherwise, the current number of vcpus is returned.
With this patch calls to virDomainSetVcpus, virDomainGetMaxVcpus and
virDomainGetVcpusFlags to return successfull again.
[1]:https://pubs.vmware.com/vi-sdk/visdk250/ReferenceGuide/vim.host.Capabi...
Signed-off-by: Marcos Paulo de Souza <marcos.souza.org(a)gmail.com>
---
Changes from v1:
* Now we only have one patch instead of two
* Change esxDomainGetVcpusFlags in order to check the flags argument, instead
* of changing esxDomainGetMaxVcpus.
src/esx/esx_driver.c | 52 ++++++++++++++++++++++++--------------------
1 file changed, 28 insertions(+), 24 deletions(-)
Since ESX is not generally well known to regular libvir-list readers,
reviewers, and/or committers - I think CC'g Matthias Bolte to ensure
he's aware of the patch in an area where he knows best is a good idea.
Especially on the ping.
If this was more simple, sure usually one of us regulars would handle
this, but you're making more than just simple changes requiring
knowledge of the underlying system that less of us have.
I've CC'd him on this response...
Still wondering how you sent mail on 8/18/18 (your ping to this thread)
which knew about the freeze on 8/28/18 - I'd say something's strange
with your system/mail config... ;-) Unless of course you've invented
the time machine!
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index c2154799fa..a1ba889326 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -2547,45 +2547,49 @@ esxDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags)
{
esxPrivate *priv = domain->conn->privateData;
esxVI_String *propertyNameList = NULL;
- esxVI_ObjectContent *hostSystem = NULL;
- esxVI_DynamicProperty *dynamicProperty = NULL;
+ esxVI_ObjectContent *obj = NULL;
+ esxVI_Int *vcpus = NULL;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_VCPU_MAXIMUM, -1);
- if (priv->maxVcpus > 0)
- return priv->maxVcpus;
-
priv->maxVcpus = -1;
if (esxVI_EnsureSession(priv->primary) < 0)
return -1;
- if (esxVI_String_AppendValueToList(&propertyNameList,
- "capability.maxSupportedVcpus") < 0
||
- esxVI_LookupHostSystemProperties(priv->primary, propertyNameList,
- &hostSystem) < 0) {
- goto cleanup;
- }
- for (dynamicProperty = hostSystem->propSet; dynamicProperty;
- dynamicProperty = dynamicProperty->_next) {
- if (STREQ(dynamicProperty->name, "capability.maxSupportedVcpus"))
{
- if (esxVI_AnyType_ExpectType(dynamicProperty->val,
- esxVI_Type_Int) < 0) {
- goto cleanup;
- }
+ if (flags & VIR_DOMAIN_VCPU_MAXIMUM) {
+ if (esxVI_String_AppendValueToList(&propertyNameList,
+
"capability.maxHostSupportedVcpus\0"
+ "capability.maxSupportedVcpus")
< 0 ||
+ esxVI_LookupHostSystemProperties(priv->primary,
+ propertyNameList, &obj) < 0 ||
+ esxVI_GetInt(obj, "capability.maxSupportedVcpus", &vcpus,
+ esxVI_Occurrence_OptionalItem) < 0)
+ goto cleanup;
- priv->maxVcpus = dynamicProperty->val->int32;
- break;
- } else {
- VIR_WARN("Unexpected '%s' property",
dynamicProperty->name);
- }
+ if (!vcpus && esxVI_GetInt(obj,
"capability.maxHostSupportedVcpus",
Can esxVI_GetInt succeed and return NULL for vcpus?
Even it can, this will cause an issue when we leave the "if" and go to
assign priv->maxVcpus from vcpus->value
Don't believe the "!vcpus &&" is necessary. I see one other usage
and
the returned pointer is not checked prior to usage, so I think you're
safe, but in the end Matthias certain knows better than I do.
+ &vcpus,
esxVI_Occurrence_RequiredItem) < 0)
+ goto cleanup;
+
+ } else {
+ if (esxVI_String_AppendValueToList(&propertyNameList,
+ "config.hardware.numCPU\0") < 0
||
This line needs one extra space of alignment
+ esxVI_LookupVirtualMachineByUuid(priv->primary,
domain->uuid,
+ propertyNameList, &obj,
+ esxVI_Occurrence_RequiredItem) < 0 ||
+ esxVI_GetInt(obj, "config.hardware.numCPU", &vcpus,
+ esxVI_Occurrence_RequiredItem) < 0)
Similarly here w/ !vcpus where it's not checked before using a couple
lines below.
John
+ goto cleanup;
}
+ priv->maxVcpus = vcpus->value;
+
cleanup:
esxVI_String_Free(&propertyNameList);
- esxVI_ObjectContent_Free(&hostSystem);
+ esxVI_ObjectContent_Free(&obj);
+ esxVI_Int_Free(&vcpus);
return priv->maxVcpus;
}