Re: [Libvir] [PATCH] output virsh log to file
Hi,
Thank you for your review.
How about this correction?
Actually I had a closer look at this patch, and there are some
problems
(thanks to Jim Meyering who pointed these out). For example:
Using sprintf into a fixed size message buffer with no other checks may
cause a buffer overflow:
+ sprintf(msg_buf, "[%d.%02d.%02d %02d:%02d:%02d ",
I corrected to use snprintf.
You should check the return value of write:
+ /* write log */
+ write(logdef.log_fd, msg_buf, msg_len);
I added the return value check and error handling.
You don't need to zero out the stat structure before calling
stat:
+ memset(&st, 0x00, sizeof(struct stat));
+ if (stat(logdef.path_buff, &st) == 0) {
Okey, I removed memset.
What happens if this call fails?
+ rename(logdef.path_buff, bak_new_path);
I added error handling.
But my broader point is: What use would this feature be, since you
can
capture the output of virsh easily using shell redirection? The Xen
'xm' command doesn't have this feature and I don't know if anyone has
asked for it.
If I use it, the redirection is no problem.
However, when our customers are made to use virsh, it is necessary to
explain the redirection.
Mostly, a lot of customers do not seem to use the redirection usually.
And, executing the command applying the redirection to customers again
when the error occurs is troublesome of customers.
Therefore, the purpose is to make it immediately correspond to
the customer's trouble report.
Thanks,
Nobuhiro Itou.
Attachments:
- virsh_log.patch (application/octet-stream — 9.3 KB)