[libvirt] [PATCH 00/12] esx: Generator improvments and general cleanups

As we're currently in feature freeze, this series is meant for 0.9.2. It includes mostly generator improvments and general cleanups. 10/12 is a race condition fix, but it's not critical because it's not that simple to trigger. Matthias

Accept all types on deserialization in order to accept all Event subtypes. This will be used for the upcoming domain event support. --- src/esx/esx_vi_generator.input | 21 ++++---- src/esx/esx_vi_generator.py | 1 - src/esx/esx_vi_types.c | 100 ++++++++++++++++++++++++++++++++++++++- src/esx/esx_vi_types.h | 34 ++++++++++++++ 4 files changed, 142 insertions(+), 14 deletions(-) diff --git a/src/esx/esx_vi_generator.input b/src/esx/esx_vi_generator.input index 98b5206..4ec9f93 100644 --- a/src/esx/esx_vi_generator.input +++ b/src/esx/esx_vi_generator.input @@ -209,16 +209,12 @@ object ElementDescription extends Description end -object Event - Int key r - Int chainId r - DateTime createdTime r - String userName r - DatacenterEventArgument datacenter i - ComputeResourceEventArgument computeResource i - HostEventArgument host i - VmEventArgument vm i - String fullFormattedMessage o +object EntityEventArgument extends EventArgument + String name r +end + + +object EventArgument end @@ -709,6 +705,11 @@ object VmDiskFileQueryFlags end +object VmEventArgument extends EntityEventArgument + ManagedObjectReference vm r +end + + object VmLogFileInfo extends FileInfo end diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index ab127a3..ac15c7f 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -1430,7 +1430,6 @@ additional_object_features = { "AutoStartDefaults" : Object.FEATURE__AN "AutoStartPowerInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__LIST, "DatastoreHostMount" : Object.FEATURE__DEEP_COPY | Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE, "DatastoreInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__DYNAMIC_CAST, - "Event" : Object.FEATURE__LIST, "FileInfo" : Object.FEATURE__DYNAMIC_CAST, "FileQuery" : Object.FEATURE__DYNAMIC_CAST, "HostConfigManager" : Object.FEATURE__ANY_TYPE, diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c index ecaecc4..3c81021 100644 --- a/src/esx/esx_vi_types.c +++ b/src/esx/esx_vi_types.c @@ -271,13 +271,14 @@ -#define ESX_VI__TEMPLATE__DESERIALIZE_EXTRA(_type, _extra, _deserialize) \ +#define ESX_VI__TEMPLATE__DESERIALIZE_EXTRA(_type, _extra1, _extra2, \ + _deserialize) \ int \ esxVI_##_type##_Deserialize(xmlNodePtr node, esxVI_##_type **ptrptr) \ { \ xmlNodePtr childNode = NULL; \ \ - _extra \ + _extra1 \ \ if (ptrptr == NULL || *ptrptr != NULL) { \ ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", \ @@ -289,6 +290,8 @@ return -1; \ } \ \ + _extra2 \ + \ for (childNode = node->children; childNode != NULL; \ childNode = childNode->next) { \ if (childNode->type != XML_ELEMENT_NODE) { \ @@ -317,7 +320,8 @@ #define ESX_VI__TEMPLATE__DESERIALIZE(_type, _deserialize) \ - ESX_VI__TEMPLATE__DESERIALIZE_EXTRA(_type, /* nothing */, _deserialize) + ESX_VI__TEMPLATE__DESERIALIZE_EXTRA(_type, /* nothing */, /* nothing */, \ + _deserialize) @@ -649,6 +653,7 @@ __FUNCTION__, esxVI_Type_ToString(type)); \ return -1; \ }, \ + /* nothing */, \ _deserialize) @@ -775,6 +780,9 @@ esxVI_Type_ToString(esxVI_Type type) case esxVI_Type_ManagedObjectReference: return "ManagedObjectReference"; + case esxVI_Type_Event: + return "Event"; + #include "esx_vi_types.generated.typetostring" case esxVI_Type_Other: @@ -807,6 +815,8 @@ esxVI_Type_FromString(const char *type) return esxVI_Type_MethodFault; } else if (STREQ(type, "ManagedObjectReference")) { return esxVI_Type_ManagedObjectReference; + } else if (STREQ(type, "Event")) { + return esxVI_Type_Event; } #include "esx_vi_types.generated.typefromstring" @@ -1666,6 +1676,90 @@ esxVI_ManagedObjectReference_Deserialize +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * VI Type: Event + */ + +/* esxVI_Event_Alloc */ +ESX_VI__TEMPLATE__ALLOC(Event) + +/* esxVI_Event_Free */ +ESX_VI__TEMPLATE__FREE(Event, +{ + esxVI_Event_Free(&item->_next); + VIR_FREE(item->_actualType); + + esxVI_Int_Free(&item->key); + esxVI_Int_Free(&item->chainId); + esxVI_DateTime_Free(&item->createdTime); + VIR_FREE(item->userName); + /* FIXME: datacenter is currently ignored */ + /* FIXME: computeResource is currently ignored */ + /* FIXME: host is currently ignored */ + esxVI_VmEventArgument_Free(&item->vm); + VIR_FREE(item->fullFormattedMessage); +}) + +/* esxVI_Event_Validate */ +ESX_VI__TEMPLATE__VALIDATE(Event, +{ + ESX_VI__TEMPLATE__PROPERTY__REQUIRE(key) + ESX_VI__TEMPLATE__PROPERTY__REQUIRE(chainId) + ESX_VI__TEMPLATE__PROPERTY__REQUIRE(createdTime) + ESX_VI__TEMPLATE__PROPERTY__REQUIRE(userName) + /* FIXME: datacenter is currently ignored */ + /* FIXME: computeResource is currently ignored */ + /* FIXME: host is currently ignored */ +}) + +/* esxVI_Event_AppendToList */ +ESX_VI__TEMPLATE__LIST__APPEND(Event) + +/* esxVI_Event_CastFromAnyType */ +ESX_VI__TEMPLATE__DYNAMIC_CAST_FROM_ANY_TYPE(Event, +{ + case esxVI_Type_Other: + /* Just accept everything here */ + break; +}) + +/* esxVI_Event_CastListFromAnyType */ +ESX_VI__TEMPLATE__LIST__CAST_FROM_ANY_TYPE(Event) + +/* esxVI_Event_Deserialize */ +ESX_VI__TEMPLATE__DESERIALIZE_EXTRA(Event, /* nothing */, +{ + (*ptrptr)->_actualType = + (char *)xmlGetNsProp(node, BAD_CAST "type", + BAD_CAST "http://www.w3.org/2001/XMLSchema-instance"); + + if ((*ptrptr)->_actualType == NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, + _("%s is missing 'type' property"), + esxVI_Type_ToString((*ptrptr)->_type)); + goto failure; + } +}, +{ + ESX_VI__TEMPLATE__PROPERTY__DESERIALIZE(Int, key) + ESX_VI__TEMPLATE__PROPERTY__DESERIALIZE(Int, chainId) + ESX_VI__TEMPLATE__PROPERTY__DESERIALIZE(DateTime, createdTime) + ESX_VI__TEMPLATE__PROPERTY__DESERIALIZE_VALUE(String, userName) + ESX_VI__TEMPLATE__PROPERTY__DESERIALIZE_IGNORE(datacenter) /* FIXME */ + ESX_VI__TEMPLATE__PROPERTY__DESERIALIZE_IGNORE(computeResource) /* FIXME */ + ESX_VI__TEMPLATE__PROPERTY__DESERIALIZE_IGNORE(host) /* FIXME */ + ESX_VI__TEMPLATE__PROPERTY__DESERIALIZE(VmEventArgument, vm) + ESX_VI__TEMPLATE__PROPERTY__DESERIALIZE_VALUE(String, fullFormattedMessage) + + /* Don't warn about unexpected properties */ + continue; +}) + +/* esxVI_Event_DeserializeList */ +ESX_VI__TEMPLATE__LIST__DESERIALIZE(Event) + + + #include "esx_vi_types.generated.c" diff --git a/src/esx/esx_vi_types.h b/src/esx/esx_vi_types.h index ac3741f..d141a38 100644 --- a/src/esx/esx_vi_types.h +++ b/src/esx/esx_vi_types.h @@ -55,8 +55,10 @@ typedef struct _esxVI_Fault esxVI_Fault; /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * VI Objects */ + typedef struct _esxVI_MethodFault esxVI_MethodFault; typedef struct _esxVI_ManagedObjectReference esxVI_ManagedObjectReference; +typedef struct _esxVI_Event esxVI_Event; # include "esx_vi_types.generated.typedef" @@ -78,6 +80,7 @@ enum _esxVI_Type { esxVI_Type_Fault, esxVI_Type_MethodFault, esxVI_Type_ManagedObjectReference, + esxVI_Type_Event, # include "esx_vi_types.generated.typeenum" @@ -345,6 +348,37 @@ int esxVI_ManagedObjectReference_Deserialize +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * VI Type: Event + */ + +struct _esxVI_Event { + esxVI_Event *_next; /* optional */ + esxVI_Type _type; /* required */ + char *_actualType; /* required */ + + esxVI_Int *key; /* required */ + esxVI_Int *chainId; /* required */ + esxVI_DateTime *createdTime; /* required */ + char *userName; /* required */ + /* FIXME: datacenter is currently ignored */ + /* FIXME: computeResource is currently ignored */ + /* FIXME: host is currently ignored */ + esxVI_VmEventArgument *vm; /* optional */ + char *fullFormattedMessage; /* optional */ +}; + +int esxVI_Event_Alloc(esxVI_Event **item); +void esxVI_Event_Free(esxVI_Event **item); +int esxVI_Event_Validate(esxVI_Event *item); +int esxVI_Event_AppendToList(esxVI_Event **list, esxVI_Event *item); +int esxVI_Event_CastFromAnyType(esxVI_AnyType *anyType, esxVI_Event **item); +int esxVI_Event_CastListFromAnyType(esxVI_AnyType *anyType, esxVI_Event **list); +int esxVI_Event_Deserialize(xmlNodePtr node, esxVI_Event **item); +int esxVI_Event_DeserializeList(xmlNodePtr node, esxVI_Event **list); + + + # include "esx_vi_types.generated.h" -- 1.7.0.4

On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Accept all types on deserialization in order to accept all Event subtypes.
This will be used for the upcoming domain event support. --- src/esx/esx_vi_generator.input | 21 ++++---- src/esx/esx_vi_generator.py | 1 - src/esx/esx_vi_types.c | 100 ++++++++++++++++++++++++++++++++++++++- src/esx/esx_vi_types.h | 34 ++++++++++++++ 4 files changed, 142 insertions(+), 14 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2011/5/13 Eric Blake <eblake@redhat.com>:
On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Accept all types on deserialization in order to accept all Event subtypes.
This will be used for the upcoming domain event support. --- src/esx/esx_vi_generator.input | 21 ++++---- src/esx/esx_vi_generator.py | 1 - src/esx/esx_vi_types.c | 100 ++++++++++++++++++++++++++++++++++++++- src/esx/esx_vi_types.h | 34 ++++++++++++++ 4 files changed, 142 insertions(+), 14 deletions(-)
ACK.
Thanks, pushed. Matthias

--- src/Makefile.am | 1 + src/esx/esx_vi_generator.py | 48 ++++++++++++++++++++++++------------------ src/esx/esx_vi_methods.c | 40 +++-------------------------------- 3 files changed, 32 insertions(+), 57 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 1eaa7d1..4fadc47 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -327,6 +327,7 @@ ESX_DRIVER_SOURCES = \ ESX_DRIVER_GENERATED = \ esx/esx_vi_methods.generated.c \ esx/esx_vi_methods.generated.h \ + esx/esx_vi_methods.generated.macro \ esx/esx_vi_types.generated.c \ esx/esx_vi_types.generated.h \ esx/esx_vi_types.generated.typedef \ diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index ac15c7f..0c1c9e0 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -26,7 +26,6 @@ import os.path - OCCURRENCE__REQUIRED_ITEM = "r" OCCURRENCE__REQUIRED_LIST = "rl" OCCURRENCE__OPTIONAL_ITEM = "o" @@ -39,10 +38,19 @@ valid_occurrences = [OCCURRENCE__REQUIRED_ITEM, OCCURRENCE__OPTIONAL_LIST, OCCURRENCE__IGNORED] +autobind_map = { "FileManager" : "fileManager", + "PerformanceManager" : "perfManager", + "PropertyCollector" : "propertyCollector", + "SearchIndex" : "searchIndex", + "SessionManager" : "sessionManager", + "VirtualDiskManager" : "virtualDiskManager" } + +autobind_map_usage = set() + -def aligned(left, right): - while len(left) < 59: +def aligned(left, right, length=59): + while len(left) < length: left += " " return left + right @@ -50,13 +58,6 @@ def aligned(left, right): class Parameter: - autobind_map = { "FileManager" : "fileManager", - "PerformanceManager" : "perfManager", - "PropertyCollector" : "propertyCollector", - "SearchIndex" : "searchIndex", - "SessionManager" : "sessionManager", - "VirtualDiskManager" : "virtualDiskManager" } - def __init__(self, type, name, occurrence): self.type = type self.occurrence = occurrence @@ -212,7 +213,8 @@ class Method: source += "ESX_VI__METHOD(%s," % self.name if self.autobind_parameter is not None: - source += " %s,\n" % Parameter.autobind_map[self.autobind_parameter.autobind_type] + autobind_map_usage.add(self.autobind_parameter.autobind_type) + source += " %s,\n" % autobind_map[self.autobind_parameter.autobind_type] else: source += " /* explicit _this */,\n" @@ -1482,6 +1484,7 @@ types_header = open_and_print(os.path.join(output_dirname, "esx_vi_types.generat types_source = open_and_print(os.path.join(output_dirname, "esx_vi_types.generated.c")) methods_header = open_and_print(os.path.join(output_dirname, "esx_vi_methods.generated.h")) methods_source = open_and_print(os.path.join(output_dirname, "esx_vi_methods.generated.c")) +methods_macro = open_and_print(os.path.join(output_dirname, "esx_vi_methods.generated.macro")) helpers_header = open_and_print(os.path.join(output_dirname, "esx_vi.generated.h")) helpers_source = open_and_print(os.path.join(output_dirname, "esx_vi.generated.c")) @@ -1586,8 +1589,6 @@ for obj in objects_by_name.values(): - - for obj in managed_objects_by_name.values(): for property in obj.properties: if property.occurrence != OCCURRENCE__IGNORED and \ @@ -1610,15 +1611,11 @@ for obj in managed_objects_by_name.values(): - - for obj in objects_by_name.values(): inherit_features(obj) - - types_typedef.write("/* Generated by esx_vi_generator.py */\n\n\n\n") types_typeenum.write("/* Generated by esx_vi_generator.py */\n\n") types_typetostring.write("/* Generated by esx_vi_generator.py */\n\n") @@ -1627,10 +1624,12 @@ types_header.write("/* Generated by esx_vi_generator.py */\n\n\n\n") types_source.write("/* Generated by esx_vi_generator.py */\n\n\n\n") methods_header.write("/* Generated by esx_vi_generator.py */\n\n\n\n") methods_source.write("/* Generated by esx_vi_generator.py */\n\n\n\n") +methods_macro.write("/* Generated by esx_vi_generator.py */\n\n\n\n") helpers_header.write("/* Generated by esx_vi_generator.py */\n\n\n\n") helpers_source.write("/* Generated by esx_vi_generator.py */\n\n\n\n") + # output enums types_typedef.write("/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *\n" + " * VI Enums\n" + @@ -1658,8 +1657,6 @@ types_typeenum.write("\n") types_typetostring.write("\n") types_typefromstring.write("\n") - - names = objects_by_name.keys() names.sort() @@ -1682,8 +1679,6 @@ types_typeenum.write("\n") types_typetostring.write("\n") types_typefromstring.write("\n") - - names = managed_objects_by_name.keys() names.sort() @@ -1705,6 +1700,17 @@ for name in names: methods_header.write(methods_by_name[name].generate_header()) methods_source.write(methods_by_name[name].generate_source()) +sorted_usage = list(autobind_map_usage) +sorted_usage.sort() + +for usage in sorted_usage: + name = autobind_map[usage] + string = aligned("#define ESX_VI__METHOD__PARAMETER__THIS__%s " % name, "\\\n", 78) + string += " ESX_VI__METHOD__PARAMETER__THIS_FROM_SERVICE(ManagedObjectReference, \\\n" + string += aligned("", "%s)\n\n\n\n" % name, 49) + + methods_macro.write(string) + # output helpers diff --git a/src/esx/esx_vi_methods.c b/src/esx/esx_vi_methods.c index 5967088..1f1780f 100644 --- a/src/esx/esx_vi_methods.c +++ b/src/esx/esx_vi_methods.c @@ -175,42 +175,6 @@ -#define ESX_VI__METHOD__PARAMETER__THIS__fileManager \ - ESX_VI__METHOD__PARAMETER__THIS_FROM_SERVICE(ManagedObjectReference, \ - fileManager) - - - -#define ESX_VI__METHOD__PARAMETER__THIS__perfManager \ - ESX_VI__METHOD__PARAMETER__THIS_FROM_SERVICE(ManagedObjectReference, \ - perfManager) - - - -#define ESX_VI__METHOD__PARAMETER__THIS__propertyCollector \ - ESX_VI__METHOD__PARAMETER__THIS_FROM_SERVICE(ManagedObjectReference, \ - propertyCollector) - - - -#define ESX_VI__METHOD__PARAMETER__THIS__searchIndex \ - ESX_VI__METHOD__PARAMETER__THIS_FROM_SERVICE(ManagedObjectReference, \ - searchIndex) - - - -#define ESX_VI__METHOD__PARAMETER__THIS__sessionManager \ - ESX_VI__METHOD__PARAMETER__THIS_FROM_SERVICE(ManagedObjectReference, \ - sessionManager) - - - -#define ESX_VI__METHOD__PARAMETER__THIS__virtualDiskManager \ - ESX_VI__METHOD__PARAMETER__THIS_FROM_SERVICE(ManagedObjectReference, \ - virtualDiskManager) - - - /* * A required parameter must be != 0 (NULL for pointers, "undefined" == 0 for * enumeration values). @@ -248,6 +212,10 @@ +#include "esx_vi_methods.generated.macro" + + + /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * VI Methods */ -- 1.7.0.4

On 05/01/2011 01:57 PM, Matthias Bolte wrote:
--- src/Makefile.am | 1 + src/esx/esx_vi_generator.py | 48 ++++++++++++++++++++++++------------------ src/esx/esx_vi_methods.c | 40 +++-------------------------------- 3 files changed, 32 insertions(+), 57 deletions(-)
I'm not sure I completely understand what this patch was trying to do, but it compiles fine, and seems like it is moving code from special case over to generated, which hopefully makes future extensions (aka the rest of your series) use less effort. Maybe a commit comment will help someone reading 'git log' down the road. ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2011/5/13 Eric Blake <eblake@redhat.com>:
On 05/01/2011 01:57 PM, Matthias Bolte wrote:
--- src/Makefile.am | 1 + src/esx/esx_vi_generator.py | 48 ++++++++++++++++++++++++------------------ src/esx/esx_vi_methods.c | 40 +++-------------------------------- 3 files changed, 32 insertions(+), 57 deletions(-)
I'm not sure I completely understand what this patch was trying to do, but it compiles fine, and seems like it is moving code from special case over to generated, which hopefully makes future extensions (aka the rest of your series) use less effort.
Maybe a commit comment will help someone reading 'git log' down the road.
True, here's what I've added: Several vSphere API methods are called on global objects like the FileManager, the PerformanceManager or the SearchIndex. The generator input file allows to mark such methods and the generator generates such method in a way that automatically handles marked parameter. This is done by some special macros. Those were manually written and this patch moves them to the generator.
ACK.
Thanks, pushed. Matthias

Don't make all object and enum types (de)serializable by default. Detect this from the input file instead. --- src/esx/esx_vi_generator.py | 167 +++++++++++++++++++++++++++++++----------- 1 files changed, 123 insertions(+), 44 deletions(-) diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 0c1c9e0..8ac1249 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -73,6 +73,14 @@ class Parameter: return self.type in predefined_enums or self.type in enums_by_name + def is_object(self): + return self.type in predefined_objects or self.type in objects_by_name + + + def is_type_generated(self): + return self.type in enums_by_name or self.type in objects_by_name + + def generate_parameter(self, is_last = False, is_header = True, offset = 0): if self.occurrence == OCCURRENCE__IGNORED: raise ValueError("invalid function parameter occurrence value '%s'" % self.occurrence) @@ -275,6 +283,14 @@ class Property: return self.type in predefined_enums or self.type in enums_by_name + def is_object(self): + return self.type in predefined_objects or self.type in objects_by_name + + + def is_type_generated(self): + return self.type in enums_by_name or self.type in objects_by_name + + def generate_struct_member(self): if self.occurrence == OCCURRENCE__IGNORED: return " /* FIXME: %s is currently ignored */\n" % self.name @@ -427,11 +443,10 @@ class Object(Base): FEATURE__SERIALIZE = (1 << 5) FEATURE__DESERIALIZE = (1 << 6) - def __init__(self, name, extends, properties, features = 0, extended_by = None): Base.__init__(self, "struct", name) self.extends = extends - self.features = features | Object.FEATURE__SERIALIZE | Object.FEATURE__DESERIALIZE + self.features = features self.properties = properties self.extended_by = extended_by @@ -1191,7 +1206,7 @@ class Enum(Base): def __init__(self, name, values, features=0): Base.__init__(self, "enum", name) self.values = values - self.features = features | Enum.FEATURE__SERIALIZE | Enum.FEATURE__DESERIALIZE + self.features = features def generate_header(self): @@ -1372,20 +1387,6 @@ def parse_method(block): -def inherit_features(obj): - if obj.extended_by is not None: - for extended_by in obj.extended_by: - objects_by_name[extended_by].features |= obj.features - - if obj.extends is not None: - objects_by_name[obj.extends].features |= obj.features - - if obj.extended_by is not None: - for extended_by in obj.extended_by: - inherit_features(objects_by_name[extended_by]) - - - def is_known_type(type): return type in predefined_objects or \ type in predefined_enums or \ @@ -1422,12 +1423,10 @@ predefined_objects = ["AnyType", "MethodFault", "ManagedObjectReference"] - additional_enum_features = { "ManagedEntityStatus" : Enum.FEATURE__ANY_TYPE, "TaskInfoState" : Enum.FEATURE__ANY_TYPE, "VirtualMachinePowerState" : Enum.FEATURE__ANY_TYPE } - additional_object_features = { "AutoStartDefaults" : Object.FEATURE__ANY_TYPE, "AutoStartPowerInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__LIST, "DatastoreHostMount" : Object.FEATURE__DEEP_COPY | Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE, @@ -1446,24 +1445,16 @@ additional_object_features = { "AutoStartDefaults" : Object.FEATURE__AN "PropertyFilterSpec" : Object.FEATURE__LIST, "ResourcePoolResourceUsage" : Object.FEATURE__ANY_TYPE, "SelectionSpec" : Object.FEATURE__DYNAMIC_CAST, + "ServiceContent" : Object.FEATURE__DESERIALIZE, "SharesInfo" : Object.FEATURE__ANY_TYPE, "TaskInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__LIST, "UserSession" : Object.FEATURE__ANY_TYPE, "VirtualDiskSpec" : Object.FEATURE__DYNAMIC_CAST, "VirtualMachineQuestionInfo" : Object.FEATURE__ANY_TYPE, - "VirtualMachineSnapshotTree" : Object.FEATURE__DEEP_COPY | Object.FEATURE__ANY_TYPE } - + "VirtualMachineSnapshotTree" : Object.FEATURE__DEEP_COPY | Object.FEATURE__ANY_TYPE, + "VmEventArgument" : Object.FEATURE__DESERIALIZE } -removed_object_features = { "DynamicProperty" : Object.FEATURE__SERIALIZE, - "LocalizedMethodFault" : Object.FEATURE__SERIALIZE, - "ObjectContent" : Object.FEATURE__SERIALIZE, - "ObjectUpdate" : Object.FEATURE__SERIALIZE, - "PropertyChange" : Object.FEATURE__SERIALIZE, - "PropertyFilterUpdate" : Object.FEATURE__SERIALIZE, - "TaskInfo" : Object.FEATURE__SERIALIZE, - "UpdateSet" : Object.FEATURE__SERIALIZE, - "VirtualMachineConfigInfo" : Object.FEATURE__SERIALIZE, - "VirtualMachineSnapshotTree" : Object.FEATURE__SERIALIZE } +removed_object_features = {} @@ -1499,6 +1490,7 @@ block = None +# parse input file for line in file(input_filename, "rb").readlines(): number += 1 @@ -1538,11 +1530,34 @@ for line in file(input_filename, "rb").readlines(): +for method in methods_by_name.values(): + # method parameter types must be serializable + for parameter in method.parameters: + if not parameter.is_type_generated(): + continue + + if parameter.is_enum(): + enums_by_name[parameter.type].features |= Enum.FEATURE__SERIALIZE + else: + objects_by_name[parameter.type].features |= Object.FEATURE__SERIALIZE + + # method return types must be deserializable + if method.returns and method.returns.is_type_generated(): + if method.returns.is_enum(): + enums_by_name[method.returns.type].features |= Enum.FEATURE__DESERIALIZE + else: + objects_by_name[method.returns.type].features |= Object.FEATURE__DESERIALIZE + + + for enum in enums_by_name.values(): # apply additional features if enum.name in additional_enum_features: enum.features |= additional_enum_features[enum.name] + if additional_enum_features[enum.name] & Enum.FEATURE__ANY_TYPE: + enum.features |= Enum.FEATURE__DESERIALIZE + for obj in objects_by_name.values(): @@ -1566,17 +1581,12 @@ for obj in objects_by_name.values(): if obj.name in additional_object_features: obj.features |= additional_object_features[obj.name] + if additional_object_features[obj.name] & Object.FEATURE__ANY_TYPE: + obj.features |= Object.FEATURE__DESERIALIZE + if obj.name in removed_object_features: obj.features &= ~removed_object_features[obj.name] - # spread deep copy onto properties - if obj.features & Object.FEATURE__DEEP_COPY: - for property in obj.properties: - if property.occurrence != OCCURRENCE__IGNORED and \ - property.type not in predefined_objects and \ - property.type in objects_by_name: - objects_by_name[property.type].features |= Object.FEATURE__DEEP_COPY - # detect extended_by relation if obj.extends is not None: extended_obj = objects_by_name[obj.extends] @@ -1589,6 +1599,80 @@ for obj in objects_by_name.values(): +def propagate_feature(obj, feature): + global features_have_changed + + if not (obj.features & feature): + return + + for property in obj.properties: + if property.occurrence == OCCURRENCE__IGNORED or \ + not property.is_type_generated(): + continue + + if property.is_enum(): + if feature == Object.FEATURE__SERIALIZE and \ + not (enums_by_name[property.type].features & Enum.FEATURE__SERIALIZE): + enums_by_name[property.type].features |= Enum.FEATURE__SERIALIZE + features_have_changed = True + elif feature == Object.FEATURE__DESERIALIZE and \ + not (enums_by_name[property.type].features & Enum.FEATURE__DESERIALIZE): + enums_by_name[property.type].features |= Enum.FEATURE__DESERIALIZE + features_have_changed = True + elif property.is_object(): + if not (objects_by_name[property.type].features & feature): + objects_by_name[property.type].features |= feature + features_have_changed = True + + if obj.name != property.type: + propagate_feature(objects_by_name[property.type], feature) + + + +def inherit_features(obj): + global features_have_changed + + if obj.extended_by is not None: + for extended_by in obj.extended_by: + previous = objects_by_name[extended_by].features + objects_by_name[extended_by].features |= obj.features + + if objects_by_name[extended_by].features != previous: + features_have_changed = True + + if obj.extends is not None: + previous = objects_by_name[obj.extends].features + objects_by_name[obj.extends].features |= obj.features + + if objects_by_name[obj.extends].features != previous: + features_have_changed = True + + if obj.extended_by is not None: + for extended_by in obj.extended_by: + inherit_features(objects_by_name[extended_by]) + + + +# there are two directions to spread features: +# 1) up and down the inheritance chain +# 2) from object types to their member property types +# spreading needs to be done alternating on both directions because they can +# affect each other +features_have_changed = True + +while features_have_changed: + features_have_changed = False + + for obj in objects_by_name.values(): + propagate_feature(obj, Object.FEATURE__DEEP_COPY) + propagate_feature(obj, Object.FEATURE__SERIALIZE) + propagate_feature(obj, Object.FEATURE__DESERIALIZE) + + for obj in objects_by_name.values(): + inherit_features(obj) + + + for obj in managed_objects_by_name.values(): for property in obj.properties: if property.occurrence != OCCURRENCE__IGNORED and \ @@ -1611,11 +1695,6 @@ for obj in managed_objects_by_name.values(): -for obj in objects_by_name.values(): - inherit_features(obj) - - - types_typedef.write("/* Generated by esx_vi_generator.py */\n\n\n\n") types_typeenum.write("/* Generated by esx_vi_generator.py */\n\n") types_typetostring.write("/* Generated by esx_vi_generator.py */\n\n") -- 1.7.0.4

On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Don't make all object and enum types (de)serializable by default. Detect this from the input file instead. --- src/esx/esx_vi_generator.py | 167 +++++++++++++++++++++++++++++++----------- 1 files changed, 123 insertions(+), 44 deletions(-)
Too bad generated files don't show up in the diffstat :)
+# there are two directions to spread features: +# 1) up and down the inheritance chain +# 2) from object types to their member property types +# spreading needs to be done alternating on both directions because they can +# affect each other +features_have_changed = True + +while features_have_changed: + features_have_changed = False + + for obj in objects_by_name.values(): + propagate_feature(obj, Object.FEATURE__DEEP_COPY) + propagate_feature(obj, Object.FEATURE__SERIALIZE) + propagate_feature(obj, Object.FEATURE__DESERIALIZE) + + for obj in objects_by_name.values(): + inherit_features(obj)
If I'm reading this correctly, this looks like the core of the algorithm - compute a closure of all features, and then only generate for where the features were needed, rather than the old code that generated features for everything whether it was needed or not. My python is not very strong, but what you did was fairly readable and seems to make sense. At any rate, it compiles. ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2011/5/13 Eric Blake <eblake@redhat.com>:
On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Don't make all object and enum types (de)serializable by default. Detect this from the input file instead. --- src/esx/esx_vi_generator.py | 167 +++++++++++++++++++++++++++++++----------- 1 files changed, 123 insertions(+), 44 deletions(-)
Too bad generated files don't show up in the diffstat :)
+# there are two directions to spread features: +# 1) up and down the inheritance chain +# 2) from object types to their member property types +# spreading needs to be done alternating on both directions because they can +# affect each other +features_have_changed = True + +while features_have_changed: + features_have_changed = False + + for obj in objects_by_name.values(): + propagate_feature(obj, Object.FEATURE__DEEP_COPY) + propagate_feature(obj, Object.FEATURE__SERIALIZE) + propagate_feature(obj, Object.FEATURE__DESERIALIZE) + + for obj in objects_by_name.values(): + inherit_features(obj)
If I'm reading this correctly, this looks like the core of the algorithm - compute a closure of all features, and then only generate for where the features were needed, rather than the old code that generated features for everything whether it was needed or not.
Yes, that's correct :)
My python is not very strong, but what you did was fairly readable and seems to make sense. At any rate, it compiles.
ACK.
Thanks, pushed. Matthias

Detect it based on usage as parameter and return type too. --- src/esx/esx_vi_generator.py | 41 +++++++++++++++++++++++++++++++---------- 1 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 8ac1249..70cf2ee 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -1428,7 +1428,7 @@ additional_enum_features = { "ManagedEntityStatus" : Enum.FEATURE__ANY_TYPE "VirtualMachinePowerState" : Enum.FEATURE__ANY_TYPE } additional_object_features = { "AutoStartDefaults" : Object.FEATURE__ANY_TYPE, - "AutoStartPowerInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__LIST, + "AutoStartPowerInfo" : Object.FEATURE__ANY_TYPE, "DatastoreHostMount" : Object.FEATURE__DEEP_COPY | Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE, "DatastoreInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__DYNAMIC_CAST, "FileInfo" : Object.FEATURE__DYNAMIC_CAST, @@ -1437,12 +1437,9 @@ additional_object_features = { "AutoStartDefaults" : Object.FEATURE__AN "HostCpuIdInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__LIST, "HostDatastoreBrowserSearchResults" : Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE, "ManagedObjectReference" : Object.FEATURE__ANY_TYPE, - "ObjectContent" : Object.FEATURE__DEEP_COPY | Object.FEATURE__LIST, - "PerfCounterInfo" : Object.FEATURE__LIST, - "PerfEntityMetric" : Object.FEATURE__LIST | Object.FEATURE__DYNAMIC_CAST, - "PerfQuerySpec" : Object.FEATURE__LIST, + "ObjectContent" : Object.FEATURE__DEEP_COPY, + "PerfEntityMetric" : Object.FEATURE__DYNAMIC_CAST, "PerfMetricIntSeries" : Object.FEATURE__DYNAMIC_CAST, - "PropertyFilterSpec" : Object.FEATURE__LIST, "ResourcePoolResourceUsage" : Object.FEATURE__ANY_TYPE, "SelectionSpec" : Object.FEATURE__DYNAMIC_CAST, "ServiceContent" : Object.FEATURE__DESERIALIZE, @@ -1541,6 +1538,15 @@ for method in methods_by_name.values(): else: objects_by_name[parameter.type].features |= Object.FEATURE__SERIALIZE + # detect list usage + if parameter.occurrence == OCCURRENCE__REQUIRED_LIST or \ + parameter.occurrence == OCCURRENCE__OPTIONAL_LIST: + if parameter.is_enum(): + report_error("unsupported usage of enum '%s' as list in '%s'" + % (parameter.type, method.name)) + else: + objects_by_name[parameter.type].features |= Object.FEATURE__LIST + # method return types must be deserializable if method.returns and method.returns.is_type_generated(): if method.returns.is_enum(): @@ -1548,6 +1554,15 @@ for method in methods_by_name.values(): else: objects_by_name[method.returns.type].features |= Object.FEATURE__DESERIALIZE + # detect list usage + if method.returns.occurrence == OCCURRENCE__REQUIRED_LIST or \ + method.returns.occurrence == OCCURRENCE__OPTIONAL_LIST: + if method.returns.is_enum(): + report_error("unsupported usage of enum '%s' as list in '%s'" + % (method.returns.type, method.name)) + else: + objects_by_name[method.returns.type].features |= Object.FEATURE__LIST + for enum in enums_by_name.values(): @@ -1572,10 +1587,16 @@ for obj in objects_by_name.values(): # detect list usage for property in obj.properties: - if (property.occurrence == OCCURRENCE__REQUIRED_LIST or \ - property.occurrence == OCCURRENCE__OPTIONAL_LIST) and \ - property.type not in predefined_objects: - objects_by_name[property.type].features |= Object.FEATURE__LIST + if not property.is_type_generated(): + continue + + if property.occurrence == OCCURRENCE__REQUIRED_LIST or \ + property.occurrence == OCCURRENCE__OPTIONAL_LIST: + if property.is_enum(): + report_error("unsupported usage of enum '%s' as list in '%s'" + % (property.type, obj.type)) + else: + objects_by_name[property.type].features |= Object.FEATURE__LIST # apply/remove additional features if obj.name in additional_object_features: -- 1.7.0.4

On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Detect it based on usage as parameter and return type too. --- src/esx/esx_vi_generator.py | 41 +++++++++++++++++++++++++++++++---------- 1 files changed, 31 insertions(+), 10 deletions(-)
Again, my python is not strong, but this compiles and makes sense based on the comments. ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2011/5/13 Eric Blake <eblake@redhat.com>:
On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Detect it based on usage as parameter and return type too. --- src/esx/esx_vi_generator.py | 41 +++++++++++++++++++++++++++++++---------- 1 files changed, 31 insertions(+), 10 deletions(-)
Again, my python is not strong, but this compiles and makes sense based on the comments.
ACK.
Thanks, pushed. Matthias

Detect it based on usage as parameter, return type and member of other object types. --- src/esx/esx_vi_generator.py | 32 ++++++++++++++++++-------------- 1 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 70cf2ee..0d1c3bd 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -449,6 +449,7 @@ class Object(Base): self.features = features self.properties = properties self.extended_by = extended_by + self.candidate_for_dynamic_cast = False if self.extended_by is not None: self.extended_by.sort() @@ -1406,13 +1407,6 @@ def open_and_print(filename): - - - - - - - predefined_enums = ["Boolean"] predefined_objects = ["AnyType", @@ -1431,22 +1425,16 @@ additional_object_features = { "AutoStartDefaults" : Object.FEATURE__AN "AutoStartPowerInfo" : Object.FEATURE__ANY_TYPE, "DatastoreHostMount" : Object.FEATURE__DEEP_COPY | Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE, "DatastoreInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__DYNAMIC_CAST, - "FileInfo" : Object.FEATURE__DYNAMIC_CAST, - "FileQuery" : Object.FEATURE__DYNAMIC_CAST, "HostConfigManager" : Object.FEATURE__ANY_TYPE, "HostCpuIdInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__LIST, "HostDatastoreBrowserSearchResults" : Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE, "ManagedObjectReference" : Object.FEATURE__ANY_TYPE, "ObjectContent" : Object.FEATURE__DEEP_COPY, - "PerfEntityMetric" : Object.FEATURE__DYNAMIC_CAST, - "PerfMetricIntSeries" : Object.FEATURE__DYNAMIC_CAST, "ResourcePoolResourceUsage" : Object.FEATURE__ANY_TYPE, - "SelectionSpec" : Object.FEATURE__DYNAMIC_CAST, "ServiceContent" : Object.FEATURE__DESERIALIZE, "SharesInfo" : Object.FEATURE__ANY_TYPE, "TaskInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__LIST, "UserSession" : Object.FEATURE__ANY_TYPE, - "VirtualDiskSpec" : Object.FEATURE__DYNAMIC_CAST, "VirtualMachineQuestionInfo" : Object.FEATURE__ANY_TYPE, "VirtualMachineSnapshotTree" : Object.FEATURE__DEEP_COPY | Object.FEATURE__ANY_TYPE, "VmEventArgument" : Object.FEATURE__DESERIALIZE } @@ -1537,6 +1525,7 @@ for method in methods_by_name.values(): enums_by_name[parameter.type].features |= Enum.FEATURE__SERIALIZE else: objects_by_name[parameter.type].features |= Object.FEATURE__SERIALIZE + objects_by_name[parameter.type].candidate_for_dynamic_cast = True # detect list usage if parameter.occurrence == OCCURRENCE__REQUIRED_LIST or \ @@ -1553,6 +1542,7 @@ for method in methods_by_name.values(): enums_by_name[method.returns.type].features |= Enum.FEATURE__DESERIALIZE else: objects_by_name[method.returns.type].features |= Object.FEATURE__DESERIALIZE + objects_by_name[method.returns.type].candidate_for_dynamic_cast = True # detect list usage if method.returns.occurrence == OCCURRENCE__REQUIRED_LIST or \ @@ -1585,11 +1575,16 @@ for obj in objects_by_name.values(): if not is_known_type(obj.extends): report_error("object '%s' extends unknown object '%s'" % (obj.name, obj.extends)) - # detect list usage for property in obj.properties: if not property.is_type_generated(): continue + if property.is_enum(): + enums_by_name[property.type].candidate_for_dynamic_cast = True + else: + objects_by_name[property.type].candidate_for_dynamic_cast = True + + # detect list usage if property.occurrence == OCCURRENCE__REQUIRED_LIST or \ property.occurrence == OCCURRENCE__OPTIONAL_LIST: if property.is_enum(): @@ -1620,6 +1615,15 @@ for obj in objects_by_name.values(): +for obj in objects_by_name.values(): + # if an object is a candidate (it is used directly as parameter or return + # type or is a member of another object) and it is extended by another + # object then this type needs the dynamic cast feature + if obj.candidate_for_dynamic_cast and obj.extended_by: + obj.features |= Object.FEATURE__DYNAMIC_CAST + + + def propagate_feature(obj, feature): global features_have_changed -- 1.7.0.4

On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Detect it based on usage as parameter, return type and member of other object types. --- src/esx/esx_vi_generator.py | 32 ++++++++++++++++++-------------- 1 files changed, 18 insertions(+), 14 deletions(-)
Very much like the last one. ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2011/5/13 Eric Blake <eblake@redhat.com>:
On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Detect it based on usage as parameter, return type and member of other object types. --- src/esx/esx_vi_generator.py | 32 ++++++++++++++++++-------------- 1 files changed, 18 insertions(+), 14 deletions(-)
Very much like the last one.
ACK.
Thanks, pushed. Matthias

Move common code from Property and Paramater into new Member class. Rename the other base class to Type. --- src/esx/esx_vi_generator.py | 95 +++++++++++++++++-------------------------- 1 files changed, 38 insertions(+), 57 deletions(-) diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 0d1c3bd..753ec0b 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -57,17 +57,11 @@ def aligned(left, right, length=59): -class Parameter: - def __init__(self, type, name, occurrence): +class Member: + def __init__(self, type, occurrence): self.type = type self.occurrence = occurrence - if ':' in name and name.startswith("_this"): - self.name, self.autobind_type = name.split(":") - else: - self.name = name - self.autobind_type = None - def is_enum(self): return self.type in predefined_enums or self.type in enums_by_name @@ -81,6 +75,31 @@ class Parameter: return self.type in enums_by_name or self.type in objects_by_name + def get_occurrence_comment(self): + if self.occurrence == OCCURRENCE__REQUIRED_ITEM: + return "/* required */" + elif self.occurrence == OCCURRENCE__REQUIRED_LIST: + return "/* required, list */" + elif self.occurrence == OCCURRENCE__OPTIONAL_ITEM: + return "/* optional */" + elif self.occurrence == OCCURRENCE__OPTIONAL_LIST: + return "/* optional, list */" + + raise ValueError("unknown occurrence value '%s'" % self.occurrence) + + + +class Parameter(Member): + def __init__(self, type, name, occurrence): + Member.__init__(self, type, occurrence) + + if ':' in name and name.startswith("_this"): + self.name, self.autobind_type = name.split(":") + else: + self.name = name + self.autobind_type = None + + def generate_parameter(self, is_last = False, is_header = True, offset = 0): if self.occurrence == OCCURRENCE__IGNORED: raise ValueError("invalid function parameter occurrence value '%s'" % self.occurrence) @@ -131,7 +150,7 @@ class Parameter: return " ESX_VI__METHOD__PARAMETER__SERIALIZE(%s, %s)\n" % (self.type, self.name) - def get_type_string(self, as_return_value = False): + def get_type_string(self, as_return_value=False): string = "" if self.type == "String" and \ @@ -152,19 +171,6 @@ class Parameter: return string - def get_occurrence_comment(self): - if self.occurrence == OCCURRENCE__REQUIRED_ITEM: - return "/* required */" - elif self.occurrence == OCCURRENCE__REQUIRED_LIST: - return "/* required, list */" - elif self.occurrence == OCCURRENCE__OPTIONAL_ITEM: - return "/* optional */" - elif self.occurrence == OCCURRENCE__OPTIONAL_LIST: - return "/* optional, list */" - - raise ValueError("unknown occurrence value '%s'" % self.occurrence) - - def get_occurrence_short_enum(self): if self.occurrence == OCCURRENCE__REQUIRED_ITEM: return "RequiredItem" @@ -272,23 +278,11 @@ class Method: -class Property: +class Property(Member): def __init__(self, type, name, occurrence): - self.type = type - self.name = name - self.occurrence = occurrence - - - def is_enum(self): - return self.type in predefined_enums or self.type in enums_by_name - - - def is_object(self): - return self.type in predefined_objects or self.type in objects_by_name + Member.__init__(self, type, occurrence) - - def is_type_generated(self): - return self.type in enums_by_name or self.type in objects_by_name + self.name = name def generate_struct_member(self): @@ -391,21 +385,8 @@ class Property: return "esxVI_%s *" % self.type - def get_occurrence_comment(self): - if self.occurrence == OCCURRENCE__REQUIRED_ITEM: - return "/* required */" - elif self.occurrence == OCCURRENCE__REQUIRED_LIST: - return "/* required, list */" - elif self.occurrence == OCCURRENCE__OPTIONAL_ITEM: - return "/* optional */" - elif self.occurrence == OCCURRENCE__OPTIONAL_LIST: - return "/* optional, list */" - - raise ValueError("unknown occurrence value '%s'" % self.occurrence) - - -class Base: +class Type: def __init__(self, kind, name): self.kind = kind self.name = name @@ -435,7 +416,7 @@ class Base: -class Object(Base): +class Object(Type): FEATURE__DYNAMIC_CAST = (1 << 1) FEATURE__LIST = (1 << 2) FEATURE__DEEP_COPY = (1 << 3) @@ -444,7 +425,7 @@ class Object(Base): FEATURE__DESERIALIZE = (1 << 6) def __init__(self, name, extends, properties, features = 0, extended_by = None): - Base.__init__(self, "struct", name) + Type.__init__(self, "struct", name) self.extends = extends self.features = features self.properties = properties @@ -898,12 +879,12 @@ class Object(Base): -class ManagedObject(Base): +class ManagedObject(Type): FEATURE__LIST = (1 << 2) def __init__(self, name, extends, properties, features=0, extended_by=None): - Base.__init__(self, "struct", name) + Type.__init__(self, "struct", name) self.extends = extends self.features = features self.properties = properties @@ -1198,14 +1179,14 @@ class ManagedObject(Base): -class Enum(Base): +class Enum(Type): FEATURE__ANY_TYPE = (1 << 1) FEATURE__SERIALIZE = (1 << 2) FEATURE__DESERIALIZE = (1 << 3) def __init__(self, name, values, features=0): - Base.__init__(self, "enum", name) + Type.__init__(self, "enum", name) self.values = values self.features = features -- 1.7.0.4

On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Move common code from Property and Paramater into new Member class.
s/Paramater/Parameter/
Rename the other base class to Type. --- src/esx/esx_vi_generator.py | 95 +++++++++++++++++-------------------------- 1 files changed, 38 insertions(+), 57 deletions(-)
ACK with the commit nit fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2011/5/13 Eric Blake <eblake@redhat.com>:
On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Move common code from Property and Paramater into new Member class.
s/Paramater/Parameter/
That's one of my common typos. It seems impossible for me to get it right in the first attempt :)
Rename the other base class to Type. --- src/esx/esx_vi_generator.py | 95 +++++++++++++++++-------------------------- 1 files changed, 38 insertions(+), 57 deletions(-)
ACK with the commit nit fixed.
Thanks, pushed with fixed typo. Matthias

Instead of specifing the type of the managed object directly specify the ServiceContent member name. This way the mapping dictionary can be removed. --- src/esx/esx_vi_generator.input | 36 ++++++++++++++++++------------------ src/esx/esx_vi_generator.py | 28 ++++++++++------------------ 2 files changed, 28 insertions(+), 36 deletions(-) diff --git a/src/esx/esx_vi_generator.input b/src/esx/esx_vi_generator.input index 4ec9f93..d927e68 100644 --- a/src/esx/esx_vi_generator.input +++ b/src/esx/esx_vi_generator.input @@ -40,9 +40,9 @@ # # The _this parameter can have a type attached to it: # -# _this:<type> +# _this:<member> # -# The <type> refers to one of the ServiceContent members. This make the +# The <member> refers to one of the ServiceContent members. This make the # generator auto-bind _this to the corresponding ServiceContent member. # @@ -782,7 +782,7 @@ end method CopyVirtualDisk_Task returns ManagedObjectReference r - ManagedObjectReference _this:VirtualDiskManager r + ManagedObjectReference _this:virtualDiskManager r String sourceName r ManagedObjectReference sourceDatacenter o String destName r @@ -793,7 +793,7 @@ end method CreateFilter returns ManagedObjectReference r - ManagedObjectReference _this:PropertyCollector r + ManagedObjectReference _this:propertyCollector r PropertyFilterSpec spec r Boolean partialUpdates r end @@ -809,7 +809,7 @@ end method CreateVirtualDisk_Task returns ManagedObjectReference r - ManagedObjectReference _this:VirtualDiskManager r + ManagedObjectReference _this:virtualDiskManager r String name r ManagedObjectReference datacenter o VirtualDiskSpec spec r @@ -817,7 +817,7 @@ end method DeleteVirtualDisk_Task returns ManagedObjectReference r - ManagedObjectReference _this:VirtualDiskManager r + ManagedObjectReference _this:virtualDiskManager r String name r ManagedObjectReference datacenter o end @@ -829,7 +829,7 @@ end method FindByIp returns ManagedObjectReference o - ManagedObjectReference _this:SearchIndex r + ManagedObjectReference _this:searchIndex r ManagedObjectReference datacenter o String ip r Boolean vmSearch r @@ -837,7 +837,7 @@ end method FindByUuid returns ManagedObjectReference o - ManagedObjectReference _this:SearchIndex r + ManagedObjectReference _this:searchIndex r ManagedObjectReference datacenter o String uuid r Boolean vmSearch r @@ -845,7 +845,7 @@ end method Login returns UserSession r - ManagedObjectReference _this:SessionManager r + ManagedObjectReference _this:sessionManager r String userName r String password r String locale o @@ -853,12 +853,12 @@ end method Logout - ManagedObjectReference _this:SessionManager r + ManagedObjectReference _this:sessionManager r end method MakeDirectory - ManagedObjectReference _this:FileManager r + ManagedObjectReference _this:fileManager r String name r ManagedObjectReference datacenter o Boolean createParentDirectories o @@ -886,7 +886,7 @@ end method QueryAvailablePerfMetric returns PerfMetricId ol - ManagedObjectReference _this:PerformanceManager r + ManagedObjectReference _this:perfManager r ManagedObjectReference entity r DateTime beginTime o DateTime endTime o @@ -895,19 +895,19 @@ end method QueryPerf returns PerfEntityMetricBase ol - ManagedObjectReference _this:PerformanceManager r + ManagedObjectReference _this:perfManager r PerfQuerySpec querySpec rl end method QueryPerfCounter returns PerfCounterInfo ol - ManagedObjectReference _this:PerformanceManager r + ManagedObjectReference _this:perfManager r Int counterId rl end method QueryVirtualDiskUuid returns String r - ManagedObjectReference _this:VirtualDiskManager r + ManagedObjectReference _this:virtualDiskManager r String name r ManagedObjectReference datacenter o end @@ -978,7 +978,7 @@ end method SessionIsActive returns Boolean r - ManagedObjectReference _this:SessionManager r + ManagedObjectReference _this:sessionManager r String sessionID r String userName r end @@ -1000,13 +1000,13 @@ end method WaitForUpdates returns UpdateSet r - ManagedObjectReference _this:PropertyCollector r + ManagedObjectReference _this:propertyCollector r String version o end method ZeroFillVirtualDisk_Task returns ManagedObjectReference r - ManagedObjectReference _this:VirtualDiskManager r + ManagedObjectReference _this:virtualDiskManager r String name r ManagedObjectReference datacenter o end diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 753ec0b..2fca929 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -38,14 +38,7 @@ valid_occurrences = [OCCURRENCE__REQUIRED_ITEM, OCCURRENCE__OPTIONAL_LIST, OCCURRENCE__IGNORED] -autobind_map = { "FileManager" : "fileManager", - "PerformanceManager" : "perfManager", - "PropertyCollector" : "propertyCollector", - "SearchIndex" : "searchIndex", - "SessionManager" : "sessionManager", - "VirtualDiskManager" : "virtualDiskManager" } - -autobind_map_usage = set() +autobind_names = set() @@ -94,16 +87,16 @@ class Parameter(Member): Member.__init__(self, type, occurrence) if ':' in name and name.startswith("_this"): - self.name, self.autobind_type = name.split(":") + self.name, self.autobind_name = name.split(":") else: self.name = name - self.autobind_type = None + self.autobind_name = None def generate_parameter(self, is_last = False, is_header = True, offset = 0): if self.occurrence == OCCURRENCE__IGNORED: raise ValueError("invalid function parameter occurrence value '%s'" % self.occurrence) - elif self.autobind_type is not None: + elif self.autobind_name is not None: return "" else: string = " " @@ -193,7 +186,7 @@ class Method: self.returns = returns for parameter in parameters: - if parameter.autobind_type is None: + if parameter.autobind_name is None: self.parameters.append(parameter) else: self.autobind_parameter = parameter @@ -227,8 +220,8 @@ class Method: source += "ESX_VI__METHOD(%s," % self.name if self.autobind_parameter is not None: - autobind_map_usage.add(self.autobind_parameter.autobind_type) - source += " %s,\n" % autobind_map[self.autobind_parameter.autobind_type] + autobind_names.add(self.autobind_parameter.autobind_name) + source += " %s,\n" % self.autobind_parameter.autobind_name else: source += " /* explicit _this */,\n" @@ -1785,11 +1778,10 @@ for name in names: methods_header.write(methods_by_name[name].generate_header()) methods_source.write(methods_by_name[name].generate_source()) -sorted_usage = list(autobind_map_usage) -sorted_usage.sort() +names = list(autobind_names) +names.sort() -for usage in sorted_usage: - name = autobind_map[usage] +for name in names: string = aligned("#define ESX_VI__METHOD__PARAMETER__THIS__%s " % name, "\\\n", 78) string += " ESX_VI__METHOD__PARAMETER__THIS_FROM_SERVICE(ManagedObjectReference, \\\n" string += aligned("", "%s)\n\n\n\n" % name, 49) -- 1.7.0.4

On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Instead of specifing the type of the managed object directly specify
s/specifing/specifying/
the ServiceContent member name. This way the mapping dictionary can be removed. --- src/esx/esx_vi_generator.input | 36 ++++++++++++++++++------------------ src/esx/esx_vi_generator.py | 28 ++++++++++------------------ 2 files changed, 28 insertions(+), 36 deletions(-)
This fell apart for me; I don't know how to fix it, so from this point on, you'll have to rebase and post v2. CC libvirt_driver_esx_la-esx_vi_methods.lo In file included from esx/esx_vi_methods.c:288:0: esx/esx_vi_methods.generated.c: In function 'esxVI_RetrieveProperties': esx/esx_vi_methods.generated.c:520:278: error: 'esxVI_ServiceContent' has no member named 'PropertyCollector' -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2011/5/13 Eric Blake <eblake@redhat.com>:
On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Instead of specifing the type of the managed object directly specify
s/specifing/specifying/
the ServiceContent member name. This way the mapping dictionary can be removed. --- src/esx/esx_vi_generator.input | 36 ++++++++++++++++++------------------ src/esx/esx_vi_generator.py | 28 ++++++++++------------------ 2 files changed, 28 insertions(+), 36 deletions(-)
This fell apart for me; I don't know how to fix it, so from this point on, you'll have to rebase and post v2.
CC libvirt_driver_esx_la-esx_vi_methods.lo In file included from esx/esx_vi_methods.c:288:0: esx/esx_vi_methods.generated.c: In function 'esxVI_RetrieveProperties': esx/esx_vi_methods.generated.c:520:278: error: 'esxVI_ServiceContent' has no member named 'PropertyCollector'
One line of change that should have been in this patch slipped into the next one :( Here's a v2 that fixes this. Matthias

On 05/14/2011 04:31 AM, Matthias Bolte wrote:
2011/5/13 Eric Blake <eblake@redhat.com>:
On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Instead of specifing the type of the managed object directly specify the ServiceContent member name. This way the mapping dictionary can be removed.
This fell apart for me; I don't know how to fix it, so from this point on, you'll have to rebase and post v2.
CC libvirt_driver_esx_la-esx_vi_methods.lo In file included from esx/esx_vi_methods.c:288:0: esx/esx_vi_methods.generated.c: In function 'esxVI_RetrieveProperties': esx/esx_vi_methods.generated.c:520:278: error: 'esxVI_ServiceContent' has no member named 'PropertyCollector'
One line of change that should have been in this patch slipped into the next one :(
Here's a v2 that fixes this.
ACK to v2. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2011/5/16 Eric Blake <eblake@redhat.com>:
On 05/14/2011 04:31 AM, Matthias Bolte wrote:
2011/5/13 Eric Blake <eblake@redhat.com>:
On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Instead of specifing the type of the managed object directly specify the ServiceContent member name. This way the mapping dictionary can be removed.
This fell apart for me; I don't know how to fix it, so from this point on, you'll have to rebase and post v2.
CC libvirt_driver_esx_la-esx_vi_methods.lo In file included from esx/esx_vi_methods.c:288:0: esx/esx_vi_methods.generated.c: In function 'esxVI_RetrieveProperties': esx/esx_vi_methods.generated.c:520:278: error: 'esxVI_ServiceContent' has no member named 'PropertyCollector'
One line of change that should have been in this patch slipped into the next one :(
Here's a v2 that fixes this.
ACK to v2.
Thanks, pushed v2. Matthias

No functional change included. --- src/esx/esx_vi_generator.input | 122 ++++++++++++++++++++-------------------- 1 files changed, 61 insertions(+), 61 deletions(-) diff --git a/src/esx/esx_vi_generator.input b/src/esx/esx_vi_generator.input index d927e68..361a6e7 100644 --- a/src/esx/esx_vi_generator.input +++ b/src/esx/esx_vi_generator.input @@ -167,7 +167,7 @@ object AutoStartPowerInfo end -object ChoiceOption extends OptionType +object ChoiceOption extends OptionType ElementDescription choiceInfo rl Int defaultIndex o end @@ -193,7 +193,7 @@ object Description end -object DeviceBackedVirtualDiskSpec extends VirtualDiskSpec +object DeviceBackedVirtualDiskSpec extends VirtualDiskSpec String device r end @@ -204,12 +204,12 @@ object DynamicProperty end -object ElementDescription extends Description +object ElementDescription extends Description String key r end -object EntityEventArgument extends EventArgument +object EntityEventArgument extends EventArgument String name r end @@ -218,7 +218,7 @@ object EventArgument end -object FileBackedVirtualDiskSpec extends VirtualDiskSpec +object FileBackedVirtualDiskSpec extends VirtualDiskSpec Long capacityKb r end @@ -241,19 +241,19 @@ object FileQueryFlags end -object FloppyImageFileInfo extends FileInfo +object FloppyImageFileInfo extends FileInfo end -object FloppyImageFileQuery extends FileQuery +object FloppyImageFileQuery extends FileQuery end -object FolderFileInfo extends FileInfo +object FolderFileInfo extends FileInfo end -object FolderFileQuery extends FileQuery +object FolderFileQuery extends FileQuery end @@ -324,7 +324,7 @@ object HostMountInfo end -object HostNasVolume extends HostFileSystemVolume +object HostNasVolume extends HostFileSystemVolume String remoteHost r String remotePath r String userName o @@ -337,7 +337,7 @@ object HostScsiDiskPartition end -object HostVmfsVolume extends HostFileSystemVolume +object HostVmfsVolume extends HostFileSystemVolume Int blockSizeMb r Int maxBlocks r Int majorVersion r @@ -348,15 +348,15 @@ object HostVmfsVolume extends HostFileSystemVolume end -object IsoImageFileInfo extends FileInfo +object IsoImageFileInfo extends FileInfo end -object IsoImageFileQuery extends FileQuery +object IsoImageFileQuery extends FileQuery end -object LocalDatastoreInfo extends DatastoreInfo +object LocalDatastoreInfo extends DatastoreInfo String path o end @@ -367,7 +367,7 @@ object LocalizedMethodFault end -object NasDatastoreInfo extends DatastoreInfo +object NasDatastoreInfo extends DatastoreInfo HostNasVolume nas o end @@ -411,7 +411,7 @@ object PerfCounterInfo end -object PerfEntityMetric extends PerfEntityMetricBase +object PerfEntityMetric extends PerfEntityMetricBase PerfSampleInfo sampleInfo ol PerfMetricSeries value ol end @@ -428,7 +428,7 @@ object PerfMetricId end -object PerfMetricIntSeries extends PerfMetricSeries +object PerfMetricIntSeries extends PerfMetricSeries Long value ol end @@ -561,11 +561,11 @@ object TaskInfo end -object TemplateConfigFileInfo extends VmConfigFileInfo +object TemplateConfigFileInfo extends VmConfigFileInfo end -object TemplateConfigFileQuery extends VmConfigFileQuery +object TemplateConfigFileQuery extends VmConfigFileQuery end @@ -653,12 +653,12 @@ object VirtualMachineSnapshotTree end -object VmConfigFileInfo extends FileInfo +object VmConfigFileInfo extends FileInfo Int configVersion o end -object VmConfigFileQuery extends FileQuery +object VmConfigFileQuery extends FileQuery VmConfigFileQueryFilter filter o VmConfigFileQueryFlags details o end @@ -674,7 +674,7 @@ object VmConfigFileQueryFlags end -object VmDiskFileInfo extends FileInfo +object VmDiskFileInfo extends FileInfo String diskType o Long capacityKb o Int hardwareVersion o @@ -683,7 +683,7 @@ object VmDiskFileInfo extends FileInfo end -object VmDiskFileQuery extends FileQuery +object VmDiskFileQuery extends FileQuery VmDiskFileQueryFilter filter o VmDiskFileQueryFlags details o end @@ -705,36 +705,36 @@ object VmDiskFileQueryFlags end -object VmEventArgument extends EntityEventArgument +object VmEventArgument extends EntityEventArgument ManagedObjectReference vm r end -object VmLogFileInfo extends FileInfo +object VmLogFileInfo extends FileInfo end -object VmLogFileQuery extends FileQuery +object VmLogFileQuery extends FileQuery end -object VmNvramFileInfo extends FileInfo +object VmNvramFileInfo extends FileInfo end -object VmNvramFileQuery extends FileQuery +object VmNvramFileQuery extends FileQuery end -object VmSnapshotFileInfo extends FileInfo +object VmSnapshotFileInfo extends FileInfo end -object VmSnapshotFileQuery extends FileQuery +object VmSnapshotFileQuery extends FileQuery end -object VmfsDatastoreInfo extends DatastoreInfo +object VmfsDatastoreInfo extends DatastoreInfo HostVmfsVolume vmfs o end @@ -743,19 +743,19 @@ end # Managed Objects # -managed object ComputeResource extends ManagedEntity +managed object ComputeResource extends ManagedEntity ManagedObjectReference host ol ManagedObjectReference resourcePool o end -managed object Datacenter extends ManagedEntity +managed object Datacenter extends ManagedEntity ManagedObjectReference hostFolder r ManagedObjectReference vmFolder r end -managed object HostSystem extends ManagedEntity +managed object HostSystem extends ManagedEntity HostConfigManager configManager r end @@ -781,7 +781,7 @@ method CancelTask end -method CopyVirtualDisk_Task returns ManagedObjectReference r +method CopyVirtualDisk_Task returns ManagedObjectReference r ManagedObjectReference _this:virtualDiskManager r String sourceName r ManagedObjectReference sourceDatacenter o @@ -792,14 +792,14 @@ method CopyVirtualDisk_Task returns ManagedObjectReference r end -method CreateFilter returns ManagedObjectReference r +method CreateFilter returns ManagedObjectReference r ManagedObjectReference _this:propertyCollector r PropertyFilterSpec spec r Boolean partialUpdates r end -method CreateSnapshot_Task returns ManagedObjectReference r +method CreateSnapshot_Task returns ManagedObjectReference r ManagedObjectReference _this r String name r String description o @@ -808,7 +808,7 @@ method CreateSnapshot_Task returns ManagedObjectReference r end -method CreateVirtualDisk_Task returns ManagedObjectReference r +method CreateVirtualDisk_Task returns ManagedObjectReference r ManagedObjectReference _this:virtualDiskManager r String name r ManagedObjectReference datacenter o @@ -816,7 +816,7 @@ method CreateVirtualDisk_Task returns ManagedObjectReference r end -method DeleteVirtualDisk_Task returns ManagedObjectReference r +method DeleteVirtualDisk_Task returns ManagedObjectReference r ManagedObjectReference _this:virtualDiskManager r String name r ManagedObjectReference datacenter o @@ -828,7 +828,7 @@ method DestroyPropertyFilter end -method FindByIp returns ManagedObjectReference o +method FindByIp returns ManagedObjectReference o ManagedObjectReference _this:searchIndex r ManagedObjectReference datacenter o String ip r @@ -836,7 +836,7 @@ method FindByIp returns ManagedObjectReference o end -method FindByUuid returns ManagedObjectReference o +method FindByUuid returns ManagedObjectReference o ManagedObjectReference _this:searchIndex r ManagedObjectReference datacenter o String uuid r @@ -844,7 +844,7 @@ method FindByUuid returns ManagedObjectReference o end -method Login returns UserSession r +method Login returns UserSession r ManagedObjectReference _this:sessionManager r String userName r String password r @@ -865,7 +865,7 @@ method MakeDirectory end -method MigrateVM_Task returns ManagedObjectReference r +method MigrateVM_Task returns ManagedObjectReference r ManagedObjectReference _this r ManagedObjectReference pool o ManagedObjectReference host o @@ -874,18 +874,18 @@ method MigrateVM_Task returns ManagedObjectReference r end -method PowerOffVM_Task returns ManagedObjectReference r +method PowerOffVM_Task returns ManagedObjectReference r ManagedObjectReference _this r end -method PowerOnVM_Task returns ManagedObjectReference r +method PowerOnVM_Task returns ManagedObjectReference r ManagedObjectReference _this r ManagedObjectReference host o end -method QueryAvailablePerfMetric returns PerfMetricId ol +method QueryAvailablePerfMetric returns PerfMetricId ol ManagedObjectReference _this:perfManager r ManagedObjectReference entity r DateTime beginTime o @@ -894,19 +894,19 @@ method QueryAvailablePerfMetric returns PerfMetricId ol end -method QueryPerf returns PerfEntityMetricBase ol +method QueryPerf returns PerfEntityMetricBase ol ManagedObjectReference _this:perfManager r PerfQuerySpec querySpec rl end -method QueryPerfCounter returns PerfCounterInfo ol +method QueryPerfCounter returns PerfCounterInfo ol ManagedObjectReference _this:perfManager r Int counterId rl end -method QueryVirtualDiskUuid returns String r +method QueryVirtualDiskUuid returns String r ManagedObjectReference _this:virtualDiskManager r String name r ManagedObjectReference datacenter o @@ -918,7 +918,7 @@ method RebootGuest end -method ReconfigVM_Task returns ManagedObjectReference r +method ReconfigVM_Task returns ManagedObjectReference r ManagedObjectReference _this r VirtualMachineConfigSpec spec r end @@ -935,7 +935,7 @@ method RefreshDatastore end -method RegisterVM_Task returns ManagedObjectReference r +method RegisterVM_Task returns ManagedObjectReference r ManagedObjectReference _this r String path r String name o @@ -945,39 +945,39 @@ method RegisterVM_Task returns ManagedObjectReference r end -method RemoveSnapshot_Task returns ManagedObjectReference r +method RemoveSnapshot_Task returns ManagedObjectReference r ManagedObjectReference _this r Boolean removeChildren r end -method RetrieveProperties returns ObjectContent ol - ManagedObjectReference _this:PropertyCollector r +method RetrieveProperties returns ObjectContent ol + ManagedObjectReference _this:propertyCollector r PropertyFilterSpec specSet rl end -method RevertToSnapshot_Task returns ManagedObjectReference r +method RevertToSnapshot_Task returns ManagedObjectReference r ManagedObjectReference _this r ManagedObjectReference host o end -method SearchDatastoreSubFolders_Task returns ManagedObjectReference r +method SearchDatastoreSubFolders_Task returns ManagedObjectReference r ManagedObjectReference _this r String datastorePath r HostDatastoreBrowserSearchSpec searchSpec o end -method SearchDatastore_Task returns ManagedObjectReference r +method SearchDatastore_Task returns ManagedObjectReference r ManagedObjectReference _this r String datastorePath r HostDatastoreBrowserSearchSpec searchSpec o end -method SessionIsActive returns Boolean r +method SessionIsActive returns Boolean r ManagedObjectReference _this:sessionManager r String sessionID r String userName r @@ -989,7 +989,7 @@ method ShutdownGuest end -method SuspendVM_Task returns ManagedObjectReference r +method SuspendVM_Task returns ManagedObjectReference r ManagedObjectReference _this r end @@ -999,13 +999,13 @@ method UnregisterVM end -method WaitForUpdates returns UpdateSet r +method WaitForUpdates returns UpdateSet r ManagedObjectReference _this:propertyCollector r String version o end -method ZeroFillVirtualDisk_Task returns ManagedObjectReference r +method ZeroFillVirtualDisk_Task returns ManagedObjectReference r ManagedObjectReference _this:virtualDiskManager r String name r ManagedObjectReference datacenter o -- 1.7.0.4

2011/5/1 Matthias Bolte <matthias.bolte@googlemail.com>:
No functional change included. --- src/esx/esx_vi_generator.input | 122 ++++++++++++++++++++-------------------- 1 files changed, 61 insertions(+), 61 deletions(-)
-method RetrieveProperties returns ObjectContent ol - ManagedObjectReference _this:PropertyCollector r +method RetrieveProperties returns ObjectContent ol + ManagedObjectReference _this:propertyCollector r PropertyFilterSpec specSet rl end
The s/_this:PropertyCollector/_this:propertyCollector/ change should have been in the previous commit. So here a fixed v2. Matthias

On 05/14/2011 04:34 AM, Matthias Bolte wrote:
2011/5/1 Matthias Bolte <matthias.bolte@googlemail.com>:
No functional change included. --- src/esx/esx_vi_generator.input | 122 ++++++++++++++++++++-------------------- 1 files changed, 61 insertions(+), 61 deletions(-)
-method RetrieveProperties returns ObjectContent ol - ManagedObjectReference _this:PropertyCollector r +method RetrieveProperties returns ObjectContent ol + ManagedObjectReference _this:propertyCollector r PropertyFilterSpec specSet rl end
The s/_this:PropertyCollector/_this:propertyCollector/ change should have been in the previous commit.
So here a fixed v2.
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2011/5/16 Eric Blake <eblake@redhat.com>:
On 05/14/2011 04:34 AM, Matthias Bolte wrote:
2011/5/1 Matthias Bolte <matthias.bolte@googlemail.com>:
No functional change included. --- src/esx/esx_vi_generator.input | 122 ++++++++++++++++++++-------------------- 1 files changed, 61 insertions(+), 61 deletions(-)
-method RetrieveProperties returns ObjectContent ol - ManagedObjectReference _this:PropertyCollector r +method RetrieveProperties returns ObjectContent ol + ManagedObjectReference _this:propertyCollector r PropertyFilterSpec specSet rl end
The s/_this:PropertyCollector/_this:propertyCollector/ change should have been in the previous commit.
So here a fixed v2.
ACK.
Thanks, pushed. Matthias

Break long lines and change spacing of keyword arguments to match Python style standards better. No functional change included. --- src/esx/esx_vi_generator.py | 369 ++++++++++++++++++++++++++++-------------- 1 files changed, 246 insertions(+), 123 deletions(-) diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 2fca929..37c76f0 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -40,6 +40,8 @@ valid_occurrences = [OCCURRENCE__REQUIRED_ITEM, autobind_names = set() +separator = "/* " + ("* " * 37) + "*\n" + def aligned(left, right, length=59): @@ -93,9 +95,10 @@ class Parameter(Member): self.autobind_name = None - def generate_parameter(self, is_last = False, is_header = True, offset = 0): + def generate_parameter(self, is_last=False, is_header=True, offset=0): if self.occurrence == OCCURRENCE__IGNORED: - raise ValueError("invalid function parameter occurrence value '%s'" % self.occurrence) + raise ValueError("invalid function parameter occurrence value '%s'" + % self.occurrence) elif self.autobind_name is not None: return "" else: @@ -116,11 +119,13 @@ class Parameter(Member): def generate_return(self, offset = 0, end_of_line = ";"): if self.occurrence == OCCURRENCE__IGNORED: - raise ValueError("invalid function parameter occurrence value '%s'" % self.occurrence) + raise ValueError("invalid function parameter occurrence value '%s'" + % self.occurrence) else: string = " " string += " " * offset - string += "%s%s)%s" % (self.get_type_string(True), self.name, end_of_line) + string += "%s%s)%s" \ + % (self.get_type_string(True), self.name, end_of_line) return aligned(string, self.get_occurrence_comment() + "\n") @@ -136,11 +141,14 @@ class Parameter(Member): def generate_serialize_code(self): if self.occurrence in [OCCURRENCE__REQUIRED_LIST, OCCURRENCE__OPTIONAL_LIST]: - return " ESX_VI__METHOD__PARAMETER__SERIALIZE_LIST(%s, %s)\n" % (self.type, self.name) + return " ESX_VI__METHOD__PARAMETER__SERIALIZE_LIST(%s, %s)\n" \ + % (self.type, self.name) elif self.type == "String": - return " ESX_VI__METHOD__PARAMETER__SERIALIZE_VALUE(String, %s)\n" % self.name + return " ESX_VI__METHOD__PARAMETER__SERIALIZE_VALUE(String, %s)\n" \ + % self.name else: - return " ESX_VI__METHOD__PARAMETER__SERIALIZE(%s, %s)\n" % (self.type, self.name) + return " ESX_VI__METHOD__PARAMETER__SERIALIZE(%s, %s)\n" \ + % (self.type, self.name) def get_type_string(self, as_return_value=False): @@ -203,7 +211,7 @@ class Method: header += parameter.generate_parameter() if self.returns is None: - header += self.parameters[-1].generate_parameter(is_last = True) + header += self.parameters[-1].generate_parameter(is_last=True) else: header += self.parameters[-1].generate_parameter() header += self.returns.generate_return() @@ -231,22 +239,30 @@ class Method: source += ",\n" for parameter in self.parameters[:-1]: - source += parameter.generate_parameter(is_header = False, offset = 9) + source += parameter.generate_parameter(is_header=False, + offset=9) if self.returns is None: - source += self.parameters[-1].generate_parameter(is_last = True, is_header = False, offset = 9) + source += self.parameters[-1].generate_parameter(is_last=True, + is_header=False, + offset=9) else: - source += self.parameters[-1].generate_parameter(is_header = False, offset = 9) - source += self.returns.generate_return(offset = 9, end_of_line = ",") + source += self.parameters[-1].generate_parameter(is_header=False, + offset=9) + source += self.returns.generate_return(offset=9, + end_of_line=",") else: source += "),\n" if self.returns is None: source += " void, /* nothing */, None,\n" elif self.returns.type == "String": - source += " String, Value, %s,\n" % self.returns.get_occurrence_short_enum() + source += " String, Value, %s,\n" \ + % self.returns.get_occurrence_short_enum() else: - source += " %s, /* nothing */, %s,\n" % (self.returns.type, self.returns.get_occurrence_short_enum()) + source += " %s, /* nothing */, %s,\n" \ + % (self.returns.type, + self.returns.get_occurrence_short_enum()) source += "{\n" @@ -322,13 +338,16 @@ class Property(Member): return " /* FIXME: %s is currently ignored */\n" % self.name elif self.occurrence in [OCCURRENCE__REQUIRED_LIST, OCCURRENCE__OPTIONAL_LIST]: - return " ESX_VI__TEMPLATE__PROPERTY__DEEP_COPY_LIST(%s, %s)\n" % (self.type, self.name) + return " ESX_VI__TEMPLATE__PROPERTY__DEEP_COPY_LIST(%s, %s)\n" \ + % (self.type, self.name) elif self.type == "String": - return " ESX_VI__TEMPLATE__PROPERTY__DEEP_COPY_VALUE(String, %s)\n" % self.name + return " ESX_VI__TEMPLATE__PROPERTY__DEEP_COPY_VALUE(String, %s)\n" \ + % self.name elif self.is_enum(): return " (*dest)->%s = src->%s;\n" % (self.name, self.name) else: - return " ESX_VI__TEMPLATE__PROPERTY__DEEP_COPY(%s, %s)\n" % (self.type, self.name) + return " ESX_VI__TEMPLATE__PROPERTY__DEEP_COPY(%s, %s)\n" \ + % (self.type, self.name) def generate_serialize_code(self): @@ -336,35 +355,46 @@ class Property(Member): return " /* FIXME: %s is currently ignored */\n" % self.name elif self.occurrence in [OCCURRENCE__REQUIRED_LIST, OCCURRENCE__OPTIONAL_LIST]: - return " ESX_VI__TEMPLATE__PROPERTY__SERIALIZE_LIST(%s, %s)\n" % (self.type, self.name) + return " ESX_VI__TEMPLATE__PROPERTY__SERIALIZE_LIST(%s, %s)\n" \ + % (self.type, self.name) elif self.type == "String": - return " ESX_VI__TEMPLATE__PROPERTY__SERIALIZE_VALUE(String, %s)\n" % self.name + return " ESX_VI__TEMPLATE__PROPERTY__SERIALIZE_VALUE(String, %s)\n" \ + % self.name else: - return " ESX_VI__TEMPLATE__PROPERTY__SERIALIZE(%s, %s)\n" % (self.type, self.name) + return " ESX_VI__TEMPLATE__PROPERTY__SERIALIZE(%s, %s)\n" \ + % (self.type, self.name) def generate_deserialize_code(self): if self.occurrence == OCCURRENCE__IGNORED: - return " ESX_VI__TEMPLATE__PROPERTY__DESERIALIZE_IGNORE(%s) /* FIXME */\n" % self.name + return " ESX_VI__TEMPLATE__PROPERTY__DESERIALIZE_IGNORE(%s) /* FIXME */\n" \ + % self.name elif self.occurrence in [OCCURRENCE__REQUIRED_LIST, OCCURRENCE__OPTIONAL_LIST]: - return " ESX_VI__TEMPLATE__PROPERTY__DESERIALIZE_LIST(%s, %s)\n" % (self.type, self.name) + return " ESX_VI__TEMPLATE__PROPERTY__DESERIALIZE_LIST(%s, %s)\n" \ + % (self.type, self.name) elif self.type == "String": - return " ESX_VI__TEMPLATE__PROPERTY__DESERIALIZE_VALUE(String, %s)\n" % self.name + return " ESX_VI__TEMPLATE__PROPERTY__DESERIALIZE_VALUE(String, %s)\n" \ + % self.name else: - return " ESX_VI__TEMPLATE__PROPERTY__DESERIALIZE(%s, %s)\n" % (self.type, self.name) + return " ESX_VI__TEMPLATE__PROPERTY__DESERIALIZE(%s, %s)\n" \ + % (self.type, self.name) def generate_lookup_code(self): if self.occurrence == OCCURRENCE__IGNORED: - return " ESX_VI__TEMPLATE__PROPERTY__CAST_FROM_ANY_TYPE_IGNORE(%s) /* FIXME */\n" % self.name + return " ESX_VI__TEMPLATE__PROPERTY__CAST_FROM_ANY_TYPE_IGNORE(%s) /* FIXME */\n" \ + % self.name elif self.occurrence in [OCCURRENCE__REQUIRED_LIST, OCCURRENCE__OPTIONAL_LIST]: - return " ESX_VI__TEMPLATE__PROPERTY__CAST_LIST_FROM_ANY_TYPE(%s, %s)\n" % (self.type, self.name) + return " ESX_VI__TEMPLATE__PROPERTY__CAST_LIST_FROM_ANY_TYPE(%s, %s)\n" \ + % (self.type, self.name) elif self.type == "String": - return " ESX_VI__TEMPLATE__PROPERTY__CAST_VALUE_FROM_ANY_TYPE(String, %s)\n" % self.name + return " ESX_VI__TEMPLATE__PROPERTY__CAST_VALUE_FROM_ANY_TYPE(String, %s)\n" \ + % self.name else: - return " ESX_VI__TEMPLATE__PROPERTY__CAST_FROM_ANY_TYPE(%s, %s)\n" % (self.type, self.name) + return " ESX_VI__TEMPLATE__PROPERTY__CAST_FROM_ANY_TYPE(%s, %s)\n" \ + % (self.type, self.name) def get_type_string(self): @@ -386,7 +416,8 @@ class Type: def generate_typedef(self): - return "typedef %s _esxVI_%s esxVI_%s;\n" % (self.kind, self.name, self.name) + return "typedef %s _esxVI_%s esxVI_%s;\n" \ + % (self.kind, self.name, self.name) def generate_typeenum(self): @@ -417,7 +448,8 @@ class Object(Type): FEATURE__SERIALIZE = (1 << 5) FEATURE__DESERIALIZE = (1 << 6) - def __init__(self, name, extends, properties, features = 0, extended_by = None): + + def __init__(self, name, extends, properties, features=0, extended_by=None): Type.__init__(self, "struct", name) self.extends = extends self.features = features @@ -436,7 +468,9 @@ class Object(Type): members += "\n" if self.extends is not None: - members += objects_by_name[self.extends].generate_struct_members(add_banner=True, struct_gap=False) + "\n" + members += objects_by_name[self.extends] \ + .generate_struct_members(add_banner=True, + struct_gap=False) + "\n" if self.extends is not None or add_banner: members += " /* %s */\n" % self.name @@ -454,7 +488,8 @@ class Object(Type): source = "" if self.extends is not None: - source += objects_by_name[self.extends].generate_free_code(add_banner=True) + "\n" + source += objects_by_name[self.extends] \ + .generate_free_code(add_banner=True) + "\n" if self.extends is not None or add_banner: source += " /* %s */\n" % self.name @@ -479,7 +514,8 @@ class Object(Type): source = "" if self.extends is not None: - source += objects_by_name[self.extends].generate_validate_code(add_banner=True) + "\n" + source += objects_by_name[self.extends] \ + .generate_validate_code(add_banner=True) + "\n" if self.extends is not None or add_banner: source += " /* %s */\n" % self.name @@ -500,7 +536,7 @@ class Object(Type): return source - def generate_dynamic_cast_code(self, is_first = True): + def generate_dynamic_cast_code(self, is_first=True): source = "" if self.extended_by is not None: @@ -510,10 +546,12 @@ class Object(Type): source += " /* %s */\n" % self.name for extended_by in self.extended_by: - source += " ESX_VI__TEMPLATE__DYNAMIC_CAST__ACCEPT(%s)\n" % extended_by + source += " ESX_VI__TEMPLATE__DYNAMIC_CAST__ACCEPT(%s)\n" \ + % extended_by for extended_by in self.extended_by: - source += objects_by_name[extended_by].generate_dynamic_cast_code(False) + source += objects_by_name[extended_by] \ + .generate_dynamic_cast_code(False) return source @@ -522,7 +560,8 @@ class Object(Type): source = "" if self.extends is not None: - source += objects_by_name[self.extends].generate_deep_copy_code(add_banner = True) + "\n" + source += objects_by_name[self.extends] \ + .generate_deep_copy_code(add_banner=True) + "\n" if self.extends is not None or add_banner: source += " /* %s */\n" % self.name @@ -543,11 +582,12 @@ class Object(Type): return source - def generate_serialize_code(self, add_banner = False): + def generate_serialize_code(self, add_banner=False): source = "" if self.extends is not None: - source += objects_by_name[self.extends].generate_serialize_code(add_banner = True) + "\n" + source += objects_by_name[self.extends] \ + .generate_serialize_code(add_banner=True) + "\n" if self.extends is not None or add_banner: source += " /* %s */\n" % self.name @@ -561,11 +601,12 @@ class Object(Type): return source - def generate_deserialize_code(self, add_banner = False): + def generate_deserialize_code(self, add_banner=False): source = "" if self.extends is not None: - source += objects_by_name[self.extends].generate_deserialize_code(add_banner=True) + "\n" + source += objects_by_name[self.extends] \ + .generate_deserialize_code(add_banner=True) + "\n" if self.extends is not None or add_banner: source += " /* %s */\n" % self.name @@ -580,7 +621,7 @@ class Object(Type): def generate_header(self): - header = "/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *\n" + header = separator header += " * VI Object: %s\n" % self.name if self.extends is not None: @@ -602,51 +643,74 @@ class Object(Type): header += "struct _esxVI_%s {\n" % self.name if self.features & Object.FEATURE__LIST: - header += aligned(" esxVI_%s *_next; " % self.name, "/* optional */\n") + header += aligned(" esxVI_%s *_next; " % self.name, + "/* optional */\n") else: - header += aligned(" esxVI_%s *_unused; " % self.name, "/* optional */\n") + header += aligned(" esxVI_%s *_unused; " % self.name, + "/* optional */\n") header += aligned(" esxVI_Type _type; ", "/* required */\n") header += self.generate_struct_members(struct_gap=True) header += "};\n\n" # functions - header += "int esxVI_%s_Alloc(esxVI_%s **item);\n" % (self.name, self.name) - header += "void esxVI_%s_Free(esxVI_%s **item);\n" % (self.name, self.name) - header += "int esxVI_%s_Validate(esxVI_%s *item);\n" % (self.name, self.name) + header += "int esxVI_%s_Alloc(esxVI_%s **item);\n" \ + % (self.name, self.name) + header += "void esxVI_%s_Free(esxVI_%s **item);\n" \ + % (self.name, self.name) + header += "int esxVI_%s_Validate(esxVI_%s *item);\n" \ + % (self.name, self.name) if self.features & Object.FEATURE__DYNAMIC_CAST: if self.extended_by is not None or self.extends is not None: - header += "esxVI_%s *esxVI_%s_DynamicCast(void *item);\n" % (self.name, self.name) + header += "esxVI_%s *esxVI_%s_DynamicCast(void *item);\n" \ + % (self.name, self.name) else: report_error("cannot add dynamic cast support for an untyped object") if self.features & Object.FEATURE__LIST: - header += "int esxVI_%s_AppendToList(esxVI_%s **list, esxVI_%s *item);\n" % (self.name, self.name, self.name) + header += "int esxVI_%s_AppendToList(esxVI_%s **list, esxVI_%s *item);\n" \ + % (self.name, self.name, self.name) if self.features & Object.FEATURE__DEEP_COPY: - header += "int esxVI_%s_DeepCopy(esxVI_%s **dst, esxVI_%s *src);\n" % (self.name, self.name, self.name) + header += "int esxVI_%s_DeepCopy(esxVI_%s **dst, esxVI_%s *src);\n" \ + % (self.name, self.name, self.name) if self.features & Object.FEATURE__LIST: - header += "int esxVI_%s_DeepCopyList(esxVI_%s **dstList, esxVI_%s *srcList);\n" % (self.name, self.name, self.name) + header += ("int esxVI_%s_DeepCopyList(esxVI_%s **dstList, " + "esxVI_%s *srcList);\n") \ + % (self.name, self.name, self.name) if self.features & Object.FEATURE__ANY_TYPE: - header += "int esxVI_%s_CastFromAnyType(esxVI_AnyType *anyType, esxVI_%s **item);\n" % (self.name, self.name) + header += ("int esxVI_%s_CastFromAnyType(esxVI_AnyType *anyType, " + "esxVI_%s **item);\n") \ + % (self.name, self.name) if self.features & Object.FEATURE__LIST: - header += "int esxVI_%s_CastListFromAnyType(esxVI_AnyType *anyType, esxVI_%s **list);\n" % (self.name, self.name) + header += ("int esxVI_%s_CastListFromAnyType(esxVI_AnyType *anyType, " + "esxVI_%s **list);\n") \ + % (self.name, self.name) if self.features & Object.FEATURE__SERIALIZE: - header += "int esxVI_%s_Serialize(esxVI_%s *item, const char *element, virBufferPtr output);\n" % (self.name, self.name) + header += ("int esxVI_%s_Serialize(esxVI_%s *item, " + "const char *element, " + "virBufferPtr output);\n") \ + % (self.name, self.name) if self.features & Object.FEATURE__LIST: - header += "int esxVI_%s_SerializeList(esxVI_%s *list, const char *element, virBufferPtr output);\n" % (self.name, self.name) + header += ("int esxVI_%s_SerializeList(esxVI_%s *list, " + "const char *element, " + "virBufferPtr output);\n") \ + % (self.name, self.name) if self.features & Object.FEATURE__DESERIALIZE: - header += "int esxVI_%s_Deserialize(xmlNodePtr node, esxVI_%s **item);\n" % (self.name, self.name) + header += "int esxVI_%s_Deserialize(xmlNodePtr node, esxVI_%s **item);\n" \ + % (self.name, self.name) if self.features & Object.FEATURE__LIST: - header += "int esxVI_%s_DeserializeList(xmlNodePtr node, esxVI_%s **list);\n" % (self.name, self.name) + header += ("int esxVI_%s_DeserializeList(xmlNodePtr node, " + "esxVI_%s **list);\n") \ + % (self.name, self.name) header += "\n\n\n" @@ -654,7 +718,7 @@ class Object(Type): def generate_source(self): - source = "/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *\n" + source = separator source += " * VI Object: %s\n" % self.name if self.extends is not None: @@ -684,8 +748,10 @@ class Object(Type): if self.features & Object.FEATURE__LIST: if self.extends is not None: - # avoid "dereferencing type-punned pointer will break strict-aliasing rules" warnings - source += " esxVI_%s *next = (esxVI_%s *)item->_next;\n\n" % (self.extends, self.extends) + # avoid "dereferencing type-punned pointer will break + # strict-aliasing rules" warnings + source += " esxVI_%s *next = (esxVI_%s *)item->_next;\n\n" \ + % (self.extends, self.extends) source += " esxVI_%s_Free(&next);\n" % self.extends source += " item->_next = (esxVI_%s *)next;\n\n" % self.name else: @@ -700,15 +766,18 @@ class Object(Type): source += "{\n" for extended_by in self.extended_by: - source += " ESX_VI__TEMPLATE__DISPATCH__FREE(%s)\n" % extended_by + source += " ESX_VI__TEMPLATE__DISPATCH__FREE(%s)\n" \ + % extended_by source += "},\n" source += "{\n" if self.features & Object.FEATURE__LIST: if self.extends is not None: - # avoid "dereferencing type-punned pointer will break strict-aliasing rules" warnings - source += " esxVI_%s *next = (esxVI_%s *)item->_next;\n\n" % (self.extends, self.extends) + # avoid "dereferencing type-punned pointer will brea + # strict-aliasing rules" warnings + source += " esxVI_%s *next = (esxVI_%s *)item->_next;\n\n" \ + % (self.extends, self.extends) source += " esxVI_%s_Free(&next);\n" % self.extends source += " item->_next = (esxVI_%s *)next;\n\n" % self.name else: @@ -758,7 +827,8 @@ class Object(Type): if self.features & Object.FEATURE__LIST: source += "/* esxVI_%s_DeepCopyList */\n" % self.name - source += "ESX_VI__TEMPLATE__LIST__DEEP_COPY(%s)\n\n" % self.name + source += "ESX_VI__TEMPLATE__LIST__DEEP_COPY(%s)\n\n" \ + % self.name else: if self.features & Object.FEATURE__DEEP_COPY: source += "/* esxVI_%s_DeepCopy */\n" % self.name @@ -766,7 +836,8 @@ class Object(Type): source += "{\n" for extended_by in self.extended_by: - source += " ESX_VI__TEMPLATE__DISPATCH__DEEP_COPY(%s)\n" % extended_by + source += " ESX_VI__TEMPLATE__DISPATCH__DEEP_COPY(%s)\n" \ + % extended_by source += "},\n" source += "{\n" @@ -777,26 +848,31 @@ class Object(Type): if self.features & Object.FEATURE__LIST: source += "/* esxVI_%s_DeepCopyList */\n" % self.name - source += "ESX_VI__TEMPLATE__LIST__DEEP_COPY(%s)\n\n" % self.name + source += "ESX_VI__TEMPLATE__LIST__DEEP_COPY(%s)\n\n" \ + % self.name # cast from any type if self.features & Object.FEATURE__ANY_TYPE: source += "/* esxVI_%s_CastFromAnyType */\n" % self.name if self.extended_by is None: - source += "ESX_VI__TEMPLATE__CAST_FROM_ANY_TYPE(%s)\n\n" % self.name + source += "ESX_VI__TEMPLATE__CAST_FROM_ANY_TYPE(%s)\n\n" \ + % self.name else: - source += "ESX_VI__TEMPLATE__DYNAMIC_CAST_FROM_ANY_TYPE(%s,\n" % self.name + source += "ESX_VI__TEMPLATE__DYNAMIC_CAST_FROM_ANY_TYPE(%s,\n" \ + % self.name source += "{\n" for extended_by in self.extended_by: - source += " ESX_VI__TEMPLATE__DISPATCH__CAST_FROM_ANY_TYPE(%s)\n" % extended_by + source += " ESX_VI__TEMPLATE__DISPATCH__CAST_FROM_ANY_TYPE(%s)\n" \ + % extended_by source += "})\n\n" if self.features & Object.FEATURE__LIST: source += "/* esxVI_%s_CastListFromAnyType */\n" % self.name - source += "ESX_VI__TEMPLATE__LIST__CAST_FROM_ANY_TYPE(%s)\n\n" % self.name + source += "ESX_VI__TEMPLATE__LIST__CAST_FROM_ANY_TYPE(%s)\n\n" \ + % self.name # serialize if self.extended_by is None: @@ -811,7 +887,8 @@ class Object(Type): if self.features & Object.FEATURE__LIST: source += "/* esxVI_%s_SerializeList */\n" % self.name - source += "ESX_VI__TEMPLATE__LIST__SERIALIZE(%s)\n\n" % self.name + source += "ESX_VI__TEMPLATE__LIST__SERIALIZE(%s)\n\n" \ + % self.name else: if self.features & Object.FEATURE__SERIALIZE: source += "/* esxVI_%s_Serialize */\n" % self.name @@ -819,7 +896,8 @@ class Object(Type): source += "{\n" for extended_by in self.extended_by: - source += " ESX_VI__TEMPLATE__DISPATCH__SERIALIZE(%s)\n" % extended_by + source += " ESX_VI__TEMPLATE__DISPATCH__SERIALIZE(%s)\n" \ + % extended_by source += "},\n" source += "{\n" @@ -830,7 +908,8 @@ class Object(Type): if self.features & Object.FEATURE__LIST: source += "/* esxVI_%s_SerializeList */\n" % self.name - source += "ESX_VI__TEMPLATE__LIST__SERIALIZE(%s)\n\n" % self.name + source += "ESX_VI__TEMPLATE__LIST__SERIALIZE(%s)\n\n" \ + % self.name # deserilaize if self.extended_by is None: @@ -845,15 +924,18 @@ class Object(Type): if self.features & Object.FEATURE__LIST: source += "/* esxVI_%s_DeserializeList */\n" % self.name - source += "ESX_VI__TEMPLATE__LIST__DESERIALIZE(%s)\n\n" % self.name + source += "ESX_VI__TEMPLATE__LIST__DESERIALIZE(%s)\n\n" \ + % self.name else: if self.features & Object.FEATURE__DESERIALIZE: source += "/* esxVI_%s_Deserialize */\n" % self.name - source += "ESX_VI__TEMPLATE__DYNAMIC_DESERIALIZE(%s,\n" % self.name + source += "ESX_VI__TEMPLATE__DYNAMIC_DESERIALIZE(%s,\n" \ + % self.name source += "{\n" for extended_by in self.extended_by: - source += " ESX_VI__TEMPLATE__DISPATCH__DESERIALIZE(%s)\n" % extended_by + source += " ESX_VI__TEMPLATE__DISPATCH__DESERIALIZE(%s)\n" \ + % extended_by source += "},\n" source += "{\n" @@ -864,7 +946,8 @@ class Object(Type): if self.features & Object.FEATURE__LIST: source += "/* esxVI_%s_DeserializeList */\n" % self.name - source += "ESX_VI__TEMPLATE__LIST__DESERIALIZE(%s)\n\n" % self.name + source += "ESX_VI__TEMPLATE__LIST__DESERIALIZE(%s)\n\n" \ + % self.name source += "\n\n" @@ -894,7 +977,8 @@ class ManagedObject(Type): members += "\n" if self.extends is not None: - members += managed_objects_by_name[self.extends].generate_struct_members(add_banner=True) + "\n" + members += managed_objects_by_name[self.extends] \ + .generate_struct_members(add_banner=True) + "\n" if self.extends is not None or add_banner: members += " /* %s */\n" % self.name @@ -912,7 +996,8 @@ class ManagedObject(Type): source = "" if self.extends is not None: - source += managed_objects_by_name[self.extends].generate_free_code(add_banner=True) + "\n" + source += managed_objects_by_name[self.extends] \ + .generate_free_code(add_banner=True) + "\n" if self.extends is not None or add_banner: source += " /* %s */\n" % self.name @@ -937,7 +1022,8 @@ class ManagedObject(Type): source = "" if self.extends is not None: - source += managed_objects_by_name[self.extends].generate_validate_code(add_banner=True) + "\n" + source += managed_objects_by_name[self.extends] \ + .generate_validate_code(add_banner=True) + "\n" if self.extends is not None or add_banner: source += " /* %s */\n" % self.name @@ -962,7 +1048,8 @@ class ManagedObject(Type): source = "" if self.extends is not None: - source += managed_objects_by_name[self.extends].generate_lookup_code1(add_banner=True) + "\n" + source += managed_objects_by_name[self.extends] \ + .generate_lookup_code1(add_banner=True) + "\n" if self.extends is not None or add_banner: source += " /* %s */\n" % self.name @@ -987,7 +1074,8 @@ class ManagedObject(Type): source = "" if self.extends is not None: - source += managed_objects_by_name[self.extends].generate_lookup_code2(add_banner=True) + "\n" + source += managed_objects_by_name[self.extends] \ + .generate_lookup_code2(add_banner=True) + "\n" if self.extends is not None or add_banner: source += " /* %s */\n" % self.name @@ -1009,7 +1097,7 @@ class ManagedObject(Type): def generate_comment(self): - comment = "/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *\n" + comment = separator comment += " * VI Managed Object: %s\n" % self.name if self.extends is not None: @@ -1020,10 +1108,12 @@ class ManagedObject(Type): if self.extended_by is not None: for extended_by in self.extended_by: if first: - comment += " * extended by %s\n" % extended_by + comment += " * extended by %s\n" \ + % extended_by first = False else: - comment += " * %s\n" % extended_by + comment += " * %s\n" \ + % extended_by comment += " */\n\n" @@ -1037,12 +1127,15 @@ class ManagedObject(Type): header += "struct _esxVI_%s {\n" % self.name if self.features & Object.FEATURE__LIST: - header += aligned(" esxVI_%s *_next; " % self.name, "/* optional */\n") + header += aligned(" esxVI_%s *_next; " % self.name, + "/* optional */\n") else: - header += aligned(" esxVI_%s *_unused; " % self.name, "/* optional */\n") + header += aligned(" esxVI_%s *_unused; " % self.name, + "/* optional */\n") header += aligned(" esxVI_Type _type; ", "/* required */\n") - header += aligned(" esxVI_ManagedObjectReference *_reference; ", "/* required */\n") + header += aligned(" esxVI_ManagedObjectReference *_reference; ", + "/* required */\n") header += "\n" header += self.generate_struct_members() @@ -1051,10 +1144,13 @@ class ManagedObject(Type): # functions header += "int esxVI_%s_Alloc(esxVI_%s **item);\n" % (self.name, self.name) header += "void esxVI_%s_Free(esxVI_%s **item);\n" % (self.name, self.name) - header += "int esxVI_%s_Validate(esxVI_%s *item, esxVI_String *selectedPropertyNameList);\n" % (self.name, self.name) + header += ("int esxVI_%s_Validate(esxVI_%s *item, " + "esxVI_String *selectedPropertyNameList);\n") \ + % (self.name, self.name) if self.features & Object.FEATURE__LIST: - header += "int esxVI_%s_AppendToList(esxVI_%s **list, esxVI_%s *item);\n" % (self.name, self.name, self.name) + header += "int esxVI_%s_AppendToList(esxVI_%s **list, esxVI_%s *item);\n" \ + % (self.name, self.name, self.name) header += "\n\n\n" @@ -1065,12 +1161,13 @@ class ManagedObject(Type): header = "" # functions - header += ("int esxVI_Lookup%s(esxVI_Context *ctx, " + - "const char *name, " + - "esxVI_ManagedObjectReference *root, " + - "esxVI_String *selectedPropertyNameList, " + - "esxVI_%s **item, " + - "esxVI_Occurrence occurrence);\n") % (self.name, self.name) + header += ("int esxVI_Lookup%s(esxVI_Context *ctx, " + "const char *name, " + "esxVI_ManagedObjectReference *root, " + "esxVI_String *selectedPropertyNameList, " + "esxVI_%s **item, " + "esxVI_Occurrence occurrence);\n") \ + % (self.name, self.name) header += "\n" @@ -1092,8 +1189,10 @@ class ManagedObject(Type): if self.features & ManagedObject.FEATURE__LIST: if self.extends is not None: - # avoid "dereferencing type-punned pointer will break strict-aliasing rules" warnings - source += " esxVI_%s *next = (esxVI_%s *)item->_next;\n\n" % (self.extends, self.extends) + # avoid "dereferencing type-punned pointer will break + # strict-aliasing rules" warnings + source += " esxVI_%s *next = (esxVI_%s *)item->_next;\n\n" \ + % (self.extends, self.extends) source += " esxVI_%s_Free(&next);\n" % self.extends source += " item->_next = (esxVI_%s *)next;\n\n" % self.name else: @@ -1117,8 +1216,10 @@ class ManagedObject(Type): if self.features & Object.FEATURE__LIST: if self.extends is not None: - # avoid "dereferencing type-punned pointer will break strict-aliasing rules" warnings - source += " esxVI_%s *next = (esxVI_%s *)item->_next;\n\n" % (self.extends, self.extends) + # avoid "dereferencing type-punned pointer will break + # strict-aliasing rules" warnings + source += " esxVI_%s *next = (esxVI_%s *)item->_next;\n\n" \ + % (self.extends, self.extends) source += " esxVI_%s_Free(&next);\n" % self.extends source += " item->_next = (esxVI_%s *)next;\n\n" % self.name else: @@ -1185,7 +1286,7 @@ class Enum(Type): def generate_header(self): - header = "/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *\n" + header = separator header += " * VI Enum: %s\n" % self.name header += " */\n\n" @@ -1200,13 +1301,19 @@ class Enum(Type): # functions if self.features & Enum.FEATURE__ANY_TYPE: - header += "int esxVI_%s_CastFromAnyType(esxVI_AnyType *anyType, esxVI_%s *item);\n" % (self.name, self.name) + header += ("int esxVI_%s_CastFromAnyType(esxVI_AnyType *anyType, " + "esxVI_%s *item);\n") \ + % (self.name, self.name) if self.features & Enum.FEATURE__SERIALIZE: - header += "int esxVI_%s_Serialize(esxVI_%s item, const char *element, virBufferPtr output);\n" % (self.name, self.name) + header += ("int esxVI_%s_Serialize(esxVI_%s item, const char *element, " + "virBufferPtr output);\n") \ + % (self.name, self.name) if self.features & Enum.FEATURE__DESERIALIZE: - header += "int esxVI_%s_Deserialize(xmlNodePtr node, esxVI_%s *item);\n" % (self.name, self.name) + header += ("int esxVI_%s_Deserialize(xmlNodePtr node, " + "esxVI_%s *item);\n") \ + % (self.name, self.name) header += "\n\n\n" @@ -1214,15 +1321,17 @@ class Enum(Type): def generate_source(self): - source = "/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *\n" + source = separator source += " * VI Enum: %s\n" % self.name source += " */\n\n" - source += "static const esxVI_Enumeration _esxVI_%s_Enumeration = {\n" % self.name + source += "static const esxVI_Enumeration _esxVI_%s_Enumeration = {\n" \ + % self.name source += " esxVI_Type_%s, {\n" % self.name for value in self.values: - source += " { \"%s\", esxVI_%s_%s },\n" % (value, self.name, capitalize_first(value)) + source += " { \"%s\", esxVI_%s_%s },\n" \ + % (value, self.name, capitalize_first(value)) source += " { NULL, -1 },\n" source += " },\n" @@ -1231,15 +1340,18 @@ class Enum(Type): # functions if self.features & Enum.FEATURE__ANY_TYPE: source += "/* esxVI_%s_CastFromAnyType */\n" % self.name - source += "ESX_VI__TEMPLATE__ENUMERATION__CAST_FROM_ANY_TYPE(%s)\n\n" % self.name + source += "ESX_VI__TEMPLATE__ENUMERATION__CAST_FROM_ANY_TYPE(%s)\n\n" \ + % self.name if self.features & Enum.FEATURE__SERIALIZE: source += "/* esxVI_%s_Serialize */\n" % self.name - source += "ESX_VI__TEMPLATE__ENUMERATION__SERIALIZE(%s)\n\n" % self.name + source += "ESX_VI__TEMPLATE__ENUMERATION__SERIALIZE(%s)\n\n" \ + % self.name if self.features & Enum.FEATURE__DESERIALIZE: source += "/* esxVI_%s_Deserialize */\n" % self.name - source += "ESX_VI__TEMPLATE__ENUMERATION__DESERIALIZE(%s)\n\n" % self.name + source += "ESX_VI__TEMPLATE__ENUMERATION__DESERIALIZE(%s)\n\n" \ + % self.name source += "\n\n" @@ -1397,20 +1509,27 @@ additional_enum_features = { "ManagedEntityStatus" : Enum.FEATURE__ANY_TYPE additional_object_features = { "AutoStartDefaults" : Object.FEATURE__ANY_TYPE, "AutoStartPowerInfo" : Object.FEATURE__ANY_TYPE, - "DatastoreHostMount" : Object.FEATURE__DEEP_COPY | Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE, - "DatastoreInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__DYNAMIC_CAST, + "DatastoreHostMount" : Object.FEATURE__DEEP_COPY | + Object.FEATURE__LIST | + Object.FEATURE__ANY_TYPE, + "DatastoreInfo" : Object.FEATURE__ANY_TYPE | + Object.FEATURE__DYNAMIC_CAST, "HostConfigManager" : Object.FEATURE__ANY_TYPE, - "HostCpuIdInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__LIST, - "HostDatastoreBrowserSearchResults" : Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE, + "HostCpuIdInfo" : Object.FEATURE__LIST | + Object.FEATURE__ANY_TYPE, + "HostDatastoreBrowserSearchResults" : Object.FEATURE__LIST | + Object.FEATURE__ANY_TYPE, "ManagedObjectReference" : Object.FEATURE__ANY_TYPE, "ObjectContent" : Object.FEATURE__DEEP_COPY, "ResourcePoolResourceUsage" : Object.FEATURE__ANY_TYPE, "ServiceContent" : Object.FEATURE__DESERIALIZE, "SharesInfo" : Object.FEATURE__ANY_TYPE, - "TaskInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__LIST, + "TaskInfo" : Object.FEATURE__LIST | + Object.FEATURE__ANY_TYPE, "UserSession" : Object.FEATURE__ANY_TYPE, "VirtualMachineQuestionInfo" : Object.FEATURE__ANY_TYPE, - "VirtualMachineSnapshotTree" : Object.FEATURE__DEEP_COPY | Object.FEATURE__ANY_TYPE, + "VirtualMachineSnapshotTree" : Object.FEATURE__DEEP_COPY | + Object.FEATURE__ANY_TYPE, "VmEventArgument" : Object.FEATURE__DESERIALIZE } removed_object_features = {} @@ -1543,11 +1662,13 @@ for obj in objects_by_name.values(): for property in obj.properties: if property.occurrence != OCCURRENCE__IGNORED and \ not is_known_type(property.type): - report_error("object '%s' contains unknown property type '%s'" % (obj.name, property.type)) + report_error("object '%s' contains unknown property type '%s'" + % (obj.name, property.type)) if obj.extends is not None: if not is_known_type(obj.extends): - report_error("object '%s' extends unknown object '%s'" % (obj.name, obj.extends)) + report_error("object '%s' extends unknown object '%s'" + % (obj.name, obj.extends)) for property in obj.properties: if not property.is_type_generated(): @@ -1676,11 +1797,13 @@ for obj in managed_objects_by_name.values(): for property in obj.properties: if property.occurrence != OCCURRENCE__IGNORED and \ not is_known_type(property.type): - report_error("object '%s' contains unknown property type '%s'" % (obj.name, property.type)) + report_error("object '%s' contains unknown property type '%s'" + % (obj.name, property.type)) if obj.extends is not None: if not is_known_type(obj.extends): - report_error("object '%s' extends unknown object '%s'" % (obj.name, obj.extends)) + report_error("object '%s' extends unknown object '%s'" + % (obj.name, obj.extends)) # detect extended_by relation if obj.extends is not None: @@ -1709,7 +1832,7 @@ helpers_source.write("/* Generated by esx_vi_generator.py */\n\n\n\n") # output enums -types_typedef.write("/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *\n" + +types_typedef.write(separator + " * VI Enums\n" + " */\n\n") @@ -1728,7 +1851,7 @@ for name in names: # output objects types_typedef.write("\n\n\n" + - "/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *\n" + + separator + " * VI Objects\n" + " */\n\n") types_typeenum.write("\n") @@ -1750,7 +1873,7 @@ for name in names: # output managed objects types_typedef.write("\n\n\n" + - "/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *\n" + + separator + " * VI Managed Objects\n" + " */\n\n") types_typeenum.write("\n") -- 1.7.0.4

On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Break long lines and change spacing of keyword arguments to match Python style standards better.
No functional change included. --- src/esx/esx_vi_generator.py | 369 ++++++++++++++++++++++++++++-------------- 1 files changed, 246 insertions(+), 123 deletions(-)
I don't know the python style standards, but I could at least review this on the grounds of looking like safe whitespace changes. ACK.
@@ -830,7 +908,8 @@ class Object(Type):
if self.features & Object.FEATURE__LIST: source += "/* esxVI_%s_SerializeList */\n" % self.name - source += "ESX_VI__TEMPLATE__LIST__SERIALIZE(%s)\n\n" % self.name + source += "ESX_VI__TEMPLATE__LIST__SERIALIZE(%s)\n\n" \ + % self.name
# deserilaize
You may want to do a followup patch to fix typos like this. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2011/5/16 Eric Blake <eblake@redhat.com>:
On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Break long lines and change spacing of keyword arguments to match Python style standards better.
No functional change included. --- src/esx/esx_vi_generator.py | 369 ++++++++++++++++++++++++++++-------------- 1 files changed, 246 insertions(+), 123 deletions(-)
I don't know the python style standards, but I could at least review this on the grounds of looking like safe whitespace changes.
ACK.
@@ -830,7 +908,8 @@ class Object(Type):
if self.features & Object.FEATURE__LIST: source += "/* esxVI_%s_SerializeList */\n" % self.name - source += "ESX_VI__TEMPLATE__LIST__SERIALIZE(%s)\n\n" % self.name + source += "ESX_VI__TEMPLATE__LIST__SERIALIZE(%s)\n\n" \ + % self.name
# deserilaize
You may want to do a followup patch to fix typos like this.
As this one was the only typo I cloud find in the generator I've just folded the fix into this patch and pushed the result. Matthias

When the session has expired then multiple threads can race while reestablishing it. This race condition is not that critical as it requires a special usage pattern to be triggerd. It can only happen when an application doesn't do API calls for quite some time (the session expires after 30 min inactivity) and then multiple threads doing simultaneous API calls and end up doing simultaneous calls to esxVI_EnsureSession. --- src/esx/esx_vi.c | 47 +++++++++++++++++++++++++++++++++++------------ src/esx/esx_vi.h | 4 +++- 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 756ff68..73413c5 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -603,6 +603,10 @@ ESX_VI__TEMPLATE__ALLOC(Context) /* esxVI_Context_Free */ ESX_VI__TEMPLATE__FREE(Context, { + if (item->sessionLock != NULL) { + virMutexDestroy(item->sessionLock); + } + esxVI_CURL_Free(&item->curl); VIR_FREE(item->url); VIR_FREE(item->ipAddress); @@ -610,6 +614,7 @@ ESX_VI__TEMPLATE__FREE(Context, VIR_FREE(item->password); esxVI_ServiceContent_Free(&item->service); esxVI_UserSession_Free(&item->session); + VIR_FREE(item->sessionLock); esxVI_Datacenter_Free(&item->datacenter); esxVI_ComputeResource_Free(&item->computeResource); esxVI_HostSystem_Free(&item->hostSystem); @@ -642,6 +647,17 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, return -1; } + if (VIR_ALLOC(ctx->sessionLock) < 0) { + virReportOOMError(); + return -1; + } + + if (virMutexInit(ctx->sessionLock) < 0) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not initialize session mutex")); + return -1; + } + if (esxVI_RetrieveServiceContent(ctx, &ctx->service) < 0) { return -1; } @@ -1558,11 +1574,18 @@ esxVI_EnsureSession(esxVI_Context *ctx) esxVI_DynamicProperty *dynamicProperty = NULL; esxVI_UserSession *currentSession = NULL; - if (ctx->session == NULL) { - ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid call")); + if (ctx->sessionLock == NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid call, no mutex")); return -1; } + virMutexLock(ctx->sessionLock); + + if (ctx->session == NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid call, no session")); + goto cleanup; + } + if (ctx->hasSessionIsActive) { /* * Use SessionIsActive to check if there is an active session for this @@ -1570,7 +1593,7 @@ esxVI_EnsureSession(esxVI_Context *ctx) */ if (esxVI_SessionIsActive(ctx, ctx->session->key, ctx->session->userName, &active) < 0) { - return -1; + goto cleanup; } if (active != esxVI_Boolean_True) { @@ -1578,11 +1601,9 @@ esxVI_EnsureSession(esxVI_Context *ctx) if (esxVI_Login(ctx, ctx->username, ctx->password, NULL, &ctx->session) < 0) { - return -1; + goto cleanup; } } - - return 0; } else { /* * Query the session manager for the current session of this connection @@ -1624,16 +1645,18 @@ esxVI_EnsureSession(esxVI_Context *ctx) "last login")); goto cleanup; } + } - result = 0; + result = 0; cleanup: - esxVI_String_Free(&propertyNameList); - esxVI_ObjectContent_Free(&sessionManager); - esxVI_UserSession_Free(¤tSession); + virMutexUnlock(ctx->sessionLock); - return result; - } + esxVI_String_Free(&propertyNameList); + esxVI_ObjectContent_Free(&sessionManager); + esxVI_UserSession_Free(¤tSession); + + return result; } diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h index 75469ab..e3d096e 100644 --- a/src/esx/esx_vi.h +++ b/src/esx/esx_vi.h @@ -185,6 +185,7 @@ int esxVI_SharedCURL_Remove(esxVI_SharedCURL *shared, esxVI_CURL *curl); */ struct _esxVI_Context { + /* All members are used read-only after esxVI_Context_Connect ... */ esxVI_CURL *curl; char *url; char *ipAddress; @@ -193,7 +194,8 @@ struct _esxVI_Context { esxVI_ServiceContent *service; esxVI_APIVersion apiVersion; esxVI_ProductVersion productVersion; - esxVI_UserSession *session; + esxVI_UserSession *session; /* ... except the session ... */ + virMutexPtr sessionLock; /* ... that is protected by this mutex */ esxVI_Datacenter *datacenter; esxVI_ComputeResource *computeResource; esxVI_HostSystem *hostSystem; -- 1.7.0.4

On 05/01/2011 01:57 PM, Matthias Bolte wrote:
When the session has expired then multiple threads can race while reestablishing it.
This race condition is not that critical as it requires a special usage pattern to be triggerd. It can only happen when an application doesn't
s/triggerd/triggered/
do API calls for quite some time (the session expires after 30 min inactivity) and then multiple threads doing simultaneous API calls and end up doing simultaneous calls to esxVI_EnsureSession. --- src/esx/esx_vi.c | 47 +++++++++++++++++++++++++++++++++++------------ src/esx/esx_vi.h | 4 +++- 2 files changed, 38 insertions(+), 13 deletions(-)
C code - I'm back in my element!
+++ b/src/esx/esx_vi.h @@ -185,6 +185,7 @@ int esxVI_SharedCURL_Remove(esxVI_SharedCURL *shared, esxVI_CURL *curl); */
struct _esxVI_Context { + /* All members are used read-only after esxVI_Context_Connect ... */ esxVI_CURL *curl; char *url; char *ipAddress; @@ -193,7 +194,8 @@ struct _esxVI_Context { esxVI_ServiceContent *service; esxVI_APIVersion apiVersion; esxVI_ProductVersion productVersion; - esxVI_UserSession *session; + esxVI_UserSession *session; /* ... except the session ... */ + virMutexPtr sessionLock; /* ... that is protected by this mutex */
ACK, and thanks for those comments. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2011/5/16 Eric Blake <eblake@redhat.com>:
On 05/01/2011 01:57 PM, Matthias Bolte wrote:
When the session has expired then multiple threads can race while reestablishing it.
This race condition is not that critical as it requires a special usage pattern to be triggerd. It can only happen when an application doesn't
s/triggerd/triggered/
Fixed.
do API calls for quite some time (the session expires after 30 min inactivity) and then multiple threads doing simultaneous API calls and end up doing simultaneous calls to esxVI_EnsureSession. --- src/esx/esx_vi.c | 47 +++++++++++++++++++++++++++++++++++------------ src/esx/esx_vi.h | 4 +++- 2 files changed, 38 insertions(+), 13 deletions(-)
C code - I'm back in my element!
+++ b/src/esx/esx_vi.h @@ -185,6 +185,7 @@ int esxVI_SharedCURL_Remove(esxVI_SharedCURL *shared, esxVI_CURL *curl); */
struct _esxVI_Context { + /* All members are used read-only after esxVI_Context_Connect ... */ esxVI_CURL *curl; char *url; char *ipAddress; @@ -193,7 +194,8 @@ struct _esxVI_Context { esxVI_ServiceContent *service; esxVI_APIVersion apiVersion; esxVI_ProductVersion productVersion; - esxVI_UserSession *session; + esxVI_UserSession *session; /* ... except the session ... */ + virMutexPtr sessionLock; /* ... that is protected by this mutex */
ACK, and thanks for those comments.
Thanks, pushed. Matthias

Just true/false is good enough for it. Also use it directly from the parsed URI instead of caching it in esxPrivate. --- src/esx/esx_driver.c | 52 ++++++++++++++++++++--------------------- src/esx/esx_private.h | 1 - src/esx/esx_storage_driver.c | 12 ++++++--- src/esx/esx_vi.c | 15 +++++------ src/esx/esx_vi.h | 8 +++--- 5 files changed, 44 insertions(+), 44 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 1f8f90b..89240dc 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -940,8 +940,6 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED) priv->maxVcpus = -1; priv->supportsVMotion = esxVI_Boolean_Undefined; priv->supportsLongMode = esxVI_Boolean_Undefined; - priv->autoAnswer = priv->parsedUri->autoAnswer ? esxVI_Boolean_True - : esxVI_Boolean_False; priv->usedCpuTimeCounterId = -1; /* @@ -1721,7 +1719,7 @@ esxDomainSuspend(virDomainPtr domain) "runtime.powerState") < 0 || esxVI_LookupVirtualMachineByUuidAndPrepareForTask (priv->primary, domain->uuid, propertyNameList, &virtualMachine, - priv->autoAnswer) < 0 || + priv->parsedUri->autoAnswer) < 0 || esxVI_GetVirtualMachinePowerState(virtualMachine, &powerState) < 0) { goto cleanup; } @@ -1735,7 +1733,7 @@ esxDomainSuspend(virDomainPtr domain) if (esxVI_SuspendVM_Task(priv->primary, virtualMachine->obj, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState, + priv->parsedUri->autoAnswer, &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } @@ -1779,7 +1777,7 @@ esxDomainResume(virDomainPtr domain) "runtime.powerState") < 0 || esxVI_LookupVirtualMachineByUuidAndPrepareForTask (priv->primary, domain->uuid, propertyNameList, &virtualMachine, - priv->autoAnswer) < 0 || + priv->parsedUri->autoAnswer) < 0 || esxVI_GetVirtualMachinePowerState(virtualMachine, &powerState) < 0) { goto cleanup; } @@ -1793,7 +1791,7 @@ esxDomainResume(virDomainPtr domain) &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState, + priv->parsedUri->autoAnswer, &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } @@ -1930,7 +1928,7 @@ esxDomainDestroy(virDomainPtr domain) "runtime.powerState") < 0 || esxVI_LookupVirtualMachineByUuidAndPrepareForTask (ctx, domain->uuid, propertyNameList, &virtualMachine, - priv->autoAnswer) < 0 || + priv->parsedUri->autoAnswer) < 0 || esxVI_GetVirtualMachinePowerState(virtualMachine, &powerState) < 0) { goto cleanup; } @@ -1944,7 +1942,7 @@ esxDomainDestroy(virDomainPtr domain) if (esxVI_PowerOffVM_Task(ctx, virtualMachine->obj, &task) < 0 || esxVI_WaitForTaskCompletion(ctx, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState, + priv->parsedUri->autoAnswer, &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } @@ -2057,7 +2055,7 @@ esxDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) "runtime.powerState") < 0 || esxVI_LookupVirtualMachineByUuidAndPrepareForTask (priv->primary, domain->uuid, propertyNameList, &virtualMachine, - priv->autoAnswer) < 0 || + priv->parsedUri->autoAnswer) < 0 || esxVI_GetVirtualMachinePowerState(virtualMachine, &powerState) < 0) { goto cleanup; } @@ -2081,7 +2079,7 @@ esxDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState, + priv->parsedUri->autoAnswer, &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } @@ -2124,7 +2122,7 @@ esxDomainSetMemory(virDomainPtr domain, unsigned long memory) if (esxVI_LookupVirtualMachineByUuidAndPrepareForTask (priv->primary, domain->uuid, NULL, &virtualMachine, - priv->autoAnswer) < 0 || + priv->parsedUri->autoAnswer) < 0 || esxVI_VirtualMachineConfigSpec_Alloc(&spec) < 0 || esxVI_ResourceAllocationInfo_Alloc(&spec->memoryAllocation) < 0 || esxVI_Long_Alloc(&spec->memoryAllocation->limit) < 0) { @@ -2138,7 +2136,7 @@ esxDomainSetMemory(virDomainPtr domain, unsigned long memory) &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState, + priv->parsedUri->autoAnswer, &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } @@ -2468,7 +2466,7 @@ esxDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus, if (esxVI_LookupVirtualMachineByUuidAndPrepareForTask (priv->primary, domain->uuid, NULL, &virtualMachine, - priv->autoAnswer) < 0 || + priv->parsedUri->autoAnswer) < 0 || esxVI_VirtualMachineConfigSpec_Alloc(&spec) < 0 || esxVI_Int_Alloc(&spec->numCPUs) < 0) { goto cleanup; @@ -2480,7 +2478,7 @@ esxDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState, + priv->parsedUri->autoAnswer, &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } @@ -2896,7 +2894,7 @@ esxDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) "runtime.powerState") < 0 || esxVI_LookupVirtualMachineByUuidAndPrepareForTask (priv->primary, domain->uuid, propertyNameList, &virtualMachine, - priv->autoAnswer) < 0 || + priv->parsedUri->autoAnswer) < 0 || esxVI_GetVirtualMachinePowerState(virtualMachine, &powerState) < 0 || esxVI_GetVirtualMachineIdentity(virtualMachine, &id, NULL, NULL) < 0) { goto cleanup; @@ -2912,7 +2910,7 @@ esxDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState, + priv->parsedUri->autoAnswer, &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } @@ -3134,7 +3132,7 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml) &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, def->uuid, esxVI_Occurrence_OptionalItem, - priv->autoAnswer, &taskInfoState, + priv->parsedUri->autoAnswer, &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } @@ -3599,7 +3597,7 @@ esxDomainSetSchedulerParameters(virDomainPtr domain, if (esxVI_LookupVirtualMachineByUuidAndPrepareForTask (priv->primary, domain->uuid, NULL, &virtualMachine, - priv->autoAnswer) < 0 || + priv->parsedUri->autoAnswer) < 0 || esxVI_VirtualMachineConfigSpec_Alloc(&spec) < 0 || esxVI_ResourceAllocationInfo_Alloc(&spec->cpuAllocation) < 0) { goto cleanup; @@ -3685,7 +3683,7 @@ esxDomainSetSchedulerParameters(virDomainPtr domain, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState, + priv->parsedUri->autoAnswer, &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } @@ -3819,7 +3817,7 @@ esxDomainMigratePerform(virDomainPtr domain, /* Lookup VirtualMachine */ if (esxVI_LookupVirtualMachineByUuidAndPrepareForTask (priv->vCenter, domain->uuid, NULL, &virtualMachine, - priv->autoAnswer) < 0) { + priv->parsedUri->autoAnswer) < 0) { goto cleanup; } @@ -3856,7 +3854,7 @@ esxDomainMigratePerform(virDomainPtr domain, &task) < 0 || esxVI_WaitForTaskCompletion(priv->vCenter, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState, + priv->parsedUri->autoAnswer, &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } @@ -4063,7 +4061,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, if (esxVI_LookupVirtualMachineByUuidAndPrepareForTask (priv->primary, domain->uuid, NULL, &virtualMachine, - priv->autoAnswer) < 0 || + priv->parsedUri->autoAnswer) < 0 || esxVI_LookupRootSnapshotTreeList(priv->primary, domain->uuid, &rootSnapshotList) < 0 || esxVI_GetSnapshotTreeByName(rootSnapshotList, def->name, @@ -4084,7 +4082,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, esxVI_Boolean_False, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState, + priv->parsedUri->autoAnswer, &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } @@ -4345,7 +4343,7 @@ esxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, unsigned int flags) &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, snapshot->domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState, + priv->parsedUri->autoAnswer, &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } @@ -4404,7 +4402,7 @@ esxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) removeChildren, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, snapshot->domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState, + priv->parsedUri->autoAnswer, &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } @@ -4449,7 +4447,7 @@ esxDomainSetMemoryParameters(virDomainPtr domain, virMemoryParameterPtr params, if (esxVI_LookupVirtualMachineByUuidAndPrepareForTask (priv->primary, domain->uuid, NULL, &virtualMachine, - priv->autoAnswer) < 0 || + priv->parsedUri->autoAnswer) < 0 || esxVI_VirtualMachineConfigSpec_Alloc(&spec) < 0 || esxVI_ResourceAllocationInfo_Alloc(&spec->memoryAllocation) < 0) { goto cleanup; @@ -4475,7 +4473,7 @@ esxDomainSetMemoryParameters(virDomainPtr domain, virMemoryParameterPtr params, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState, + priv->parsedUri->autoAnswer, &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } diff --git a/src/esx/esx_private.h b/src/esx/esx_private.h index 7ce237e..3fcc001 100644 --- a/src/esx/esx_private.h +++ b/src/esx/esx_private.h @@ -41,7 +41,6 @@ typedef struct _esxPrivate { int32_t maxVcpus; esxVI_Boolean supportsVMotion; esxVI_Boolean supportsLongMode; /* aka x86_64 */ - esxVI_Boolean autoAnswer; int32_t usedCpuTimeCounterId; } esxPrivate; diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index 9e4dd9e..d45b13f 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -1114,7 +1114,8 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, esxVI_VirtualDiskSpec_DynamicCast(virtualDiskSpec), &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, NULL, esxVI_Occurrence_None, - priv->autoAnswer, &taskInfoState, + priv->parsedUri->autoAnswer, + &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } @@ -1315,7 +1316,8 @@ esxStorageVolumeCreateXMLFrom(virStoragePoolPtr pool, const char *xmldesc, NULL, esxVI_Boolean_False, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, NULL, esxVI_Occurrence_None, - priv->autoAnswer, &taskInfoState, + priv->parsedUri->autoAnswer, + &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } @@ -1402,7 +1404,8 @@ esxStorageVolumeDelete(virStorageVolPtr volume, unsigned int flags) priv->primary->datacenter->_reference, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, NULL, - esxVI_Occurrence_None, priv->autoAnswer, + esxVI_Occurrence_None, + priv->parsedUri->autoAnswer, &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } @@ -1450,7 +1453,8 @@ esxStorageVolumeWipe(virStorageVolPtr volume, unsigned int flags) priv->primary->datacenter->_reference, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, NULL, - esxVI_Occurrence_None, priv->autoAnswer, + esxVI_Occurrence_None, + priv->parsedUri->autoAnswer, &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 73413c5..7f0b0a8 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -2482,7 +2482,7 @@ int esxVI_LookupVirtualMachineByUuidAndPrepareForTask (esxVI_Context *ctx, const unsigned char *uuid, esxVI_String *propertyNameList, esxVI_ObjectContent **virtualMachine, - esxVI_Boolean autoAnswer) + bool autoAnswer) { int result = -1; esxVI_String *completePropertyNameList = NULL; @@ -2884,7 +2884,7 @@ int esxVI_LookupAndHandleVirtualMachineQuestion(esxVI_Context *ctx, const unsigned char *uuid, esxVI_Occurrence occurrence, - esxVI_Boolean autoAnswer, + bool autoAnswer, esxVI_Boolean *blocked) { int result = -1; @@ -3204,7 +3204,7 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, datastorePathWithoutFileName, searchSpec, &task) < 0 || esxVI_WaitForTaskCompletion(ctx, task, NULL, esxVI_Occurrence_None, - esxVI_Boolean_False, &taskInfoState, + false, &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } @@ -3349,7 +3349,7 @@ esxVI_LookupDatastoreContentByDatastoreName datastorePath, searchSpec, &task) < 0 || esxVI_WaitForTaskCompletion(ctx, task, NULL, esxVI_Occurrence_None, - esxVI_Boolean_False, &taskInfoState, + false, &taskInfoState, &taskInfoErrorMessage) < 0) { goto cleanup; } @@ -3554,7 +3554,7 @@ int esxVI_HandleVirtualMachineQuestion (esxVI_Context *ctx, esxVI_ManagedObjectReference *virtualMachine, esxVI_VirtualMachineQuestionInfo *questionInfo, - esxVI_Boolean autoAnswer, esxVI_Boolean *blocked) + bool autoAnswer, esxVI_Boolean *blocked) { int result = -1; esxVI_ElementDescription *elementDescription = NULL; @@ -3597,7 +3597,7 @@ esxVI_HandleVirtualMachineQuestion possibleAnswers = virBufferContentAndReset(&buffer); } - if (autoAnswer == esxVI_Boolean_True) { + if (autoAnswer) { if (possibleAnswers == NULL) { ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, _("Pending question blocks virtual machine execution, " @@ -3662,8 +3662,7 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, esxVI_ManagedObjectReference *task, const unsigned char *virtualMachineUuid, esxVI_Occurrence virtualMachineOccurrence, - esxVI_Boolean autoAnswer, - esxVI_TaskInfoState *finalState, + bool autoAnswer, esxVI_TaskInfoState *finalState, char **errorMessage) { int result = -1; diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h index e3d096e..9e37d86 100644 --- a/src/esx/esx_vi.h +++ b/src/esx/esx_vi.h @@ -396,7 +396,7 @@ int esxVI_LookupVirtualMachineByName(esxVI_Context *ctx, const char *name, int esxVI_LookupVirtualMachineByUuidAndPrepareForTask (esxVI_Context *ctx, const unsigned char *uuid, esxVI_String *propertyNameList, esxVI_ObjectContent **virtualMachine, - esxVI_Boolean autoAnswer); + bool autoAnswer); int esxVI_LookupDatastoreList(esxVI_Context *ctx, esxVI_String *propertyNameList, esxVI_ObjectContent **datastoreList); @@ -427,7 +427,7 @@ int esxVI_LookupPendingTaskInfoListByVirtualMachine int esxVI_LookupAndHandleVirtualMachineQuestion(esxVI_Context *ctx, const unsigned char *uuid, esxVI_Occurrence occurrence, - esxVI_Boolean autoAnswer, + bool autoAnswer, esxVI_Boolean *blocked); int esxVI_LookupRootSnapshotTreeList @@ -463,13 +463,13 @@ int esxVI_HandleVirtualMachineQuestion (esxVI_Context *ctx, esxVI_ManagedObjectReference *virtualMachine, esxVI_VirtualMachineQuestionInfo *questionInfo, - esxVI_Boolean autoAnswer, esxVI_Boolean *blocked); + bool autoAnswer, esxVI_Boolean *blocked); int esxVI_WaitForTaskCompletion(esxVI_Context *ctx, esxVI_ManagedObjectReference *task, const unsigned char *virtualMachineUuid, esxVI_Occurrence virtualMachineOccurrence, - esxVI_Boolean autoAnswer, + bool autoAnswer, esxVI_TaskInfoState *finalState, char **errorMessage); -- 1.7.0.4

On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Just true/false is good enough for it. Also use it directly from the parsed URI instead of caching it in esxPrivate. --- src/esx/esx_driver.c | 52 ++++++++++++++++++++--------------------- src/esx/esx_private.h | 1 - src/esx/esx_storage_driver.c | 12 ++++++--- src/esx/esx_vi.c | 15 +++++------ src/esx/esx_vi.h | 8 +++--- 5 files changed, 44 insertions(+), 44 deletions(-)
+++ b/src/esx/esx_private.h @@ -41,7 +41,6 @@ typedef struct _esxPrivate { int32_t maxVcpus; esxVI_Boolean supportsVMotion; esxVI_Boolean supportsLongMode; /* aka x86_64 */ - esxVI_Boolean autoAnswer; int32_t usedCpuTimeCounterId; } esxPrivate;
+++ b/src/esx/esx_vi.c @@ -2482,7 +2482,7 @@ int esxVI_LookupVirtualMachineByUuidAndPrepareForTask (esxVI_Context *ctx, const unsigned char *uuid, esxVI_String *propertyNameList, esxVI_ObjectContent **virtualMachine, - esxVI_Boolean autoAnswer) + bool autoAnswer)
Everything else is fallout from these two categories of changes (more than one method signature affected, and lots of callers adjusted). ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2011/5/16 Eric Blake <eblake@redhat.com>:
On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Just true/false is good enough for it. Also use it directly from the parsed URI instead of caching it in esxPrivate. --- src/esx/esx_driver.c | 52 ++++++++++++++++++++--------------------- src/esx/esx_private.h | 1 - src/esx/esx_storage_driver.c | 12 ++++++--- src/esx/esx_vi.c | 15 +++++------ src/esx/esx_vi.h | 8 +++--- 5 files changed, 44 insertions(+), 44 deletions(-)
+++ b/src/esx/esx_private.h @@ -41,7 +41,6 @@ typedef struct _esxPrivate { int32_t maxVcpus; esxVI_Boolean supportsVMotion; esxVI_Boolean supportsLongMode; /* aka x86_64 */ - esxVI_Boolean autoAnswer; int32_t usedCpuTimeCounterId; } esxPrivate;
+++ b/src/esx/esx_vi.c @@ -2482,7 +2482,7 @@ int esxVI_LookupVirtualMachineByUuidAndPrepareForTask (esxVI_Context *ctx, const unsigned char *uuid, esxVI_String *propertyNameList, esxVI_ObjectContent **virtualMachine, - esxVI_Boolean autoAnswer) + bool autoAnswer)
Everything else is fallout from these two categories of changes (more than one method signature affected, and lots of callers adjusted).
ACK.
Thanks, pushed. Matthias

--- src/esx/esx_driver.c | 6 ++---- src/esx/esx_vi.c | 34 +++++++++++++++------------------- src/esx/esx_vi.h | 12 +++++------- 3 files changed, 22 insertions(+), 30 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 89240dc..4a9f474 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1511,8 +1511,7 @@ esxNumberOfDomains(virConnectPtr conn) } return esxVI_LookupNumberOfDomainsByPowerState - (priv->primary, esxVI_VirtualMachinePowerState_PoweredOn, - esxVI_Boolean_False); + (priv->primary, esxVI_VirtualMachinePowerState_PoweredOn, false); } @@ -2865,8 +2864,7 @@ esxNumberOfDefinedDomains(virConnectPtr conn) } return esxVI_LookupNumberOfDomainsByPowerState - (priv->primary, esxVI_VirtualMachinePowerState_PoweredOn, - esxVI_Boolean_True); + (priv->primary, esxVI_VirtualMachinePowerState_PoweredOn, true); } diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 7f0b0a8..60bbb42 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -2016,7 +2016,7 @@ esxVI_GetManagedObjectReference(esxVI_ObjectContent *objectContent, int esxVI_LookupNumberOfDomainsByPowerState(esxVI_Context *ctx, esxVI_VirtualMachinePowerState powerState, - esxVI_Boolean inverse) + bool inverse) { bool success = false; esxVI_String *propertyNameList = NULL; @@ -2044,10 +2044,8 @@ esxVI_LookupNumberOfDomainsByPowerState(esxVI_Context *ctx, goto cleanup; } - if ((inverse != esxVI_Boolean_True && - powerState_ == powerState) || - (inverse == esxVI_Boolean_True && - powerState_ != powerState)) { + if ((!inverse && powerState_ == powerState) || + ( inverse && powerState_ != powerState)) { count++; } } else { @@ -2488,7 +2486,7 @@ esxVI_LookupVirtualMachineByUuidAndPrepareForTask esxVI_String *completePropertyNameList = NULL; esxVI_VirtualMachineQuestionInfo *questionInfo = NULL; esxVI_TaskInfo *pendingTaskInfoList = NULL; - esxVI_Boolean blocked = esxVI_Boolean_Undefined; + bool blocked; if (esxVI_String_DeepCopyList(&completePropertyNameList, propertyNameList) < 0 || @@ -2884,8 +2882,7 @@ int esxVI_LookupAndHandleVirtualMachineQuestion(esxVI_Context *ctx, const unsigned char *uuid, esxVI_Occurrence occurrence, - bool autoAnswer, - esxVI_Boolean *blocked) + bool autoAnswer, bool *blocked) { int result = -1; esxVI_ObjectContent *virtualMachine = NULL; @@ -3553,8 +3550,8 @@ esxVI_LookupAutoStartPowerInfoList(esxVI_Context *ctx, int esxVI_HandleVirtualMachineQuestion (esxVI_Context *ctx, esxVI_ManagedObjectReference *virtualMachine, - esxVI_VirtualMachineQuestionInfo *questionInfo, - bool autoAnswer, esxVI_Boolean *blocked) + esxVI_VirtualMachineQuestionInfo *questionInfo, bool autoAnswer, + bool *blocked) { int result = -1; esxVI_ElementDescription *elementDescription = NULL; @@ -3563,12 +3560,12 @@ esxVI_HandleVirtualMachineQuestion int answerIndex = 0; char *possibleAnswers = NULL; - if (blocked == NULL || *blocked != esxVI_Boolean_Undefined) { + if (blocked == NULL) { ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); return -1; } - *blocked = esxVI_Boolean_False; + *blocked = false; if (questionInfo->choice->choiceInfo != NULL) { for (elementDescription = questionInfo->choice->choiceInfo; @@ -3604,7 +3601,7 @@ esxVI_HandleVirtualMachineQuestion "question is '%s', no possible answers"), questionInfo->text); - *blocked = esxVI_Boolean_True; + *blocked = true; goto cleanup; } else if (answerChoice == NULL) { ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, @@ -3613,7 +3610,7 @@ esxVI_HandleVirtualMachineQuestion "default answer is specified"), questionInfo->text, possibleAnswers); - *blocked = esxVI_Boolean_True; + *blocked = true; goto cleanup; } @@ -3639,7 +3636,7 @@ esxVI_HandleVirtualMachineQuestion questionInfo->text); } - *blocked = esxVI_Boolean_True; + *blocked = true; goto cleanup; } @@ -3677,7 +3674,7 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, esxVI_PropertyChange *propertyChange = NULL; esxVI_AnyType *propertyValue = NULL; esxVI_TaskInfoState state = esxVI_TaskInfoState_Undefined; - esxVI_Boolean blocked = esxVI_Boolean_Undefined; + bool blocked; esxVI_TaskInfo *taskInfo = NULL; if (errorMessage == NULL || *errorMessage != NULL) { @@ -3735,13 +3732,12 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, } if (taskInfo->cancelable == esxVI_Boolean_True) { - if (esxVI_CancelTask(ctx, task) < 0 && - blocked == esxVI_Boolean_True) { + if (esxVI_CancelTask(ctx, task) < 0 && blocked) { VIR_ERROR0(_("Cancelable task is blocked by an " "unanswered question but cancelation " "failed")); } - } else if (blocked == esxVI_Boolean_True) { + } else if (blocked) { VIR_ERROR0(_("Non-cancelable task is blocked by an " "unanswered question")); } diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h index 9e37d86..8677ca0 100644 --- a/src/esx/esx_vi.h +++ b/src/esx/esx_vi.h @@ -351,7 +351,7 @@ int esxVI_GetManagedObjectReference(esxVI_ObjectContent *objectContent, int esxVI_LookupNumberOfDomainsByPowerState (esxVI_Context *ctx, esxVI_VirtualMachinePowerState powerState, - esxVI_Boolean inverse); + bool inverse); int esxVI_GetVirtualMachineIdentity(esxVI_ObjectContent *virtualMachine, int *id, char **name, unsigned char *uuid); @@ -427,8 +427,7 @@ int esxVI_LookupPendingTaskInfoListByVirtualMachine int esxVI_LookupAndHandleVirtualMachineQuestion(esxVI_Context *ctx, const unsigned char *uuid, esxVI_Occurrence occurrence, - bool autoAnswer, - esxVI_Boolean *blocked); + bool autoAnswer, bool *blocked); int esxVI_LookupRootSnapshotTreeList (esxVI_Context *ctx, const unsigned char *virtualMachineUuid, @@ -460,10 +459,9 @@ int esxVI_LookupAutoStartPowerInfoList(esxVI_Context *ctx, esxVI_AutoStartPowerInfo **powerInfoList); int esxVI_HandleVirtualMachineQuestion - (esxVI_Context *ctx, - esxVI_ManagedObjectReference *virtualMachine, - esxVI_VirtualMachineQuestionInfo *questionInfo, - bool autoAnswer, esxVI_Boolean *blocked); + (esxVI_Context *ctx, esxVI_ManagedObjectReference *virtualMachine, + esxVI_VirtualMachineQuestionInfo *questionInfo, bool autoAnswer, + bool *blocked); int esxVI_WaitForTaskCompletion(esxVI_Context *ctx, esxVI_ManagedObjectReference *task, -- 1.7.0.4

On 05/01/2011 01:57 PM, Matthias Bolte wrote:
--- src/esx/esx_driver.c | 6 ++---- src/esx/esx_vi.c | 34 +++++++++++++++------------------- src/esx/esx_vi.h | 12 +++++------- 3 files changed, 22 insertions(+), 30 deletions(-)
I got a trivial rebase error when testing this out:
@@ -3735,13 +3732,12 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, }
if (taskInfo->cancelable == esxVI_Boolean_True) { - if (esxVI_CancelTask(ctx, task) < 0 && - blocked == esxVI_Boolean_True) { + if (esxVI_CancelTask(ctx, task) < 0 && blocked) { VIR_ERROR0(_("Cancelable task is blocked by an "
now that the context has VIR_ERROR instead of VIR_ERROR0. But I'm assuming you've already picked up on that. ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2011/5/16 Eric Blake <eblake@redhat.com>:
On 05/01/2011 01:57 PM, Matthias Bolte wrote:
--- src/esx/esx_driver.c | 6 ++---- src/esx/esx_vi.c | 34 +++++++++++++++------------------- src/esx/esx_vi.h | 12 +++++------- 3 files changed, 22 insertions(+), 30 deletions(-)
I got a trivial rebase error when testing this out:
@@ -3735,13 +3732,12 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, }
if (taskInfo->cancelable == esxVI_Boolean_True) { - if (esxVI_CancelTask(ctx, task) < 0 && - blocked == esxVI_Boolean_True) { + if (esxVI_CancelTask(ctx, task) < 0 && blocked) { VIR_ERROR0(_("Cancelable task is blocked by an "
now that the context has VIR_ERROR instead of VIR_ERROR0. But I'm assuming you've already picked up on that.
ACK.
Thanks, pushed. Matthias

2011/5/1 Matthias Bolte <matthias.bolte@googlemail.com>:
As we're currently in feature freeze, this series is meant for 0.9.2.
It includes mostly generator improvments and general cleanups.
10/12 is a race condition fix, but it's not critical because it's not that simple to trigger.
Matthias
Ping for review :) Matthias
participants (2)
-
Eric Blake
-
Matthias Bolte