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

[Laine Stump]

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 at 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.