[libvirt] [PATCH v2] qemu_domain: add timestamp in tainting of guests log

[John Ferlan]

On 01/20/2017 03:46 AM, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao at gmail.com>
> 
> We lacked of timestamp in tainting of guests log,
> which bring troubles for finding guest issues:
> such as whether a guest powerdown caused by qemu-monitor-command
> or others issues inside guests.
> If we had timestamp in tainting of guests log,
> it would be helpful when checking guest's /var/log/messages.
> 
> Signed-off-by: Chen Hanxiao <chenhanxiao at gmail.com>
> ---
> v2:
>    update logic flow of virDomainObjTaint check
> 
>  src/qemu/qemu_domain.c | 79 +++++++++++++++++++++++++++-----------------------
>  1 file changed, 42 insertions(+), 37 deletions(-)
> 

ACK and pushed with one minor change below

John

> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index c676042..2bc023c 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3999,46 +3999,51 @@ void qemuDomainObjTaint(virQEMUDriverPtr driver,
>  {
>      virErrorPtr orig_err = NULL;
>      bool closeLog = false;
> +    char *timestamp = NULL;
> +    char uuidstr[VIR_UUID_STRING_BUFLEN];
>  
> -    if (virDomainObjTaint(obj, taint)) {
> -        char uuidstr[VIR_UUID_STRING_BUFLEN];
> -        virUUIDFormat(obj->def->uuid, uuidstr);
> -
> -        VIR_WARN("Domain id=%d name='%s' uuid=%s is tainted: %s",
> -                 obj->def->id,
> -                 obj->def->name,
> -                 uuidstr,
> -                 virDomainTaintTypeToString(taint));
> -
> -        /* We don't care about errors logging taint info, so
> -         * preserve original error, and clear any error that
> -         * is raised */
> -        orig_err = virSaveLastError();
> -        if (logCtxt == NULL) {
> -            logCtxt = qemuDomainLogContextNew(driver, obj,
> -                                              QEMU_DOMAIN_LOG_CONTEXT_MODE_ATTACH);
> -            if (!logCtxt) {
> -                if (orig_err) {
> -                    virSetError(orig_err);
> -                    virFreeError(orig_err);
> -                }
> -                VIR_WARN("Unable to open domainlog");
> -                return;
> -            }
> -            closeLog = true;
> -        }
> +    if (!virDomainObjTaint(obj, taint))
> +        return;
> +
> +    virUUIDFormat(obj->def->uuid, uuidstr);
> +
> +    VIR_WARN("Domain id=%d name='%s' uuid=%s is tainted: %s",
> +             obj->def->id,
> +             obj->def->name,
> +             uuidstr,
> +             virDomainTaintTypeToString(taint));
>  
> -        if (qemuDomainLogContextWrite(logCtxt,
> -                                      "Domain id=%d is tainted: %s\n",
> -                                      obj->def->id,
> -                                      virDomainTaintTypeToString(taint)) < 0)
> -            virResetLastError();
> -        if (closeLog)
> -            qemuDomainLogContextFree(logCtxt);
> -        if (orig_err) {
> -            virSetError(orig_err);
> -            virFreeError(orig_err);
> +    if ((timestamp = virTimeStringNow()) == NULL)

use "if (!(timestamp = virTimeStringNow()))" and move after the orig_err
since we don't what to lose the original error...


> +        goto cleanup;
> +
> +    /* We don't care about errors logging taint info, so
> +     * preserve original error, and clear any error that
> +     * is raised */
> +    orig_err = virSaveLastError();
> +    if (logCtxt == NULL) {
> +        logCtxt = qemuDomainLogContextNew(driver, obj,
> +                                          QEMU_DOMAIN_LOG_CONTEXT_MODE_ATTACH);
> +        if (!logCtxt) {
> +            VIR_WARN("Unable to open domainlog");
> +            goto cleanup;
>          }
> +        closeLog = true;
> +    }
> +
> +    if (qemuDomainLogContextWrite(logCtxt,
> +                                  "%s: Domain id=%d is tainted: %s\n",
> +                                  timestamp,
> +                                  obj->def->id,
> +                                  virDomainTaintTypeToString(taint)) < 0)
> +        virResetLastError();
> +
> + cleanup:
> +    VIR_FREE(timestamp);
> +    if (closeLog)
> +        qemuDomainLogContextFree(logCtxt);
> +    if (orig_err) {
> +        virSetError(orig_err);
> +        virFreeError(orig_err);
>      }
>  }
>  
>