On 09.12.2013 07:23, Michael Chapman wrote:
> virtlockd's argv[0] may not contain a full path to the binary, however
> it should contain something that can be looked up in the PATH. Use
> execvp() to do path lookup when re-executing ourselves.
>
> After re-execution, we must not attempt to daemonize again. It's not
> only unnecessary, but it also means we end up with the wrong PID and so
> we can't validate the state file.
>
> Instead, build a new argv for the new program that does not include
> --daemon.
>
> Signed-off-by: Michael Chapman <mike@xxxxxxxxxxxxxxxxx>
> ---
> src/locking/lock_daemon.c | 49 +++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
> index ae09ef8..32ca4d6 100644
> --- a/src/locking/lock_daemon.c
> +++ b/src/locking/lock_daemon.c
> @@ -1000,6 +1000,7 @@ cleanup:
>
> static int
> virLockDaemonPreExecRestart(virNetServerPtr srv,
> + const char *argv0,
> char **argv)
> {
> virJSONValuePtr child;
> @@ -1075,7 +1076,7 @@ virLockDaemonPreExecRestart(virNetServerPtr srv,
> goto cleanup;
> }
>
> - if (execv(argv[0], argv) < 0) {
> + if (execvp(argv0, argv) < 0) {
> virReportSystemError(errno, "%s",
> _("Unable to restart self"));
While this chunk is okay,
> goto cleanup;
> @@ -1389,11 +1390,47 @@ int main(int argc, char **argv) {
> virNetServerUpdateServices(lockDaemon->srv, true);
> virNetServerRun(lockDaemon->srv);
>
> - if (execRestart &&
> - virLockDaemonPreExecRestart(lockDaemon->srv, argv) < 0)
> - ret = VIR_LOCK_DAEMON_ERR_REEXEC;
> - else
> - ret = 0;
> + ret = VIR_LOCK_DAEMON_ERR_NONE;
> + if (execRestart) {
> + char **restart_argv = NULL;
> + size_t count = 0;
> +
> + if (VIR_EXPAND_N(restart_argv, count, 1) < 0)
> + goto no_memory;
> + if (VIR_STRDUP(restart_argv[count - 1], argv[0]) < 0)
> + goto no_memory;
> + if (verbose) {
> + if (VIR_EXPAND_N(restart_argv, count, 1) < 0)
> + goto no_memory;
> + if (VIR_STRDUP(restart_argv[count - 1], "--verbose") < 0)
> + goto no_memory;
> + }
> + if (remote_config_file && !implicit_conf) {
> + if (VIR_EXPAND_N(restart_argv, count, 2) < 0)
> + goto no_memory;
> + if (VIR_STRDUP(restart_argv[count - 2], "--config") < 0)
> + goto no_memory;
> + if (VIR_STRDUP(restart_argv[count - 1], remote_config_file) < 0)
> + goto no_memory;
> + }
> + if (pid_file) {
> + if (VIR_EXPAND_N(restart_argv, count, 2) < 0)
> + goto no_memory;
> + if (VIR_STRDUP(restart_argv[count - 2], "--pid-file") < 0)
> + goto no_memory;
> + if (VIR_STRDUP(restart_argv[count - 1], pid_file) < 0)
> + goto no_memory;
> + }
> + if (VIR_EXPAND_N(restart_argv, count, 1) < 0)
> + goto no_memory;
> +
> + if (virLockDaemonPreExecRestart(lockDaemon->srv, argv[0], restart_argv) < 0)
> + ret = VIR_LOCK_DAEMON_ERR_REEXEC;
> +
> + while (count)
> + VIR_FREE(restart_argv[--count]);
> + VIR_FREE(restart_argv);
> + }
>
> cleanup:
> virObjectUnref(lockProgram);
>
This one isn't. It requires us to expand it whenever a new cmd line
argument is invented. How about copying all arguments but "--daemon" or
"-d"?
(Just an explicit note - I'm not pushing this one now)
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list