Re: [Qemu-devel] [PATCH v4] Add timestamp to error message

[Eric Blake]

On 06/27/2013 04:21 AM, Laszlo Ersek wrote:
> comments below
> 
> On 06/27/13 04:08, Seiji Aguchi wrote:
>> [Issue]
>> When we offer a customer support service and a problem happens
>> in a customer's system, we try to understand the problem by
>> comparing what the customer reports with message logs of the
>> customer's system.
>>

>> +extern void qemu_get_timestamp_str(char (*timestr)[]);
> 
> (a) The type of "timestr" is a valid one, it says "pointer to array of
> unknown size". The array type is an incomplete type (its size is
> unknown), but pointers can point to incomplete types (like to an opaque
> struct, which is also an incomplete type).
> 
> It's however quite unusual to write something like this, when a simple
> pointer-to-char would do.

Agreed; I made a similar comment against v3.

>>      error_print_loc();
>>      va_start(ap, fmt);
> 
> Does this print the timestamp to all kinds of monitors too? On stderr,
> the timestamp is great. When printed to an "interactive monitor", it
> could also help post-mortem debugging. But would it not confuse libvirt
> eg.? (Apologies if this has been discussed before.)

Libvirt would love to unconditionally enable timestamps in log output;
having coordination of any qemu log dumped alongside other libvirt log
output will indeed benefit from having a common timestamp synchronization.

Regarding monitor output, libvirt uses the QMP monitor, not the HMP
monitor, and this had better not prepend text to QMP output (since that
would result in non-JSON output).  It could be added as an additional
dictionary member within each QMP command, which would not impact
libvirt (which ignores dictionary elements it is not expecting, or
future libvirt could see if a timestamp was passed back).  HMP
passthrough commands called via QMP's 'human-monitor-command' might then
have the timestamp included, but that's not an issue since HMP output is
already not guaranteed stable.  But if you DO want to add it to QMP
return values, then it needs a followup patch.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature