On 06/26/2013 04:20 PM, 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. >
> +#include "qemu-common.h" > + > +/* "1970-01-01T00:00:00.999999Z" + '\0' */ > +#define TIMESTAMP_LEN 28 Self-documenting constants are nicer: #define TIMESTAMP_LEN (sizeof("1970-01-01T00:00:00.999999Z")+1) > +extern void qemu_get_timestamp_str(char (*timestr)[]); Eww. I had a tough time parsing this signature. Isn't it the same as the more legible: extern void qemu_get_timestamp_str(char **timestr); Furthermore, isn't the idea that you DON'T want to return a malloc'd string, but require that the user pre-allocate storage that we write into (since malloc during a log message reporting OOM is liable to fail, but we still want the log to be best effort)? But then why do you need a pointer to an array? Wouldn't it be better as: extern void qemu_get_timestamp_str(char *timestr, size_t len) where len is the length of timestr, and where the comments document that len should be at least TIMESTAMP_LEN to avoid truncation (or even assert() if it is not)? > +++ b/qemu-options.hx > @@ -3102,3 +3102,15 @@ HXCOMM This is the last statement. Insert new options > before this line! > STEXI > @end table > ETEXI > + > +DEF("msg", HAS_ARG, QEMU_OPTION_msg, > + "-msg [timestamp=on|off]\n" > + " output message with timestamp (default: off)\n", > + QEMU_ARCH_ALL) Did you test that the existing query-command-line-options QMP command will list this option (just making sure that libvirt will be able to know when to use this option). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature