Arik Hadas has posted comments on this change.

Change subject: core: lock disks when migrating VMs
......................................................................


Patch Set 2: (3 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java
Line 36:     private static final long serialVersionUID = -89419649366187512L;
Line 37:     private Guid _vdsDestinationId;
Line 38:     protected boolean forcedMigrationForNonMigratableVM;
Line 39:     private List<Disk> pluggedVmDisks;
Line 40:     private Map<String, Pair<String, String>> sharedLockMap;
this map will always be null when calling getSharedLocks so this class-field 
can be removed
Line 41: 
Line 42:     /**
Line 43:      * Used to log the migration error.
Line 44:      */


Line 299:         }
Line 300:     }
Line 301: 
Line 302:     @Override
Line 303:     protected Map<String, Pair<String, String>> getSharedLocks() {
I removed such code that calls the parent lately - maybe we should think of an 
improvement to the infrastructure that will collect all locks in the hierarchy. 
adding such code here can be useful if the parent's locks will be relevant, but 
in our inheritance model, it might not be the case - and then we'll take 
unnecessary locks without noticing..
Line 304:         if (sharedLockMap == null) {
Line 305:             List<Disk> disksToLock = getVmPluggedDisks();
Line 306:             if (!disksToLock.isEmpty()) {
Line 307:                 sharedLockMap = new HashMap<String, Pair<String, 
String>>(disksToLock.size());


Line 305:             List<Disk> disksToLock = getVmPluggedDisks();
Line 306:             if (!disksToLock.isEmpty()) {
Line 307:                 sharedLockMap = new HashMap<String, Pair<String, 
String>>(disksToLock.size());
Line 308:                 for (Disk disk : disksToLock) {
Line 309:                     sharedLockMap.put(disk.getId().toString(), 
LockMessagesMatchUtil.DISK);
maybe adding more informative error message instead of the default one?
Line 310:                 }
Line 311:             }
Line 312:         }
Line 313:         return sharedLockMap;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9a335c3269400b355acd566c8f39e4a50fa237b
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: Vered Volansky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to