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@xxxxxxxxxxxxxxxxxxxxxx>
>>> ---
>>> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list