On 01/16/2017 04:19 AM, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> ---
> 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
> }
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
