Dan Kenigsberg has posted comments on this change.

Change subject: bootstrap: Return recovery error during lvm bootstrap
......................................................................


Patch Set 4: Code-Review-1

(2 comments)

....................................................
File vdsm/clientIF.py
Line 108:         self._prepareBindings()
Line 109: 
Line 110:     @property
Line 111:     def ready(self):
Line 112:         return (self.irs is None or self.irs.ready) and not 
self._recovery
I know that I am to blame about this one, but still:

Let's assume that _recovery changes to False immediately, but HSM() 
initialization is slow. ready() may return True as long as self.irs is None, 
and then fall back (for a while) to False. Wouldn't this confuse Engine?
Line 113: 
Line 114:     def contEIOVms(self, sdUUID, isDomainStateValid):
Line 115:         # This method is called everytime the onDomainStateChange
Line 116:         # event is emitted, this event is emitted even when a domain 
goes


....................................................
File vdsm/storage/hsm.py
Line 373:         def storageRefresh():
Line 374:             try:
Line 375:                 lvm.bootstrap(refreshlvs=blockSD.SPECIAL_LVS)
Line 376:             finally:
Line 377:                 self._ready = True
wouldn't we want to signal that only when storageRefresh is done?

The race has existed before lvm.bootstrap() but did not show up. Now we have an 
opportunity to close it down.

Also, why do we want to consider ourselves "ready" on an exception? If  
lvm.bootstrap() raised before deactivating orphan LVs, we do not want to use 
this host's storage.
Line 378:                 self.log.debug("HSM is ready")
Line 379: 
Line 380:             sdCache.refreshStorage()
Line 381: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id74468917c5b7c05d4183854e2f1255de98325dc
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Eduardo <ewars...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to