On Wed, 2011-11-02 at 02:58 -0400, Chun Yan Liu wrote:
> Stefano, could you review the revised patch and share your comments?
> Thanks.
>
>
> >>> Chunyan Liu <cyliu@xxxxxxxx> 10/28/2011 9:27 PM >>>
> Start qemu-nbd to mount non-raw qdisk in dom0 so that xl can create PV
> guest with qcow/qcow2 disk image and using pygrub.
> v2: use fork and exec instead of system(3)
>
> Signed-off-by: Chunyan Liu <cyliu@xxxxxxxx>
>
> diff -r b4cf57bbc3fb tools/libxl/libxl.c
> --- a/tools/libxl/libxl.cThu Oct 20 15:24:46 2011 +0800
> +++ b/tools/libxl/libxl.cFri Oct 28 20:50:36 2011 +0800
> @@ -1077,6 +1077,58 @@ out_free:
> libxl__free_all(&gc);
> return rc;
> }
> +static int fork_exec(char *arg0, char **args)
> +{
> + pid_t pid;
> + int status;
> +
> + pid = fork();
This needs to be libxl_fork, I think. Perhaps even this whole thing
should be libxl__spawn_spawn (not sure about that)?
> + if (pid < 0)
> + return -1;
> + else if (pid == 0){
> + execvp(arg0, args);
> + exit(127);
> + }
> + sleep(1);
Why do you need this sleep?
> + while (waitpid(pid, &status, 0) < 0) {
> + if (errno != EINTR) {
> + status = -1;
> + break;
> + }
> + }
> +
> + return status;
> +}
> +
> +static char * nbd_mount_disk(libxl__gc *gc, libxl_device_disk *disk)
> +{
> + int i;
> + int nbds_max = 16;
> + char *nbd_dev = NULL;
> + char *args[] = {"qemu-nbd","-c",NULL,NULL,NULL};
"-r" perhaps?
> + char *ret = NULL;
> +
> + for (i = 0; i < nbds_max; i++) {
> + nbd_dev = libxl__sprintf(gc, "/dev/nbd%d", i);
We can't get qemu-nbd to find a free device on our behalf and tell us
what it was?
> + args[2] = libxl__sprintf(gc, "%s", nbd_dev);
> + args[3] = libxl__sprintf(gc, "%s", disk->pdev_path);
> + if (fork_exec(args[0], args) == 0) {
> + ret = strdup(nbd_dev);
> + break;
> + }
> + }
> +
> + return ret;
> +}
> +
> +static int nbd_unmount_disk(libxl__gc *gc, char *diskpath) {
> + char *args[] = {"qemu-nbd","-d",NULL,NULL};
> + args[2] = libxl__sprintf(gc, "%s", diskpath);
> + if (fork_exec(args[0], args))
> + return 0;
> + else
> + return ERROR_FAIL;
> +}
>
> char * libxl_device_disk_local_attach(libxl_ctx *ctx,
> libxl_device_disk *disk)
> {
> @@ -1084,6 +1136,7 @@ char * libxl_device_disk_local_attach(li
> char *dev = NULL;
> char *ret = NULL;
> int rc;
> + char *mdev = NULL;
>
> rc = libxl__device_disk_set_backend(&gc, disk);
> if (rc) goto out;
> @@ -1118,8 +1171,12 @@ char * libxl_device_disk_local_attach(li
> break;
> case LIBXL_DISK_BACKEND_QDISK:
> if (disk->format != LIBXL_DISK_FORMAT_RAW) {
> - LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally"
> - " attach a qdisk image if the format is
> not raw");
> + LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "attaching a
> non-raw qdisk image to domain 0\n");
> + mdev = nbd_mount_disk(&gc, disk);
> + if (mdev)
> + dev = mdev;
> + else
> + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "fail to mount
> image with qemu-nbd");
> break;
> }
> LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching
> qdisk %s\n",
> @@ -1135,11 +1192,13 @@ char * libxl_device_disk_local_attach(li
> out:
> if (dev != NULL)
> ret = strdup(dev);
> + if (mdev)
> + free(mdev);
free(NULL) is acceptable.
> libxl__free_all(&gc);
> return ret;
> }
>
> -int libxl_device_disk_local_detach(libxl_ctx *ctx, libxl_device_disk
> *disk)
> +int libxl_device_disk_local_detach(libxl_ctx *ctx, libxl_device_disk
> *disk, char *diskpath)
> {
> /* Nothing to do for PHYSTYPE_PHY. */
This should be moved into the switch which you have added e.g.
case LIBXL_DISKBACK_END_PHY:
/* nothing to do */
break;
>
> @@ -1147,7 +1206,22 @@ int libxl_device_disk_local_detach(libxl
> * For other device types assume that the blktap2 process is
> * needed by the soon to be started domain and do nothing.
should be another explicit case statement.
> */
> + libxl__gc gc = LIBXL_INIT_GC(ctx);
> + int ret;
Please declare these at the top of the function.
>
> + switch (disk->backend) {
> + case LIBXL_DISK_BACKEND_QDISK:
> + if (disk->format != LIBXL_DISK_FORMAT_RAW) {
> + LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Locally detach a
> non-raw "
> + "qdisk image");
> + ret = nbd_unmount_disk(&gc, diskpath);
> + return ret;
> + }
> + default:
> + break;
> + }
> +
> + libxl__free_all(&gc);
> return 0;
> }
>
> diff -r b4cf57bbc3fb tools/libxl/libxl.h
> --- a/tools/libxl/libxl.hThu Oct 20 15:24:46 2011 +0800
> +++ b/tools/libxl/libxl.hFri Oct 28 20:50:36 2011 +0800
> @@ -390,7 +390,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, u
> * Make a disk available in this domain. Returns path to a device.
> */
> char * libxl_device_disk_local_attach(libxl_ctx *ctx,
> libxl_device_disk *disk);
> -int libxl_device_disk_local_detach(libxl_ctx *ctx, libxl_device_disk
> *disk);
> +int libxl_device_disk_local_detach(libxl_ctx *ctx, libxl_device_disk
> *disk, char *diskpath);
>
> int libxl_device_nic_init(libxl_device_nic *nic, int dev_num);
> int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid,
> libxl_device_nic *nic);
> diff -r b4cf57bbc3fb tools/libxl/libxl_bootloader.c
> --- a/tools/libxl/libxl_bootloader.cThu Oct 20 15:24:46 2011 +0800
> +++ b/tools/libxl/libxl_bootloader.cFri Oct 28 20:50:36 2011 +0800
> @@ -424,7 +424,7 @@ int libxl_run_bootloader(libxl_ctx *ctx,
> rc = 0;
> out_close:
> if (diskpath) {
> - libxl_device_disk_local_detach(ctx, disk);
> + libxl_device_disk_local_detach(ctx, disk, diskpath);
> free(diskpath);
> }
> if (fifo_fd > -1)
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home