On Wed, Apr 07, 2010 at 12:00:01PM +0200, Matthias Bolte wrote:
Fix invalid code generating in esx_vi_generator.py regarding deep copy types that contain enum properties.
Add strptime and timegm to bootstrap.conf. Both are used to convert a xsd:dateTime to calendar time. --- bootstrap.conf | 2 + src/esx/esx_driver.c | 468 +++++++++++++++++++++++++++++++++++++--- src/esx/esx_vi.c | 290 +++++++++++++++++++++++++ src/esx/esx_vi.h | 27 +++ src/esx/esx_vi_generator.input | 12 + src/esx/esx_vi_generator.py | 25 ++- src/esx/esx_vi_methods.c | 86 ++++++++ src/esx/esx_vi_methods.h | 14 ++ src/esx/esx_vi_types.c | 99 +++++++++ src/esx/esx_vi_types.h | 12 + 10 files changed, 990 insertions(+), 45 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf index ac2f8e6..ca9332d 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -52,9 +52,11 @@ stpcpy strchrnul strndup strerror +strptime strsep sys_stat time_r +timegm useless-if-before-free vasprintf verify
Okay, IIRC the environment checks for LGPL licence compat
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index eb06555..5272654 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c
pure formatting changes on this module [...]
+static virDomainSnapshotPtr +esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, + unsigned int flags ATTRIBUTE_UNUSED) +{ [...] +} +
Looks fine
+ +static char * +esxDomainSnapshotDumpXML(virDomainSnapshotPtr snapshot, + unsigned int flags ATTRIBUTE_UNUSED) +{ + esxPrivate *priv = snapshot->domain->conn->privateData; + esxVI_VirtualMachineSnapshotTree *rootSnapshotList = NULL; + esxVI_VirtualMachineSnapshotTree *snapshotTree = NULL; + esxVI_VirtualMachineSnapshotTree *snapshotTreeParent = NULL; + virDomainSnapshotDef def; + char uuid_string[VIR_UUID_STRING_BUFLEN] = ""; + char *xml = NULL; + + memset(&def, 0, sizeof (virDomainSnapshotDef)); + + if (esxVI_EnsureSession(priv->host) < 0) { + goto failure; + } + + if (esxVI_LookupRootSnapshotTreeList(priv->host, snapshot->domain->uuid, + &rootSnapshotList) < 0 || + esxVI_GetSnapshotTreeByName(rootSnapshotList, snapshot->name, + &snapshotTree, &snapshotTreeParent, + esxVI_Occurrence_RequiredItem) < 0) { + goto failure; + } + + def.name = snapshot->name; + def.description = snapshotTree->description; + def.parent = snapshotTreeParent != NULL ? snapshotTreeParent->name : NULL; + + if (esxVI_DateTime_ConvertToCalendarTime(snapshotTree->createTime, + &def.creationTime) < 0) { + goto failure; + } + + def.state = esxVI_VirtualMachinePowerState_ConvertToLibvirt + (snapshotTree->state); + + virUUIDFormat(snapshot->domain->uuid, uuid_string); + + xml = virDomainSnapshotDefFormat(uuid_string, &def, 0); + + cleanup: + esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotList); + + return xml; + + failure: + VIR_FREE(xml); + + goto cleanup; +} +
Okay, I we will need to check if virDomainSnapshotDef ever grow to get new fields, but the memset should prevent problems anyway.
+ +static int +esxDomainSnapshotNum(virDomainPtr domain, unsigned int flags ATTRIBUTE_UNUSED) +{ [...] +} +
looks fine but we should probably raise an error if flags != 0 since this is not supported in this API level
+ +static int +esxDomainSnapshotListNames(virDomainPtr domain, char **names, int nameslen, + unsigned int flags ATTRIBUTE_UNUSED) +{ [..] +} +
same here
+ +static virDomainSnapshotPtr +esxDomainSnapshotLookupByName(virDomainPtr domain, const char *name, + unsigned int flags ATTRIBUTE_UNUSED) +{ + esxPrivate *priv = domain->conn->privateData; + esxVI_VirtualMachineSnapshotTree *rootSnapshotTreeList = NULL; + esxVI_VirtualMachineSnapshotTree *snapshotTree = NULL; + esxVI_VirtualMachineSnapshotTree *snapshotTreeParent = NULL; + virDomainSnapshotPtr snapshot = NULL; + + if (esxVI_EnsureSession(priv->host) < 0) { + goto failure; + } + + if (esxVI_LookupRootSnapshotTreeList(priv->host, domain->uuid, + &rootSnapshotTreeList) < 0 || + esxVI_GetSnapshotTreeByName(rootSnapshotTreeList, name, &snapshotTree, + &snapshotTreeParent, + esxVI_Occurrence_RequiredItem) < 0) { + goto failure; + } + + snapshot = virGetDomainSnapshot(domain, name); + + cleanup: + esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotTreeList); + + return snapshot; + + failure: + snapshot = NULL; + + goto cleanup; +} +
seems the failure path does a unecessary write to snapshot which may get pointed out by automatic tools
+ +static int +esxDomainHasCurrentSnapshot(virDomainPtr domain, + unsigned int flags ATTRIBUTE_UNUSED) +{ [...] +} +
flag check for 0
+static virDomainSnapshotPtr +esxDomainSnapshotCurrent(virDomainPtr domain, + unsigned int flags ATTRIBUTE_UNUSED)
flag check for 0
+{ +
extra empty line here it seems :-)
+ virDomainSnapshotPtr snapshot = NULL; + esxPrivate *priv = domain->conn->privateData; + esxVI_VirtualMachineSnapshotTree *currentSnapshotTree = NULL; + + if (esxVI_EnsureSession(priv->host) < 0) { + goto failure; + } + + if (esxVI_LookupCurrentSnapshotTree(priv->host, domain->uuid, + ¤tSnapshotTree, + esxVI_Occurrence_RequiredItem) < 0) { + goto failure; + } + + snapshot = virGetDomainSnapshot(domain, currentSnapshotTree->name); + + cleanup: + esxVI_VirtualMachineSnapshotTree_Free(¤tSnapshotTree); + + return snapshot; + + failure: + snapshot = NULL;
seems redundant here too
+ goto cleanup; +} + + + +static int +esxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, + unsigned int flags ATTRIBUTE_UNUSED) +{
flag check :-)
+ int result = 0; [...] +} + + + +static int +esxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) +{ [...] +} + + + [...] diff --git a/src/esx/esx_vi_generator.input b/src/esx/esx_vi_generator.input index 06dddbf..9c545eb 100644 --- a/src/esx/esx_vi_generator.input +++ b/src/esx/esx_vi_generator.input @@ -424,3 +424,15 @@ object VirtualMachineQuestionInfo ChoiceOption choice r VirtualMachineMessage message i end + + +object VirtualMachineSnapshotTree + ManagedObjectReference snapshot r + ManagedObjectReference vm r + String name r + String description r + DateTime createTime r + VirtualMachinePowerState state r + Boolean quiesced r + VirtualMachineSnapshotTree childSnapshotList ol +end
very concice :-) [...]
+int +esxVI_DateTime_ConvertToCalendarTime(esxVI_DateTime *dateTime, + time_t *secondsSinceEpoch) +{ + char value[64] = ""; + char *tmp; + struct tm tm; + int milliseconds; + char sign; + int tz_hours; + int tz_minutes; + int tz_offset = 0; + + if (dateTime == NULL || secondsSinceEpoch == NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + + if (virStrcpyStatic(value, dateTime->value) == NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, + _("xsd:dateTime value '%s' too long for destination"), + dateTime->value); + return -1; + }
Ouch, they are using XSD dateTime type !
+ /* expected format: 2010-04-05T12:13:55.316789+02:00 */ + tmp = strptime(value, "%Y-%m-%dT%H:%M:%S", &tm);
well timezone and fractional second are optional in XSD dateTime http://www.w3.org/TR/xmlschema-2/#dateTime so theorically some of those last fields my be missing and break this code :-\ the optional leading '-' should not be used here but ... [...]
+ /* + * xsd:dateTime represents local time relative to the timezone given + * as offset. pretend the local time is in UTC and use timegm in order
and optional :-)
+ * to avoid interference with the timezone to this computer. + * apply timezone correction afterwards, because it's simpler than + * handling all the possible over- and underflows when trying to apply + * it to the tm struct. + */ + *secondsSinceEpoch = timegm(&tm) - tz_offset; + + return 0; +} +
Nothing major, the flags attribute should be checked since we do that now, and a couple of cleanups. Once done, ACK thanks for getting there so fast :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/