Eduardo has posted comments on this change.
Change subject: Stop spm if refresh fail on bad parameters.
......................................................................
Patch Set 1: (2 inline comments)
....................................................
File vdsm/storage/hsm.py
Line 788: se.StoragePoolActionError(
Line 789: "spUUID=%s, msdUUID=%s, masterVersion=%s" %
Line 790: (spUUID, msdUUID, masterVersion)))
Line 791: self.getPool(spUUID) # Validate that is the correct pool.
Line 792: vars.task.getExclusiveLock(STORAGE, spUUID)
This change certainly opens the debate.
Is needed because the host pool view is changed during it.
Which pool ops can be started after this call and should operate concurrently
in an uncertain state of the pool?
If this is the spm should be no difference because the state should be already
known but in error that be lead to the pool disconnection.
I addition all the spm ops take the exclusive lock.
This verb can be starved sending concurrent pool shared lock operations:
updateVM, removeVM, uploadVM, getVmsList, getImageDomainsList. Should be
considered if these operations needs pool lock or lock in the MSD or lock-free.
In addition the shared lock is taken by
getStoragePoolInfo : No sense to run concurrently with refresh.
Line 793: pool = self.getPool(spUUID)
Line 794: try:
Line 795: self.validateSdUUID(msdUUID)
Line 796: pool.refresh(msdUUID, masterVersion)
Line 796: pool.refresh(msdUUID, masterVersion)
Line 797: except (se.StorageDomainAccessError,
se.StorageDomainDoesNotExist,
Line 798: se.StoragePoolWrongMaster):
Line 799: self.log.error("refreshStoragePool failed", exc_info=True)
Line 800: pool.stopSpm()
If not, calling stopSpm is innocuous.
Line 801: self._disconnectPool(pool, pool.id, pool.scsiKey, False)
Line 802: raise
Line 803:
Line 804: if pool.hsmMailer:
--
To view, visit http://gerrit.ovirt.org/13930
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f85c3e731551b6974483cffe38c7ac37281370b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Daniel Paikov <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
