Re: [libvirt] [PATCH] qemu: add support for error messages greater than 1024 characters

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(a)virtualopensystems.com&gt; >> --- >> 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.