Re: [libvirt] Internal error message from netcf when trying to start libvirtd

On Tue, Oct 15, 2013 at 06:03:27PM +0200, Christophe Fergeau wrote: > Hey, > > Due to a configuration issue on my system, libvirtd is not starting on my > system (not complaining about this!): > 2013-10-15 15:40:51.024+0000: 10222: info : libvirt version: 1.1.3 > 2013-10-15 15:40:51.024+0000: 10222: error : virNetTLSContextCheckCertFile:117 : Cannot read CA certificate > '/home/teuf/usr/etc/pki/CA/cacert.pem': No such file or directory > > However, before libvirtd exits, I get another error message from the netcf > code, which is unexpected this time: > 2013-10-15 15:49:18.361+0000: 10222: error : netcfStateCleanup:105 : > internal error: Attempt to close netcf state driver already closed > > This message comes from the call of virStateCleanup() at the end of main() > in libvirtd.c. virStateCleanup() should not be called before > daemonStateInit() has been called in main. > After this call, things get more ugly as daemonStateInit() calls > virStateInitialize() from a thread, so there's probably a small window for > virStateInitialize() and virStateCleanup() running concurrently if an error > occurs between the call to daemonStateInit() and the call to > virNetServerRun(). > > I'm sending this email rather than a patch as I'm not sure what is the best > way to fix it. The easy way would be for virStateCleanup() to be a noop > when virStateInitialize() hasn't been called (iow remove the error message > from netcfStateCleanup). However, this would leave this small race > condition around (which is not that bad as it would only occurs in > situations when the daemon fails to start). So another approach would be to > set a vir_state_initialized boolean once the thread has called > ivrStateInitialize, and only call virStateCleanup() when it's set. > > Or maybe there's a 3rd way to fix this? > > Let me know if you have any guidance into the best way to fix this, Yeah, I've known about this issue for a while and its a bit horrible to solve. I think we probably need to have a global mutex to serialize the virStateInitialize, virStateReload and virStateCleanup calls so that none of them can run in parallel. I'd have virGlobalInit create the mutex instance, since we know that is run from thread safe context. Then make the virState* calls hold that mutex while executing. We don't want virStateCleanup to skip execution if virStateInitialize has failed though - every callback in virStateCleanup should be written to be safe if its corresponding init function hasn't run. Just the mutex serialization ought to be sufficient I think. Daniel