| Message ID | 20140602175345.235004414@amt.cnet |
|---|---|
| State | New |
| Headers | show |
On 06/02/2014 11:51 AM, mtosatti@redhat.com wrote: > It is necessary to reset RTC interrupt reinjection backlog if > guest time is synchronized via a different mechanism, such as > QGA's guest-set-time command. > > Failing to do so causes both corrections to be applied (summed), > resulting in an incorrect guest time. > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > > Index: qemu/hw/timer/mc146818rtc.c Still no --- separator between your commit message and the patch body. Are you using 'git send-email'? > Index: qemu/qapi-schema.json > =================================================================== > --- qemu.orig/qapi-schema.json > +++ qemu/qapi-schema.json > @@ -4722,3 +4722,15 @@ > 'btn' : 'InputBtnEvent', > 'rel' : 'InputMoveEvent', > 'abs' : 'InputMoveEvent' } } > + > +## > +# @: rtc-reset-reinjection s/: // to resemble most other commands > +# > +# This command will reset RTC's interrupt reinjection backlog. s/RTC's/the RTC/ > +# Can be used if another mechanism to synchronize guest time > +# is in effect, for example QEMU guest agents guest-set-time s/agents/agent's/ > +# command. > +# > +# Since: 2.1 > +## > +{ 'command': 'rtc-reset-reinjection' } > Index: qemu/qmp-commands.hx > =================================================================== > --- qemu.orig/qmp-commands.hx > +++ qemu/qmp-commands.hx > @@ -3572,3 +3572,26 @@ Example: > } } ] } > > EQMP > + > +#if defined (TARGET_I386) > + { > + .name = "rtc_reset_reinjection", s/rtc_reset_reinjection/rtc-reset-reinjection/ > + .args_type = "", > + .mhandler.cmd_new = qmp_marshal_input_rtc_reset_reinjection, > + }, > +#endif > + > +SQMP > +rtc-reset-reinjection > +---------------------
On Mon, Jun 02, 2014 at 01:31:29PM -0600, Eric Blake wrote: > On 06/02/2014 11:51 AM, mtosatti@redhat.com wrote: > > It is necessary to reset RTC interrupt reinjection backlog if > > guest time is synchronized via a different mechanism, such as > > QGA's guest-set-time command. > > > > Failing to do so causes both corrections to be applied (summed), > > resulting in an incorrect guest time. > > > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > > > > Index: qemu/hw/timer/mc146818rtc.c > > Still no --- separator between your commit message and the patch body. > Are you using 'git send-email'? OK, diffstats on every patch. > > Index: qemu/qapi-schema.json > > =================================================================== > > --- qemu.orig/qapi-schema.json > > +++ qemu/qapi-schema.json > > @@ -4722,3 +4722,15 @@ > > 'btn' : 'InputBtnEvent', > > 'rel' : 'InputMoveEvent', > > 'abs' : 'InputMoveEvent' } } > > + > > +## > > +# @: rtc-reset-reinjection > > s/: // to resemble most other commands Several commands have ":". What is the correct syntax and why? > > +# This command will reset RTC's interrupt reinjection backlog. > > s/RTC's/the RTC/ > > > +# Can be used if another mechanism to synchronize guest time > > +# is in effect, for example QEMU guest agents guest-set-time > > s/agents/agent's/ Fixed. > > +# command. > > +# > > +# Since: 2.1 > > +## > > +{ 'command': 'rtc-reset-reinjection' } > > Index: qemu/qmp-commands.hx > > =================================================================== > > --- qemu.orig/qmp-commands.hx > > +++ qemu/qmp-commands.hx > > @@ -3572,3 +3572,26 @@ Example: > > } } ] } > > > > EQMP > > + > > +#if defined (TARGET_I386) > > + { > > + .name = "rtc_reset_reinjection", > > s/rtc_reset_reinjection/rtc-reset-reinjection/ This is a function name.
On 06/02/2014 02:20 PM, Marcelo Tosatti wrote: > On Mon, Jun 02, 2014 at 01:31:29PM -0600, Eric Blake wrote: >> On 06/02/2014 11:51 AM, mtosatti@redhat.com wrote: >>> It is necessary to reset RTC interrupt reinjection backlog if >>> guest time is synchronized via a different mechanism, such as >>> QGA's guest-set-time command. >>> >>> + >>> +## >>> +# @: rtc-reset-reinjection >> >> s/: // to resemble most other commands > > Several commands have ":". What is the correct syntax and why? Alas, we don't have any automated program that strips these stylized comments and turns them into formal documentation. But the goal is that some day we might, at which point, being consistent in our style is the most likely to be successful. The prevalent style appears to be: ## # @command: # # Short summary # # @foo: describe mandatory option foo # # @bar: #optional describe optional option bar, and its default value # if omitted # # Returns: what to expect from the command # # Since: version it was introduced ## { 'command' ... } Although I will admit that '@command' vs. '@command:' didn't have a clear winner. Maybe someone with OCD wants to do a pure cleanup patch to get the file into a consistent state? Until then, I'm pointing out where things are definitely different (your '@: command' was an outlier) >>> EQMP >>> + >>> +#if defined (TARGET_I386) >>> + { >>> + .name = "rtc_reset_reinjection", >> >> s/rtc_reset_reinjection/rtc-reset-reinjection/ > > This is a function name. No, it is a QMP command name. See "send-key" for an example.
Index: qemu/hw/timer/mc146818rtc.c =================================================================== --- qemu.orig/hw/timer/mc146818rtc.c +++ qemu/hw/timer/mc146818rtc.c @@ -26,6 +26,7 @@ #include "sysemu/sysemu.h" #include "hw/timer/mc146818rtc.h" #include "qapi/visitor.h" +#include "qmp-commands.h" #ifdef TARGET_I386 #include "hw/i386/apic.h" @@ -84,6 +85,9 @@ typedef struct RTCState { Notifier clock_reset_notifier; LostTickPolicy lost_tick_policy; Notifier suspend_notifier; +#ifdef TARGET_I386 + QLIST_ENTRY(RTCState) link; +#endif } RTCState; static void rtc_set_time(RTCState *s); @@ -522,6 +526,20 @@ static void rtc_get_time(RTCState *s, st rtc_from_bcd(s, s->cmos_data[RTC_CENTURY]) * 100 - 1900; } +#ifdef TARGET_I386 +static QLIST_HEAD(, RTCState) rtc_devices = + QLIST_HEAD_INITIALIZER(rtc_devices); + +void qmp_rtc_reset_reinjection(Error **errp) +{ + RTCState *s; + + QLIST_FOREACH(s, &rtc_devices, link) { + s->irq_coalesced = 0; + } +} +#endif + static void rtc_set_time(RTCState *s) { struct tm tm; @@ -911,6 +929,10 @@ ISADevice *rtc_init(ISABus *bus, int bas } else { isa_init_irq(isadev, &s->irq, RTC_ISA_IRQ); } +#ifdef TARGET_I386 + QLIST_INSERT_HEAD(&rtc_devices, s, link); +#endif + return isadev; } Index: qemu/qapi-schema.json =================================================================== --- qemu.orig/qapi-schema.json +++ qemu/qapi-schema.json @@ -4722,3 +4722,15 @@ 'btn' : 'InputBtnEvent', 'rel' : 'InputMoveEvent', 'abs' : 'InputMoveEvent' } } + +## +# @: rtc-reset-reinjection +# +# This command will reset RTC's interrupt reinjection backlog. +# Can be used if another mechanism to synchronize guest time +# is in effect, for example QEMU guest agents guest-set-time +# command. +# +# Since: 2.1 +## +{ 'command': 'rtc-reset-reinjection' } Index: qemu/qmp-commands.hx =================================================================== --- qemu.orig/qmp-commands.hx +++ qemu/qmp-commands.hx @@ -3572,3 +3572,26 @@ Example: } } ] } EQMP + +#if defined (TARGET_I386) + { + .name = "rtc_reset_reinjection", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_input_rtc_reset_reinjection, + }, +#endif + +SQMP +rtc-reset-reinjection +--------------------- + +Reset RTC's interrupt reinjection backlog. + +Arguments: None. + +Example: + +-> { "execute": "rtc-reset-reinjection" } +<- { "return": {} } + +EQMP
It is necessary to reset RTC interrupt reinjection backlog if guest time is synchronized via a different mechanism, such as QGA's guest-set-time command. Failing to do so causes both corrections to be applied (summed), resulting in an incorrect guest time. Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>