On Mon, Dec 16, 2013 at 09:50:30AM -0500, Cole Robinson wrote:

On 12/16/2013 04:27 AM, Laine Stump wrote:

On 12/14/2013 07:15 PM, Cole Robinson wrote:

On 12/11/2013 03:33 PM, Michele Paolino wrote:

In libvirt, the default error message length is 1024 bytes. This is not
enough for qemu to print long error messages such as the list of
supported ARM machine models (more than 1700 chars). This is
raised when the machine entry in the XML file is wrong, but there may
be now or in future other verbose error messages.
The above patch enables libvirt to print error messages >1024 for qemu.

Signed-off-by: Michele Paolino <m.paolino@virtualopensystems.com>
---
 src/qemu/qemu_process.c |   23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index bd9546e..c2e2136 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1904,10 +1904,25 @@ cleanup:
          * a possible read of the fd in the monitor code doesn't influence this
          * error delivery option */
         ignore_value(lseek(logfd, pos, SEEK_SET));
-        qemuProcessReadLog(logfd, buf + len, buf_size - len - 1, 0, true);
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("process exited while connecting to monitor: %s"),
-                       buf);
+        len = qemuProcessReadLog(logfd, buf + len, buf_size - len - 1, 0, true);
+
+        /* virReportError error buffer is limited to 1024 byte*/
+        if (len < 1024){
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                            _("process exited while connecting to monitor: %s"),
+                            buf);
+        } else {
+             if (STRPREFIX(buf, "Supported machines are:"))
+                 virReportError(VIR_ERR_INTERNAL_ERROR,
+                     _("process exited while connecting to monitor:"
+                       "please check machine model"));
+             else
+                 virReportError(VIR_ERR_INTERNAL_ERROR,
+                     _("process exited while connecting to monitor"));
+
+             VIR_ERROR("%s", buf);
+        }
+
         ret = -1;
     }
 

This kind of error scraping is a slipper slop IMO, and for this particular
case I don't think it's even that interesting.

I definitely agree with that.

Libvirt already parses the machine types for each qemu emulator and reports it
in virsh capabilities XML. Seems reasonable that we could validate the XML
machine type at define time and catch an invalid machine type error long
before we even try and start the VM.

Do we really want to refuse to define a particular guest just because
the host where we're defining it doesn't currently support the given
machine type? That's yet another slippery slope - for example, should we
also be validating all the devices in <hostdev> at definition time? I
think the proper time to do that validation is at domain start time when
we should have the proper qemu binary (and necessary drivers/hardware)
available.

My suggestion for define time was because IIRC that's where we do the
capabilities arch + os type + domain type validation as well.

Doing validation at that level is nice because we don't need to duplicate it
in each driver, but it also sucks when qemu is accidentally uninstalled and
libvirt is restarted, it now refuses to parse all those pre-existing qemu/kvm
configs, and virsh list --all is empty to the great confusion of users.

Yeah, that is actually quite a big problem I'd like us to fix one day
by moving that validation out of the define stage into the start stage

Daniel