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

Federico Simoncelli fsimonce at redhat.com
Wed Jun 25 14:41:33 UTC 2014


Federico Simoncelli 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)
> Right - but it the domain is not available, we must fail.
No. The old forcedDetachStorageDomain behavior should be maintained. Maybe it's not clear what we're doing here:

1. remove sdUUID from the pool, this is the original use case (the domain may not be reachable, it may be destroyed or lost, cannot be produced)

2. remove spUUID from a domain that may not be in the current pool (forcedDetachSD will be gracefully skipped, the domain is found and produced and the spUUID is removed)

If you enforce the domain to be present you're breaking the first use case (that actually proved to be really useful for many users in the mailing lists).
Line 761:         dom.acquireHostId(pool.id)
Line 762:         try:
Line 763:             dom.acquireClusterLock(pool.id)
Line 764:             try:


Line 767:             finally:
Line 768:                 dom.releaseClusterLock()
Line 769:         finally:
Line 770:             dom.releaseHostId(pool.id)
Line 771:         pool.forcedDetachSD(sdUUID)
> Why?
Because we're adding something new and I don't want it to prevent the old use case to work because of a bug.
Line 772: 
Line 773:     @public
Line 774:     def detachStorageDomain(self, sdUUID, spUUID, msdUUID=None,
Line 775:                             masterVersion=None, options=None):


-- 
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