* [Qemu-devel] [PATCH v2] vl.c: move pidfile creation up the line
@ 2016-11-02 14:18 ` Michael Tokarev
0 siblings, 0 replies; 8+ messages in thread
From: Michael Tokarev @ 2016-11-02 14:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, Markus Armbruster, Michael Tokarev
With current code, pid file is open after various
sockets, chardevs, fsdevs and the like. This causes
interesting effects, for example when monitor is a
unix-socket, and another qemu instance is already
running, new qemu first "damages" the socket and
next complain that it can't acquire the pid file and
exits, making running qemu unreachable.
Move pid file creation earlier, right after the call
to os_daemonize(), where we know our process id (pid).
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
vl.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
v2: move the pid file creation even earlier, as per
comment by Markus Armbruster
diff --git a/vl.c b/vl.c
index 368510f..ce7e998 100644
--- a/vl.c
+++ b/vl.c
@@ -4058,6 +4058,11 @@ int main(int argc, char **argv, char **envp)
os_daemonize();
+ if (pid_file && qemu_create_pidfile(pid_file) != 0) {
+ error_report("could not acquire pid file: %s", strerror(errno));
+ exit(1);
+ }
+
if (qemu_init_main_loop(&main_loop_err)) {
error_report_err(main_loop_err);
exit(1);
@@ -4335,11 +4340,6 @@ int main(int argc, char **argv, char **envp)
}
#endif
- if (pid_file && qemu_create_pidfile(pid_file) != 0) {
- error_report("could not acquire pid file: %s", strerror(errno));
- exit(1);
- }
-
if (qemu_opts_foreach(qemu_find_opts("device"),
device_help_func, NULL, NULL)) {
exit(0);
--
2.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH v2] vl.c: move pidfile creation up the line
2016-11-02 14:18 ` [Qemu-devel] " Michael Tokarev
@ 2016-11-02 14:22 ` Daniel P. Berrange
-1 siblings, 0 replies; 8+ messages in thread
From: Daniel P. Berrange @ 2016-11-02 14:22 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-devel, qemu-trivial, Markus Armbruster
On Wed, Nov 02, 2016 at 05:18:50PM +0300, Michael Tokarev wrote:
> With current code, pid file is open after various
> sockets, chardevs, fsdevs and the like. This causes
> interesting effects, for example when monitor is a
> unix-socket, and another qemu instance is already
> running, new qemu first "damages" the socket and
> next complain that it can't acquire the pid file and
> exits, making running qemu unreachable.
>
> Move pid file creation earlier, right after the call
> to os_daemonize(), where we know our process id (pid).
>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
> vl.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2] vl.c: move pidfile creation up the line
@ 2016-11-02 14:22 ` Daniel P. Berrange
0 siblings, 0 replies; 8+ messages in thread
From: Daniel P. Berrange @ 2016-11-02 14:22 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-devel, qemu-trivial, Markus Armbruster
On Wed, Nov 02, 2016 at 05:18:50PM +0300, Michael Tokarev wrote:
> With current code, pid file is open after various
> sockets, chardevs, fsdevs and the like. This causes
> interesting effects, for example when monitor is a
> unix-socket, and another qemu instance is already
> running, new qemu first "damages" the socket and
> next complain that it can't acquire the pid file and
> exits, making running qemu unreachable.
>
> Move pid file creation earlier, right after the call
> to os_daemonize(), where we know our process id (pid).
>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
> vl.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [PATCH v2] vl.c: move pidfile creation up the line
2016-11-02 14:18 ` [Qemu-devel] " Michael Tokarev
@ 2016-11-03 7:35 ` Markus Armbruster
-1 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2016-11-03 7:35 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-devel, qemu-trivial
Michael Tokarev <mjt@tls.msk.ru> writes:
> With current code, pid file is open after various
> sockets, chardevs, fsdevs and the like. This causes
> interesting effects, for example when monitor is a
> unix-socket, and another qemu instance is already
> running, new qemu first "damages" the socket and
> next complain that it can't acquire the pid file and
> exits, making running qemu unreachable.
>
> Move pid file creation earlier, right after the call
> to os_daemonize(), where we know our process id (pid).
>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
> vl.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> v2: move the pid file creation even earlier, as per
> comment by Markus Armbruster
>
> diff --git a/vl.c b/vl.c
> index 368510f..ce7e998 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4058,6 +4058,11 @@ int main(int argc, char **argv, char **envp)
>
> os_daemonize();
>
> + if (pid_file && qemu_create_pidfile(pid_file) != 0) {
> + error_report("could not acquire pid file: %s", strerror(errno));
> + exit(1);
> + }
> +
> if (qemu_init_main_loop(&main_loop_err)) {
> error_report_err(main_loop_err);
> exit(1);
> @@ -4335,11 +4340,6 @@ int main(int argc, char **argv, char **envp)
> }
> #endif
>
> - if (pid_file && qemu_create_pidfile(pid_file) != 0) {
> - error_report("could not acquire pid file: %s", strerror(errno));
> - exit(1);
> - }
> -
> if (qemu_opts_foreach(qemu_find_opts("device"),
> device_help_func, NULL, NULL)) {
> exit(0);
Right after os_daemonize() is as early as possible. There might be
stuff happening before os_daemonize() that shouldn't, but I'm not
demanding you search for it to get this patch in.
The wider problem: we spread around parsing, checking and acting upon
command line options pretty carelessly, even though checking after
daemonize and acting before it is problematic.
Regardless,
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH v2] vl.c: move pidfile creation up the line
@ 2016-11-03 7:35 ` Markus Armbruster
0 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2016-11-03 7:35 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-devel, qemu-trivial
Michael Tokarev <mjt@tls.msk.ru> writes:
> With current code, pid file is open after various
> sockets, chardevs, fsdevs and the like. This causes
> interesting effects, for example when monitor is a
> unix-socket, and another qemu instance is already
> running, new qemu first "damages" the socket and
> next complain that it can't acquire the pid file and
> exits, making running qemu unreachable.
>
> Move pid file creation earlier, right after the call
> to os_daemonize(), where we know our process id (pid).
>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
> vl.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> v2: move the pid file creation even earlier, as per
> comment by Markus Armbruster
>
> diff --git a/vl.c b/vl.c
> index 368510f..ce7e998 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4058,6 +4058,11 @@ int main(int argc, char **argv, char **envp)
>
> os_daemonize();
>
> + if (pid_file && qemu_create_pidfile(pid_file) != 0) {
> + error_report("could not acquire pid file: %s", strerror(errno));
> + exit(1);
> + }
> +
> if (qemu_init_main_loop(&main_loop_err)) {
> error_report_err(main_loop_err);
> exit(1);
> @@ -4335,11 +4340,6 @@ int main(int argc, char **argv, char **envp)
> }
> #endif
>
> - if (pid_file && qemu_create_pidfile(pid_file) != 0) {
> - error_report("could not acquire pid file: %s", strerror(errno));
> - exit(1);
> - }
> -
> if (qemu_opts_foreach(qemu_find_opts("device"),
> device_help_func, NULL, NULL)) {
> exit(0);
Right after os_daemonize() is as early as possible. There might be
stuff happening before os_daemonize() that shouldn't, but I'm not
demanding you search for it to get this patch in.
The wider problem: we spread around parsing, checking and acting upon
command line options pretty carelessly, even though checking after
daemonize and acting before it is problematic.
Regardless,
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-trivial] [PATCH v2] vl.c: move pidfile creation up the line
2016-11-02 14:18 ` [Qemu-devel] " Michael Tokarev
@ 2016-11-03 8:41 ` Paolo Bonzini
-1 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2016-11-03 8:41 UTC (permalink / raw)
To: Michael Tokarev, qemu-devel; +Cc: qemu-trivial, Markus Armbruster
On 02/11/2016 15:18, Michael Tokarev wrote:
> With current code, pid file is open after various
> sockets, chardevs, fsdevs and the like. This causes
> interesting effects, for example when monitor is a
> unix-socket, and another qemu instance is already
> running, new qemu first "damages" the socket and
> next complain that it can't acquire the pid file and
> exits, making running qemu unreachable.
>
> Move pid file creation earlier, right after the call
> to os_daemonize(), where we know our process id (pid).
>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
> vl.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> v2: move the pid file creation even earlier, as per
> comment by Markus Armbruster
>
> diff --git a/vl.c b/vl.c
> index 368510f..ce7e998 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4058,6 +4058,11 @@ int main(int argc, char **argv, char **envp)
>
> os_daemonize();
>
> + if (pid_file && qemu_create_pidfile(pid_file) != 0) {
> + error_report("could not acquire pid file: %s", strerror(errno));
> + exit(1);
> + }
> +
> if (qemu_init_main_loop(&main_loop_err)) {
> error_report_err(main_loop_err);
> exit(1);
> @@ -4335,11 +4340,6 @@ int main(int argc, char **argv, char **envp)
> }
> #endif
>
> - if (pid_file && qemu_create_pidfile(pid_file) != 0) {
> - error_report("could not acquire pid file: %s", strerror(errno));
> - exit(1);
> - }
> -
> if (qemu_opts_foreach(qemu_find_opts("device"),
> device_help_func, NULL, NULL)) {
> exit(0);
>
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH v2] vl.c: move pidfile creation up the line
@ 2016-11-03 8:41 ` Paolo Bonzini
0 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2016-11-03 8:41 UTC (permalink / raw)
To: Michael Tokarev, qemu-devel; +Cc: qemu-trivial, Markus Armbruster
On 02/11/2016 15:18, Michael Tokarev wrote:
> With current code, pid file is open after various
> sockets, chardevs, fsdevs and the like. This causes
> interesting effects, for example when monitor is a
> unix-socket, and another qemu instance is already
> running, new qemu first "damages" the socket and
> next complain that it can't acquire the pid file and
> exits, making running qemu unreachable.
>
> Move pid file creation earlier, right after the call
> to os_daemonize(), where we know our process id (pid).
>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
> vl.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> v2: move the pid file creation even earlier, as per
> comment by Markus Armbruster
>
> diff --git a/vl.c b/vl.c
> index 368510f..ce7e998 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4058,6 +4058,11 @@ int main(int argc, char **argv, char **envp)
>
> os_daemonize();
>
> + if (pid_file && qemu_create_pidfile(pid_file) != 0) {
> + error_report("could not acquire pid file: %s", strerror(errno));
> + exit(1);
> + }
> +
> if (qemu_init_main_loop(&main_loop_err)) {
> error_report_err(main_loop_err);
> exit(1);
> @@ -4335,11 +4340,6 @@ int main(int argc, char **argv, char **envp)
> }
> #endif
>
> - if (pid_file && qemu_create_pidfile(pid_file) != 0) {
> - error_report("could not acquire pid file: %s", strerror(errno));
> - exit(1);
> - }
> -
> if (qemu_opts_foreach(qemu_find_opts("device"),
> device_help_func, NULL, NULL)) {
> exit(0);
>
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread