Liron Ar has posted comments on this change.

Change subject: engine: Populate error message placeholder in 
HotPlugDiskToVmCommand
......................................................................


Patch Set 3:

(1 comment)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java
Line 165:             exclusiveLock.put(getDisk().getId().toString(),
Line 166:                     
LockMessagesMatchUtil.makeLockingPair(LockingGroup.DISK,
Line 167:                             
VdcBllMessages.ACTION_TYPE_FAILED_DISKS_LOCKED));
Line 168:             addCanDoActionMessage(String.format("$%1$s %2$s", 
"diskAliases", getDiskAlias()));
Line 169: 
a. it's unneeded to format and replace also the placeholder, if it's copied 
from somewhere let's change it there as well.

b.do you try to solve a situation in which the disk is locked 
in memory or in the db? or both?
 
1. if it's locked in the db and the message isn't displayed correctly, 
it should be fixed where this message is returned.


2. if it's locked in the memory, having this here won't solve your problem as 
it's added as CDA messages placeholder replacement and not as replacement for 
the message passed to the lockmanagerutil so your issue won't be resolved. it 
should be done as - 

use the other overload of LockMessagesMatchUtil.makeLockingPair and pass to it 
the following as the message: 
VdcBllMessages.ACTION_TYPE_FAILED_DISKS_LOCKED.name() + 
String.format("$diskAliases %1$s", getDiskAlias())
Line 170:             if (getDisk().isBoot()) {
Line 171:                 exclusiveLock.put(getVmId().toString(),
Line 172:                     
LockMessagesMatchUtil.makeLockingPair(LockingGroup.VM_DISK_BOOT,
Line 173:                             
VdcBllMessages.ACTION_TYPE_FAILED_OBJECT_LOCKED));


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iff5224477b8ba547ea7dcde9069ec8874fa8ed24
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to