Quoting Marc-André Lureau (2018-08-25 08:57:23) > Simplify the code around qmp_dispatch(): > - rely on qmp_dispatch/check_obj() for message checking > - have a single send_response() point > - constify send_response() argument > > It changes a couple of error messages: > > * When @req isn't a dictionary, from > Invalid JSON syntax > to > QMP input must be a JSON object > > * When @req lacks member "execute", from > this feature or command is not currently supported > to > QMP input lacks member 'execute' > > CC: Michael Roth <mdr...@linux.vnet.ibm.com> > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> > --- > qga/main.c | 47 +++++++++-------------------------------------- > 1 file changed, 9 insertions(+), 38 deletions(-) > > diff --git a/qga/main.c b/qga/main.c > index 6d70242d05..f0ec035996 100644 > --- a/qga/main.c > +++ b/qga/main.c > @@ -544,15 +544,15 @@ fail: > #endif > } > > -static int send_response(GAState *s, QDict *payload) > +static int send_response(GAState *s, const QDict *rsp) > { > const char *buf; > QString *payload_qstr, *response_qstr; > GIOStatus status; > > - g_assert(payload && s->channel); > + g_assert(rsp && s->channel); > > - payload_qstr = qobject_to_json(QOBJECT(payload)); > + payload_qstr = qobject_to_json(QOBJECT(rsp)); > if (!payload_qstr) { > return -EINVAL; > } > @@ -578,53 +578,24 @@ static int send_response(GAState *s, QDict *payload) > return 0; > } > > -static void process_command(GAState *s, QDict *req) > -{ > - QDict *rsp; > - int ret; > - > - g_assert(req); > - g_debug("processing command"); > - rsp = qmp_dispatch(&ga_commands, QOBJECT(req), false); > - if (rsp) { > - ret = send_response(s, rsp); > - if (ret < 0) { > - g_warning("error sending response: %s", strerror(-ret)); > - } > - qobject_unref(rsp); > - }
We used to check here for the success-response=false scenario (e.g. guest-shutdown qga command). Now we pass the result of qmp_dispatch() directly to send_response(), which looks like that might cause an assertion now. Do we need to add handling for this? > -} > - > /* handle requests/control events coming in over the channel */ > static void process_event(void *opaque, QObject *obj, Error *err) > { > GAState *s = opaque; > - QDict *req, *rsp; > + QDict *rsp; > int ret; > > g_debug("process_event: called"); > assert(!obj != !err); > if (err) { > - goto err; > - } > - req = qobject_to(QDict, obj); > - if (!req) { > - error_setg(&err, "Input must be a JSON object"); > - goto err; > - } > - if (!qdict_haskey(req, "execute")) { > - g_warning("unrecognized payload format"); > - error_setg(&err, QERR_UNSUPPORTED); > - goto err; > + rsp = qmp_error_response(err); > + goto end; > } > > - process_command(s, req); > - qobject_unref(obj); > - return; > + g_debug("processing command"); > + rsp = qmp_dispatch(&ga_commands, obj, false); > > -err: > - g_warning("failed to parse event: %s", error_get_pretty(err)); > - rsp = qmp_error_response(err); > +end: > ret = send_response(s, rsp); > if (ret < 0) { > g_warning("error sending error response: %s", strerror(-ret)); > -- > 2.18.0.547.g1d89318c48 >