fog / fog-libvirt

libvirt provider for fog
MIT License
16 stars 42 forks source link

catch Libvirt exceptions in list_domains to protect against race conditions #19

Closed sford closed 8 years ago

sford commented 8 years ago

This is a followup to #15. This fixes two more areas that are susceptible to Libvirt exceptions when libvirt is being used by multiple processes. For example, multiple instances of vagrant with the vagrant-libvirt plugin.

I have been running CI tests with this change for the past couple days with pretty good success.

sford commented 8 years ago

Hey @plribeiro3000, any thoughts on this PR? Like #15 , this PR helps fix issues when using fog-libvirt in parallel.

plribeiro3000 commented 8 years ago

Maybe you could have a private method for the rescue part? I feel it very hard to follow this code, specially with an unless in the end.

Maybe something like:

def retrieve
  yield
rescue ::Libvirt::RetrieveError, ::Libvirt::Error
  nil
end
sford commented 8 years ago

@plribeiro3000,

Thanks for the feedback! Took me a while to jump back on this, but I refactored the code to use yield. I think it is a lot cleaner.

plribeiro3000 commented 8 years ago

Yes. It is indeed. Thx for bearing with me on this. 👍