Change in vdsm[master]: detach: Support force detach on orphaned Storage Domains.

nsoffer at redhat.com nsoffer at redhat.com
Wed Jun 25 15:18:24 UTC 2014


Nir Soffer has posted comments on this change.

Change subject: detach: Support force detach on orphaned Storage Domains.
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.ovirt.org/#/c/29042/4/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 756:         vars.task.getExclusiveLock(STORAGE, spUUID)
Line 757:         pool = self.getPool(spUUID)
Line 758:         if sdUUID == pool.masterDomain.sdUUID:
Line 759:             raise se.CannotDetachMasterStorageDomain(sdUUID)
Line 760:         dom = sdCache.produce(sdUUID=sdUUID)
> IMO because when you run forcedDetachStorageDomain you may expect that the 
This is true if this delete the domain, but this just modify single line in the domain meta data (or vg tag).
Line 761:         dom.acquireHostId(pool.id)
Line 762:         try:
Line 763:             dom.acquireClusterLock(pool.id)
Line 764:             try:


Line 757:         pool = self.getPool(spUUID)
Line 758:         if sdUUID == pool.masterDomain.sdUUID:
Line 759:             raise se.CannotDetachMasterStorageDomain(sdUUID)
Line 760:         dom = sdCache.produce(sdUUID=sdUUID)
Line 761:         dom.acquireHostId(pool.id)
> Please move this block into a different private method.
Sure this is better - but why not move into StorageDomain.forceDetach() ?
Line 762:         try:
Line 763:             dom.acquireClusterLock(pool.id)
Line 764:             try:
Line 765:                 for domPoolUUID in dom.getPools():


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4a6fb01775f7c834a3466e45b9b7ed9b4bd5c3be
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipchuk at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipchuk at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer 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