Re: [libvirt] [RFC PATCH 4/8] Snapshot internal methods.

2010/4/2 Chris Lalancette <clalance(a)redhat.com&gt;: > Signed-off-by: Chris Lalancette <clalance(a)redhat.com&gt; > --- > 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?

> +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.

> +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.