[libvirt] [RFC PATCH 4/8] Snapshot internal methods.
Matthias Bolte
matthias.bolte at googlemail.com
Fri Apr 2 13:25:10 UTC 2010
2010/4/2 Chris Lalancette <clalance at redhat.com>:
> Signed-off-by: Chris Lalancette <clalance at 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?
> +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.
> + if (creation == NULL) {
> + /* there was no creation time in an existing snapshot; this
> + * should never happen
> + */
> + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("missing creationTime from existing snapshot"));
> + goto cleanup;
> + }
> +
> + if (strptime(creation, "%Y-%m-%d_%T", &time_info) == NULL) {
> + /* we saw a creation time we couldn't parse. We shouldn't
> + * ever see this, so throw an error
> + */
> + virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not convert '%s' to a time value"),
> + creation);
> + goto cleanup;
> + }
> + def->creationTime = mktime(&time_info);
mktime converts the time_info into Unix time in UTC and uses the
current timezone set for the system, doesn't it?
So you'll get different results in different timezones for the same
time string 2010-04-02_12:14:42.
Client and server in different timezones. With QEMU the time parsing
and formating is done on the server side in server local time and this
gets transfered to the client without further modification. With ESX I
request the snapshots createTime, that's in server local time
annotated with a timezone offset. I need to convert that into UTC and
assign it to def->creationTime. virDomainSnapshotDefFormat later on
will convert it into client local time, because the ESX driver is
client side. So you'll get different time string, even if both QEMU
and ESX take a snapshot in the same timezone at the same time, if the
client is in another timezone.
> + def->parent = virXPathString("string(./parent/name)", ctxt);
> +
> + state = virXPathString("string(./state)", ctxt);
> + if (state == NULL) {
> + /* there was no state in an existing snapshot; this
> + * should never happen
> + */
> + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("missing state from existing snapshot"));
> + goto cleanup;
> + }
> + def->state = virDomainStateTypeFromString(state);
> + if (def->state < 0) {
> + virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Invalid state '%s' in domain snapshot XML"),
> + state);
> + goto cleanup;
> + }
> + }
> + else {
> + gettimeofday(&tv, NULL);
> + def->creationTime = tv.tv_sec;
> + }
> +
> + ret = def;
> +
> +cleanup:
> + VIR_FREE(creation);
> + VIR_FREE(state);
> + if (ctxt)
> + xmlXPathFreeContext(ctxt);
> + if (ret == NULL)
> + virDomainSnapshotDefFree(def);
> + xmlFreeDoc(xml);
> +
> + return ret;
> +}
> +
> +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.
> + virBufferVSprintf(&buf, " <creationTime>%s</creationTime>\n", timestr);
Same note about the capital T applies here.
> + virBufferAddLit(&buf, " <domain>\n");
> + virBufferVSprintf(&buf, " <uuid>%s</uuid>\n", domain_uuid);
> + virBufferAddLit(&buf, " </domain>\n");
> + virBufferAddLit(&buf, "</domainsnapshot>\n");
> +
> + if (virBufferError(&buf)) {
> + virBufferFreeAndReset(&buf);
> + virReportOOMError();
> + return NULL;
> + }
> +
> + return virBufferContentAndReset(&buf);
> +}
> +
Matthias
More information about the libvir-list
mailing list