[PATCH v2] Add VM info to improve error log message for qemu monitor
Rohit Kumar
rohit.kumar3 at nutanix.com
Mon Jan 3 12:06:35 UTC 2022
Ping. Please look at this patch.
Link:
https://listman.redhat.com/archives/libvir-list/2021-December/msg00891.html
On 23/12/21 12:09 pm, Rohit Kumar wrote:
> This patch is to determine the VM which had IO or socket hangup error.
> Accessing directly vm->def->name inside qemuMonitorIO() or qemuMonitorSend()
> might leads to illegal access as we are out of 'vm' context and vm->def might
> not exist. Adding a field "domainName" inside mon object to access vm name
> and initialising it when creating mon object.
>
> Signed-off-by: Rohit Kumar <rohit.kumar3 at nutanix.com>
> ---
> diff to v1:
> - Adding a field domainName inside _qemuMonitor struct for accessing vm name
> instead of directly accessing mon->vm->def->name.
> - Link to v1: https://listman.redhat.com/archives/libvir-list/2021-December/msg00217.html
> - Talked with peter on RFC and he suggested me to add a field inside
> monitor struct to get VM name.
>
> src/qemu/qemu_monitor.c | 47 +++++++++++++++++++++++++----------------
> 1 file changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index dda6ae9796..c3a0227795 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -80,6 +80,7 @@ struct _qemuMonitor {
> GSource *watch;
>
> virDomainObj *vm;
> + char *domainName;
>
> qemuMonitorCallbacks *cb;
> void *callbackOpaque;
> @@ -243,6 +244,7 @@ qemuMonitorDispose(void *obj)
> virCondDestroy(&mon->notify);
> g_free(mon->buffer);
> g_free(mon->balloonpath);
> + g_free(mon->domainName);
> }
>
>
> @@ -530,13 +532,18 @@ qemuMonitorIO(GSocket *socket G_GNUC_UNUSED,
> qemuMonitor *mon = opaque;
> bool error = false;
> bool hangup = false;
> + virDomainObj *vm = NULL;
> + char *vmName = NULL;
>
> virObjectRef(mon);
>
> + vm = mon->vm;
> + vmName = mon->domainName;
> +
> /* lock access to the monitor and protect fd */
> virObjectLock(mon);
> #if DEBUG_IO
> - VIR_DEBUG("Monitor %p I/O on socket %p cond %d", mon, socket, cond);
> + VIR_DEBUG("Monitor %p I/O on socket %p cond %d vm=%p name=%s", mon, socket, cond, vm, NULLSTR(vmName));
> #endif
> if (mon->fd == -1 || !mon->watch) {
> virObjectUnlock(mon);
> @@ -583,8 +590,8 @@ qemuMonitorIO(GSocket *socket G_GNUC_UNUSED,
>
> if (!error && !mon->goteof &&
> cond & G_IO_ERR) {
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("Invalid file descriptor while waiting for monitor"));
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("%s: Invalid file descriptor while waiting for monitor"), NULLSTR(vmName));
> mon->goteof = true;
> }
> }
> @@ -609,13 +616,14 @@ qemuMonitorIO(GSocket *socket G_GNUC_UNUSED,
> virResetLastError();
> } else {
> if (virGetLastErrorCode() == VIR_ERR_OK && !mon->goteof)
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("Error while processing monitor IO"));
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("%s: Error while processing monitor IO"), NULLSTR(vmName));
> virCopyLastError(&mon->lastError);
> virResetLastError();
> }
>
> - VIR_DEBUG("Error on monitor %s", NULLSTR(mon->lastError.message));
> + VIR_DEBUG("Error on monitor %s mon=%p vm=%p name=%s",
> + NULLSTR(mon->lastError.message), mon, vm, NULLSTR(vmName));
> /* If IO process resulted in an error & we have a message,
> * then wakeup that waiter */
> if (mon->msg && !mon->msg->finished) {
> @@ -631,22 +639,22 @@ qemuMonitorIO(GSocket *socket G_GNUC_UNUSED,
> * will try to acquire the virDomainObj *mutex next */
> if (mon->goteof) {
> qemuMonitorEofNotifyCallback eofNotify = mon->cb->eofNotify;
> - virDomainObj *vm = mon->vm;
>
> /* Make sure anyone waiting wakes up now */
> virCondSignal(&mon->notify);
> virObjectUnlock(mon);
> - VIR_DEBUG("Triggering EOF callback");
> + VIR_DEBUG("Triggering EOF callback mon=%p vm=%p name=%s",
> + mon, vm, NULLSTR(vmName));
> (eofNotify)(mon, vm, mon->callbackOpaque);
> virObjectUnref(mon);
> } else if (error) {
> qemuMonitorErrorNotifyCallback errorNotify = mon->cb->errorNotify;
> - virDomainObj *vm = mon->vm;
>
> /* Make sure anyone waiting wakes up now */
> virCondSignal(&mon->notify);
> virObjectUnlock(mon);
> - VIR_DEBUG("Triggering error callback");
> + VIR_DEBUG("Triggering error callback mon=%p vm=%p name=%s",
> + mon, vm, NULLSTR(vmName));
> (errorNotify)(mon, vm, mon->callbackOpaque);
> virObjectUnref(mon);
> } else {
> @@ -694,6 +702,7 @@ qemuMonitorOpenInternal(virDomainObj *vm,
> mon->fd = fd;
> mon->context = g_main_context_ref(context);
> mon->vm = virObjectRef(vm);
> + mon->domainName = g_strdup(vm->def->name);
> mon->waitGreeting = true;
> mon->cb = cb;
> mon->callbackOpaque = opaque;
> @@ -932,17 +941,19 @@ qemuMonitorSend(qemuMonitor *mon,
> qemuMonitorMessage *msg)
> {
> int ret = -1;
> + virDomainObj *vm = mon->vm;
> + char *vmName = mon->domainName;
>
> /* Check whether qemu quit unexpectedly */
> if (mon->lastError.code != VIR_ERR_OK) {
> - VIR_DEBUG("Attempt to send command while error is set %s",
> - NULLSTR(mon->lastError.message));
> + VIR_DEBUG("Attempt to send command while error is set %s mon=%p vm=%p name=%s",
> + NULLSTR(mon->lastError.message), mon, vm, NULLSTR(vmName));
> virSetError(&mon->lastError);
> return -1;
> }
> if (mon->goteof) {
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("End of file from qemu monitor"));
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("%s: End of file from qemu monitor"), NULLSTR(vmName));
> return -1;
> }
>
> @@ -955,15 +966,15 @@ qemuMonitorSend(qemuMonitor *mon,
>
> while (!mon->msg->finished) {
> if (virCondWait(&mon->notify, &mon->parent.lock) < 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("Unable to wait on monitor condition"));
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("%s: Unable to wait on monitor condition"), NULLSTR(vmName));
> goto cleanup;
> }
> }
>
> if (mon->lastError.code != VIR_ERR_OK) {
> - VIR_DEBUG("Send command resulted in error %s",
> - NULLSTR(mon->lastError.message));
> + VIR_DEBUG("Send command resulted in error %s mon=%p vm=%p name=%s",
> + NULLSTR(mon->lastError.message), mon, vm, NULLSTR(vmName));
> virSetError(&mon->lastError);
> goto cleanup;
> }
More information about the libvir-list
mailing list