On 01/20/2017 03:46 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 /var/log/messages.

Signed-off-by: Chen Hanxiao <chenhanxiao@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); } }