io/command: use glib GSpawn, instead of open-coding fork/exec

Simplify qio_channel_command_new_spawn() with GSpawn API. This will
allow to build for WIN32 in the following patches.

As pointed out by Daniel Berrangé: there is a change in semantics here
too. The current code only touches stdin/stdout/stderr. Any other FDs
which do NOT have O_CLOEXEC set will be inherited. With the new code,
all FDs except stdin/out/err will be explicitly closed, because we don't
set the flag G_SPAWN_LEAVE_DESCRIPTORS_OPEN. The only place we use
QIOChannelCommand today is the migration exec: protocol, and that is
only declared to use stdin/stdout.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20221006113657.2656108-5-marcandre.lureau@redhat.com>
This commit is contained in:
Marc-André Lureau 2022-10-06 15:36:55 +04:00
parent bb06b0143b
commit a95570e3e4
2 changed files with 18 additions and 87 deletions

View File

@ -41,7 +41,7 @@ struct QIOChannelCommand {
QIOChannel parent; QIOChannel parent;
int writefd; int writefd;
int readfd; int readfd;
pid_t pid; GPid pid;
}; };

View File

@ -31,7 +31,7 @@
* qio_channel_command_new_pid: * qio_channel_command_new_pid:
* @writefd: the FD connected to the command's stdin * @writefd: the FD connected to the command's stdin
* @readfd: the FD connected to the command's stdout * @readfd: the FD connected to the command's stdout
* @pid: the PID of the running child command * @pid: the PID/HANDLE of the running child command
* @errp: pointer to a NULL-initialized error object * @errp: pointer to a NULL-initialized error object
* *
* Create a channel for performing I/O with the * Create a channel for performing I/O with the
@ -50,7 +50,7 @@
static QIOChannelCommand * static QIOChannelCommand *
qio_channel_command_new_pid(int writefd, qio_channel_command_new_pid(int writefd,
int readfd, int readfd,
pid_t pid) GPid pid)
{ {
QIOChannelCommand *ioc; QIOChannelCommand *ioc;
@ -69,94 +69,24 @@ qio_channel_command_new_spawn(const char *const argv[],
int flags, int flags,
Error **errp) Error **errp)
{ {
pid_t pid = -1; g_autoptr(GError) err = NULL;
int stdinfd[2] = { -1, -1 }; GPid pid = 0;
int stdoutfd[2] = { -1, -1 }; GSpawnFlags gflags = G_SPAWN_CLOEXEC_PIPES | G_SPAWN_DO_NOT_REAP_CHILD;
int devnull = -1; int stdinfd = -1, stdoutfd = -1;
bool stdinnull = false, stdoutnull = false;
QIOChannelCommand *ioc;
flags = flags & O_ACCMODE; flags = flags & O_ACCMODE;
gflags |= flags == O_WRONLY ? G_SPAWN_STDOUT_TO_DEV_NULL : 0;
if (flags == O_RDONLY) { if (!g_spawn_async_with_pipes(NULL, (char **)argv, NULL, gflags, NULL, NULL,
stdinnull = true; &pid,
} flags == O_RDONLY ? NULL : &stdinfd,
if (flags == O_WRONLY) { flags == O_WRONLY ? NULL : &stdoutfd,
stdoutnull = true; NULL, &err)) {
error_setg(errp, "%s", err->message);
return NULL;
} }
if (stdinnull || stdoutnull) { return qio_channel_command_new_pid(stdinfd, stdoutfd, pid);
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);
}
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);
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 */ #else /* WIN32 */
@ -221,7 +151,7 @@ static void qio_channel_command_init(Object *obj)
QIOChannelCommand *ioc = QIO_CHANNEL_COMMAND(obj); QIOChannelCommand *ioc = QIO_CHANNEL_COMMAND(obj);
ioc->readfd = -1; ioc->readfd = -1;
ioc->writefd = -1; ioc->writefd = -1;
ioc->pid = -1; ioc->pid = 0;
} }
static void qio_channel_command_finalize(Object *obj) static void qio_channel_command_finalize(Object *obj)
@ -239,6 +169,7 @@ static void qio_channel_command_finalize(Object *obj)
#ifndef WIN32 #ifndef WIN32
qio_channel_command_abort(ioc, NULL); qio_channel_command_abort(ioc, NULL);
#endif #endif
g_spawn_close_pid(ioc->pid);
} }
} }