Change in ovirt-engine[ovirt-engine-4.1]: backend: new exclusive lock for SyncLunsCommand

[Code Review]

>From Tal Nisan <tnisan at 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 at 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 at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Daniel Erez <derez at redhat.com>
Gerrit-Reviewer: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>