Maor Lipchuk has posted comments on this change.

Change subject: core: WIP: verify iscsi lun connection when adding lun disk
......................................................................


Patch Set 4: (1 inline comment)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
Line 128: 
Line 129:                 try {
Line 130:                     return StorageHelperDirector.getInstance()
Line 131:                             
.getItem(getStorageDomain().getstorage_type())
Line 132:                             .ConnectStorageToLunByVdsId(null, 
vdss.get(0).getId(), lun, getStoragePool().getId());
(As we discussed)
Using the first host as an indicator to determine whether this disk is valid or 
not, might be confusing for the user.

Please don't forget to add a log (after wip) that an attempt connecting a lun 
to a specific host has been made.

Consider the alternative solutions:
 1. Do the connect in the execute phase on all the active hosts and start to 
connect each one until one succeeded. at the end we can print an audit log 
listing the problematic hosts.
 2. Get the specific host from the client, to do the connect.
Line 133:                 } catch (VdcBLLException e) {
Line 134:                     log.error(e);
Line 135:                     return false;
Line 136:                 }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8aa07be1fd6d6021481c8c0291458aff403304ee
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to