Roger Pau Monne
2012-May-10 12:28 UTC
[PATCH v2] libxl: make libxl_device_{vkb, vfb}_add async operations.
Add an ao parameter to both functions, since they may become slow in the future. Also move the functions to libxl_device.c, as done with disk and nic add fuctions, so libxl_device_{vkb,vfb}_add are mere wrappers arround libxl__device_{vkb,vfb}_add, that can be called from a running AO operation. Quite a lot of code motion here also, and integration with the new AO domain creation. This should be applied after my "libxl: call hotplug scripts from libxl for vif" series. Changes since v1: * Everything as far as I can tell. Signed-off-by: Roger Pau Monne <roger.pau@citrix.com> --- tools/libxl/libxl.c | 148 +++++++----------------------------------- tools/libxl/libxl.h | 6 +- tools/libxl/libxl_create.c | 100 ++++++++++++++++++++++++++-- tools/libxl/libxl_device.c | 147 +++++++++++++++++++++++++++++++++++++++++ tools/libxl/libxl_dm.c | 4 +- tools/libxl/libxl_internal.h | 13 ++++- 6 files changed, 281 insertions(+), 137 deletions(-) diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index 5a681b1..f50ecaf 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -1962,69 +1962,25 @@ int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb) return 0; } -static int libxl__device_from_vkb(libxl__gc *gc, uint32_t domid, - libxl_device_vkb *vkb, - libxl__device *device) -{ - device->backend_devid = vkb->devid; - device->backend_domid = vkb->backend_domid; - device->backend_kind = LIBXL__DEVICE_KIND_VKBD; - device->devid = vkb->devid; - device->domid = domid; - device->kind = LIBXL__DEVICE_KIND_VKBD; - - return 0; -} - -int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb) +int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb, + const libxl_asyncop_how *ao_how) { - GC_INIT(ctx); - flexarray_t *front; - flexarray_t *back; - libxl__device device; + AO_CREATE(ctx, domid, ao_how); + libxl__ao_device *device; int rc; - rc = libxl__device_vkb_setdefault(gc, vkb); - if (rc) goto out; - - front = flexarray_make(16, 1); - if (!front) { - rc = ERROR_NOMEM; + GCNEW(device); + libxl__init_ao_device(device, ao, NULL); + device->callback = libxl__device_cb; + rc = libxl__device_vkb_add(egc, domid, vkb, device); + if (rc) { + LOGE(ERROR, "unable to add vkb %d", vkb->devid); goto out; } - back = flexarray_make(16, 1); - if (!back) { - rc = ERROR_NOMEM; - goto out_free; - } - - rc = libxl__device_from_vkb(gc, domid, vkb, &device); - if (rc != 0) goto out_free; - - flexarray_append(back, "frontend-id"); - flexarray_append(back, libxl__sprintf(gc, "%d", domid)); - flexarray_append(back, "online"); - flexarray_append(back, "1"); - flexarray_append(back, "state"); - flexarray_append(back, libxl__sprintf(gc, "%d", 1)); - flexarray_append(back, "domain"); - flexarray_append(back, libxl__domid_to_name(gc, domid)); - - flexarray_append(front, "backend-id"); - flexarray_append(front, libxl__sprintf(gc, "%d", vkb->backend_domid)); - flexarray_append(front, "state"); - flexarray_append(front, libxl__sprintf(gc, "%d", 1)); - libxl__device_generic_add(gc, &device, - libxl__xs_kvs_of_flexarray(gc, back, back->count), - libxl__xs_kvs_of_flexarray(gc, front, front->count)); - rc = 0; -out_free: - flexarray_free(back); - flexarray_free(front); out: - GC_FREE; - return rc; + if (rc) return AO_ABORT(rc); + return AO_INPROGRESS; } int libxl_device_vkb_remove(libxl_ctx *ctx, uint32_t domid, @@ -2089,81 +2045,25 @@ int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb) return 0; } -static int libxl__device_from_vfb(libxl__gc *gc, uint32_t domid, - libxl_device_vfb *vfb, - libxl__device *device) -{ - device->backend_devid = vfb->devid; - device->backend_domid = vfb->backend_domid; - device->backend_kind = LIBXL__DEVICE_KIND_VFB; - device->devid = vfb->devid; - device->domid = domid; - device->kind = LIBXL__DEVICE_KIND_VFB; - return 0; -} - -int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb) +int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb, + const libxl_asyncop_how *ao_how) { - GC_INIT(ctx); - flexarray_t *front; - flexarray_t *back; - libxl__device device; + AO_CREATE(ctx, domid, ao_how); + libxl__ao_device *device; int rc; - rc = libxl__device_vfb_setdefault(gc, vfb); - if (rc) goto out; - - front = flexarray_make(16, 1); - if (!front) { - rc = ERROR_NOMEM; + GCNEW(device); + libxl__init_ao_device(device, ao, NULL); + device->callback = libxl__device_cb; + rc = libxl__device_vfb_add(egc, domid, vfb, device); + if (rc) { + LOGE(ERROR, "unable to add vfb %d", vfb->devid); goto out; } - back = flexarray_make(16, 1); - if (!back) { - rc = ERROR_NOMEM; - goto out_free; - } - - rc = libxl__device_from_vfb(gc, domid, vfb, &device); - if (rc != 0) goto out_free; - - flexarray_append_pair(back, "frontend-id", libxl__sprintf(gc, "%d", domid)); - flexarray_append_pair(back, "online", "1"); - flexarray_append_pair(back, "state", libxl__sprintf(gc, "%d", 1)); - flexarray_append_pair(back, "domain", libxl__domid_to_name(gc, domid)); - flexarray_append_pair(back, "vnc", - libxl_defbool_val(vfb->vnc.enable) ? "1" : "0"); - flexarray_append_pair(back, "vnclisten", vfb->vnc.listen); - flexarray_append_pair(back, "vncpasswd", vfb->vnc.passwd); - flexarray_append_pair(back, "vncdisplay", - libxl__sprintf(gc, "%d", vfb->vnc.display)); - flexarray_append_pair(back, "vncunused", - libxl_defbool_val(vfb->vnc.findunused) ? "1" : "0"); - flexarray_append_pair(back, "sdl", - libxl_defbool_val(vfb->sdl.enable) ? "1" : "0"); - flexarray_append_pair(back, "opengl", - libxl_defbool_val(vfb->sdl.opengl) ? "1" : "0"); - if (vfb->sdl.xauthority) { - flexarray_append_pair(back, "xauthority", vfb->sdl.xauthority); - } - if (vfb->sdl.display) { - flexarray_append_pair(back, "display", vfb->sdl.display); - } - - flexarray_append_pair(front, "backend-id", - libxl__sprintf(gc, "%d", vfb->backend_domid)); - flexarray_append_pair(front, "state", libxl__sprintf(gc, "%d", 1)); - libxl__device_generic_add(gc, &device, - libxl__xs_kvs_of_flexarray(gc, back, back->count), - libxl__xs_kvs_of_flexarray(gc, front, front->count)); - rc = 0; -out_free: - flexarray_free(front); - flexarray_free(back); out: - GC_FREE; - return rc; + if (rc) return AO_ABORT(rc); + return AO_INPROGRESS; } int libxl_device_vfb_remove(libxl_ctx *ctx, uint32_t domid, diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 6689baf..0497175 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -700,14 +700,16 @@ int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic, libxl_nicinfo *nicinfo); /* Keyboard */ -int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb); +int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb, + const libxl_asyncop_how *ao_how); int libxl_device_vkb_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb, const libxl_asyncop_how *ao_how); int libxl_device_vkb_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb); /* Framebuffer */ -int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb); +int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb, + const libxl_asyncop_how *ao_how); int libxl_device_vfb_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb, const libxl_asyncop_how *ao_how); diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 69f4e4d..974ed89 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -602,6 +602,11 @@ static void domcreate_bootloader_done(libxl__egc *egc, static void domcreate_disk_connected(libxl__egc *egc, libxl__ao_device *aorm); +static void domcreate_misc_connected(libxl__egc *egc, libxl__ao_device *aorm); + +static void domcreate_start_devmodel(libxl__egc *egc, + libxl__domain_create_state *dcs); + static void domcreate_console_available(libxl__egc *egc, libxl__domain_create_state *dcs); @@ -775,7 +780,6 @@ static void domcreate_disk_connected(libxl__egc *egc, libxl__ao_device *aorm) const uint32_t domid = dcs->guest_domid; libxl_domain_config *const d_config = dcs->guest_config; libxl__domain_build_state *const state = &dcs->build_state; - libxl_ctx *const ctx = CTX; ret = libxl__ao_device_check_last(gc, aorm, dcs->devices, dcs->num_devices, &last); @@ -797,8 +801,8 @@ static void domcreate_disk_connected(libxl__egc *egc, libxl__ao_device *aorm) switch (d_config->c_info.type) { case LIBXL_DOMAIN_TYPE_HVM: { - libxl__device_console console; libxl_device_vkb vkb; + libxl__device_console console; ret = init_console_info(&console, 0); if ( ret ) @@ -806,10 +810,94 @@ static void domcreate_disk_connected(libxl__egc *egc, libxl__ao_device *aorm) libxl__device_console_add(gc, domid, &console, state); libxl__device_console_dispose(&console); + GCNEW_ARRAY(dcs->devices, 1); + dcs->num_devices = 1; + libxl__init_ao_device(&dcs->devices[0], ao, &dcs->devices); + dcs->devices[0].callback = domcreate_misc_connected; libxl_device_vkb_init(&vkb); - libxl_device_vkb_add(ctx, domid, &vkb); + libxl__device_vkb_add(egc, domid, &vkb, &dcs->devices[0]); libxl_device_vkb_dispose(&vkb); + return; + } + case LIBXL_DOMAIN_TYPE_PV: + { + GCNEW_ARRAY(dcs->devices, d_config->num_vfbs*2); + dcs->num_devices = d_config->num_vfbs*2; + for (i = 0; i < d_config->num_vfbs; i++) { + libxl__init_ao_device(&dcs->devices[i], ao, &dcs->devices); + dcs->devices[i].callback = domcreate_misc_connected; + libxl__init_ao_device(&dcs->devices[i+1], ao, &dcs->devices); + dcs->devices[i+1].callback = domcreate_misc_connected; + libxl__device_vfb_add(egc, domid, &d_config->vfbs[i], + &dcs->devices[i]); + libxl__device_vkb_add(egc, domid, &d_config->vkbs[i], + &dcs->devices[i+1]); + } + + if (!dcs->num_devices) + domcreate_start_devmodel(egc, dcs); + + return; + } + default: + ret = ERROR_INVAL; + goto error_out; + } + abort(); /* not reached */ + + error_out: + assert(ret); + domcreate_complete(egc, dcs, ret); +} + +static void domcreate_misc_connected(libxl__egc *egc, libxl__ao_device *aorm) +{ + STATE_AO_GC(aorm->ao); + libxl__domain_create_state *dcs = CONTAINER_OF(aorm->base, *dcs, devices); + int last, ret = 0; + + aorm->state = DEVICE_FINISHED; + + ret = libxl__ao_device_check_last(gc, aorm, dcs->devices, + dcs->num_devices, &last); + if (!last) return; + if (last && ret) { + LOGE(ERROR, "error connecting misc devices"); + goto error_out; + } + + domcreate_start_devmodel(egc, dcs); + return; + +error_out: + assert(ret); + domcreate_complete(egc, dcs, ret); +} + +static void domcreate_start_devmodel(libxl__egc *egc, + libxl__domain_create_state *dcs) +{ + STATE_AO_GC(dcs->ao); + int ret; + + /* convenience aliases */ + const uint32_t domid = dcs->guest_domid; + libxl_domain_config *const d_config = dcs->guest_config; + libxl__domain_build_state *const state = &dcs->build_state; + + /* We might be going to call libxl__spawn_local_dm, or _spawn_stub_dm. + * Fill in any field required by either, including both relevant + * callbacks (_spawn_stub_dm will overwrite our trespass if needed). */ + dcs->dmss.dm.spawn.ao = ao; + dcs->dmss.dm.guest_config = dcs->guest_config; + dcs->dmss.dm.build_state = &dcs->build_state; + dcs->dmss.dm.callback = domcreate_devmodel_started; + dcs->dmss.callback = domcreate_devmodel_started; + + switch (d_config->c_info.type) { + case LIBXL_DOMAIN_TYPE_HVM: + { dcs->dmss.dm.guest_domid = domid; if (libxl_defbool_val(d_config->b_info.device_model_stubdomain)) libxl__spawn_stub_dm(egc, &dcs->dmss); @@ -822,11 +910,6 @@ static void domcreate_disk_connected(libxl__egc *egc, libxl__ao_device *aorm) int need_qemu = 0; libxl__device_console console; - for (i = 0; i < d_config->num_vfbs; i++) { - libxl_device_vfb_add(ctx, domid, &d_config->vfbs[i]); - libxl_device_vkb_add(ctx, domid, &d_config->vkbs[i]); - } - ret = init_console_info(&console, 0); if ( ret ) goto error_out; @@ -862,6 +945,7 @@ static void domcreate_disk_connected(libxl__egc *egc, libxl__ao_device *aorm) domcreate_complete(egc, dcs, ret); } + static void domcreate_devmodel_started(libxl__egc *egc, libxl__dm_spawn_state *dmss, int ret) diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c index 6b431a2..f2c4d50 100644 --- a/tools/libxl/libxl_device.c +++ b/tools/libxl/libxl_device.c @@ -205,6 +205,33 @@ int libxl__device_from_nic(libxl__gc *gc, uint32_t domid, return 0; } +int libxl__device_from_vkb(libxl__gc *gc, uint32_t domid, + libxl_device_vkb *vkb, + libxl__device *device) +{ + device->backend_devid = vkb->devid; + device->backend_domid = vkb->backend_domid; + device->backend_kind = LIBXL__DEVICE_KIND_VKBD; + device->devid = vkb->devid; + device->domid = domid; + device->kind = LIBXL__DEVICE_KIND_VKBD; + + return 0; +} + +int libxl__device_from_vfb(libxl__gc *gc, uint32_t domid, + libxl_device_vfb *vfb, + libxl__device *device) +{ + device->backend_devid = vfb->devid; + device->backend_domid = vfb->backend_domid; + device->backend_kind = LIBXL__DEVICE_KIND_VFB; + device->devid = vfb->devid; + device->domid = domid; + device->kind = LIBXL__DEVICE_KIND_VFB; + return 0; +} + int libxl__device_disk_add(libxl__egc *egc, uint32_t domid, libxl_device_disk *disk, libxl__ao_device *aorm) @@ -445,6 +472,126 @@ out: return rc; } +int libxl__device_vkb_add(libxl__egc *egc, uint32_t domid, + libxl_device_vkb *vkb, libxl__ao_device *aorm) +{ + STATE_AO_GC(aorm->ao); + flexarray_t *front; + flexarray_t *back; + libxl__device *device; + int rc; + + rc = libxl__device_vkb_setdefault(gc, vkb); + if (rc) goto out; + + front = flexarray_make(16, 1); + if (!front) { + rc = ERROR_NOMEM; + goto out; + } + back = flexarray_make(16, 1); + if (!back) { + rc = ERROR_NOMEM; + goto out_free; + } + + GCNEW(device); + rc = libxl__device_from_vkb(gc, domid, vkb, device); + if (rc != 0) goto out_free; + + flexarray_append(back, "frontend-id"); + flexarray_append(back, GCSPRINTF("%d", domid)); + flexarray_append(back, "online"); + flexarray_append(back, "1"); + flexarray_append(back, "state"); + flexarray_append(back, GCSPRINTF("%d", 1)); + flexarray_append(back, "domain"); + flexarray_append(back, libxl__domid_to_name(gc, domid)); + + flexarray_append(front, "backend-id"); + flexarray_append(front, GCSPRINTF("%d", vkb->backend_domid)); + flexarray_append(front, "state"); + flexarray_append(front, GCSPRINTF("%d", 1)); + + libxl__device_generic_add(gc, device, + libxl__xs_kvs_of_flexarray(gc, back, back->count), + libxl__xs_kvs_of_flexarray(gc, front, front->count)); + rc = 0; +out_free: + flexarray_free(back); + flexarray_free(front); +out: + aorm->rc = rc; + aorm->callback(egc, aorm); + return rc; +} + +int libxl__device_vfb_add(libxl__egc *egc, uint32_t domid, + libxl_device_vfb *vfb, libxl__ao_device *aorm) +{ + STATE_AO_GC(aorm->ao); + flexarray_t *front; + flexarray_t *back; + libxl__device *device; + int rc; + + rc = libxl__device_vfb_setdefault(gc, vfb); + if (rc) goto out; + + front = flexarray_make(16, 1); + if (!front) { + rc = ERROR_NOMEM; + goto out; + } + back = flexarray_make(16, 1); + if (!back) { + rc = ERROR_NOMEM; + goto out_free; + } + + GCNEW(device); + rc = libxl__device_from_vfb(gc, domid, vfb, device); + if (rc != 0) goto out_free; + + flexarray_append_pair(back, "frontend-id", GCSPRINTF("%d", domid)); + flexarray_append_pair(back, "online", "1"); + flexarray_append_pair(back, "state", GCSPRINTF("%d", 1)); + flexarray_append_pair(back, "domain", libxl__domid_to_name(gc, domid)); + flexarray_append_pair(back, "vnc", + libxl_defbool_val(vfb->vnc.enable) ? "1" : "0"); + flexarray_append_pair(back, "vnclisten", vfb->vnc.listen); + flexarray_append_pair(back, "vncpasswd", vfb->vnc.passwd); + flexarray_append_pair(back, "vncdisplay", + GCSPRINTF("%d", vfb->vnc.display)); + flexarray_append_pair(back, "vncunused", + libxl_defbool_val(vfb->vnc.findunused) ? "1" : "0"); + flexarray_append_pair(back, "sdl", + libxl_defbool_val(vfb->sdl.enable) ? "1" : "0"); + flexarray_append_pair(back, "opengl", + libxl_defbool_val(vfb->sdl.opengl) ? "1" : "0"); + if (vfb->sdl.xauthority) { + flexarray_append_pair(back, "xauthority", vfb->sdl.xauthority); + } + if (vfb->sdl.display) { + flexarray_append_pair(back, "display", vfb->sdl.display); + } + + flexarray_append_pair(front, "backend-id", + GCSPRINTF("%d", vfb->backend_domid)); + flexarray_append_pair(front, "state", GCSPRINTF("%d", 1)); + + libxl__device_generic_add(gc, device, + libxl__xs_kvs_of_flexarray(gc, back, back->count), + libxl__xs_kvs_of_flexarray(gc, front, front->count)); + rc = 0; +out_free: + flexarray_free(front); + flexarray_free(back); +out: + aorm->rc = rc; + aorm->callback(egc, aorm); + return rc; +} typedef struct { libxl__gc *gc; diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index c0d7ca1..ed3aae4 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -859,10 +859,10 @@ static void spawn_stub_disk_connected(libxl__egc *egc, libxl__ao_device *aorm) */ libxl__device_nic_setdefault(gc, &dm_config->vifs[i]); } - ret = libxl_device_vfb_add(ctx, dm_domid, &dm_config->vfbs[0]); + ret = libxl_device_vfb_add(ctx, dm_domid, &dm_config->vfbs[0], 0); if (ret) goto out; - ret = libxl_device_vkb_add(ctx, dm_domid, &dm_config->vkbs[0]); + ret = libxl_device_vkb_add(ctx, dm_domid, &dm_config->vkbs[0], 0); if (ret) goto out; diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 91eba63..d57200f 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -788,6 +788,12 @@ _hidden int libxl__device_from_nic(libxl__gc *gc, uint32_t domid, _hidden int libxl__device_from_disk(libxl__gc *gc, uint32_t domid, libxl_device_disk *disk, libxl__device *device); +_hidden int libxl__device_from_vkb(libxl__gc *gc, uint32_t domid, + libxl_device_vkb *vkb, + libxl__device *device); +_hidden int libxl__device_from_vfb(libxl__gc *gc, uint32_t domid, + libxl_device_vfb *vfb, + libxl__device *device); _hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor); _hidden int libxl__device_disk_dev_number(const char *virtpath, @@ -981,7 +987,12 @@ _hidden int libxl__device_disk_add(libxl__egc *egc, uint32_t domid, _hidden int libxl__device_nic_add(libxl__egc *egc, uint32_t domid, libxl_device_nic *nic, libxl__ao_device *aorm); - +_hidden int libxl__device_vkb_add(libxl__egc *egc, uint32_t domid, + libxl_device_vkb *vkb, + libxl__ao_device *aorm); +_hidden int libxl__device_vfb_add(libxl__egc *egc, uint32_t domid, + libxl_device_vfb *vfb, + libxl__ao_device *aorm); /* *----- spawn ----- * -- 1.7.7.5 (Apple Git-26)
Ian Jackson
2012-May-10 16:38 UTC
Re: [PATCH v2] libxl: make libxl_device_{vkb, vfb}_add async operations.
Roger Pau Monne writes ("[Xen-devel] [PATCH v2] libxl: make libxl_device_{vkb, vfb}_add async operations."):> Also move the functions to libxl_device.c, as done with disk and nic > add fuctions, so libxl_device_{vkb,vfb}_add are mere wrappers arround > libxl__device_{vkb,vfb}_add, that can be called from a running AO > operation. Quite a lot of code motion here also, and integration with > the new AO domain creation. > > This should be applied after my "libxl: call hotplug scripts from > libxl for vif" series.Can you please (a) add it to the end of your series and (b) separate out the code motion ? As it is it is difficult to review. As a guideline, I would recommend trying to actually read the patch, by eye, before you post it. If you find you can''t make head or tail of it then probably we won''t be able to either :-). Thanks, Ian.