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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list