From Tal Nisan <tnisan@redhat.com>:

Tal Nisan has submitted this change and it was merged. Change subject: backend: new exclusive lock for SyncLunsCommand ...................................................................... backend: new exclusive lock for SyncLunsCommand This patch replaces the group of the exclusive lock that is used in SyncLunsInfoForBlockStorageDomainCommand (hereafter called "SyncLunsCommand") from SYNC_LUNS to STORAGE in order to avoid running it in parallel with ExtendSANStorageDomainCommand and ReduceSANStorageDomainDevicesCommand. To understand why it would corrupt the db, think about the following scenario: 1. There exists a block storage domain, sd1, and it is composed from one lun, lun1. 1. Thread A enters SyncLunsCommand::executeCommand, performs the call to getLunsFromVgInfo() and stores the information about lun1 in lunsFromVgInfo. 2. Thread B starts and completes the execution of ExtendSANStorageDomainCommand, so now sd1 is composed from two luns, lun1 and lun2. 3. Thread A executes the next line and stores the luns of sd1, lun1 and lun2, in lunsFromDb. 4. Thread A continues and at the end removes lun2 from the luns table, since it assumes that it's an unused and irrelevant lun. A similar case would happen with ReduceSANStorageDomainDevicesCommand (the only difference is that we will end up with another lun that should not have remained in the db). Since 13 other commands use a lock with the group STORAGE, and since we should make sure that some important commands won't get stuck when running in parallel with SyncLunsCommand, here's a list of all them: **Irrelevant commands** Following are commands that can't run in parallel with SyncLunsCommand because they will be blocked in the can do action anyway. Remember that executeCommand requires an active block domain. 1. DetachStorageDomainFromPoolCommand - sd should be on maintenance. 2. ForceRemoveStorageDomainCommand - sd cannot be active. 3. RemoveStorageDomainCommand - sd should be detached. 4. ImportHostedEngineStorageDomainCommand - sd still doesn't exist. 5. ActivateStorageDomainCommand - calls to SyncLunsCommand right after freeing the STORAGE lock. 6. AddExistingBlockStorageDomainCommand - sd still doesn't exist. 7. AttachStorageDomainToPoolCommand - sd should be detached. **Commands that we don't want to run in parallel with SyncLunsCommand** 1. ExtendSANStorageDomainCommand 2. ReduceSANStorageDomainDevicesCommand **Other commands that will collide with SyncLunsCommand** Following are commands that will collide with SyncLunsCommand, and thus the first command that will get the lock will succeed to run: 1. DeactivateStorageDomainCommand 2. ProcessOvfUpdateForStorageDomainCommand 3. ScanStorageForUnregisteredDisksCommand 4. UpdateStorageServerConnectionCommand Since SyncLunsCommand is not system initiated but user initiated, IMHO the risk that these commands will clash is presumably low. Change-Id: Ie0d56c593c5847e164f90d885a6711cd77c9c486 Related-To: https://bugzilla.redhat.com/1455548 Signed-off-by: Idan Shaby <ishaby@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/domain/SyncLunsInfoForBlockStorageDomainCommand.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/locks/LockingGroup.java 2 files changed, 1 insertion(+), 2 deletions(-) Approvals: Jenkins CI: Passed CI tests Allon Mureinik: Looks good to me, approved Idan Shaby: Verified -- To view, visit https://gerrit.ovirt.org/78056 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ie0d56c593c5847e164f90d885a6711cd77c9c486 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-4.1 Gerrit-Owner: Idan Shaby <ishaby@redhat.com> Gerrit-Reviewer: Allon Mureinik <amureini@redhat.com> Gerrit-Reviewer: Daniel Erez <derez@redhat.com> Gerrit-Reviewer: Idan Shaby <ishaby@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Tal Nisan <tnisan@redhat.com> Gerrit-Reviewer: gerrit-hooks <automation@ovirt.org>