On 06/05/2014 06:22 AM, Wenchao Xia wrote:
This patch also eliminates build time warning caused by no caller
of monitor_qapi_event_throttle().
Again, my suggestion on 6/29 could avoid that warning; if you use that
workaround, don't clean it until 29/29, but you can drop this paragraph
of this commit.
Signed-off-by: Wenchao Xia <address@hidden>
---
docs/qmp/qmp-events.txt | 16 ----------------
hw/ppc/spapr_rtas.c | 3 ++-
hw/timer/mc146818rtc.c | 3 ++-
include/sysemu/sysemu.h | 2 --
monitor.c | 4 +++-
qapi-event.json | 13 +++++++++++++
vl.c | 9 ---------
7 files changed, 20 insertions(+), 30 deletions(-)
Yay - the first event with data, so I get to see what the generator does.
The .h file shows this signature:
void qapi_event_send_rtc_change(int64_t offset,
Error **errp);
and the .c has this code:
void qapi_event_send_rtc_change(int64_t offset,
Error **errp)
{
QDict *qmp;
Error *local_err = NULL;
QMPEventFuncEmit emit;
QmpOutputVisitor *qov;
Visitor *v;
QObject *obj;
emit = qmp_event_get_func_emit();
if (!emit) {
return;
}
qmp = qmp_event_build_dict("RTC_CHANGE");
qov = qmp_output_visitor_new();
g_assert(qov);
v = qmp_output_get_visitor(qov);
g_assert(v);
/* Fake visit, as if all member are under a structure */
Grammar error; guess I have more comments on 3/29.
visit_start_struct(v, NULL, "", "RTC_CHANGE", 0, &local_err);
if (local_err) {
goto clean;
}
Hmm, qmp_output_start_struct() never sets errp.
visit_type_int(v, &offset, "offset", &local_err);
if (local_err) {
goto clean;
}
Likewise, qmp_output_type_int never sets errp.
visit_end_struct(v, &local_err);
if (local_err) {
goto clean;
}
And neither does qmp_end_struct.
obj = qmp_output_get_qobject(qov);
g_assert(obj != NULL);
qdict_put_obj(qmp, "data", obj);
emit(QAPI_EVENT_RTC_CHANGE, qmp, &local_err);
And I already mentioned earlier that the only implementation of the
emit() callback never sets local_err (and probably doesn't even need it
as a parameter).
clean:
qmp_output_visitor_cleanup(qov);
error_propagate(errp, local_err);
QDECREF(qmp);
}
If you change the earlier 3 instances to just use visit_...(,
&error_abort), you can completely ditch the local_err variable, drop the
clean: label, and overall have a much shorter generated function.
@@ -93,7 +94,7 @@ static void rtas_set_time_of_day(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
tm.tm_sec = rtas_ld(args, 5);
/* Just generate a monitor event for the change */
- rtc_change_mon_event(&tm);
+ qapi_event_send_rtc_change(qemu_timedate_diff(&tm), NULL);
spapr->rtc_offset = qemu_timedate_diff(&tm);
Eww. This makes me worry whether grabbing qemu_timedate_diff() two times
in a row may cause a different value to be reported than what is stored
locally. Please grab it only once into a local variable, then copy that
value into both locations.
Once again, all callers of qapi_event_send_rtc_change() are passing a
NULL errp to silently ignore errors; and I just audited that no errors
happen anyways.
+ monitor_qapi_event_throttle(QAPI_EVENT_RTC_CHANGE, 1000);
+
qmp_event_set_func_emit(monitor_qapi_event_queue);
}
@@ -737,7 +740,6 @@ monitor_protocol_event_throttle(MonitorEvent event,
static void monitor_protocol_event_init(void)
{
/* Limit RTC & BALLOON events to 1 per second */
- monitor_protocol_event_throttle(QEVENT_RTC_CHANGE, 1000);
monitor_protocol_event_throttle(QEVENT_BALLOON_CHANGE, 1000);
monitor_protocol_event_throttle(QEVENT_WATCHDOG, 1000);
...furthermore, the OLD comment was wrong (it forgot about the watchdog
event). You could get around that by rewording the comment to just say
'limit guest-triggerable events to 1 per second' without calling out
what those events are.
+# @RTC_CHANGE
+#
+# Emitted when the guest changes the RTC time.
+#
+# @offset: offset between base RTC clock (as specified by -rtc base), and
+# new RTC clock value
+#
+# Since: 2.1
0.14.0