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
