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

On Tue, Jan 04, 2022 at 14:10:49 +0530, Rohit Kumar wrote: > > On 03/01/22 10:12 pm, Peter Krempa wrote: > > On Wed, Dec 22, 2021 at 22:39:21 -0800, 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(a)nutanix.com&gt; > > > --- [...] > > > @@ -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)); > > Same here. Additionally did you ever get to a situation where vmName > > would be NULL? > There might be a situation when g_strdup() fails to allocate vm name, for > e.g. if host ran out of memory ? No, g_strdup on out of memory condition abort()s the program completely. The only time when g_strdup returns NULL is if the argument is NULL. > Let me know your thoughts on this, I would be happy to remove NULLSTR if > it's not the case. That depends if you happened to see whether all callers avoid passing NULL name for the VM which is very likely.