<html><head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body> <div class="moz-cite-prefix">On 07/12/21 8:44 pm, Peter Krempa wrote: </div> <blockquote type="cite" cite="mid:Ya96aFjmHNI0cM0K@angien.pipo.sk"> <pre class="moz-quote-pre" wrap="">On Tue, Dec 07, 2021 at 15:08:02 +0000, Daniel P. Berrangé wrote: </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">On Tue, Dec 07, 2021 at 04:02:11PM +0100, Peter Krempa wrote: </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">On Tue, Dec 07, 2021 at 14:53:20 +0000, Daniel P. Berrangé wrote: </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">On Tue, Dec 07, 2021 at 05:34:00AM -0800, Rohit Kumar wrote: </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">This patch is to determine the VM which had IO or socket hangup error. Signed-off-by: Rohit Kumar <a class="moz-txt-link-rfc2396E" href="mailto:rohit.kumar3@nutanix.com"><rohit.kumar3@nutanix.com></a> --- 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); + } </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> 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. </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> 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. </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> I'd be in favour of /consistently/ having 'mon=%p vm=%p' for all monitor debug logs though. </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> Use %p for 'vm' is fine as it doesn't break the locking boundary, but the patch is doing %s for vm->def->name, which, if done properly would make the code way more complex with questionable benefits.</pre> </blockquote> I wanted to add vm Name in with "End of File from qemu Monitor error" atleast, just to find which vm had a socket hangup. How should i go further on this ? To add vm name, we can do this steps : 1.) Pass virDomainObjPtr in this function after locking this. Just like we do in qemuDomainObjEnterMonitorInternal. 2.) Add a vm_name field in mon object and populate vm_name in qemuDomainObjEnterMonitorInternal, since that is still vm context.<span style="color: rgb(209, 210, 211); font-family: Slack-Lato, appleLogo, sans-serif; font-size: 15px; font-style: normal; font-variant-ligatures: common-ligatures; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: left; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(26, 29, 33); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;"> These will be complex to do. Or should i just add 'mon=%p vm=%p' to every monitor debug logs ? <blockquote type="cite" cite="mid:Ya96aFjmHNI0cM0K@angien.pipo.sk"> <pre class="moz-quote-pre" wrap=""> </pre> </blockquote> </body> </html>