* [PATCH] io/command: implement portable spawn
@ 2022-09-01 11:15 marcandre.lureau
2022-09-01 13:00 ` Daniel P. Berrangé
0 siblings, 1 reply; 3+ messages in thread
From: marcandre.lureau @ 2022-09-01 11:15 UTC (permalink / raw)
To: qemu-devel; +Cc: berrange, bin.meng, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Using GLib spawn API is both simpler and portable.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
io/channel-command.c | 115 ++++++++-----------------------------------
1 file changed, 21 insertions(+), 94 deletions(-)
diff --git a/io/channel-command.c b/io/channel-command.c
index 9f2f4a1793..c069732105 100644
--- a/io/channel-command.c
+++ b/io/channel-command.c
@@ -26,7 +26,6 @@
#include "qemu/sockets.h"
#include "trace.h"
-#ifndef WIN32
/**
* qio_channel_command_new_pid:
* @writefd: the FD connected to the command's stdin
@@ -69,107 +68,35 @@ qio_channel_command_new_spawn(const char *const argv[],
int flags,
Error **errp)
{
- pid_t pid = -1;
- int stdinfd[2] = { -1, -1 };
- int stdoutfd[2] = { -1, -1 };
- int devnull = -1;
- bool stdinnull = false, stdoutnull = false;
+ g_autoptr(GError) err = NULL;
+ GPid pid = 0;
QIOChannelCommand *ioc;
+ GSpawnFlags gflags = G_SPAWN_CLOEXEC_PIPES;
+ int stdinfd = -1, stdoutfd = -1;
flags = flags & O_ACCMODE;
-
- if (flags == O_RDONLY) {
- stdinnull = true;
- }
- if (flags == O_WRONLY) {
- stdoutnull = true;
- }
-
- if (stdinnull || stdoutnull) {
- devnull = open("/dev/null", O_RDWR);
- if (devnull < 0) {
- error_setg_errno(errp, errno,
- "Unable to open /dev/null");
- goto error;
- }
- }
-
- if ((!stdinnull && !g_unix_open_pipe(stdinfd, FD_CLOEXEC, NULL)) ||
- (!stdoutnull && !g_unix_open_pipe(stdoutfd, FD_CLOEXEC, NULL))) {
- error_setg_errno(errp, errno,
- "Unable to open pipe");
- goto error;
- }
-
- pid = qemu_fork(errp);
- if (pid < 0) {
- goto error;
- }
-
- if (pid == 0) { /* child */
- dup2(stdinnull ? devnull : stdinfd[0], STDIN_FILENO);
- dup2(stdoutnull ? devnull : stdoutfd[1], STDOUT_FILENO);
- /* Leave stderr connected to qemu's stderr */
-
- if (!stdinnull) {
- close(stdinfd[0]);
- close(stdinfd[1]);
- }
- if (!stdoutnull) {
- close(stdoutfd[0]);
- close(stdoutfd[1]);
- }
- if (devnull != -1) {
- close(devnull);
- }
-
- execv(argv[0], (char * const *)argv);
- _exit(1);
+ gflags |= flags == O_WRONLY ? G_SPAWN_STDOUT_TO_DEV_NULL : 0;
+
+ if (!g_spawn_async_with_pipes(NULL, (char **)argv, NULL, gflags, NULL, NULL,
+ &pid,
+ flags == O_RDONLY ? NULL : &stdinfd,
+ flags == O_WRONLY ? NULL : &stdoutfd,
+ NULL, &err)) {
+ error_setg(errp, "%s", err->message);
+ return NULL;
}
- if (!stdinnull) {
- close(stdinfd[0]);
- }
- if (!stdoutnull) {
- close(stdoutfd[1]);
- }
- ioc = qio_channel_command_new_pid(stdinnull ? devnull : stdinfd[1],
- stdoutnull ? devnull : stdoutfd[0],
- pid);
- trace_qio_channel_command_new_spawn(ioc, argv[0], flags);
+ ioc = qio_channel_command_new_pid(stdinfd,
+ stdoutfd,
+#ifdef _WIN32
+ GetProcessId(pid)
+#else
+ pid
+#endif
+ );
return ioc;
-
- error:
- if (devnull != -1) {
- close(devnull);
- }
- if (stdinfd[0] != -1) {
- close(stdinfd[0]);
- }
- if (stdinfd[1] != -1) {
- close(stdinfd[1]);
- }
- if (stdoutfd[0] != -1) {
- close(stdoutfd[0]);
- }
- if (stdoutfd[1] != -1) {
- close(stdoutfd[1]);
- }
- return NULL;
-}
-
-#else /* WIN32 */
-QIOChannelCommand *
-qio_channel_command_new_spawn(const char *const argv[],
- int flags,
- Error **errp)
-{
- error_setg_errno(errp, ENOSYS,
- "Command spawn not supported on this platform");
- return NULL;
}
-#endif /* WIN32 */
#ifndef WIN32
static int qio_channel_command_abort(QIOChannelCommand *ioc,
--
2.37.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] io/command: implement portable spawn
2022-09-01 11:15 [PATCH] io/command: implement portable spawn marcandre.lureau
@ 2022-09-01 13:00 ` Daniel P. Berrangé
2022-09-01 13:19 ` Marc-André Lureau
0 siblings, 1 reply; 3+ messages in thread
From: Daniel P. Berrangé @ 2022-09-01 13:00 UTC (permalink / raw)
To: marcandre.lureau; +Cc: qemu-devel, bin.meng
On Thu, Sep 01, 2022 at 03:15:53PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Using GLib spawn API is both simpler and portable.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> io/channel-command.c | 115 ++++++++-----------------------------------
> 1 file changed, 21 insertions(+), 94 deletions(-)
> + ioc = qio_channel_command_new_pid(stdinfd,
> + stdoutfd,
> +#ifdef _WIN32
> + GetProcessId(pid)
> +#else
> + pid
> +#endif
> + );
THe pid parameter is declared as 'pid_t' but GetProcessId returns
DWORD - are those types guaranteed compatible.
Also the pid passed into qio_channel_command_new_pid is used
by qio_channel_command_close/abort, to kill off the process,
but this code is stubbed out in WIN32 and this patch hasn't
provided an impl.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] io/command: implement portable spawn
2022-09-01 13:00 ` Daniel P. Berrangé
@ 2022-09-01 13:19 ` Marc-André Lureau
0 siblings, 0 replies; 3+ messages in thread
From: Marc-André Lureau @ 2022-09-01 13:19 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, bin.meng
Hi
On Thu, Sep 1, 2022 at 5:00 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Thu, Sep 01, 2022 at 03:15:53PM +0400, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Using GLib spawn API is both simpler and portable.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > io/channel-command.c | 115 ++++++++-----------------------------------
> > 1 file changed, 21 insertions(+), 94 deletions(-)
>
> > + ioc = qio_channel_command_new_pid(stdinfd,
> > + stdoutfd,
> > +#ifdef _WIN32
> > + GetProcessId(pid)
> > +#else
> > + pid
> > +#endif
> > + );
>
> THe pid parameter is declared as 'pid_t' but GetProcessId returns
> DWORD - are those types guaranteed compatible.
I think pid_t is mingw specific, and is defined as int64.
(windows crt uses int, apparently)
>
> Also the pid passed into qio_channel_command_new_pid is used
> by qio_channel_command_close/abort, to kill off the process,
> but this code is stubbed out in WIN32 and this patch hasn't
> provided an impl.
ok, I'll update the patch and actually test it on win32 too ;)
>
>
> With regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-01 13:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-01 11:15 [PATCH] io/command: implement portable spawn marcandre.lureau
2022-09-01 13:00 ` Daniel P. Berrangé
2022-09-01 13:19 ` Marc-André Lureau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).