[virt-tools-list] [virt-viewer][PATCH] Revert "virt-viewer: set keepAlive on libvirt connection"

Thu Jul 16 16:01:14 UTC 2015

----- Original Message -----
> From: "Christophe Fergeau" <cfergeau at redhat.com>
> To: "Fabiano Fidencio" <ffidenci at redhat.com>
> Cc: virt-tools-list at redhat.com
> Sent: Thursday, July 16, 2015 5:19:32 PM
> Subject: Re: [virt-tools-list] [virt-viewer][PATCH] Revert "virt-viewer: set keepAlive on libvirt connection"
> 
> On Thu, Jul 16, 2015 at 11:01:06AM -0400, Fabiano Fidencio wrote:
> > 
> > 
> > ----- Original Message -----
> > > From: "Fabiano Fidencio" <ffidenci at redhat.com>
> > > To: virt-tools-list at redhat.com
> > > Sent: Thursday, July 16, 2015 4:45:42 PM
> > > Subject: Re: [virt-tools-list] [virt-viewer][PATCH] Revert "virt-viewer:
> > > set keepAlive on libvirt connection"
> > > 
> > > Howdy!
> > > 
> > > ----- Original Message -----
> > > > From: "Fabiano Fidêncio" <fidencio at redhat.com>
> > > > To: virt-tools-list at redhat.com
> > > > Cc: "Fabiano Fidêncio" <fidencio at redhat.com>
> > > > Sent: Thursday, July 16, 2015 2:04:33 PM
> > > > Subject: [virt-viewer][PATCH] Revert "virt-viewer: set keepAlive on
> > > > libvirt
> > > > connection"
> > > > 
> > > > This reverts commit 08378ec4dc3623792c64a3bae6279eac1c3c153e.
> > > > 
> > > > The commit in question was done in order to fix rhbz#1164052, but has
> > > > been proven that it's not necessary depending on the libvirt version on
> > > > the remote host.
> > > > Considering we don't want to keep a workaround that can, actually, hide
> > > > a proper bug that must be fixed (or in virt-viewer, or elsewhere),
> > > > reverting this patch seems the safest option for now.
> > > 
> > > Jonathon asked me on IRC:
> > > 11:08 <~ jjongsma> fidencio, any additional context about that patch
> > > reverting the keepalive?
> > > 11:08 <~ jjongsma> which versions is it unnecessary for? How did you find
> > > that? etc.
> > > 11:13 <@ fidencio> jjongsma: hmmm. I will add more info there.
> > > 
> > > So, to be added to the commit message:
> > > 
> > > The patch was not necessary when connecting to a rhel6 host
> > > (libvirt-0.10.2-54) neither connecting to a fedora22 host
> > > (libvirt-1.2.13.1-2). The problem could be reproduce on rhel7 host
> > > (libvirt-1.2.17-1). And these are the systems that I have installed on my
> > > remote host machine.
> > 
> > Actually, I still can reproduce the issue on a fedora22 machine.
> > Hmm. Maybe would be better adding a big comment there than reverting the
> > patch.
> 
> Maybe it's a change of behaviour in newer libvirt releases? older ones
> did not need keep alive, newer ones do?

Hmm. Maybe.
https://bugzilla.redhat.com/show_bug.cgi?id=832081#c21

The patch seems to be hiding a crash on libvirt ... see these 2 backtraces (related to rhbz#1243228):

With the patch applied:
(gdb) where
#0  0x00007efcae715095 in g_io_create_watch () from /lib64/libglib-2.0.so.0
#1  0x00007efcae7150ef in g_io_add_watch_full () from /lib64/libglib-2.0.so.0
#2  0x00000000004275ba in virt_viewer_events_update_handle (watch=<optimized out>, events=1) at 
virt-viewer-events.c:158
#3  0x00007efcb1a62dce in virNetSocketUpdateIOCallback (sock=0x1e75c00, events=1) at rpc/virnetsocket.c:1981
#4  0x00007efcb1a50113 in virNetClientIOUpdateCallback (client=<optimized out>, enableCallback=<optimized out>) at 
rpc/virnetclient.c:1639
#5  0x00007efcb1a50f82 in virNetClientIO (thiscall=0x20e0170, client=0x1f2e060) at rpc/virnetclient.c:1793
#6  virNetClientSendInternal (client=client at entry=0x1f2e060, msg=msg at entry=0x20e0100, 
expectReply=expectReply at entry=false, nonBlock=nonBlock at entry=true) at rpc/virnetclient.c:1962
#7  0x00007efcb1a52413 in virNetClientSendNonBlock (client=0x1f2e060, msg=msg at entry=0x20e0100) at 
rpc/virnetclient.c:2036
#8  0x00007efcb1a5243d in virNetClientKeepAliveSendCB (opaque=<optimized out>, msg=0x20e0100) at 
rpc/virnetclient.c:293
#9  0x00007efcb1a5ba02 in virKeepAliveTimer (timer=<optimized out>, opaque=0x20d3d00) at rpc/virkeepalive.c:176
#10 0x00000000004272e9 in virt_viewer_events_dispatch_timeout (opaque=0x1e6cd30) at virt-viewer-events.c:233
#11 0x00007efcae7231b3 in g_timeout_dispatch () from /lib64/libglib-2.0.so.0
#12 0x00007efcae72279a in g_main_context_dispatch () from /lib64/libglib-2.0.so.0
#13 0x00007efcae722ae8 in g_main_context_iterate.isra.24 () from /lib64/libglib-2.0.so.0
#14 0x00007efcae722dba in g_main_loop_run () from /lib64/libglib-2.0.so.0
#15 0x00007efcb054a045 in gtk_main () from /lib64/libgtk-3.so.0
#16 0x0000000000410a9c in main (argc=1, argv=0x7ffde58a7978) at virt-viewer-main.c:124

Without:
(gdb) where
#0  0x00007f2dba9bf2f0 in virClassIsDerivedFrom (klass=0x48, parent=0x2bd45c0) at util/virobject.c:169
#1  0x00007f2dba9bf62e in virObjectIsClass (anyobj=anyobj at entry=0x2bd3860, klass=<optimized out>) at 
util/virobject.c:365
#2  0x00007f2dba9bf654 in virObjectLock (anyobj=0x2bd3860) at util/virobject.c:317
#3  0x00007f2dba994da5 in virDispatchError (conn=conn at entry=0x2bd3860) at util/virerror.c:620
#4  0x00007f2dbaa80958 in virConnectDomainEventDeregisterAny (conn=0x2bd3860, callbackID=0) at libvirt-domain.c:9282
#5  0x00000000004294d2 in virt_viewer_dispose (object=0x2c2b180) at virt-viewer.c:644
#6  0x00007f2db7aaaa82 in g_object_unref () from /lib64/libgobject-2.0.so.0
#7  0x0000000000410a22 in main (argc=1, argv=0x7ffc6a948c38) at virt-viewer-main.c:130

An important not is that I was not able to reproduce none of the crashes by myself. :-(

Best Regards,
--
Fabiano Fidêncio