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

Re: [Xen-devel]xl create PV guest with qcow/qcow2 disk images fail



On Wed, 2 Nov 2011, Chun Yan Liu wrote:
> Stefano, could you review the revised patch and share your comments? Thanks.

Sure.
 
> >>> 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();
> +    if (pid < 0)
> +        return -1;
> +    else if (pid == 0){
> +        execvp(arg0, args);
> +        exit(127);
> +    }
> +    sleep(1);

In a following email you wrote that without the sleep the device is
"not prepared yet".
What do you mean by that?
The device is present but reading/writing to it returns an error?
If so, rather than a sleep we need an explicit wait for the device
to be ready. Even trying to read from the device in a loop until it
succeeds would be better than a sleep. At least we would know exactly
what we are doing and why we are doing it.


> +    while (waitpid(pid, &status, 0) < 0) {
> +        if (errno != EINTR) {
> +            status = -1;
> +            break;
> +        }
> +    }
> +    return status;
> +}

Here you are waiting for the death of qemu-nbd; I thought that qemu-nbd
needs to be kept running?


> +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};
> +    char *ret = NULL;
> +
> +    for (i = 0; i < nbds_max; i++) {
> +        nbd_dev = libxl__sprintf(gc, "/dev/nbd%d", i);
> +        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;
> +}

This is not great. I would read /proc/partitions instead.
Also keep in mind that xl works on BSD now so at the very least you need
to ifdef all the linux specific code.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

 


Rackspace

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