[libvirt] [PATCH] qemu: Record timestamp for qemu domain log

[Eric Blake]

On 11/01/2010 06:46 AM, Osier Yang wrote:
> Currently only support domain start and domain shutdown, for domain
> start, adding timestamp before qemu command line, for domain shutdown,
> just say it's shutting down with timestamp.
> 
> * src/qemu/qemu_driver.c
> ---
>  src/qemu/qemu_driver.c |   42 +++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 41 insertions(+), 1 deletions(-)
> 
> +    } else if (safewrite(logfile, timestamp, strlen(timestamp)) < 0) {
> +        VIR_FREE(timestamp);
> +        VIR_WARN("Unable to write timestamp to logfile: %s",
> +                 virStrerror(errno, ebuf, sizeof ebuf));

VIR_FREE might clobber errno.  Swap these two lines.

> @@ -4178,6 +4188,31 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver,
>      virErrorPtr orig_err;
>      virDomainDefPtr def;
>      int i;
> +    int logfile = -1;
> +    char *timestamp;
> +    char ebuf[1024];
> +
> +    VIR_DEBUG0("Creating domain log file");
> +    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();
> +            goto cleanup;

Hmm.  The rest of this function is best-effort cleanup, since there is
no return value.  If you run out of memory while trying to cleanup, you
should STILL try and clean up the rest of the resources freed later in
this function, rather than immediately jumping to your cleanup: handler.

> +        } else {
> +            strcat(timestamp, "shutting down\n");

Ouch. Buffer overflow - you can't guarantee whether timestamp was
overallocated with enough memory for you to blindly strcat() data onto
the end of it.

> +
> +            if (safewrite(logfile, timestamp, strlen(timestamp)) < 0) {
> +                VIR_WARN("Unable to write timestamp to logfile: %s",
> +                          virStrerror(errno, ebuf, sizeof ebuf));
> +            }
> +
> +            VIR_FREE(timestamp);
> +        }
> +    }
> 
>      VIR_DEBUG("Shutting down VM '%s' pid=%d migrated=%d",
>                vm->def->name, vm->pid, migrated);

Also, I think you want your attempts to write to a log file to occur
after this VIR_DEBUG statement.

> @@ -4315,6 +4350,11 @@ retry:
>          virSetError(orig_err);
>          virFreeError(orig_err);
>      }
> +
> +cleanup:
> +    if (VIR_CLOSE(logfile) < 0)
> +        VIR_WARN("Unable to close logfile: %s",
> +                 virStrerror(errno, ebuf, sizeof ebuf));
>  }

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20101101/1d224e62/attachment-0001.sig>