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.kum...@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.
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.


Reply via email to