Change in vdsm[master]: vdsm: check libvirt availability at startup

fromani at redhat.com fromani at redhat.com
Thu Nov 27 11:18:58 UTC 2014


Francesco Romani has posted comments on this change.

Change subject: vdsm: check libvirt availability at startup
......................................................................


Patch Set 2: Code-Review-1

(2 comments)

need to write better commit message, still not clear and too long.

http://gerrit.ovirt.org/#/c/35596/2//COMMIT_MSG
Commit Message:

Line 27: although unlikely, that VDSM respawns faster than
Line 28: libvirt, especially on overloaded systems.
Line 29: 
Line 30: This patch adds an explicit check about libvirt
Line 31: availability, and makes VDSM panic() if no connection
> But what's the benefit of dying early? Which latency is optimized by this p
Apologies for still not clear despite overly long commit message. Will improve.

The core issue here is there is no protection if open_connection() fails. The only thing we do is log the stack trace and that's it.

No recovery will take place and the host will be reported in the 'recovery' state forever.

See this stack trace as an example:
http://fpaste.org/154365/

(I probably need to rewrite the commit message including it).
Line 32: at all is possible.
Line 33: It must be noted that vulnerability windows
Line 34: still exists, as in theory libvirt may crash anytime
Line 35: (e.g. just after the initial connection check).


http://gerrit.ovirt.org/#/c/35596/2/vdsm/vdsm
File vdsm/vdsm:

Line 83:         conn = libvirtconnection.get()
Line 84:     except Exception:
Line 85:         conn = None
Line 86:     if not conn:
Line 87:         utils.panic("failed to connect to libvirt")
> Any specific reason to avoid panic in except clause?
Probably not, if I make sure that libvirtconnection.open_connection() always return a valid connection object or raises.
To put differently, I need to make sure it cannot return None. Will check
Line 88: 
Line 89:     if config.getboolean('irs', 'irs_enable'):
Line 90:         try:
Line 91:             irs = Dispatcher(HSM())


-- 
To view, visit http://gerrit.ovirt.org/35596
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I36137a424427b03a5dd3f24b5a7af1af454d0a23
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list