Quoting Basil Salman (2019-01-13 04:05:29) > Sometimes qemu-ga fails to send a response to client due to memory allocation > issues due to a large response message, this can be experienced while trying > to read large number of bytes using QMP command guest-file-read.
send_response has 2 areas that can fail: 1) When formatting the QDict *rsp from qmp_dispatch() into JSON via qobject_to_json(): payload_qstr = qobject_to_json(QOBJECT(payload)); if (!payload_qstr) { return -EINVAL; } But we can only reach that via qobject_to_json() calling qstring_new() and that returning NULL. The qstring's initial size is independent of the actual payload size. So I don't see how a large read would induce this. There is other code in qobject_to_json() -> to_json() that could maybe hit an allocation failure once it start converting the payload to JSON, but AFAICT that would cause a crash of qemu/qemu-ga once g_realloc() finally fails to grow the qstring in qstring_append(), not an error return. So I don't think it's a bad idea to generate an error response like you're doing in your patch for future cases, but I don't see how it is reachable in the current code (even without the fix in patch 1). Do you have a particular reproducer for this specific failure? Are you sure it wasn't just the entire guest agent process crashing? 2) The other error you can get from send_response() is if there's a problem writing things out to the actual communication channel, in which case sending another error response likely won't help. > > Added a check to send an error response to qemu-ga client in such cases. > > Signed-off-by: Basil Salman <ba...@daynix.com> > --- > qga/main.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/qga/main.c b/qga/main.c > index 87a0711c14..964275c40c 100644 > --- a/qga/main.c > +++ b/qga/main.c > @@ -561,6 +561,8 @@ static void process_command(GAState *s, QDict *req) > { > QDict *rsp; > int ret; > + QDict *ersp; > + Error *err = NULL; > > g_assert(req); > g_debug("processing command"); > @@ -569,9 +571,20 @@ static void process_command(GAState *s, QDict *req) > ret = send_response(s, rsp); > if (ret < 0) { > g_warning("error sending response: %s", strerror(-ret)); > + goto err; > } > qobject_unref(rsp); > } > + return; > +err: > + error_setg(&err, "Insufficient system resources exist to " > + "complete the requested service"); > + ersp = qmp_error_response(err); > + ret = send_response(s, ersp); > + if (ret < 0) { > + g_warning("error sending error response: %s", strerror(-ret)); > + } > + qobject_unref(ersp); > } > > /* handle requests/control events coming in over the channel */ > -- > 2.17.2 >