[libvirt] [PATCH] qemu: Fix segmentation fault on reconnect

[Marc Hartmayer]

On Fri, Jun 08, 2018 at 01:26 PM +0200, Erik Skultety <eskultet at redhat.com> wrote:
> On Fri, Jun 08, 2018 at 12:49:07PM +0200, Marc Hartmayer wrote:
>> On start up of libvirtd the worker pool of the QEMU driver must be
>> initialized before trying to reconnect to all the running QEMU
>> instances. Otherwise segmentation faults can occur if there are QEMU
>> monitor events emitted.
>>
>>  #0  __GI___pthread_mutex_lock (mutex=mutex at entry=0x40) at ../nptl/pthread_mutex_lock.c:67
>>  #1  0x000003fffdba9e62 in virMutexLock (m=m at entry=0x40) at ../../src/util/virthread.c:89
>>  #2  0x000003fffdbab2dc in virThreadPoolSendJob (pool=0x0, priority=priority at entry=0, jobData=0x1000b7210) at ../../src/util/virthreadpool.c:386
>>  #3  0x000003ffd8343b70 in qemuProcessHandleSerialChanged (mon=<optimized out>, vm=0x3ff70074340, devAlias=<optimized out>, connected=<optimized out>, opaque=0x3ff70013f70) at ../../src/qemu/qemu_process.c:1480
>>  #4  0x000003ffd836a776 in qemuMonitorEmitSerialChange (mon=mon at entry=0x3ff500013f0, devAlias=devAlias at entry=0x1000b6940 "channel0", connected=<optimized out>) at ../../src/qemu/qemu_monitor.c:1558
>>  #5  0x000003ffd8378e52 in qemuMonitorJSONHandleSerialChange (mon=0x3ff500013f0, data=0x1000b79a0) at ../../src/qemu/qemu_monitor_json.c:1133
>>  #6  0x000003ffd8378930 in qemuMonitorJSONIOProcessEvent (mon=<optimized out>, mon at entry=0x3ff500013f0, obj=obj at entry=0x1000b6780) at ../../src/qemu/qemu_monitor_json.c:182
>>  #7  0x000003ffd837edee in qemuMonitorJSONIOProcessLine (mon=0x3ff500013f0, mon at entry=<error reading variable: value has been optimized out>, line=0x1000b8760 "{\"timestamp\": {\"seconds\": 1528444997, \"microseconds\": 125231}, \"event\": \"VSERPORT_CHANGE\", \"data\": {\"open\": false, \"id\": \"channel0\"}}", msg=0x3ff917f9850, msg at entry=<error reading variable: value has been optimized out>) at ../../src/qemu/qemu_monitor_json.c:211
>>  #8  0x000003ffd837ef86 in qemuMonitorJSONIOProcess (mon=<optimized out>, mon at entry=0x3ff500013f0, data=0x10008c4c0 "{\"return\": [{\"arch\": \"other\", \"current\": true, \"props\": {\"core-id\": 0}, \"CPU\": 0, \"qom_path\": \"/machine/unattached/device[0]\", \"halted\": false, \"thread_id\": 131501}, {\"arch\": \"other\", \"current\": false"..., len=477, msg=<optimized out>, msg at entry=0x3ff917f9850) at ../../src/qemu/qemu_monitor_json.c:253
>>  #9  0x000003ffd836757a in qemuMonitorIOProcess (mon=mon at entry=0x3ff500013f0) at ../../src/qemu/qemu_monitor.c:447
>>  #10 0x000003ffd836863e in qemuMonitorIO (watch=<error reading variable: value has been optimized out>, fd=<optimized out>, events=0, events at entry=<error reading variable: value has been optimized out>, opaque=0x3ff500013f0, opaque at entry=<error reading variable: value has been optimized out>) at ../../src/qemu/qemu_monitor.c:702
>>  #11 0x000003fffdb4033a in virEventPollDispatchHandles (nfds=<optimized out>, fds=0x1000b7160) at ../../src/util/vireventpoll.c:508
>>  #12 0x000003fffdb4055e in virEventPollRunOnce () at ../../src/util/vireventpoll.c:657
>>  #13 0x000003fffdb3e782 in virEventRunDefaultImpl () at ../../src/util/virevent.c:327
>>  #14 0x000003fffdc89400 in virNetDaemonRun (dmn=0x1000b2510) at ../../src/rpc/virnetdaemon.c:850
>>  #15 0x000000010002a816 in main (argc=<optimized out>, argv=<optimized out>) at ../../src/remote/remote_daemon.c:1460
>>
>> Signed-off-by: Marc Hartmayer <mhartmay at linux.ibm.com>
>> Reviewed-by: Bjoern Walk <bwalk at linux.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy at linux.ibm.com>
>> ---
>>  src/qemu/qemu_driver.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 971cf3c2750a..28769878cc58 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -932,12 +932,15 @@ qemuStateInitialize(bool privileged,
>>                              qemuDomainManagedSaveLoad,
>>                              qemu_driver);
>>
>> -    qemuProcessReconnectAll(qemu_driver);
>> -
>> +    /* must be initialized before trying to reconnect to all the
>> +     * running domains since there might occur some QEMU monitor
>> +     * events that will be dispatched to the worker pool */
>>      qemu_driver->workerPool = virThreadPoolNew(0, 1, 0, qemuProcessEventHandler, qemu_driver);
>>      if (!qemu_driver->workerPool)
>>          goto error;
>>
>> +    qemuProcessReconnectAll(qemu_driver);
>> +
>>      virNWFilterRegisterCallbackDriver(&qemuCallbackDriver);
>>      return 0;
>
> Wow, how could we have missed this for so long...
>
> Reviewed-by: Erik Skultety <eskultet at redhat.com>
>
> PS: I'll strip some bits of the backtrace so it looks more compact in the
> history before pushing.

Okay :) Thanks for the review.

>
-- 
Beste Grüße / Kind regards
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294