<html> <head> <meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type"> </head> <body text="#000000" bgcolor="#FFFFFF"> <div class="moz-cite-prefix">On 16/12/2013 16:00, Daniel P. Berrange wrote: </div> <blockquote cite="mid:20131216150036.GK30120@redhat.com" type="cite"> <pre wrap="">On Mon, Dec 16, 2013 at 09:50:30AM -0500, Cole Robinson wrote: </pre> <blockquote type="cite"> <pre wrap="">On 12/16/2013 04:27 AM, Laine Stump wrote: </pre> <blockquote type="cite"> <pre wrap="">On 12/14/2013 07:15 PM, Cole Robinson wrote: </pre> <blockquote type="cite"> <pre wrap="">On 12/11/2013 03:33 PM, Michele Paolino wrote: </pre> <blockquote type="cite"> <pre wrap="">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 <a class="moz-txt-link-rfc2396E" href="mailto:m.paolino@virtualopensystems.com"><m.paolino@virtualopensystems.com></a> --- 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; } </pre> </blockquote> <pre wrap="">This kind of error scraping is a slipper slop IMO, and for this particular case I don't think it's even that interesting. </pre> </blockquote> <pre wrap=""> I definitely agree with that. </pre> <blockquote type="cite"> <pre wrap=""> 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. </pre> </blockquote> <pre wrap=""> 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. </pre> </blockquote> <pre wrap=""> 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.</pre> </blockquote> </blockquote> Indeed. In my opinion start time is the best choice because it's more flexible and gives to the user the possibility to catch a more precise error message. <blockquote cite="mid:20131216150036.GK30120@redhat.com" type="cite"> <blockquote type="cite"> <pre wrap=""> </pre> </blockquote> <pre wrap=""> 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 </pre> </blockquote> <div class="moz-signature">-- Michele Paolino, Virtualization R&D Engineer Virtual Open Systems Open Source KVM Virtualization Developments Multicore Systems Virtualization Porting Services Web: <a href="http://www.virtualopensystems.com/">www.virtualopensystems.</a><a href="http://www.virtualopensystems.com/">com</a> </div> </body> </html>