2010/4/2 Chris Lalancette <clalance@xxxxxxxxxx>:
> On 04/02/2010 09:25 AM, Matthias Bolte wrote:
>> 2010/4/2 Chris Lalancette <clalance@xxxxxxxxxx>:
>>> Signed-off-by: Chris Lalancette <clalance@xxxxxxxxxx>
>>> ---
>>> 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
>>
>>> +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.
I missed currentMemory and since it's already a mixture of all styles
lets just keep 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.
>
> 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?
>
I think the problem is that struct tm doesn't contain a timezone
filed. Therefore, strptime parses a time that is not fixed in a
timezone. So even If we would use a time format like
2010-04-02_12:30:58+0200
it won't help with strptime. strptime knows %z for timezone part, but
just ignores it.
mktime operates in local time. The man page says:
"The mktime() function converts a broken-down time structure,
expressed as local time, to calendar time representation"
So gmtime_r doesn't help.
I think we could use seconds since the Epoch from time() as the actual
value, and attach a human readable version (from strftime with "%a, %d
%b %Y %H:%M:%S %z" in RFC 2822 format) in a comment like this:
<creationTime>1270221648</creationTime><!-- Fri, 02 Apr 2010
17:20:48 +0200 -->
That way we don't need to deal with the subtle timezone stuff as
seconds since the Epoch is in UTC and we have a human readable version
in virsh snapshot-dumpxml for example.
Applications then can take the seconds since the Epoch value and
format it as they like to local time or what ever they prefer.
Matthias
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list