Message ID | 1357466820-12860-3-git-send-email-lilei@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
On 01/06/2013 03:06 AM, Lei Li wrote: > Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> > --- > qga/commands-posix.c | 12 ++++++++++++ > qga/qapi-schema.json | 17 +++++++++++++++++ > 2 files changed, 29 insertions(+), 0 deletions(-) > > +++ b/qga/qapi-schema.json > @@ -100,6 +100,23 @@ > 'utc-offset': 'int' } } > > ## > +# @guest-get-time: > +# > +# Get the information about host time in UTC and the > +# UTC offset. About the host time, or about the guest time? In other words, doesn't this command exist for the host to ask the guest what time the _guest_ thinks it is, so that the host can then decide whether to issue a followup command to tell the guest to adjust its time? > +# > +# This command tries to get the host time which is > +# presumably correct, since need to be able to resynchronize > +# clock to host in guest. > +# > +# Returns: @HostTimeInfo on success. For that matter, should we name the type in patch 1/3 'TimeInfo', instead of 'HostTimeInfo', as it is not intrinsically tied to host or guest, but more a function of who is being queried?
On Sun, 6 Jan 2013 18:06:59 +0800 Lei Li <lilei@linux.vnet.ibm.com> wrote: > Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> > --- > qga/commands-posix.c | 12 ++++++++++++ > qga/qapi-schema.json | 17 +++++++++++++++++ > 2 files changed, 29 insertions(+), 0 deletions(-) > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index 26b0fa0..190199d 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -109,6 +109,18 @@ static HostTimeInfo *get_host_time(void) > return host_time; > } > > +struct HostTimeInfo *qmp_guest_get_time(Error **errp) > +{ > + HostTimeInfo *host_time = get_host_time(); The command is called guest_get_time() and runs in the guest, but it returns HostTimeInfo. Is this correct? > + > + if (!host_time) { > + error_set(errp, QERR_QGA_COMMAND_FAILED, "Failed to get host time"); > + return NULL; > + } > + > + return host_time; > +} > + > typedef struct GuestFileHandle { > uint64_t id; > FILE *fh; > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json > index 7793aff..4a8b93c 100644 > --- a/qga/qapi-schema.json > +++ b/qga/qapi-schema.json > @@ -100,6 +100,23 @@ > 'utc-offset': 'int' } } > > ## > +# @guest-get-time: > +# > +# Get the information about host time in UTC and the > +# UTC offset. > +# > +# This command tries to get the host time which is > +# presumably correct, since need to be able to resynchronize > +# clock to host in guest. > +# > +# Returns: @HostTimeInfo on success. > +# > +# Since 1.4 > +## > +{ 'command': 'guest-get-time', > + 'returns': 'HostTimeInfo' } > + > +## > # @GuestAgentCommandInfo: > # > # Information about guest agent commands.
On 01/08/2013 06:04 AM, Eric Blake wrote: > On 01/06/2013 03:06 AM, Lei Li wrote: >> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> >> --- >> qga/commands-posix.c | 12 ++++++++++++ >> qga/qapi-schema.json | 17 +++++++++++++++++ >> 2 files changed, 29 insertions(+), 0 deletions(-) >> >> +++ b/qga/qapi-schema.json >> @@ -100,6 +100,23 @@ >> 'utc-offset': 'int' } } >> >> ## >> +# @guest-get-time: >> +# >> +# Get the information about host time in UTC and the >> +# UTC offset. > About the host time, or about the guest time? In other words, doesn't > this command exist for the host to ask the guest what time the _guest_ > thinks it is, so that the host can then decide whether to issue a > followup command to tell the guest to adjust its time? No, this command is for getting host time. You might want to take a look at the RFC and the reply from Mike I sent few days ago for suggestions and discussions. http://article.gmane.org/gmane.comp.emulators.qemu/186126 >> +# >> +# This command tries to get the host time which is >> +# presumably correct, since need to be able to resynchronize >> +# clock to host in guest. >> +# >> +# Returns: @HostTimeInfo on success. > For that matter, should we name the type in patch 1/3 'TimeInfo', > instead of 'HostTimeInfo', as it is not intrinsically tied to host or > guest, but more a function of who is being queried? Yes, it make sense. Luiz feel confused about this 'HostTimeInfo' too, I think 'TimeInfo' might be a good idea. :)
On 01/09/2013 09:33 PM, Luiz Capitulino wrote: > On Sun, 6 Jan 2013 18:06:59 +0800 > Lei Li <lilei@linux.vnet.ibm.com> wrote: > >> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> >> --- >> qga/commands-posix.c | 12 ++++++++++++ >> qga/qapi-schema.json | 17 +++++++++++++++++ >> 2 files changed, 29 insertions(+), 0 deletions(-) >> >> diff --git a/qga/commands-posix.c b/qga/commands-posix.c >> index 26b0fa0..190199d 100644 >> --- a/qga/commands-posix.c >> +++ b/qga/commands-posix.c >> @@ -109,6 +109,18 @@ static HostTimeInfo *get_host_time(void) >> return host_time; >> } >> >> +struct HostTimeInfo *qmp_guest_get_time(Error **errp) >> +{ >> + HostTimeInfo *host_time = get_host_time(); > The command is called guest_get_time() and runs in the guest, but it returns > HostTimeInfo. Is this correct? Okay, looks like this 'HostTimeInfo' brings a lots of confusion. I will change it to 'TimeInfo' as I replied to another patch. >> + >> + if (!host_time) { >> + error_set(errp, QERR_QGA_COMMAND_FAILED, "Failed to get host time"); >> + return NULL; >> + } >> + >> + return host_time; >> +} >> + >> typedef struct GuestFileHandle { >> uint64_t id; >> FILE *fh; >> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json >> index 7793aff..4a8b93c 100644 >> --- a/qga/qapi-schema.json >> +++ b/qga/qapi-schema.json >> @@ -100,6 +100,23 @@ >> 'utc-offset': 'int' } } >> >> ## >> +# @guest-get-time: >> +# >> +# Get the information about host time in UTC and the >> +# UTC offset. >> +# >> +# This command tries to get the host time which is >> +# presumably correct, since need to be able to resynchronize >> +# clock to host in guest. >> +# >> +# Returns: @HostTimeInfo on success. >> +# >> +# Since 1.4 >> +## >> +{ 'command': 'guest-get-time', >> + 'returns': 'HostTimeInfo' } >> + >> +## >> # @GuestAgentCommandInfo: >> # >> # Information about guest agent commands.
On Fri, Jan 11, 2013 at 03:37:26PM +0800, Lei Li wrote: > On 01/08/2013 06:04 AM, Eric Blake wrote: > >On 01/06/2013 03:06 AM, Lei Li wrote: > >>Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> > >>--- > >> qga/commands-posix.c | 12 ++++++++++++ > >> qga/qapi-schema.json | 17 +++++++++++++++++ > >> 2 files changed, 29 insertions(+), 0 deletions(-) > >> > >>+++ b/qga/qapi-schema.json > >>@@ -100,6 +100,23 @@ > >> 'utc-offset': 'int' } } > >> ## > >>+# @guest-get-time: > >>+# > >>+# Get the information about host time in UTC and the > >>+# UTC offset. > >About the host time, or about the guest time? In other words, doesn't > >this command exist for the host to ask the guest what time the _guest_ > >thinks it is, so that the host can then decide whether to issue a > >followup command to tell the guest to adjust its time? > > No, this command is for getting host time. You might want to take a look at > the RFC and the reply from Mike I sent few days ago for suggestions and > discussions. > > http://article.gmane.org/gmane.comp.emulators.qemu/186126 As mentioned elsewhere, what your guest-get-time implementation is actually returning is the guest time. But that's exactly what we want, since it provides all the information the host needs to calculate what value it should pass to guest-set-time (really we just need the utc-offset for this, but the other values are useful for other use cases. In fact, if you document guest-*-time to always return values relative to UTC 1970 epoch, we don't even actually need *that* unless we need to change the guest utc-offset for some reason, but again, nice to handle other use cases) > > >>+# > >>+# This command tries to get the host time which is > >>+# presumably correct, since need to be able to resynchronize > >>+# clock to host in guest. > >>+# > >>+# Returns: @HostTimeInfo on success. > >For that matter, should we name the type in patch 1/3 'TimeInfo', > >instead of 'HostTimeInfo', as it is not intrinsically tied to host or > >guest, but more a function of who is being queried? > > Yes, it make sense. Luiz feel confused about this 'HostTimeInfo' too, > I think 'TimeInfo' might be a good idea. :) Fourth'd :) > > > > -- > Lei >
diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 26b0fa0..190199d 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -109,6 +109,18 @@ static HostTimeInfo *get_host_time(void) return host_time; } +struct HostTimeInfo *qmp_guest_get_time(Error **errp) +{ + HostTimeInfo *host_time = get_host_time(); + + if (!host_time) { + error_set(errp, QERR_QGA_COMMAND_FAILED, "Failed to get host time"); + return NULL; + } + + return host_time; +} + typedef struct GuestFileHandle { uint64_t id; FILE *fh; diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index 7793aff..4a8b93c 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -100,6 +100,23 @@ 'utc-offset': 'int' } } ## +# @guest-get-time: +# +# Get the information about host time in UTC and the +# UTC offset. +# +# This command tries to get the host time which is +# presumably correct, since need to be able to resynchronize +# clock to host in guest. +# +# Returns: @HostTimeInfo on success. +# +# Since 1.4 +## +{ 'command': 'guest-get-time', + 'returns': 'HostTimeInfo' } + +## # @GuestAgentCommandInfo: # # Information about guest agent commands.
Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> --- qga/commands-posix.c | 12 ++++++++++++ qga/qapi-schema.json | 17 +++++++++++++++++ 2 files changed, 29 insertions(+), 0 deletions(-)