Re: [libvirt] [PATCH] esx: Add support for memtune/min_guarantee
2010/11/9 Daniel Veillard <veillard(a)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