Skip to content

Commit

Permalink
libxl: Fix domain soft reset state handling
Browse files Browse the repository at this point in the history
In do_domain_soft_reset(), a `libxl__domain_suspend_state' is used
without been properly initialised and disposed of. This lead do a
abort() in libxl due to the `dsps.qmp' state been used before been
initialised:
    libxl__ev_qmp_send: Assertion `ev->state == qmp_state_disconnected || ev->state == qmp_state_connected' failed.

Once initialised, `dsps' also needs to be disposed of as the `qmp'
state might still be in the `Connected' state in the callback for
libxl__domain_suspend_device_model(). So this patch adds
libxl__domain_suspend_dispose() which can be called from the two
places where we need to dispose of `dsps'.

Reported-by: Olaf Hering <olaf@aepfle.de>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Ian Jackson <iwj@xenproject.org>
Tested-by: Olaf Hering <olaf@aepfle.de>
  • Loading branch information
anthonyper-ctx authored and jbeulich committed Mar 18, 2021
1 parent 21657ad commit dae3c3e
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 7 deletions.
11 changes: 8 additions & 3 deletions tools/libs/light/libxl_create.c
Original file line number Diff line number Diff line change
Expand Up @@ -2179,9 +2179,7 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
state->console_tty = libxl__strdup(gc, console_tty);

dss->ao = ao;
dss->domid = dss->dsps.domid = domid;
dss->dsps.dm_savefile = GCSPRINTF(LIBXL_DEVICE_MODEL_SAVE_FILE".%d",
domid);
dss->domid = domid;

rc = libxl__save_emulator_xenstore_data(dss, &srs->toolstack_buf,
&srs->toolstack_len);
Expand All @@ -2191,6 +2189,11 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
}

dss->dsps.ao = ao;
dss->dsps.domid = domid;
dss->dsps.live = false;
rc = libxl__domain_suspend_init(egc, &dss->dsps, d_config->b_info.type);
if (rc)
goto out;
dss->dsps.callback_device_model_done = soft_reset_dm_suspended;
libxl__domain_suspend_device_model(egc, &dss->dsps); /* must be last */

Expand All @@ -2209,6 +2212,8 @@ static void soft_reset_dm_suspended(libxl__egc *egc,
CONTAINER_OF(dsps, *srs, dss.dsps);
libxl__app_domain_create_state *cdcs = &srs->cdcs;

libxl__domain_suspend_dispose(gc, dsps);

/*
* Ask all backends to disconnect by removing the domain from
* xenstore. On the creation path the domain will be introduced to
Expand Down
15 changes: 11 additions & 4 deletions tools/libs/light/libxl_dom_suspend.c
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,16 @@ int libxl__domain_suspend_init(libxl__egc *egc,
return rc;
}

void libxl__domain_suspend_dispose(libxl__gc *gc,
libxl__domain_suspend_state *dsps)
{
libxl__xswait_stop(gc, &dsps->pvcontrol);
libxl__ev_evtchn_cancel(gc, &dsps->guest_evtchn);
libxl__ev_xswatch_deregister(gc, &dsps->guest_watch);
libxl__ev_time_deregister(gc, &dsps->guest_timeout);
libxl__ev_qmp_dispose(gc, &dsps->qmp);
}

/*----- callbacks, called by xc_domain_save -----*/

void libxl__domain_suspend_device_model(libxl__egc *egc,
Expand Down Expand Up @@ -388,10 +398,7 @@ static void domain_suspend_common_done(libxl__egc *egc,
{
EGC_GC;
assert(!libxl__xswait_inuse(&dsps->pvcontrol));
libxl__ev_evtchn_cancel(gc, &dsps->guest_evtchn);
libxl__ev_xswatch_deregister(gc, &dsps->guest_watch);
libxl__ev_time_deregister(gc, &dsps->guest_timeout);
libxl__ev_qmp_dispose(gc, &dsps->qmp);
libxl__domain_suspend_dispose(gc, dsps);
dsps->callback_common_done(egc, dsps, rc);
}

Expand Down
2 changes: 2 additions & 0 deletions tools/libs/light/libxl_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -3617,6 +3617,8 @@ struct libxl__domain_suspend_state {
int libxl__domain_suspend_init(libxl__egc *egc,
libxl__domain_suspend_state *dsps,
libxl_domain_type type);
void libxl__domain_suspend_dispose(libxl__gc *gc,
libxl__domain_suspend_state *dsps);

/* calls dsps->callback_device_model_done when done
* may synchronously calls this callback */
Expand Down

0 comments on commit dae3c3e

Please sign in to comment.