Re: [PATCH] qga: fix assert regression on guest-shutdown

[Christian Ehrhardt]

On Tue, Jun 9, 2020 at 1:15 PM Christian Ehrhardt <christian.ehrhardt@canonical.com> wrote:

On Thu, Jun 4, 2020 at 3:43 PM Christian Ehrhardt <christian.ehrhardt@canonical.com> wrote:

On Thu, Jun 4, 2020 at 11:46 AM Marc-André Lureau <marcandre.lureau@redhat.com> wrote:

Since commit 781f2b3d1e ("qga: process_event() simplification"),
send_response() is called unconditionally, but will assert when "rsp" is
NULL. This may happen with QCO_NO_SUCCESS_RESP commands, such as
"guest-shutdown".

Fixes: 781f2b3d1e5ef389b44016a897fd55e7a780bf35
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Reported-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qga/main.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qga/main.c b/qga/main.c
index f0e454f28d3..3febf3b0fdf 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -531,7 +531,11 @@ static int send_response(GAState *s, const QDict *rsp)
     QString *payload_qstr, *response_qstr;
     GIOStatus status;

-    g_assert(rsp && s->channel);
+    g_assert(s->channel);
+
+    if (!rsp) {
+        return 0;
+    }

Thanks Marc-André,

LGTM and should fix the issues I was seeing.

Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>

In the meantime I also got to test this against the initially reported issue, LGTM as well (ran as no-change backport onto 4.2).

Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>

This LGTM with 2*reviews 1*tested and 11 days on the list without any negative feedback.

I just wanted to re-check if there is anything else left for this to be committed?