Francesco Romani has posted comments on this change.

Change subject: vm: handle missing domains on recovery
......................................................................


Patch Set 4:

(4 comments)

http://gerrit.ovirt.org/#/c/25275/4//COMMIT_MSG
Commit Message:

Line 21: Id495f6047ba658c2b04da19bd7bf76425b3b9659
Line 22: 
Line 23: In this case, the reconnection attempt to libvirt from VDSM
Line 24: will cause a domain lookup fail, but we still want
Line 25: to create an hollow VM object, granted it is in status Down.
> Please consider replacing with a paragraph like:
Will amend.
Line 26: 
Line 27: This patch add an explicit check in the recovery path to
Line 28: make sure that a VM is either created with a valid libvirt
Line 29: domain handle, or it is reported as Down so the engine


Line 23: In this case, the reconnection attempt to libvirt from VDSM
Line 24: will cause a domain lookup fail, but we still want
Line 25: to create an hollow VM object, granted it is in status Down.
Line 26: 
Line 27: This patch add an explicit check in the recovery path to
> add->adds
Done
Line 28: make sure that a VM is either created with a valid libvirt
Line 29: domain handle, or it is reported as Down so the engine
Line 30: can collect its state and explicitely destroy it.
Line 31: 


http://gerrit.ovirt.org/#/c/25275/4/vdsm/vm.py
File vdsm/vm.py:

Line 2348:                 # we cannot continue without a libvirt connection 
handle
Line 2349:                 # to avoid state desync or worse split-brain 
scenarios.
Line 2350:                 if isinstance(e, libvirt.libvirtError) and \
Line 2351:                    e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
Line 2352:                     raise LibvirtConnectionError()
> I am ashamed to admit for how long did the exception name confused me... Th
Right, will fix.
Line 2353:                 elif not self.recovering:
Line 2354:                     raise
Line 2355:                 else:
Line 2356:                     self.log.info("Skipping errors on recovery", 
exc_info=True)


Line 2378:             self.saveState()
Line 2379:         except LibvirtConnectionError:
Line 2380:             self.recovering = False
Line 2381:             # we cannot ever deal with this, not even on recovery.
Line 2382:             self.setDownStatus(ERROR, 
vmexitreason.LIBVIRT_CONNECT_FAILED)
> here, too, please find a name that would not throw the poor reviewer to the
My bad, will fix.
Line 2383:         except Exception as e:
Line 2384:             if self.recovering:
Line 2385:                 self.log.info("Skipping errors on recovery", 
exc_info=True)
Line 2386:             else:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I00ef12883c8035209de0f273925eb8603d6b6da8
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to