[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v6 05/13] libxl: convert libxl__device_disk_local_attach to an async op



On Thu, 2012-06-14 at 13:21 +0100, Roger Pau Monne wrote:
> +void libxl__device_disk_local_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;
> 
>      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;
> +
> +                libxl__prepare_ao_device(ao, aodev);
> +                aodev->action = DEVICE_DISCONNECT;
> +                aodev->dev = device;
> +                aodev->callback = local_device_detach_cb;
> +                aodev->force = 0;
> +                libxl__initiate_device_remove(egc, aodev);
>              }

You need
                else 
                        dls->callback(...)

here otherwise in the case where the qdisk attach just used the raw file
directly it hangs.

You might want to consolidate both this and the default/PHYSTYPE_PHY
case which follows into a single "nothing to do, call callback" path?

> -            break;
> +            return;
>          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;
> +            dls->callback(egc, dls, rc);
> +            return;
>      }
> 
> +out:
> +    assert(rc);
> +    dls->callback(egc, dls, rc);
> +    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);
> +
> +    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:
> +    dls->callback(egc, dls, aodev->rc);
> +    return;
>  }
> 
>  
> /******************************************************************************/
> diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
> index 7ebc0df..182a975 100644
> --- a/tools/libxl/libxl_bootloader.c
> +++ b/tools/libxl/libxl_bootloader.c
> @@ -224,11 +224,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 +244,40 @@ 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_fisnihed_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);
> +
> +    if (bl->diskpath) {
> +        bl->dls.callback = bootloader_fisnihed_cb;
> +        libxl__device_disk_local_detach(egc, &bl->dls);
> +        return;
> +    }
> +
> +    bl->callback(egc, bl, rc);
> +}
> +
> +static void bootloader_fisnihed_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);
> +
> +    free(bl->diskpath);
> +    bl->diskpath = 0;
> +
> +    if (rc) {
> +        LOG(ERROR, "unable to detach locally attached disk");
> +    }
> +
>      bl->callback(egc, bl, rc);
>  }
> 
> @@ -275,6 +300,16 @@ 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);
> +
> +static void bootloader_disk_failed_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 +317,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,13 +378,38 @@ 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;
> -        goto out;
> +    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_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");
> +        dls->callback = bootloader_disk_failed_cb;
> +        libxl__device_disk_local_detach(egc, dls);
> +        return;
>      }
> 
> +    bl->diskpath = bl->dls.diskpath;
> +
>      LOG(DEBUG, "Config bootloader value: %s", info->u.pv.bootloader);
> 
>      if ( !strcmp(info->u.pv.bootloader, "/usr/bin/pygrub") )
> @@ -389,11 +448,23 @@ void libxl__bootloader_run(libxl__egc *egc, 
> libxl__bootloader_state *bl)
> 
>   out:
>      assert(rc);
> - out_ok:
> -    free(logfile_tmp);
>      bootloader_callback(egc, bl, rc);
>  }
> 
> +static void bootloader_disk_failed_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, "failed to detach locally attached disk");
> +    }
> +
> +    bootloader_callback(egc, bl, ERROR_FAIL);
> +}
> +
>  static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op)
>  {
>      libxl__bootloader_state *bl = CONTAINER_OF(op, *bl, openpty);
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 0000d6b..85c21b4 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1245,17 +1245,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,
>          xs_transaction_t t, 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 {
> @@ -1775,6 +1764,35 @@ 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);
> +
> +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_attach */
> +    char *diskpath;
> +    /* private for implementation of local detach */
> +    libxl__ao_device aodev;
> +};
> +
> +/*
> + * Make a disk available in this (the control) domain. Calls dls->callback
> + * when finished.
> + */
> +_hidden void libxl__device_disk_local_attach(libxl__egc *egc,
> +                                             libxl__disk_local_state *dls);
> +_hidden void libxl__device_disk_local_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;
> @@ -1872,7 +1890,7 @@ struct libxl__bootloader_state {
>       * the caller using libxl__device_disk_local_detach, but only
>       * after the domain's kernel and initramfs have been loaded into
>       * memory and the file references disposed of. */
> -    libxl_device_disk localdisk;
> +    libxl__disk_local_state dls;
>      uint32_t domid;
>      /* outputs:
>       *  - caller must initialise kernel and ramdisk to point to file
> --
> 1.7.7.5 (Apple Git-26)
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.