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

Re: [Xen-devel] [PATCH 1/6] libxl: libxl_device_disk_local_attach return a new libxl_device_disk



On Tue, 2012-03-27 at 14:59 +0100, Stefano Stabellini wrote:
> The caller passes an additional pre-allocated libxl_device_disk struct
> to libxl_device_disk_local_attach, that it is going to fill it with
> informations about the new locally attached disk.
> The new libxl_device_disk should be passed to
> libxl_device_disk_local_detach afterwards.
> 
> This patch is just about adding the new parameter and updating the
> caller.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> ---
>  tools/libxl/libxl.c            |   46 
> ++++++++++++++++++++++++++--------------
>  tools/libxl/libxl.h            |    3 +-
>  tools/libxl/libxl_bootloader.c |    9 +++++--
>  3 files changed, 38 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 5344366..d33fbdf 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1644,63 +1644,77 @@ out:
>      return ret;
>  }
>  
> -char * libxl_device_disk_local_attach(libxl_ctx *ctx, libxl_device_disk 
> *disk)
> +char * libxl_device_disk_local_attach(libxl_ctx *ctx, const 
> libxl_device_disk *disk,
> +        libxl_device_disk **new_disk)

I think this would be better as a caller allocated libxl_device_disk *.
That's much simpler since the caller can just put it on the stack or in
an existing datastructure (I expect a suitable one comes into being with
IanJ's async patch series).

Do we need this as a public facing API? Does anything use it? I think it
should be an implementation detail of libxl_device_disk_add where domid
== SELF? That would also allow you to use the gc here.

>  {
>      GC_INIT(ctx);
>      char *dev = NULL;
>      char *ret = NULL;
>      int rc;
> -
> -    rc = libxl__device_disk_setdefault(gc, disk);
> +    libxl_device_disk *tmpdisk = (libxl_device_disk *)
> +        malloc(sizeof(libxl_device_disk));
> +    if (tmpdisk == NULL) goto out;
> +
> +    *new_disk = tmpdisk;
> +    memcpy(tmpdisk, disk, sizeof(libxl_device_disk));
> +    if (tmpdisk->pdev_path != NULL)
> +        tmpdisk->pdev_path = strdup(tmpdisk->pdev_path);
> +    if (tmpdisk->script != NULL)
> +        tmpdisk->script = strdup(tmpdisk->script);
> +    tmpdisk->vdev = NULL;
> +
> +    rc = libxl__device_disk_setdefault(gc, tmpdisk);
>      if (rc) goto out;
>  
> -    switch (disk->backend) {
> +    switch (tmpdisk->backend) {
>          case LIBXL_DISK_BACKEND_PHY:
>              LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching PHY disk 
> %s",
> -                       disk->pdev_path);
> -            dev = disk->pdev_path;
> +                       tmpdisk->pdev_path);
> +            dev = tmpdisk->pdev_path;
>              break;
>          case LIBXL_DISK_BACKEND_TAP:
> -            switch (disk->format) {
> +            switch (tmpdisk->format) {
>              case LIBXL_DISK_FORMAT_RAW:
>                  /* optimise away the early tapdisk attach in this case */
>                  LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching"
>                             " tap disk %s directly (ie without using blktap)",
> -                           disk->pdev_path);
> -                dev = disk->pdev_path;
> +                           tmpdisk->pdev_path);
> +                dev = tmpdisk->pdev_path;
>                  break;
>              case LIBXL_DISK_FORMAT_VHD:
> -                dev = libxl__blktap_devpath(gc, disk->pdev_path,
> -                                            disk->format);
> +                dev = libxl__blktap_devpath(gc, tmpdisk->pdev_path,
> +                                            tmpdisk->format);
>                  break;
>              case LIBXL_DISK_FORMAT_QCOW:
>              case LIBXL_DISK_FORMAT_QCOW2:
>                  abort(); /* prevented by libxl__device_disk_set_backend */
>              default:
>                  LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> -                           "unrecognized disk format: %d", disk->format);
> +                           "unrecognized disk format: %d", tmpdisk->format);
>                  break;
>              }
>              break;
>          case LIBXL_DISK_BACKEND_QDISK:
> -            if (disk->format != LIBXL_DISK_FORMAT_RAW) {
> +            if (tmpdisk->format != LIBXL_DISK_FORMAT_RAW) {
>                  LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally"
>                             " attach a qdisk image if the format is not raw");
>                  break;
>              }
>              LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching qdisk %s\n",
>                         disk->pdev_path);
> -            dev = disk->pdev_path;
> +            dev = tmpdisk->pdev_path;
>              break;
>          default:
>              LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
> -                "type: %d", disk->backend);
> +                "type: %d", tmpdisk->backend);
>              break;
>      }
>  
>   out:
> -    if (dev != NULL)
> +    if (dev != NULL) {
>          ret = strdup(dev);
> +        tmpdisk->vdev = strdup(tmpdisk->vdev);
> +    }
>      GC_FREE;
>      return ret;
>  }
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 6b69030..d297b04 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -540,7 +540,8 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, 
> libxl_device_disk *disk);
>   * Make a disk available in this (the control) domain. Returns path to
>   * a device.
>   */
> -char * libxl_device_disk_local_attach(libxl_ctx *ctx, libxl_device_disk 
> *disk);
> +char * libxl_device_disk_local_attach(libxl_ctx *ctx, const 
> libxl_device_disk *disk,
> +        libxl_device_disk **new_disk);
>  int libxl_device_disk_local_detach(libxl_ctx *ctx, libxl_device_disk *disk);
>  
>  /* Network Interfaces */
> diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
> index 2774062..0abc31f 100644
> --- a/tools/libxl/libxl_bootloader.c
> +++ b/tools/libxl/libxl_bootloader.c
> @@ -330,6 +330,7 @@ int libxl_run_bootloader(libxl_ctx *ctx,
>      char *fifo = NULL;
>      char *diskpath = NULL;
>      char **args = NULL;
> +    libxl_device_disk *tmpdisk = NULL;
>  
>      char tempdir_template[] = "/var/run/libxl/bl.XXXXXX";
>      char *tempdir;
> @@ -386,7 +387,7 @@ int libxl_run_bootloader(libxl_ctx *ctx,
>          goto out_close;
>      }
>  
> -    diskpath = libxl_device_disk_local_attach(ctx, disk);
> +    diskpath = libxl_device_disk_local_attach(ctx, disk, &tmpdisk);
>      if (!diskpath) {
>          goto out_close;
>      }
> @@ -452,9 +453,11 @@ int libxl_run_bootloader(libxl_ctx *ctx,
>  
>      rc = 0;
>  out_close:
> -    if (diskpath) {
> -        libxl_device_disk_local_detach(ctx, disk);
> +    if (diskpath)
>          free(diskpath);
> +    if (tmpdisk) {
> +        libxl_device_disk_local_detach(ctx, tmpdisk);
> +        free(tmpdisk);
>      }
>      if (fifo_fd > -1)
>          close(fifo_fd);



_______________________________________________
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®.