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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list