Re: [Xen-devel] Xen 4.2.1 live migration with qemu device model

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [Xen-devel] Xen 4.2.1 live migration with qemu device 
model"):
> libxl__xs_read_checked will always either initialise the variable
> (perhaps to NULL) or return an error. On both callsites we check for
> error and "goto out".

Yes.  And the error handling case after the strcmp does indeed handle
got_ret==NULL but the test fails to guard against this before running
strcmp.

> I think the crash is because the code uses got_ret without checking if
> it was NULL, which can happen if the path is not present. Ian (J) does
> that make sense as something which is allowed to happen?

Yes.  I think it is an error somewhere.  I don't understand how this
is happening.  This situation might occur if the qemu crashed and two
separate attempts were made to send a logdirty command, I guess.

> As I said in my early mail I'm not sure why you are getting here at all
> though.

Right.

I think the patch below fixes the segfault but it doesn't fix the
underlying cause.

Ian.

Subject: libxl: qemu trad logdirty: Tolerate ENOENT on ret path

It can happen in error conditions that lds->ret_path doesn't exist,
and libxl__xs_read_checked signals this by setting got_ret=NULL.  If
this happens, fail without crashing.

Reported-by: Alex Bligh <alex@xxxxxxxxxxx>,
Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 95da18e..7586a6c 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -725,7 +725,7 @@ static void 
domain_suspend_switch_qemu_xen_traditional_logdirty
             rc = libxl__xs_read_checked(gc, t, lds->ret_path, &got_ret);
             if (rc) goto out;
 
-            if (strcmp(got, got_ret)) {
+            if (!got_ret || strcmp(got, got_ret)) {
                 LOG(ERROR,"controlling logdirty: qemu was already sent"
                     " command `%s' (xenstore path `%s') but result is `%s'",
                     got, lds->cmd_path, got_ret ? got_ret : "<none>");

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel