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.
static void qemudShutdownVMDaemon(struct qemud_driver *driver, virDomainObjPtr vm, int migrated) { @@ -4243,10 +4253,44 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver, virErrorPtr orig_err; virDomainDefPtr def; int i; + int logfile = -1; + char *timestamp; + char ebuf[1024];
VIR_DEBUG("Shutting down VM '%s' pid=%d migrated=%d", vm->def->name, vm->pid, migrated);
+ if ((logfile = qemudLogFD(driver, vm->def->name)) < 0) { + /* To not break the normal domain shutdown process, skip the + * timestamp log writing if failed on opening log file. */ + VIR_WARN("Unable to open logfile: %s", + virStrerror(errno, ebuf, sizeof ebuf)); + } else { + if ((timestamp = virTimestamp()) == NULL) { + virReportOOMError(); + } else { + char *shutdownMsg; + + if ((virAsprintf(&shutdownMsg, "%s: shutting down\n", + timestamp)) < 0) { + virReportOOMError(); + } else { + if (safewrite(logfile, shutdownMsg, strlen(shutdownMsg)) < 0) { + VIR_WARN("Unable to write shutdownMsg to logfile: %s", + virStrerror(errno, ebuf, sizeof ebuf)); + } + + VIR_FREE(shutdownMsg); + }
This could easily avoid needing to alloc the shutdownMsg, eg #define POSTFIX ": shutting down\n" if (safewrite(logfile, timestamp, strlen(timestamp)) < 0 || safewrite(logfile, POSTFIX, strlen(POSTFIX)) < 0) ... It might be nice to include a ': starting up\n' postfix in the startup timestamp message too Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|