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
