On 01/16/2017 04:19 AM, Chen Hanxiao wrote:

From: Chen Hanxiao <chenhanxiao@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 logs(eg. /var/log/messages).

Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- src/qemu/qemu_domain.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 35baffb..21c90a9 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4001,6 +4001,7 @@ void qemuDomainObjTaint(virQEMUDriverPtr driver, bool closeLog = false;

if (virDomainObjTaint(obj, taint)) { + char *timestamp = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(obj->def->uuid, uuidstr);

@@ -4018,27 +4019,31 @@ void qemuDomainObjTaint(virQEMUDriverPtr driver,

I think rather than the current flow: 1. Move timestamp decl to the top (because #2 is moving cleanup label) 2. Move cleanup to the bottom of the function (to be consistent w/ other functions) 3. Move the timestamp fetch to before the LogContextNew If you really wanted to be creative... Create a patch 1 which reverses the virDomainObjTaint logic to be: if (!virDomainObjTaint(obj, taint)) return; Then move the subsequent lines 4 spaces to the left (e.g. reduce indentation).

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; + goto cleanup; } closeLog = true; }

+ if ((timestamp = virTimeStringNow()) == NULL) + goto cleanup; + if (qemuDomainLogContextWrite(logCtxt, - "Domain id=%d is tainted: %s\n", + "%s: Domain id=%d is tainted: %s\n", + timestamp, obj->def->id, virDomainTaintTypeToString(taint)) < 0) virResetLastError(); + + cleanup: if (closeLog) qemuDomainLogContextFree(logCtxt); if (orig_err) { virSetError(orig_err); virFreeError(orig_err); } + + VIR_FREE(timestamp);

Move this before the "closeLog" check (and the whole cleanup hunk moves outside the if statement - just looks strange inside

}

The order would be: cleanup: VIR_FREE(timestamp); if (closeLog) qemuDomainLogContextFree(logCtxt); if (orig_err) { virSetError(orig_err); virFreeError(orig_err); } John

}