On 04/02/2010 09:25 AM, Matthias Bolte wrote:
2010/4/2 Chris Lalancette <clalance@redhat.com>:
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/conf/domain_conf.c | 402 ++++++++++++++++++++++++++++++++++++++++++++-- src/conf/domain_conf.h | 53 ++++++ src/libvirt_private.syms | 10 ++ 3 files changed, 455 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e260dce..1971b9a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c
@@ -6096,22 +6101,25 @@ static virDomainObjPtr virDomainLoadConfig(virCapsPtr caps,
if ((configFile = virDomainConfigFile(configDir, name)) == NULL) goto error; - if ((autostartLink = virDomainConfigFile(autostartDir, name)) == NULL) - goto error; - - if ((autostart = virFileLinkPointsTo(autostartLink, configFile)) < 0) - goto error; - if (!(def = virDomainDefParseFile(caps, configFile, VIR_DOMAIN_XML_INACTIVE))) goto error;
- if ((dom = virDomainFindByName(doms, def->name))) { - virDomainObjUnlock(dom); - dom = NULL; - newVM = 0; + /* if the domain is already in our hashtable, we don't need to do + * anything further + */ + if ((dom = virDomainFindByUUID(doms, def->uuid))) { + VIR_FREE(configFile); + virDomainDefFree(def); + return dom; }
+ if ((autostartLink = virDomainConfigFile(autostartDir, name)) == NULL) + goto error; + + if ((autostart = virFileLinkPointsTo(autostartLink, configFile)) < 0) + goto error; + if (!(dom = virDomainAssignDef(caps, doms, def, false))) goto error;
What's the reason for this reordering? It seems to be independent from snapshots. Maybe it should go into a separate patch?
Oops, yeah, I forgot to split that out. It's either a feature of def/newDef that I don't understand, or it's a bugfix. That being said, I'm not sure I even need this bugfix anymore, so I'll retest and split it into a separate patch if I still need it.
+virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, + int newSnapshot) +{ + xmlXPathContextPtr ctxt = NULL; + xmlDocPtr xml = NULL; + xmlNodePtr root; + virDomainSnapshotDefPtr def = NULL; + virDomainSnapshotDefPtr ret = NULL; + char *creation = NULL, *state = NULL; + struct timeval tv; + struct tm time_info; + char timestr[100]; + + xml = virXMLParse(NULL, xmlStr, "domainsnapshot.xml"); + if (!xml) { + virDomainReportError(VIR_ERR_XML_ERROR, + "%s",_("failed to parse snapshot xml document")); + return NULL; + } + + if ((root = xmlDocGetRootElement(xml)) == NULL) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("missing root element")); + goto cleanup; + } + + if (!xmlStrEqual(root->name, BAD_CAST "domainsnapshot")) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("incorrect root element")); + goto cleanup; + } + + ctxt = xmlXPathNewContext(xml); + if (ctxt == NULL) { + virReportOOMError(); + goto cleanup; + } + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + goto cleanup; + } + + ctxt->node = root; + + def->name = virXPathString("string(./name)", ctxt); + if (def->name == NULL) { + /* make up a name */ + gettimeofday(&tv, NULL); + localtime_r(&tv.tv_sec, &time_info); + strftime(timestr, sizeof(timestr), "%F_%T", &time_info); + def->name = strdup(timestr); + } + if (def->name == NULL) { + virReportOOMError(); + goto cleanup; + } + + def->description = virXPathString("string(./description)", ctxt); + + if (!newSnapshot) { + creation = virXPathString("string(./creationTime)", ctxt);
I think it should be creationtime or creation_time, but not creationTime.
Taking the domain XML as an example, all 3 styles are used (e.g. currentMemory, on_poweroff, and seclabel). I don't really care too much, so I'll do whatever is more comfortable. <snip>
+char *virDomainSnapshotDefFormat(char *domain_uuid, + virDomainSnapshotDefPtr def) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char timestr[100]; + struct tm time_info; + + virBufferAddLit(&buf, "<domainsnapshot>\n"); + virBufferVSprintf(&buf, " <name>%s</name>\n", def->name); + if (def->description) + virBufferVSprintf(&buf, " <description>%s</description>\n", + def->description); + virBufferVSprintf(&buf, " <state>%s</state>\n", + virDomainStateTypeToString(def->state)); + if (def->parent) { + virBufferAddLit(&buf, " <parent>\n"); + virBufferVSprintf(&buf, " <name>%s</name>\n", def->parent); + virBufferAddLit(&buf, " </parent>\n"); + } + localtime_r(&def->creationTime, &time_info); + strftime(timestr, sizeof(timestr), "%F_%T", &time_info);
Again, you handle the time in local time. You could use %F_%T%z to include the local time zone and then handle that in the parsing function to get a correct UTC time back.
Maybe a better solution is to just store the Unix time in seconds in UTC (as returned by the time() function) in the XML and let applications do the conversion into a more human readable format and take care of timezone stuff. This way we get rid of the timezone problem at the libvirt level at all.
Well, I was sort of shooting for that by defining the field to be UTC (which I obviously mis-implemented with localtime_r). It would be nice to have a human-readable string in the XML, though, which is why I didn't just use seconds since the Epoch. Can I just use gmtime_r() here to get the time in UTC, and then strptime and mktime above will do the right thing? -- Chris Lalancette