[libvirt] [PATCH] build: update gnulib for pipe on mingw

* .gnulib: Update to latest. * bootstrap.conf (gnulib_modules): Import pipe-posix for mingw. * src/remote/remote_driver.c (pipe): Drop dead macro. --- Matthias complianed on IRC that mingw builds are broken since they unconditionally build virCommand, which unconditionally uses pipe() and waitpid() even though those are missing on mingw. I've fixed pipe() in gnulib. waitpid() will need a bit more invasive fix; I'm thinking of creating: struct virPid { #ifdef WIN32 HANDLE pid; #else pid_t pid; #endif }; /* Wait for a child process to complete. */ int virPidWait(virPid pid, int *status) { #ifdef WIN32 whatever windows provides #else return waitpid(pid.pid, status, 0); #endif } or something like that. * .gnulib 6491120...e2f1471 (3):
pipe-posix: new module * build-aux/gendocs.sh: restore x bit autoupdate
.gnulib | 2 +- bootstrap.conf | 1 + src/remote/remote_driver.c | 5 ----- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/.gnulib b/.gnulib index 6491120..e2f1471 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 64911207854610668b480939469282fdaeb96f74 +Subproject commit e2f1471b021a285916339a73bc12c6b44dbf9a76 diff --git a/bootstrap.conf b/bootstrap.conf index 2ad1957..dbed752 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -48,6 +48,7 @@ mktempd netdb perror physmem +pipe-posix poll posix-shell pthread diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e6eb9b5..fae191c 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -86,11 +86,6 @@ #define VIR_FROM_THIS VIR_FROM_REMOTE -#ifdef WIN32 -# define pipe(fds) _pipe(fds,4096, _O_BINARY) -#endif - - static int inside_daemon = 0; struct remote_thread_call; -- 1.7.3.2

On 12/10/2010 03:55 PM, Eric Blake wrote:
I've fixed pipe() in gnulib. waitpid() will need a bit more invasive fix; I'm thinking of creating:
struct virPid { #ifdef WIN32 HANDLE pid; #else pid_t pid; #endif };
Not quite necessary; _spawn() in mingw returns a pid_t, and _cwait() in mingw comes pretty close to approximating waitpid(). But still some gymnastics to perform. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

* .gnulib: Update to latest. * bootstrap.conf (gnulib_modules): Import pipe-posix for mingw. * src/remote/remote_driver.c (pipe) [WIN32]: Drop dead macro. * daemon/event.c (pipe) [WIN32]: Drop dead function. --- v2: also fix daemon, and add two more patches that allow ./autobuild.sh to once again pass a mingw cross-build. .gnulib | 2 +- bootstrap.conf | 1 + daemon/event.c | 6 ------ src/remote/remote_driver.c | 5 ----- 4 files changed, 2 insertions(+), 12 deletions(-) diff --git a/.gnulib b/.gnulib index 6491120..e2f1471 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 64911207854610668b480939469282fdaeb96f74 +Subproject commit e2f1471b021a285916339a73bc12c6b44dbf9a76 diff --git a/bootstrap.conf b/bootstrap.conf index 2ad1957..dbed752 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -48,6 +48,7 @@ mktempd netdb perror physmem +pipe-posix poll posix-shell pthread diff --git a/daemon/event.c b/daemon/event.c index 01cb674..89ca9f0 100644 --- a/daemon/event.c +++ b/daemon/event.c @@ -621,12 +621,6 @@ static void virEventHandleWakeup(int watch ATTRIBUTE_UNUSED, virMutexUnlock(&eventLoop.lock); } -#ifdef WIN32 -static inline int pipe(int fd[2]) { - return _pipe(fd, 4096, 0); -} -#endif - int virEventInit(void) { if (virMutexInit(&eventLoop.lock) < 0) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e6eb9b5..fae191c 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -86,11 +86,6 @@ #define VIR_FROM_THIS VIR_FROM_REMOTE -#ifdef WIN32 -# define pipe(fds) _pipe(fds,4096, _O_BINARY) -#endif - - static int inside_daemon = 0; struct remote_thread_call; -- 1.7.3.2

* src/util/util.h (virWaitPid): New prototype. * src/util/util.c (virWaitPid): Implement it. * src/util/command.c (virCommandWait): Use it. * src/libvirt_private.syms: Export it. --- Should I convert over most remaining instances of waitpid() and add a 'make syntax-check' entry? The only place that can't convert is lxc_controller.c (it uses WNOHANG instead of 0), but lxc is already Linux-specific, so it can be exempted from a syntax-check. src/libvirt_private.syms | 1 + src/util/command.c | 3 +-- src/util/util.c | 20 ++++++++++++++++++++ src/util/util.h | 5 +++++ 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b24ca70..840588a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -862,6 +862,7 @@ virStrcpy; virStrncpy; virTimestamp; virVasprintf; +virWaitPid; # uuid.h diff --git a/src/util/command.c b/src/util/command.c index f9d475e..257b9cb 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -1204,8 +1204,7 @@ virCommandWait(virCommandPtr cmd, int *exitstatus) /* Wait for intermediate process to exit */ - while ((ret = waitpid(cmd->pid, &status, 0)) == -1 && - errno == EINTR); + ret = virWaitPid(cmd->pid, &status); if (ret == -1) { virReportSystemError(errno, _("unable to wait for process %d"), diff --git a/src/util/util.c b/src/util/util.c index 1b5bc68..7eb8662 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -876,7 +876,19 @@ virRunWithHook(const char *const*argv, return ret; } +/* Perform a blocking wait for the child process to complete, and put + * the child's exit status in *status. Return 0 if successful, -1 on + * failure to wait. */ +int virWaitPid(pid_t pid, int *status) +{ + int ret; + while ((ret = waitpid(pid, status, 0) == -1) && errno == EINTR); + + return ret < 0 ? -1 : 0; +} + #else /* WIN32 */ +# include <process.h> int virSetCloseExec(int fd ATTRIBUTE_UNUSED) { @@ -940,6 +952,14 @@ virFork(pid_t *pid) return -1; } +/* Perform a blocking wait for the child process to complete, and put + * the child's exit status in *status. Return 0 if successful, -1 on + * failure to wait. */ +int virWaitPid(pid_t pid, int *status) +{ + return _cwait(status, pid, 0) < 0 ? -1 : 0; +} + #endif /* WIN32 */ int diff --git a/src/util/util.h b/src/util/util.h index ef4fc13..16f7f54 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -53,6 +53,11 @@ enum { int virSetNonBlock(int fd) ATTRIBUTE_RETURN_CHECK; int virSetCloseExec(int fd) ATTRIBUTE_RETURN_CHECK; +/* Perform a blocking wait for the child process to complete, and put + * the child's exit status in *status. Return 0 if successful, -1 on + * failure to wait. */ +int virWaitPid(pid_t pid, int *status); + /* This will execute in the context of the first child * after fork() but before execve() */ typedef int (*virExecHook)(void *data); -- 1.7.3.2

On 12/10/2010 05:07 PM, Eric Blake wrote:
* src/util/util.h (virWaitPid): New prototype. * src/util/util.c (virWaitPid): Implement it. * src/util/command.c (virCommandWait): Use it. * src/libvirt_private.syms: Export it. ---
Should I convert over most remaining instances of waitpid() and add a 'make syntax-check' entry? The only place that can't convert is lxc_controller.c (it uses WNOHANG instead of 0), but lxc is already Linux-specific, so it can be exempted from a syntax-check.
self-NACK. Gnulib provides a waitpid() module, that we should just use instead of redoing it ourselves. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Allows compilation, but no creation of child processes yet. Take it one step at a time. * src/util/util.c (virExecWithHook) [WIN32]: New dummy function. * src/libvirt_private.syms: Export it. --- src/libvirt_private.syms | 1 + src/util/util.c | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 840588a..2c17fd1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -811,6 +811,7 @@ virEventAddHandle; virEventRemoveHandle; virExec; virExecDaemonize; +virExecWithHook; virFileAbsPath; virFileDeletePid; virFileExists; diff --git a/src/util/util.c b/src/util/util.c index 7eb8662..6c83955 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -925,6 +925,28 @@ virExec(const char *const*argv ATTRIBUTE_UNUSED, } int +virExecWithHook(const char *const*argv ATTRIBUTE_UNUSED, + const char *const*envp ATTRIBUTE_UNUSED, + const fd_set *keepfd ATTRIBUTE_UNUSED, + pid_t *retpid ATTRIBUTE_UNUSED, + int infd ATTRIBUTE_UNUSED, + int *outfd ATTRIBUTE_UNUSED, + int *errfd ATTRIBUTE_UNUSED, + int flags ATTRIBUTE_UNUSED, + virExecHook hook ATTRIBUTE_UNUSED, + void *data ATTRIBUTE_UNUSED, + char *pidfile ATTRIBUTE_UNUSED) +{ + /* Some day we can implement pieces of virExec on top of _spawn() + * or CreateProcess(), but we can't implement everything, since + * mingw completely lacks fork(), so we cannot run hook code in + * the child. */ + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virExec is not implemented for WIN32")); + return -1; +} + +int virExecDaemonize(const char *const*argv ATTRIBUTE_UNUSED, const char *const*envp ATTRIBUTE_UNUSED, const fd_set *keepfd ATTRIBUTE_UNUSED, -- 1.7.3.2

* .gnulib: Update to latest. * bootstrap.conf (gnulib_modules): Import pipe-posix and waitpid for mingw. * src/remote/remote_driver.c (pipe) [WIN32]: Drop dead macro. * daemon/event.c (pipe) [WIN32]: Drop dead function. --- v3: also import waitpid module v2: fix compilation error in daemon v1: initial patch .gnulib | 2 +- bootstrap.conf | 2 ++ daemon/event.c | 6 ------ src/remote/remote_driver.c | 5 ----- 4 files changed, 3 insertions(+), 12 deletions(-) diff --git a/.gnulib b/.gnulib index 6491120..980f9d2 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 64911207854610668b480939469282fdaeb96f74 +Subproject commit 980f9d2ceb43f9d86ea57db0367e569267c8571b diff --git a/bootstrap.conf b/bootstrap.conf index 2ad1957..282768b 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -48,6 +48,7 @@ mktempd netdb perror physmem +pipe-posix poll posix-shell pthread @@ -76,6 +77,7 @@ usleep vasprintf verify vc-list-files +waitpid ' # Additional xgettext options to use. Use "\\\newline" to break lines. diff --git a/daemon/event.c b/daemon/event.c index 01cb674..89ca9f0 100644 --- a/daemon/event.c +++ b/daemon/event.c @@ -621,12 +621,6 @@ static void virEventHandleWakeup(int watch ATTRIBUTE_UNUSED, virMutexUnlock(&eventLoop.lock); } -#ifdef WIN32 -static inline int pipe(int fd[2]) { - return _pipe(fd, 4096, 0); -} -#endif - int virEventInit(void) { if (virMutexInit(&eventLoop.lock) < 0) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e6eb9b5..fae191c 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -86,11 +86,6 @@ #define VIR_FROM_THIS VIR_FROM_REMOTE -#ifdef WIN32 -# define pipe(fds) _pipe(fds,4096, _O_BINARY) -#endif - - static int inside_daemon = 0; struct remote_thread_call; -- 1.7.3.3

2010/12/13 Eric Blake <eblake@redhat.com>:
* .gnulib: Update to latest. * bootstrap.conf (gnulib_modules): Import pipe-posix and waitpid for mingw. * src/remote/remote_driver.c (pipe) [WIN32]: Drop dead macro. * daemon/event.c (pipe) [WIN32]: Drop dead function. ---
v3: also import waitpid module v2: fix compilation error in daemon v1: initial patch
ACK. Matthias

Allows compilation, but no creation of child processes yet. Take it one step at a time. * src/util/util.c (virExecWithHook) [WIN32]: New dummy function. * src/libvirt_private.syms: Export it. --- v3: tweak comments v2: patch introduced into series src/libvirt_private.syms | 1 + src/util/util.c | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b24ca70..0e3033d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -811,6 +811,7 @@ virEventAddHandle; virEventRemoveHandle; virExec; virExecDaemonize; +virExecWithHook; virFileAbsPath; virFileDeletePid; virFileExists; diff --git a/src/util/util.c b/src/util/util.c index 1b5bc68..41dbefd 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -913,6 +913,28 @@ virExec(const char *const*argv ATTRIBUTE_UNUSED, } int +virExecWithHook(const char *const*argv ATTRIBUTE_UNUSED, + const char *const*envp ATTRIBUTE_UNUSED, + const fd_set *keepfd ATTRIBUTE_UNUSED, + pid_t *retpid ATTRIBUTE_UNUSED, + int infd ATTRIBUTE_UNUSED, + int *outfd ATTRIBUTE_UNUSED, + int *errfd ATTRIBUTE_UNUSED, + int flags ATTRIBUTE_UNUSED, + virExecHook hook ATTRIBUTE_UNUSED, + void *data ATTRIBUTE_UNUSED, + char *pidfile ATTRIBUTE_UNUSED) +{ + /* XXX: Some day we can implement pieces of virCommand/virExec on + * top of _spawn() or CreateProcess(), but we can't implement + * everything, since mingw completely lacks fork(), so we cannot + * run hook code in the child. */ + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virExec is not implemented for WIN32")); + return -1; +} + +int virExecDaemonize(const char *const*argv ATTRIBUTE_UNUSED, const char *const*envp ATTRIBUTE_UNUSED, const fd_set *keepfd ATTRIBUTE_UNUSED, -- 1.7.3.3

2010/12/13 Eric Blake <eblake@redhat.com>:
Allows compilation, but no creation of child processes yet. Take it one step at a time.
* src/util/util.c (virExecWithHook) [WIN32]: New dummy function. * src/libvirt_private.syms: Export it. ---
v3: tweak comments v2: patch introduced into series
src/libvirt_private.syms | 1 + src/util/util.c | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 0 deletions(-)
ACK. Matthias

On 12/13/2010 01:59 PM, Matthias Bolte wrote:
2010/12/13 Eric Blake <eblake@redhat.com>:
Allows compilation, but no creation of child processes yet. Take it one step at a time.
* src/util/util.c (virExecWithHook) [WIN32]: New dummy function. * src/libvirt_private.syms: Export it. ---
v3: tweak comments v2: patch introduced into series
src/libvirt_private.syms | 1 + src/util/util.c | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 0 deletions(-)
ACK.
Thanks; I've pushed both mingw-related fixes. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Matthias Bolte