2010/11/9 Daniel Veillard <veillard@redhat.com>:
On Sun, Nov 07, 2010 at 01:40:42PM +0100, Matthias Bolte wrote:
Also add a test case for the VMX handling of it. [...] + if (esxVI_LookupVirtualMachineByUuidAndPrepareForTask + (priv->primary, domain->uuid, NULL, &virtualMachine, + priv->autoAnswer) < 0 || [...] + if (esxVI_String_AppendValueToList + (&propertyNameList, "config.memoryAllocation.reservation") < 0 || + esxVI_LookupVirtualMachineByUuid(priv->primary, domain->uuid,
Small nitpick to prove I read the patch :-) It seems your editor tend to cut lines in different way if within a condition than if within a normal expression, I would have put (&propertyNameList, on the same line as the if ( ...
Well, if the parameter list cannot be split at a ',' because the parameters would still be too long then I put the opening '(' at the next line, two spaces indented relative to the function name. This is independent from the context of the function call. No inconsistency here IMHO :)
[...]
+int +esxVI_GetLong(esxVI_ObjectContent *objectContent, const char *propertyName, + esxVI_Long **value, esxVI_Occurrence occurence) +{ + esxVI_DynamicProperty *dynamicProperty; + + if (value == NULL || *value != NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + + for (dynamicProperty = objectContent->propSet; dynamicProperty != NULL; + dynamicProperty = dynamicProperty->_next) { + if (STREQ(dynamicProperty->name, propertyName)) { + if (esxVI_Long_CastFromAnyType(dynamicProperty->val, value) < 0) {
Hum is esxVI_Long_CastFromAnyType raising an error if failing, because we don't rais an error in the caller if esxVI_GetLong() fails.
Yes, basically all functions in the ESX driver raise errors themselves. esxVI_Long_CastFromAnyType is no exception here, it's just no visible in the patch :) The only exceptions are esxUtil_UnescapeHex and esxUtil_ParseVirtualMachineIDString. esxUtil_UnescapeHex doesn't do it because the caller can report a better error message (with more context) and esxUtil_ParseVirtualMachineIDString has no reason to do so... and I'll change that. Thanks for asking :)
ACK,
Daniel
Thanks, pushed. Matthias