于 2010年11月16日 23:18, Daniel P. Berrange 写道:
On Tue, Nov 16, 2010 at 10:54:34PM +0800, Osier Yang wrote:
于 2010年11月16日 21:17, Daniel P. Berrange 写道:
On Tue, Nov 16, 2010 at 04:11:40PM +0800, Osier Yang wrote:
Currently only support domain start and shutdown, for domain start,
record timestamp before the qemu command line, and for domain shutdown,
just say it's shutting down with timestamp.
* src/qemu/qemu_driver.c
---
src/qemu/qemu_driver.c | 47
+++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fd61864..423befb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3877,6 +3877,7 @@ static int qemudStartVMDaemon(virConnectPtr conn,
char ebuf[1024];
char *pidfile = NULL;
int logfile = -1;
+ char *timestamp;
qemuDomainObjPrivatePtr priv = vm->privateData;
struct qemudHookData hookData;
@@ -4084,7 +4085,17 @@ static int qemudStartVMDaemon(virConnectPtr conn,
goto cleanup;
}
+ if ((timestamp = virTimestamp()) == NULL) {
+ virReportOOMError();
+ goto cleanup;
+ } else if (safewrite(logfile, timestamp, strlen(timestamp))< 0) {
+ VIR_WARN("Unable to write timestamp to logfile: %s",
+ virStrerror(errno, ebuf, sizeof ebuf));
+ VIR_FREE(timestamp);
+ }
Surely we want the VIR_FREE(timestamp) outside the 'else if'. This
code is only freeing the memory upon write failure.
In 'if'branch, "timestamp" here returned by "virTimestamp" is
NULL, no need to free it.
And if it really needs to be freed, it should be freed
in "virTimestamp", "virTimestamp" invokes "virAsprintf"
there, but "virAsprintf" guarantees *strp is NULL upon
failure. Isn't it? just recall you reminded me before.. :-)
Consider expanding the 'else if' into a fully bracketed expression
if ((timestamp = virTimestamp()) == NULL) {
virReportOOMError();
goto cleanup;
} else {
if (safewrite(logfile, timestamp, strlen(timestamp))< 0) {
VIR_WARN("Unable to write timestamp to logfile: %s",
virStrerror(errno, ebuf, sizeof ebuf));
VIR_FREE(timestamp);
}
}
The 'else' of the second 'if' statement is missing the VIR_FREE
for timestamp
yep, right, thanks. just replied with the updated patch.
Regards,
Daniel
- Osier
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
