10:30 a.m.
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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/