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

[Peter Krempa]

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

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.

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131216/3a535c1e/attachment-0001.sig>