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>
> > ---
[...]
> > @@ -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.