On Mon, Dec 16, 2013 at 10:35:58AM +0100, Peter Krempa wrote:
On 12/16/13 10:27, 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.
I'd say we want to do stuff like this when starting a VM rather than defining it. We have a precedent where we check the count of CPUs supported by the emulator when we start the VM rather than at define time. This allows us to craft a useful error message but doesn't forbid to define a guest and then upgrade the emulator to a version that supports all the stuff necessary.
Agreed, doing it at start time is best IMHO. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|