* [Qemu-devel] Changes since v2 -qga: Add `guest-get-timezone` command @ 2017-03-22 17:16 Vinzenz 'evilissimo' Feenstra 2017-03-22 17:16 ` [Qemu-devel] [PATCH v2] qga: " Vinzenz 'evilissimo' Feenstra 0 siblings, 1 reply; 5+ messages in thread From: Vinzenz 'evilissimo' Feenstra @ 2017-03-22 17:16 UTC (permalink / raw) To: qemu-devel; +Cc: eblake, mdroth Updated the documentation according to comments of eblake ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v2] qga: Add `guest-get-timezone` command 2017-03-22 17:16 [Qemu-devel] Changes since v2 -qga: Add `guest-get-timezone` command Vinzenz 'evilissimo' Feenstra @ 2017-03-22 17:16 ` Vinzenz 'evilissimo' Feenstra 2017-03-23 12:40 ` sameeh jubran 0 siblings, 1 reply; 5+ messages in thread From: Vinzenz 'evilissimo' Feenstra @ 2017-03-22 17:16 UTC (permalink / raw) To: qemu-devel; +Cc: eblake, mdroth, Vinzenz Feenstra From: Vinzenz Feenstra <vfeenstr@redhat.com> Adds a new command `guest-get-timezone` reporting the currently configured timezone on the system. The information on what timezone is currently is configured is useful in case of Windows VMs where the offset of the hardware clock is required to have the same offset. This can be used for management systems like `oVirt` to detect the timezone difference and warn administrators of the misconfiguration. Signed-off-by: Vinzenz Feenstra <vfeenstr@redhat.com> --- qga/commands.c | 19 +++++++++++++++++++ qga/qapi-schema.json | 26 ++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/qga/commands.c b/qga/commands.c index 4d92946..83d7f99 100644 --- a/qga/commands.c +++ b/qga/commands.c @@ -499,3 +499,22 @@ int ga_parse_whence(GuestFileWhence *whence, Error **errp) error_setg(errp, "invalid whence code %"PRId64, whence->u.value); return -1; } + +GuestTimezone *qmp_guest_get_timezone(Error **errp) +{ + GuestTimezone *info = g_new0(GuestTimezone, 1); + GTimeZone *tz = g_time_zone_new_local(); + gint32 interval = g_time_zone_find_interval(tz, G_TIME_TYPE_STANDARD, 0); + gchar const *name = g_time_zone_get_abbreviation(tz, interval); + if (name != NULL) { + info->offset = g_time_zone_get_offset(tz, interval) / 60; + info->zone = g_strdup(name); + } else { + error_setg(errp, "Timezone lookup failed"); + g_free(info); + info = NULL; + } + g_time_zone_unref(tz); + return info; +} + diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index a02dbf2..62d6909 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -1042,3 +1042,29 @@ 'data': { 'path': 'str', '*arg': ['str'], '*env': ['str'], '*input-data': 'str', '*capture-output': 'bool' }, 'returns': 'GuestExec' } + + +## +# @GuestTimezone: +# +# @zone: Timezone name +# @offset: Offset to UTC in minutes. For western timezones the offset has a +# negative value and for eastern the offset is positive value +# +# Since: 2.10 +## +{ 'struct': 'GuestTimezone', + 'data': { 'zone': 'str', 'offset': 'int' } } + + +## +# @guest-get-timezone: +# +# Retrieves the timezone information from the guest. +# +# Returns: A GuestTimezone dictionary. +# +# Since: 2.10 +## +{ 'command': 'guest-get-timezone', + 'returns': 'GuestTimezone' } -- 2.9.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2] qga: Add `guest-get-timezone` command 2017-03-22 17:16 ` [Qemu-devel] [PATCH v2] qga: " Vinzenz 'evilissimo' Feenstra @ 2017-03-23 12:40 ` sameeh jubran 2017-03-23 12:51 ` Marc-André Lureau 0 siblings, 1 reply; 5+ messages in thread From: sameeh jubran @ 2017-03-23 12:40 UTC (permalink / raw) To: Vinzenz 'evilissimo' Feenstra, qemu-devel; +Cc: eblake, mdroth I have tested the patch on Windows 2012 R2 and it seems to be working good. Please see some comments inline. On 3/22/2017 7:16 PM, Vinzenz 'evilissimo' Feenstra wrote: > From: Vinzenz Feenstra <vfeenstr@redhat.com> > > Adds a new command `guest-get-timezone` reporting the currently > configured timezone on the system. The information on what timezone is > currently is configured is useful in case of Windows VMs where the > offset of the hardware clock is required to have the same offset. This > can be used for management systems like `oVirt` to detect the timezone > difference and warn administrators of the misconfiguration. > > Signed-off-by: Vinzenz Feenstra <vfeenstr@redhat.com> > --- > qga/commands.c | 19 +++++++++++++++++++ > qga/qapi-schema.json | 26 ++++++++++++++++++++++++++ > 2 files changed, 45 insertions(+) > > diff --git a/qga/commands.c b/qga/commands.c > index 4d92946..83d7f99 100644 > --- a/qga/commands.c > +++ b/qga/commands.c > @@ -499,3 +499,22 @@ int ga_parse_whence(GuestFileWhence *whence, Error **errp) > error_setg(errp, "invalid whence code %"PRId64, whence->u.value); > return -1; > } > + > +GuestTimezone *qmp_guest_get_timezone(Error **errp) > +{ > + GuestTimezone *info = g_new0(GuestTimezone, 1); Check for null > + GTimeZone *tz = g_time_zone_new_local(); Check for null > + gint32 interval = g_time_zone_find_interval(tz, G_TIME_TYPE_STANDARD, 0); Check if interval is -1 > + gchar const *name = g_time_zone_get_abbreviation(tz, interval); > + if (name != NULL) { > + info->offset = g_time_zone_get_offset(tz, interval) / 60; g_time_zone_get_offset returns the offset in seconds, dividing by 60 will give us the offset in minutes. Is there a reason to display it in minutes instead of hours? Either way I think you should display the units in the output. And 60 is a magic number, Even though it is very clear, I think it is better to add a define for that or check if glib already has one. > + info->zone = g_strdup(name); > + } else { > + error_setg(errp, "Timezone lookup failed"); > + g_free(info); > + info = NULL; > + } > + g_time_zone_unref(tz); > + return info; > +} > + > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json > index a02dbf2..62d6909 100644 > --- a/qga/qapi-schema.json > +++ b/qga/qapi-schema.json > @@ -1042,3 +1042,29 @@ > 'data': { 'path': 'str', '*arg': ['str'], '*env': ['str'], > '*input-data': 'str', '*capture-output': 'bool' }, > 'returns': 'GuestExec' } > + > + > +## > +# @GuestTimezone: > +# > +# @zone: Timezone name > +# @offset: Offset to UTC in minutes. For western timezones the offset has a > +# negative value and for eastern the offset is positive value > +# > +# Since: 2.10 > +## > +{ 'struct': 'GuestTimezone', > + 'data': { 'zone': 'str', 'offset': 'int' } } > + > + > +## > +# @guest-get-timezone: > +# > +# Retrieves the timezone information from the guest. > +# > +# Returns: A GuestTimezone dictionary. > +# > +# Since: 2.10 > +## > +{ 'command': 'guest-get-timezone', > + 'returns': 'GuestTimezone' } ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2] qga: Add `guest-get-timezone` command 2017-03-23 12:40 ` sameeh jubran @ 2017-03-23 12:51 ` Marc-André Lureau 2017-03-23 13:26 ` Vinzenz Feenstra 0 siblings, 1 reply; 5+ messages in thread From: Marc-André Lureau @ 2017-03-23 12:51 UTC (permalink / raw) To: sameeh jubran, Vinzenz 'evilissimo' Feenstra, qemu-devel; +Cc: mdroth Hi On Thu, Mar 23, 2017 at 4:41 PM sameeh jubran <sameeh@daynix.com> wrote: > I have tested the patch on Windows 2012 R2 and it seems to be working good. > > Please see some comments inline. > > > On 3/22/2017 7:16 PM, Vinzenz 'evilissimo' Feenstra wrote: > > From: Vinzenz Feenstra <vfeenstr@redhat.com> > > > > Adds a new command `guest-get-timezone` reporting the currently > > configured timezone on the system. The information on what timezone is > > currently is configured is useful in case of Windows VMs where the > > offset of the hardware clock is required to have the same offset. This > > can be used for management systems like `oVirt` to detect the timezone > > difference and warn administrators of the misconfiguration. > > > > Signed-off-by: Vinzenz Feenstra <vfeenstr@redhat.com> > > --- > > qga/commands.c | 19 +++++++++++++++++++ > > qga/qapi-schema.json | 26 ++++++++++++++++++++++++++ > > 2 files changed, 45 insertions(+) > > > > diff --git a/qga/commands.c b/qga/commands.c > > index 4d92946..83d7f99 100644 > > --- a/qga/commands.c > > +++ b/qga/commands.c > > @@ -499,3 +499,22 @@ int ga_parse_whence(GuestFileWhence *whence, Error > **errp) > > error_setg(errp, "invalid whence code %"PRId64, whence->u.value); > > return -1; > > } > > + > > +GuestTimezone *qmp_guest_get_timezone(Error **errp) > > +{ > > + GuestTimezone *info = g_new0(GuestTimezone, 1); > Check for null > > + GTimeZone *tz = g_time_zone_new_local(); > Check for null > > + gint32 interval = g_time_zone_find_interval(tz, > G_TIME_TYPE_STANDARD, 0); > Check if interval is -1 > > + gchar const *name = g_time_zone_get_abbreviation(tz, interval); > > + if (name != NULL) { > > + info->offset = g_time_zone_get_offset(tz, interval) / 60; > g_time_zone_get_offset returns the offset in seconds, dividing by 60 > will give us the offset in minutes. > Is there a reason to display it in minutes instead of hours? > Either way I think you should display the units in the output. > > And 60 is a magic number, Even though it is very clear, I think it is > better to add a define for that or check if glib already has one. > Instead, why not follow glib API and return seconds? > > + info->zone = g_strdup(name); > > + } else { > > + error_setg(errp, "Timezone lookup failed"); > > + g_free(info); > > + info = NULL; > > + } > > + g_time_zone_unref(tz); > > + return info; > > +} > > + > > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json > > index a02dbf2..62d6909 100644 > > --- a/qga/qapi-schema.json > > +++ b/qga/qapi-schema.json > > @@ -1042,3 +1042,29 @@ > > 'data': { 'path': 'str', '*arg': ['str'], '*env': ['str'], > > '*input-data': 'str', '*capture-output': 'bool' }, > > 'returns': 'GuestExec' } > > + > > + > > +## > > +# @GuestTimezone: > > +# > > +# @zone: Timezone name > > +# @offset: Offset to UTC in minutes. For western timezones the offset > has a > > +# negative value and for eastern the offset is positive value > > +# > > +# Since: 2.10 > > +## > > +{ 'struct': 'GuestTimezone', > > + 'data': { 'zone': 'str', 'offset': 'int' } } > > + > > + > > +## > > +# @guest-get-timezone: > > +# > > +# Retrieves the timezone information from the guest. > > +# > > +# Returns: A GuestTimezone dictionary. > > +# > > +# Since: 2.10 > > +## > > +{ 'command': 'guest-get-timezone', > > + 'returns': 'GuestTimezone' } > > -- Marc-André Lureau ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2] qga: Add `guest-get-timezone` command 2017-03-23 12:51 ` Marc-André Lureau @ 2017-03-23 13:26 ` Vinzenz Feenstra 0 siblings, 0 replies; 5+ messages in thread From: Vinzenz Feenstra @ 2017-03-23 13:26 UTC (permalink / raw) To: Marc-André Lureau; +Cc: sameeh jubran, QEMU Developers, mdroth > On Mar 23, 2017, at 1:51 PM, Marc-André Lureau <marcandre.lureau@gmail.com> wrote: > > Hi > > On Thu, Mar 23, 2017 at 4:41 PM sameeh jubran <sameeh@daynix.com <mailto:sameeh@daynix.com>> wrote: > I have tested the patch on Windows 2012 R2 and it seems to be working good. > > Please see some comments inline. > > > On 3/22/2017 7:16 PM, Vinzenz 'evilissimo' Feenstra wrote: > > From: Vinzenz Feenstra <vfeenstr@redhat.com <mailto:vfeenstr@redhat.com>> > > > > Adds a new command `guest-get-timezone` reporting the currently > > configured timezone on the system. The information on what timezone is > > currently is configured is useful in case of Windows VMs where the > > offset of the hardware clock is required to have the same offset. This > > can be used for management systems like `oVirt` to detect the timezone > > difference and warn administrators of the misconfiguration. > > > > Signed-off-by: Vinzenz Feenstra <vfeenstr@redhat.com <mailto:vfeenstr@redhat.com>> > > --- > > qga/commands.c | 19 +++++++++++++++++++ > > qga/qapi-schema.json | 26 ++++++++++++++++++++++++++ > > 2 files changed, 45 insertions(+) > > > > diff --git a/qga/commands.c b/qga/commands.c > > index 4d92946..83d7f99 100644 > > --- a/qga/commands.c > > +++ b/qga/commands.c > > @@ -499,3 +499,22 @@ int ga_parse_whence(GuestFileWhence *whence, Error **errp) > > error_setg(errp, "invalid whence code %"PRId64, whence->u.value); > > return -1; > > } > > + > > +GuestTimezone *qmp_guest_get_timezone(Error **errp) > > +{ > > + GuestTimezone *info = g_new0(GuestTimezone, 1); > Check for null > > + GTimeZone *tz = g_time_zone_new_local(); > Check for null > > + gint32 interval = g_time_zone_find_interval(tz, G_TIME_TYPE_STANDARD, 0); > Check if interval is -1 Will add the checks above in v3 > > + gchar const *name = g_time_zone_get_abbreviation(tz, interval); > > + if (name != NULL) { > > + info->offset = g_time_zone_get_offset(tz, interval) / 60; > g_time_zone_get_offset returns the offset in seconds, dividing by 60 > will give us the offset in minutes. > Is there a reason to display it in minutes instead of hours? Yes, there are offsets that are in minutes, some are 45 minutes, some 30 minutes so hours aren’t enough. > Either way I think you should display the units in the output. > > And 60 is a magic number, Even though it is very clear, I think it is > better to add a define for that or check if glib already has one. > > Instead, why not follow glib API and return seconds? In an earlier iteration I did use a Windows API function and the offset value in Windows is returned in minutes. See here the documentation for the ‘Bias’ fields: https://msdn.microsoft.com/en-us/library/windows/desktop/ms725481(v=vs.85).aspx <https://msdn.microsoft.com/en-us/library/windows/desktop/ms725481(v=vs.85).aspx> That was for me the main motivation, plus the seconds granularity is not required for the offsets, minutes is granular enough. However I could change it to seconds of course, seems more reasonable in the light of the glib support for it. > > > + info->zone = g_strdup(name); > > + } else { > > + error_setg(errp, "Timezone lookup failed"); > > + g_free(info); > > + info = NULL; > > + } > > + g_time_zone_unref(tz); > > + return info; > > +} > > + > > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json > > index a02dbf2..62d6909 100644 > > --- a/qga/qapi-schema.json > > +++ b/qga/qapi-schema.json > > @@ -1042,3 +1042,29 @@ > > 'data': { 'path': 'str', '*arg': ['str'], '*env': ['str'], > > '*input-data': 'str', '*capture-output': 'bool' }, > > 'returns': 'GuestExec' } > > + > > + > > +## > > +# @GuestTimezone: > > +# > > +# @zone: Timezone name > > +# @offset: Offset to UTC in minutes. For western timezones the offset has a > > +# negative value and for eastern the offset is positive value > > +# > > +# Since: 2.10 > > +## > > +{ 'struct': 'GuestTimezone', > > + 'data': { 'zone': 'str', 'offset': 'int' } } > > + > > + > > +## > > +# @guest-get-timezone: > > +# > > +# Retrieves the timezone information from the guest. > > +# > > +# Returns: A GuestTimezone dictionary. > > +# > > +# Since: 2.10 > > +## > > +{ 'command': 'guest-get-timezone', > > + 'returns': 'GuestTimezone' } > > -- > Marc-André Lureau ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-23 13:26 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-03-22 17:16 [Qemu-devel] Changes since v2 -qga: Add `guest-get-timezone` command Vinzenz 'evilissimo' Feenstra 2017-03-22 17:16 ` [Qemu-devel] [PATCH v2] qga: " Vinzenz 'evilissimo' Feenstra 2017-03-23 12:40 ` sameeh jubran 2017-03-23 12:51 ` Marc-André Lureau 2017-03-23 13:26 ` Vinzenz Feenstra
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).