[libvirt] [PATCH] qemu: Add support for changing timeout value to open unix monitor socket
I have fixed all comments except: "We prefer passing the driver as an argument instead". I thought of it at first but I looked at the function call graph and it's not that easy. and you did the same in src/qemu/qemu_process.c:115
Adding an option to change monitor socket opening timeout the current default is 3 seconds and in some cases it's not enough Signed-off-by: Pavel Fux <pavel@stratoscale.com> CC: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu.conf | 7 +++++++ src/qemu/qemu_conf.c | 2 ++ src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_monitor.c | 13 +++++++++++++ 4 files changed, 24 insertions(+) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 0f0a24c..1b96077 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -409,3 +409,10 @@ # Defaults to -1. # #seccomp_sandbox = 1 + + +#If you sometimes get the message "monitor socket did not show up: No such file or directory" +#that could be because qemu did not wait enough time, you can try increasing +#this timeout, the default is 3 seconds +# +#monitor_socket_open_timeout = 30 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 7c3f317..4f35801 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -520,6 +520,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, GET_VALUE_LONG("seccomp_sandbox", cfg->seccompSandbox); + GET_VALUE_LONG("monitor_socket_open_timeout", cfg->monitorSocketOpenTimeout); + ret = 0; cleanup: diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 77d3d2f..625e69d 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -148,6 +148,8 @@ struct _virQEMUDriverConfig { unsigned int keepAliveCount; int seccompSandbox; + + int monitorSocketOpenTimeout; }; /* Main driver state */ diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 1b1d4a1..86e3dba 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -29,6 +29,7 @@ #include <fcntl.h> #include "qemu_monitor.h" +#include "qemu_conf.h" #include "qemu_monitor_text.h" #include "qemu_monitor_json.h" #include "virerror.h" @@ -47,6 +48,8 @@ #define DEBUG_IO 0 #define DEBUG_RAW_IO 0 +extern virQEMUDriverPtr qemu_driver; + struct _qemuMonitor { virObjectLockable parent; @@ -252,6 +255,16 @@ qemuMonitorOpenUnix(const char *monitor, pid_t cpid) int monfd; int timeout = 3; /* In seconds */ int ret, i = 0; + virQEMUDriverConfigPtr cfg = NULL; + + if (qemu_driver != NULL){ + cfg = virQEMUDriverGetConfig(qemu_driver); + if (cfg->monitorSocketOpenTimeout != 0){ + timeout = cfg->monitorSocketOpenTimeout; + } + virObjectUnref(cfg); + cfg = NULL; + } if ((monfd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) { virReportSystemError(errno, -- 1.8.3.1
On Thu, Jan 02, 2014 at 06:29:35PM +0200, Pavel Fux wrote:
Adding an option to change monitor socket opening timeout the current default is 3 seconds and in some cases it's not enough
This should change the *default* to something way bigger than 3 seconds, since lots of people are hitting this. Just look at the dozen Ubuntu forum posts and many other bug reports: https://www.google.co.uk/search?q="monitor+socket+did+not+show+up" Sure it can be configurable if you want, but I don't want to have to tell each and every libguestfs user that they should change this to something sensible. Make the default something much larger. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
On Tue, Jan 07, 2014 at 10:28:41AM +0000, Richard W.M. Jones wrote:
On Thu, Jan 02, 2014 at 06:29:35PM +0200, Pavel Fux wrote:
Adding an option to change monitor socket opening timeout the current default is 3 seconds and in some cases it's not enough
This should change the *default* to something way bigger than 3 seconds, since lots of people are hitting this. Just look at the dozen Ubuntu forum posts and many other bug reports:
https://www.google.co.uk/search?q="monitor+socket+did+not+show+up"
Sure it can be configurable if you want, but I don't want to have to tell each and every libguestfs user that they should change this to something sensible. Make the default something much larger.
I agree with you here, we could change it to at least 5 seconds *and* make it configurable. I'll propose the change of the default in separate patch and try reviewing this ASAP. Martin
On Tue, Jan 07, 2014 at 11:37:11AM +0100, Martin Kletzander wrote:
On Tue, Jan 07, 2014 at 10:28:41AM +0000, Richard W.M. Jones wrote:
On Thu, Jan 02, 2014 at 06:29:35PM +0200, Pavel Fux wrote:
Adding an option to change monitor socket opening timeout the current default is 3 seconds and in some cases it's not enough
This should change the *default* to something way bigger than 3 seconds, since lots of people are hitting this. Just look at the dozen Ubuntu forum posts and many other bug reports:
https://www.google.co.uk/search?q="monitor+socket+did+not+show+up"
Sure it can be configurable if you want, but I don't want to have to tell each and every libguestfs user that they should change this to something sensible. Make the default something much larger.
I agree with you here, we could change it to at least 5 seconds *and* make it configurable. I'll propose the change of the default in separate patch and try reviewing this ASAP.
Is there a reason not to make it 30 seconds? The only downside I can see is that if qemu is really hanging/broken users will have to wait 30 seconds to see that it is hanging, versus waiting 3 or 5 seconds. I can't see that is much of a drawback. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
On Tue, Jan 07, 2014 at 10:39:26AM +0000, Richard W.M. Jones wrote:
On Tue, Jan 07, 2014 at 11:37:11AM +0100, Martin Kletzander wrote:
On Tue, Jan 07, 2014 at 10:28:41AM +0000, Richard W.M. Jones wrote:
On Thu, Jan 02, 2014 at 06:29:35PM +0200, Pavel Fux wrote:
Adding an option to change monitor socket opening timeout the current default is 3 seconds and in some cases it's not enough
This should change the *default* to something way bigger than 3 seconds, since lots of people are hitting this. Just look at the dozen Ubuntu forum posts and many other bug reports:
https://www.google.co.uk/search?q="monitor+socket+did+not+show+up"
Sure it can be configurable if you want, but I don't want to have to tell each and every libguestfs user that they should change this to something sensible. Make the default something much larger.
I agree with you here, we could change it to at least 5 seconds *and* make it configurable. I'll propose the change of the default in separate patch and try reviewing this ASAP.
Is there a reason not to make it 30 seconds?
The only downside I can see is that if qemu is really hanging/broken users will have to wait 30 seconds to see that it is hanging, versus waiting 3 or 5 seconds. I can't see that is much of a drawback.
No, that's the only one. Unfortunately many users can get nervy when they have to wait more than necessary amount of time to see that something is not working. Let's see if someone else has an opinion, otherwise I'll try to talk to others and propose a reasonable value with some explanations of the decision. Martin
On 01/07/2014 05:39 AM, Richard W.M. Jones wrote:
On Tue, Jan 07, 2014 at 11:37:11AM +0100, Martin Kletzander wrote:
On Tue, Jan 07, 2014 at 10:28:41AM +0000, Richard W.M. Jones wrote:
On Thu, Jan 02, 2014 at 06:29:35PM +0200, Pavel Fux wrote:
Adding an option to change monitor socket opening timeout the current default is 3 seconds and in some cases it's not enough
This should change the *default* to something way bigger than 3 seconds, since lots of people are hitting this. Just look at the dozen Ubuntu forum posts and many other bug reports:
https://www.google.co.uk/search?q="monitor+socket+did+not+show+up"
Sure it can be configurable if you want, but I don't want to have to tell each and every libguestfs user that they should change this to something sensible. Make the default something much larger.
I agree with you here, we could change it to at least 5 seconds *and* make it configurable. I'll propose the change of the default in separate patch and try reviewing this ASAP.
Is there a reason not to make it 30 seconds?
The only downside I can see is that if qemu is really hanging/broken users will have to wait 30 seconds to see that it is hanging, versus waiting 3 or 5 seconds. I can't see that is much of a drawback.
I agree that bumping the timeout to 30 seconds should be fine: qemu actually hanging before the monitor socket is setup is something I never hear about, so the large timeout shouldn't matter much in practice. So ACK to raising it to 30. - Cole
participants (4)
-
Cole Robinson -
Martin Kletzander -
Pavel Fux -
Richard W.M. Jones