[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
2011/11/7 Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>: >> +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. I really don't understand why a sleep is needed before executing waitpid, I would understand that you wait before accessing the device (although, as Stefano noticed, it's not the preferred way), but this wait should not be done here, in fact I think the fork_exec function should be added to libxl_exec.c and made public. I have a patch with a libxl__forkexec function prepared for submission, which might come in handy here. >> +ÂÂÂ 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}; >> +ÂÂÂ 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. I would rather say that xl is closer to working on BSD now, but nbd is not supported on BSD (neither is qdisk), so anything related to nbd should be keept as Linux specific code. Regards, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |