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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 04/01/22 2:17 pm, Peter Krempa wrote:
On Tue, Jan 04, 2022 at 09:45:30 +0100, Peter Krempa wrote:
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@xxxxxxxxxxx>
---
[...]

@@ -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.
Alternatively you can do g_strdup(NULLSTR(vm->def->name)). Thus the
domain name variable will hold a copy of "<null>" as the name. Since
it's for error messages only this is tolerable and allows you to avoid
all the other NULLSTR usage.
Sure, this would be much better, I will update this. Thanks for the suggestion.





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux