[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH 24/27] libxl: convert console callback to libxl_asyncprogress_how
Remove the old console callback. (Its reentrancy properties were troublesome: in principle, the event loop might be reentered during the callback, and the libxl__domain_create_state swept out from under the feet of the domain creation.) As a side effect of the new code arrangements, the console callback for non-bootloader-using PV guests now occurs near the end of domain creation, in the same place as for HVM guests, rather than near the start. The new arrangements should in principle mean that by the time the console is described as ready by the callback, the xenstore key is indeed ready. So in the future the timeout might be removed from the console client. Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx> --- tools/libxl/libxl.h | 17 ++++++----- tools/libxl/libxl_bootloader.c | 3 ++ tools/libxl/libxl_create.c | 59 +++++++++++++++++++++++----------------- tools/libxl/libxl_internal.h | 6 +++- tools/libxl/libxl_types.idl | 2 + tools/libxl/xl_cmdimpl.c | 25 ++++++++++------- 6 files changed, 67 insertions(+), 45 deletions(-) diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index ba0f4de..979940a 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -509,18 +509,19 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version, int libxl_ctx_free(libxl_ctx *ctx /* 0 is OK */); /* domain related functions */ -typedef int (*libxl_console_ready)(libxl_ctx *ctx, uint32_t domid, void *priv); - /* fixme-ao Need to review this API. If we keep it, the reentrancy - * properties need to be documented but they may turn out to be too - * awkward */ int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config, - libxl_console_ready cb, void *priv, uint32_t *domid, - const libxl_asyncop_how *ao_how); + uint32_t *domid, + const libxl_asyncop_how *ao_how, + const libxl_asyncprogress_how *aop_console_how); int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config, - libxl_console_ready cb, void *priv, uint32_t *domid, int restore_fd, - const libxl_asyncop_how *ao_how); + const libxl_asyncop_how *ao_how, + const libxl_asyncprogress_how *aop_console_how); + /* A progress report will be made via ao_console_how, of type + * domain_create_console_available, when the domain's primary + * console is available and can be connected to. + */ void libxl_domain_config_init(libxl_domain_config *d_config); void libxl_domain_config_dispose(libxl_domain_config *d_config); diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c index 1534bae..8436c07 100644 --- a/tools/libxl/libxl_bootloader.c +++ b/tools/libxl/libxl_bootloader.c @@ -377,6 +377,9 @@ static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op) goto out; } + if (bl->console_available) + bl->console_available(egc, bl); + int bootloader_master = libxl__carefd_fd(bl->ptys[0].master); int xenconsole_master = libxl__carefd_fd(bl->ptys[1].master); diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 63cb430..14721eb 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -569,10 +569,15 @@ static int store_libxl_entry(libxl__gc *gc, uint32_t domid, static void domcreate_devmodel_started(libxl__egc *egc, libxl__dm_spawn_state *dmss, int rc); +static void domcreate_bootloader_console_available(libxl__egc *egc, + libxl__bootloader_state *bl); static void domcreate_bootloader_done(libxl__egc *egc, libxl__bootloader_state *bl, int rc); +static void domcreate_console_available(libxl__egc *egc, + libxl__domain_create_state *dcs); + /* Our own function to clean up and call the user's callback. * The final call in the sequence. */ static void domcreate_complete(libxl__egc *egc, @@ -590,8 +595,6 @@ static void initiate_domain_create(libxl__egc *egc, /* convenience aliases */ libxl_domain_config *const d_config = dcs->guest_config; const int restore_fd = dcs->restore_fd; - const libxl_console_ready cb = dcs->console_cb; - void *const priv = dcs->console_cb_priv; memset(&dcs->build_state, 0, sizeof(dcs->build_state)); domid = 0; @@ -610,11 +613,6 @@ static void initiate_domain_create(libxl__egc *egc, dcs->guest_domid = domid; dcs->dmss.dm.guest_domid = 0; /* means we haven't spawned */ - if ( d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV && cb ) { - ret = (*cb)(ctx, domid, priv); - if (ret) goto error_out; - } - ret = libxl__domain_build_info_setdefault(gc, &d_config->b_info); if (ret) goto error_out; @@ -629,6 +627,7 @@ static void initiate_domain_create(libxl__egc *egc, if (restore_fd < 0 && bootdisk) { dcs->bl.callback = domcreate_bootloader_done; + dcs->bl.console_available = domcreate_bootloader_console_available; dcs->bl.info = &d_config->b_info, dcs->bl.disk = bootdisk; dcs->bl.domid = dcs->guest_domid; @@ -644,6 +643,21 @@ error_out: domcreate_complete(egc, dcs, ret); } +static void domcreate_bootloader_console_available(libxl__egc *egc, + libxl__bootloader_state *bl) +{ + libxl__domain_create_state *dcs = CONTAINER_OF(bl, *dcs, bl); + STATE_AO_GC(bl->ao); + domcreate_console_available(egc, dcs); +} + +static void domcreate_console_available(libxl__egc *egc, + libxl__domain_create_state *dcs) { + libxl__ao_progress_report(egc, dcs->ao, &dcs->aop_console_how, + NEW_EVENT(egc, DOMAIN_CREATE_CONSOLE_AVAILABLE, + dcs->guest_domid)); +} + static void domcreate_bootloader_done(libxl__egc *egc, libxl__bootloader_state *bl, int ret) @@ -779,8 +793,6 @@ static void domcreate_devmodel_started(libxl__egc *egc, /* convenience aliases */ libxl_domain_config *const d_config = dcs->guest_config; - const libxl_console_ready cb = dcs->console_cb; - void *const priv = dcs->console_cb_priv; if (ret) { LIBXL__LOG(ctx, LIBXL__LOG_ERROR, @@ -818,12 +830,7 @@ static void domcreate_devmodel_started(libxl__egc *egc, goto error_out; } } - if ( cb && (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM || - (d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV && - d_config->b_info.u.pv.bootloader ))) { - ret = (*cb)(ctx, domid, priv); - if (ret) goto error_out; - } + domcreate_console_available(egc, dcs); domcreate_complete(egc, dcs, 0); return; @@ -863,8 +870,9 @@ static void domain_create_cb(libxl__egc *egc, int rc, uint32_t domid); static int do_domain_create(libxl_ctx *ctx, libxl_domain_config *d_config, - libxl_console_ready cb, void *priv, uint32_t *domid, - int restore_fd, const libxl_asyncop_how *ao_how) + uint32_t *domid, + int restore_fd, const libxl_asyncop_how *ao_how, + const libxl_asyncprogress_how *aop_console_how) { AO_CREATE(ctx, 0, ao_how); libxl__app_domain_create_state *cdcs; @@ -873,9 +881,8 @@ static int do_domain_create(libxl_ctx *ctx, libxl_domain_config *d_config, cdcs->dcs.ao = ao; cdcs->dcs.guest_config = d_config; cdcs->dcs.restore_fd = restore_fd; - cdcs->dcs.console_cb = cb; - cdcs->dcs.console_cb_priv = priv; cdcs->dcs.callback = domain_create_cb; + libxl__ao_progress_gethow(&cdcs->dcs.aop_console_how, aop_console_how); cdcs->domid_out = domid; initiate_domain_create(egc, &cdcs->dcs); @@ -897,19 +904,21 @@ static void domain_create_cb(libxl__egc *egc, } int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config, - libxl_console_ready cb, void *priv, uint32_t *domid, - const libxl_asyncop_how *ao_how) + const libxl_asyncop_how *ao_how, + const libxl_asyncprogress_how *aop_console_how) { - return do_domain_create(ctx, d_config, cb, priv, domid, -1, ao_how); + return do_domain_create(ctx, d_config, domid, -1, + ao_how, aop_console_how); } int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config, - libxl_console_ready cb, void *priv, uint32_t *domid, int restore_fd, - const libxl_asyncop_how *ao_how) + const libxl_asyncop_how *ao_how, + const libxl_asyncprogress_how *aop_console_how) { - return do_domain_create(ctx, d_config, cb, priv, domid, restore_fd, ao_how); + return do_domain_create(ctx, d_config, domid, restore_fd, + ao_how, aop_console_how); } /* diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index a407317..3aa0ed7 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -1699,11 +1699,14 @@ int libxl__openptys(libxl__openpty_state *op, typedef struct libxl__bootloader_state libxl__bootloader_state; typedef void libxl__run_bootloader_callback(libxl__egc*, libxl__bootloader_state*, int rc); +typedef void libxl__bootloader_console_callback(libxl__egc*, + libxl__bootloader_state*); struct libxl__bootloader_state { /* caller must fill these in, and they must all remain valid */ libxl__ao *ao; libxl__run_bootloader_callback *callback; + libxl__bootloader_console_callback *console_available; libxl_domain_build_info *info; /* u.pv.{kernel,ramdisk,cmdline} updated */ libxl_device_disk *disk; uint32_t domid; @@ -1739,9 +1742,8 @@ struct libxl__domain_create_state { libxl__ao *ao; libxl_domain_config *guest_config; int restore_fd; - libxl_console_ready console_cb; - void *console_cb_priv; libxl__domain_create_cb *callback; + libxl_asyncprogress_how aop_console_how; /* private to domain_create */ int guest_domid; libxl__domain_build_state build_state; diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 68599cb..551e367 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -459,6 +459,7 @@ libxl_event_type = Enumeration("event_type", [ (2, "DOMAIN_DEATH"), (3, "DISK_EJECT"), (4, "OPERATION_COMPLETE"), + (5, "DOMAIN_CREATE_CONSOLE_AVAILABLE"), ]) libxl_ev_user = UInt(64) @@ -484,4 +485,5 @@ libxl_event = Struct("event",[ ("operation_complete", Struct(None, [ ("rc", integer), ])), + ("domain_create_console_available", Struct(None, [])), ]))]) diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 621aeaf..b91a2d5 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -1457,16 +1457,18 @@ static int freemem(libxl_domain_build_info *b_info) return ERROR_NOMEM; } -static int autoconnect_console(libxl_ctx *ctx, uint32_t domid, void *priv) +static void autoconnect_console(libxl_ctx *ctx, libxl_event *ev, void *priv) { pid_t *pid = priv; + libxl_event_free(ctx, ev); + *pid = fork(); if (*pid < 0) { perror("unable to fork xenconsole"); - return ERROR_FAIL; + return; } else if (*pid > 0) - return 0; + return; postfork(); @@ -1533,7 +1535,7 @@ static int create_domain(struct domain_create *dom_info) int config_len = 0; int restore_fd = -1; int status = 0; - libxl_console_ready cb; + const libxl_asyncprogress_how *autoconnect_console_how; pid_t child_console_pid = -1; struct save_file_header hdr; @@ -1687,24 +1689,27 @@ start: goto error_out; } + libxl_asyncprogress_how autoconnect_console_how_buf; if ( dom_info->console_autoconnect ) { - cb = autoconnect_console; + autoconnect_console_how_buf.callback = autoconnect_console; + autoconnect_console_how_buf.for_callback = &child_console_pid; + autoconnect_console_how = &autoconnect_console_how_buf; }else{ - cb = NULL; + autoconnect_console_how = 0; } if ( restore_file ) { ret = libxl_domain_create_restore(ctx, &d_config, - cb, &child_console_pid, - &domid, restore_fd, 0); + &domid, restore_fd, + 0, autoconnect_console_how); /* * On subsequent reboot etc we should create the domain, not * restore/migrate-receive it again. */ restore_file = NULL; }else{ - ret = libxl_domain_create_new(ctx, &d_config, - cb, &child_console_pid, &domid, 0); + ret = libxl_domain_create_new(ctx, &d_config, &domid, + 0, autoconnect_console_how); } if ( ret ) goto error_out; -- 1.7.2.5 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |