[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v13 03/11] libxl: convert libxl__device_disk_local_attach to an async op
This will be needed in future patches, when libxl__device_disk_add becomes async also. Create a new status structure that defines the local attach of a disk device and use it in libxl__device_disk_local_attach. This is done in this patch to split the changes introduced when libxl__device_disk_add becomes async. Changes since v11: * Correctly pass error in libxl__device_disk_local_initiate_detach. Changes since v10: * Convert breaks to gotos in error paths of libxl__device_disk_local_initiate_attach. * Unify exit path of libxl__device_disk_local_detach. * Redo some comments. Changes since v9: * Fixed state changes comments on header of functions. * Rework libxl__device_disk_local_init to just set rc to 0, and perform the initialization in the callers context. * If attach fails, perform a detach, so the state of the local_device on the callback is Attached if rc == 0, or Idle if rc != 0. * Free diskpath inside the detach callback. Changes since v6: * Added better comments. * Instead of passing the xs_transaction arround several functions, put all the xs related operations inside device_disk_add. * Make sure libxl__device_disk_local_initiate_detach either calls the async device destroy function, or the callback. Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx> Signed-off-by: Roger Pau Monne <roger.pau@xxxxxxxxxx> --- tools/libxl/libxl.c | 98 ++++++++++++++++++++++++++++++--------- tools/libxl/libxl_bootloader.c | 73 ++++++++++++++++++++++++------ tools/libxl/libxl_internal.h | 72 +++++++++++++++++++++++------ 3 files changed, 192 insertions(+), 51 deletions(-) diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index 5565a7b..f7fca95 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -2191,19 +2191,19 @@ static char * libxl__alloc_vdev(libxl__gc *gc, void *get_vdev_user, return NULL; } -char * libxl__device_disk_local_attach(libxl__gc *gc, - const libxl_device_disk *in_disk, - libxl_device_disk *disk, - const char *blkdev_start) +void libxl__device_disk_local_initiate_attach(libxl__egc *egc, + libxl__disk_local_state *dls) { - libxl_ctx *ctx = gc->owner; + STATE_AO_GC(dls->ao); + libxl_ctx *ctx = CTX; char *dev = NULL, *be_path = NULL; - char *ret = NULL; int rc; libxl__device device; + const libxl_device_disk *in_disk = dls->in_disk; + libxl_device_disk *disk = &dls->disk; + const char *blkdev_start = dls->blkdev_start; - if (in_disk->pdev_path == NULL) - return NULL; + assert(in_disk->pdev_path); memcpy(disk, in_disk, sizeof(libxl_device_disk)); disk->pdev_path = libxl__strdup(gc, in_disk->pdev_path); @@ -2239,7 +2239,8 @@ char * libxl__device_disk_local_attach(libxl__gc *gc, default: LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk format: %d", disk->format); - break; + rc = ERROR_FAIL; + goto out; } break; case LIBXL_DISK_BACKEND_QDISK: @@ -2248,6 +2249,7 @@ char * libxl__device_disk_local_attach(libxl__gc *gc, libxl__alloc_vdev, (void *) blkdev_start)) { LOG(ERROR, "libxl_device_disk_add failed"); + rc = ERROR_FAIL; goto out; } dev = GCSPRINTF("/dev/%s", disk->vdev); @@ -2259,7 +2261,8 @@ char * libxl__device_disk_local_attach(libxl__gc *gc, default: LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend " "type: %d", disk->backend); - break; + rc = ERROR_FAIL; + goto out; } if (disk->vdev != NULL) { @@ -2272,39 +2275,88 @@ char * libxl__device_disk_local_attach(libxl__gc *gc, goto out; } if (dev != NULL) - ret = strdup(dev); - return ret; + dls->diskpath = libxl__strdup(gc, dev); + + dls->callback(egc, dls, 0); + return; out: - libxl__device_disk_local_detach(gc, disk); - return NULL; + assert(rc); + dls->rc = rc; + libxl__device_disk_local_initiate_detach(egc, dls); + return; } -int libxl__device_disk_local_detach(libxl__gc *gc, libxl_device_disk *disk) +/* Callbacks for local detach */ + +static void local_device_detach_cb(libxl__egc *egc, libxl__ao_device *aodev); + +void libxl__device_disk_local_initiate_detach(libxl__egc *egc, + libxl__disk_local_state *dls) { + STATE_AO_GC(dls->ao); int rc = 0; + libxl_device_disk *disk = &dls->disk; + libxl__device *device; + libxl__ao_device *aodev = &dls->aodev; + libxl__prepare_ao_device(ao, aodev); + + if (!dls->diskpath) goto out; switch (disk->backend) { case LIBXL_DISK_BACKEND_QDISK: if (disk->vdev != NULL) { - libxl_device_disk_remove(gc->owner, LIBXL_TOOLSTACK_DOMID, - disk, 0); - /* fixme-ao */ - rc = libxl_device_disk_destroy(gc->owner, - LIBXL_TOOLSTACK_DOMID, disk, 0); + GCNEW(device); + rc = libxl__device_from_disk(gc, LIBXL_TOOLSTACK_DOMID, + disk, device); + if (rc != 0) goto out; + + aodev->action = DEVICE_DISCONNECT; + aodev->dev = device; + aodev->callback = local_device_detach_cb; + aodev->force = 0; + libxl__initiate_device_remove(egc, aodev); + return; } - break; + /* disk->vdev == NULL; fall through */ default: /* * Nothing to do for PHYSTYPE_PHY. * For other device types assume that the blktap2 process is * needed by the soon to be started domain and do nothing. */ - break; + goto out; } +out: + aodev->rc = rc; + local_device_detach_cb(egc, aodev); + return; +} - return rc; +static void local_device_detach_cb(libxl__egc *egc, libxl__ao_device *aodev) +{ + STATE_AO_GC(aodev->ao); + libxl__disk_local_state *dls = CONTAINER_OF(aodev, *dls, aodev); + int rc; + + if (aodev->rc) { + LOGE(ERROR, "unable to %s %s with id %u", + aodev->action == DEVICE_CONNECT ? "add" : "remove", + libxl__device_kind_to_string(aodev->dev->kind), + aodev->dev->devid); + goto out; + } + +out: + /* + * If there was an error in dls->rc, it means we have been called from + * a failed execution of libxl__device_disk_local_initiate_attach, + * so return the original error. + */ + rc = dls->rc ? dls->rc : aodev->rc; + dls->callback(egc, dls, rc); + return; } /******************************************************************************/ diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c index 7ebc0df..ef5a91b 100644 --- a/tools/libxl/libxl_bootloader.c +++ b/tools/libxl/libxl_bootloader.c @@ -75,7 +75,7 @@ static void make_bootloader_args(libxl__gc *gc, libxl__bootloader_state *bl, } } - ARG(bl->diskpath); + ARG(bl->dls.diskpath); /* Sentinel for execv */ ARG(NULL); @@ -206,8 +206,9 @@ static int parse_bootloader_result(libxl__egc *egc, void libxl__bootloader_init(libxl__bootloader_state *bl) { assert(bl->ao); - bl->diskpath = NULL; + bl->dls.diskpath = NULL; bl->openpty.ao = bl->ao; + bl->dls.ao = bl->ao; bl->ptys[0].master = bl->ptys[0].slave = 0; bl->ptys[1].master = bl->ptys[1].slave = 0; libxl__ev_child_init(&bl->child); @@ -224,11 +225,6 @@ static void bootloader_cleanup(libxl__egc *egc, libxl__bootloader_state *bl) if (bl->outputpath) libxl__remove_file(gc, bl->outputpath); if (bl->outputdir) libxl__remove_directory(gc, bl->outputdir); - if (bl->diskpath) { - libxl__device_disk_local_detach(gc, &bl->localdisk); - free(bl->diskpath); - bl->diskpath = 0; - } libxl__domaindeathcheck_stop(gc,&bl->deathcheck); libxl__datacopier_kill(&bl->keystrokes); libxl__datacopier_kill(&bl->display); @@ -249,10 +245,32 @@ static void bootloader_setpaths(libxl__gc *gc, libxl__bootloader_state *bl) bl->outputpath = GCSPRINTF(XEN_RUN_DIR "/bootloader.%"PRIu32".out", domid); } +/* Callbacks */ + +static void bootloader_local_detached_cb(libxl__egc *egc, + libxl__disk_local_state *dls, + int rc); + static void bootloader_callback(libxl__egc *egc, libxl__bootloader_state *bl, int rc) { bootloader_cleanup(egc, bl); + + bl->dls.callback = bootloader_local_detached_cb; + libxl__device_disk_local_initiate_detach(egc, &bl->dls); +} + +static void bootloader_local_detached_cb(libxl__egc *egc, + libxl__disk_local_state *dls, + int rc) +{ + STATE_AO_GC(dls->ao); + libxl__bootloader_state *bl = CONTAINER_OF(dls, *bl, dls); + + if (rc) { + LOG(ERROR, "unable to detach locally attached disk"); + } + bl->callback(egc, bl, rc); } @@ -275,6 +293,12 @@ static void bootloader_abort(libxl__egc *egc, /*----- main flow of control -----*/ +/* Callbacks */ + +static void bootloader_disk_attached_cb(libxl__egc *egc, + libxl__disk_local_state *dls, + int rc); + void libxl__bootloader_run(libxl__egc *egc, libxl__bootloader_state *bl) { STATE_AO_GC(bl->ao); @@ -282,7 +306,6 @@ void libxl__bootloader_run(libxl__egc *egc, libxl__bootloader_state *bl) uint32_t domid = bl->domid; char *logfile_tmp = NULL; int rc, r; - const char *bootloader; libxl__bootloader_init(bl); @@ -344,10 +367,34 @@ void libxl__bootloader_run(libxl__egc *egc, libxl__bootloader_state *bl) goto out; } - bl->diskpath = libxl__device_disk_local_attach(gc, bl->disk, &bl->localdisk, - info->blkdev_start); - if (!bl->diskpath) { - rc = ERROR_FAIL; + + /* This sets the state of the dls struct from Undefined to Idle */ + libxl__device_disk_local_init(&bl->dls); + bl->dls.ao = ao; + bl->dls.in_disk = bl->disk; + bl->dls.blkdev_start = info->blkdev_start; + bl->dls.callback = bootloader_disk_attached_cb; + libxl__device_disk_local_initiate_attach(egc, &bl->dls); + return; + + out: + assert(rc); + out_ok: + free(logfile_tmp); + bootloader_callback(egc, bl, rc); +} + +static void bootloader_disk_attached_cb(libxl__egc *egc, + libxl__disk_local_state *dls, + int rc) +{ + STATE_AO_GC(dls->ao); + libxl__bootloader_state *bl = CONTAINER_OF(dls, *bl, dls); + const libxl_domain_build_info *info = bl->info; + const char *bootloader; + + if (rc) { + LOG(ERROR, "failed to attach local disk for bootloader execution"); goto out; } @@ -389,8 +436,6 @@ void libxl__bootloader_run(libxl__egc *egc, libxl__bootloader_state *bl) out: assert(rc); - out_ok: - free(logfile_tmp); bootloader_callback(egc, bl, rc); } diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 2cc3f86..2bef0af 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -1336,17 +1336,6 @@ _hidden int libxl__device_from_disk(libxl__gc *gc, uint32_t domid, _hidden int libxl__device_disk_add(libxl__gc *gc, uint32_t domid, libxl_device_disk *disk); -/* - * Make a disk available in this (the control) domain. Returns path to - * a device. - */ -_hidden char * libxl__device_disk_local_attach(libxl__gc *gc, - const libxl_device_disk *in_disk, - libxl_device_disk *new_disk, - const char *blkdev_start); -_hidden int libxl__device_disk_local_detach(libxl__gc *gc, - libxl_device_disk *disk); - _hidden char *libxl__uuid2string(libxl__gc *gc, const libxl_uuid uuid); struct libxl__xen_console_reader { @@ -1943,6 +1932,62 @@ struct libxl__ao_devices { _hidden void libxl__initiate_device_remove(libxl__egc *egc, libxl__ao_device *aodev); +/*----- local disk attach: attach a disk locally to run the bootloader -----*/ + +typedef struct libxl__disk_local_state libxl__disk_local_state; +typedef void libxl__disk_local_state_callback(libxl__egc*, + libxl__disk_local_state*, + int rc); + +/* A libxl__disk_local_state may be in the following states: + * Undefined, Idle, Attaching, Attached, Detaching. + */ +struct libxl__disk_local_state { + /* filled by the user */ + libxl__ao *ao; + const libxl_device_disk *in_disk; + libxl_device_disk disk; + const char *blkdev_start; + libxl__disk_local_state_callback *callback; + /* filled by libxl__device_disk_local_initiate_attach */ + char *diskpath; + /* private for implementation of local detach */ + libxl__ao_device aodev; + int rc; +}; + +/* + * Prepares a dls for use. + * State Undefined -> Idle + */ +static inline void libxl__device_disk_local_init(libxl__disk_local_state *dls) +{ + dls->rc = 0; +} + +/* Make a disk available in this (the control) domain. Always calls + * dls->callback when finished. + * State Idle -> Attaching + * + * The state of dls on entry to the callback depends on the value + * of rc passed to the callback: + * rc == 0: Attached if rc == 0 + * rc != 0: Idle + */ +_hidden void libxl__device_disk_local_initiate_attach(libxl__egc *egc, + libxl__disk_local_state *dls); + +/* Disconnects a disk device form the control domain. If the passed + * dls is not attached (or has already been detached), + * libxl__device_disk_local_initiate_detach will just call the callback + * directly. + * State Idle/Attached -> Detaching + * + * The state of dls on entry to the callback is Idle. + */ +_hidden void libxl__device_disk_local_initiate_detach(libxl__egc *egc, + libxl__disk_local_state *dls); + /*----- datacopier: copies data from one fd to another -----*/ typedef struct libxl__datacopier_state libxl__datacopier_state; @@ -2132,12 +2177,12 @@ struct libxl__bootloader_state { /* Should be zeroed by caller on entry. Will be filled in by * bootloader machinery; represents the local attachment of the * disk for the benefit of the bootloader. Must be detached by - * the caller using libxl__device_disk_local_detach. + * the caller using libxl__device_disk_local_initiate_detach. * (This is safe to do after ->callback() has happened since * the domain's kernel and initramfs will have been copied * out of the guest's disk into a temporary directory, mapped * as file references, and deleted. */ - libxl_device_disk localdisk; + libxl__disk_local_state dls; uint32_t domid; /* outputs: * - caller must initialise kernel and ramdisk to point to file @@ -2149,7 +2194,6 @@ struct libxl__bootloader_state { const char *cmdline; /* private to libxl__run_bootloader */ char *outputpath, *outputdir, *logfile; - char *diskpath; /* not from gc, represents actually attached disk */ libxl__openpty_state openpty; libxl__openpty_result ptys[2]; /* [0] is for bootloader */ libxl__ev_child child; -- 1.7.7.5 (Apple Git-26) _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |