[PATCH] Add VM info to improve error log message for qemu monitor

[Peter Krempa]

On Tue, Dec 07, 2021 at 14:53:20 +0000, Daniel P. Berrangé wrote:
> On Tue, Dec 07, 2021 at 05:34:00AM -0800, Rohit Kumar wrote:
> > This patch is to determine the VM which had IO or socket hangup error.
> > 
> > Signed-off-by: Rohit Kumar <rohit.kumar3 at nutanix.com>
> > ---
> >  src/qemu/qemu_monitor.c | 46 +++++++++++++++++++++++++----------------
> >  1 file changed, 28 insertions(+), 18 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> > index 75e0e4ed92..d36db280d9 100644
> > --- a/src/qemu/qemu_monitor.c
> > +++ b/src/qemu/qemu_monitor.c
> > @@ -530,13 +530,19 @@ qemuMonitorIO(GSocket *socket G_GNUC_UNUSED,
> >      qemuMonitor *mon = opaque;
> >      bool error = false;
> >      bool hangup = false;
> > +    virDomainObj *vm = mon->vm;
> > +    g_autofree char *vmName = NULL;
> > +
> > +    if (vm != NULL && vm->def != NULL) {
> > +        vmName = g_strdup(vm->def->name);
> > +    }
> 
> This looks a little questionable.
> 
> Although we hold a reference on 'vm', this code doesn't do anything
> to protect its access of 'vm->def'. If we were protected when accesing
> vm->def, then we wouldn't need to strdup it anyway.

Additionally we also regularly log the monitor pointer and VM name when
entering the monitor context:

See qemuDomainObjEnterMonitorInternal:

VIR_DEBUG("Entering monitor (mon=%p vm=%p name=%s)",
          priv->mon, obj, obj->def->name);

In that place we are correctly still in the 'vm' context so we can
reference the name.

This call is usually very close to any other monitor calls or can be
easily looked up, so I don't think it's worth to increase the complexity
of the monitor code just to put the VM name in every single debug
message.