[libvirt] [PATCH] qemu: Properly report a startup timeout error
The timeout errors were unconditionally being overwritten by the less helpful 'unable to start guest' error. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_driver.c | 17 +++++++---------- 1 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4374291..2172c99 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1358,15 +1358,15 @@ qemudReadLogOutput(virConnectPtr conn, buf[got] = '\0'; if (got == buflen-1) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("Out of space while reading %s log output"), - what); + _("Out of space while reading %s log output: %s"), + what, buf); return -1; } if (isdead) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("Process exited while reading %s log output"), - what); + _("Process exited while reading %s log output: %s"), + what, buf); return -1; } @@ -1378,7 +1378,8 @@ qemudReadLogOutput(virConnectPtr conn, } qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("Timed out while reading %s log output"), what); + _("Timed out while reading %s log output: %s"), + what, buf); return -1; } @@ -1557,12 +1558,8 @@ qemudWaitForMonitor(virConnectPtr conn, virStrerror(errno, ebuf, sizeof ebuf)); } - if (ret < 0) { - /* Unexpected end of file - inform user of QEMU log data */ - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("unable to start guest: %s"), buf); + if (ret < 0) return -1; - } VIR_DEBUG("Connect monitor to %p '%s'", vm, vm->def->name); if (qemuConnectMonitor(vm) < 0) -- 1.6.5.2
Currently the timeout for reading startup output is 3 seconds. If the host is under any sort of load, we can easily trigger this. Lets bump it to 30 seconds. Since the polling loop checks to see if the process has died, we shouldn't erroneously hit this timeout if qemu bombs (only if it is stuck in some infinite loop). Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2172c99..a2dab0c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1551,7 +1551,7 @@ qemudWaitForMonitor(virConnectPtr conn, ret = qemudReadLogOutput(conn, vm, logfd, buf, sizeof(buf), qemudFindCharDevicePTYs, - "console", 3); + "console", 30); if (close(logfd) < 0) { char ebuf[4096]; VIR_WARN(_("Unable to close logfile: %s"), -- 1.6.5.2
Currently the timeout for reading startup output is 3 seconds. If the host is under any sort of load, we can easily trigger this. Lets bump it to 30 seconds.
Since the polling loop checks to see if the process has died, we shouldn't erroneously hit this timeout if qemu bombs (only if it is stuck in some infinite loop).
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
It wouldn't be hard to make it fail even with 30 seconds timeout but then it doesn't really make sense to try to start a new guest on a host which is under such a big load. I think we should be fine with 30 seconds unless we want to optimize for stuck qemu. ACK
On Wed, Feb 10, 2010 at 10:49:40AM +0100, Jiri Denemark wrote:
Currently the timeout for reading startup output is 3 seconds. If the host is under any sort of load, we can easily trigger this. Lets bump it to 30 seconds.
Since the polling loop checks to see if the process has died, we shouldn't erroneously hit this timeout if qemu bombs (only if it is stuck in some infinite loop).
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
It wouldn't be hard to make it fail even with 30 seconds timeout but then it doesn't really make sense to try to start a new guest on a host which is under such a big load. I think we should be fine with 30 seconds unless we want to optimize for stuck qemu.
Hum, 30 seconds sounds a lot to me, I hope this won't mean a user waiting for a 30 second timeout in some weird cases. But we were hitting the 3s one way too easilly, going all th way to 30 should allow to detect if/when this timeout reflects at the user level and clean those case up so ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Tue, Feb 09, 2010 at 03:31:41PM -0500, Cole Robinson wrote:
The timeout errors were unconditionally being overwritten by the less helpful 'unable to start guest' error.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_driver.c | 17 +++++++---------- 1 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4374291..2172c99 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1358,15 +1358,15 @@ qemudReadLogOutput(virConnectPtr conn, buf[got] = '\0'; if (got == buflen-1) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("Out of space while reading %s log output"), - what); + _("Out of space while reading %s log output: %s"), + what, buf); return -1; }
if (isdead) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("Process exited while reading %s log output"), - what); + _("Process exited while reading %s log output: %s"), + what, buf); return -1; }
@@ -1378,7 +1378,8 @@ qemudReadLogOutput(virConnectPtr conn, }
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("Timed out while reading %s log output"), what); + _("Timed out while reading %s log output: %s"), + what, buf); return -1; }
@@ -1557,12 +1558,8 @@ qemudWaitForMonitor(virConnectPtr conn, virStrerror(errno, ebuf, sizeof ebuf)); }
- if (ret < 0) { - /* Unexpected end of file - inform user of QEMU log data */ - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("unable to start guest: %s"), buf); + if (ret < 0) return -1; - }
VIR_DEBUG("Connect monitor to %p '%s'", vm, vm->def->name); if (qemuConnectMonitor(vm) < 0)
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 02/10/2010 05:49 AM, Daniel Veillard wrote:
On Tue, Feb 09, 2010 at 03:31:41PM -0500, Cole Robinson wrote:
The timeout errors were unconditionally being overwritten by the less helpful 'unable to start guest' error.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_driver.c | 17 +++++++---------- 1 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4374291..2172c99 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1358,15 +1358,15 @@ qemudReadLogOutput(virConnectPtr conn, buf[got] = '\0'; if (got == buflen-1) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("Out of space while reading %s log output"), - what); + _("Out of space while reading %s log output: %s"), + what, buf); return -1; }
if (isdead) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("Process exited while reading %s log output"), - what); + _("Process exited while reading %s log output: %s"), + what, buf); return -1; }
@@ -1378,7 +1378,8 @@ qemudReadLogOutput(virConnectPtr conn, }
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("Timed out while reading %s log output"), what); + _("Timed out while reading %s log output: %s"), + what, buf); return -1; }
@@ -1557,12 +1558,8 @@ qemudWaitForMonitor(virConnectPtr conn, virStrerror(errno, ebuf, sizeof ebuf)); }
- if (ret < 0) { - /* Unexpected end of file - inform user of QEMU log data */ - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("unable to start guest: %s"), buf); + if (ret < 0) return -1; - }
VIR_DEBUG("Connect monitor to %p '%s'", vm, vm->def->name); if (qemuConnectMonitor(vm) < 0)
ACK,
Daniel
Thanks, I've pushed these two patches. - Cole
participants (3)
-
Cole Robinson -
Daniel Veillard -
Jiri Denemark