Liron Aravot has posted comments on this change.
Change subject: core: fix redundant storage server conn in db
......................................................................
Patch Set 6: (1 inline comment)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveStorageServerConnectionCommand.java
Line 13:
Line 14: @Override
Line 15: protected void executeCommand() {
Line 16: //disconnect the connection from vdsm
Line 17: boolean isStorageDisconnectSucceeded = disconnectStorage();
Alissa, as MIchael wrote in earlier patchset - this call might throw exception
and the rest won't be executed -
why the connection shouldn't be removed from the db regardless, what if we had
network error on the vdsm call? we want to keep the db record?
In my opinion the logic here should be "remove from db and attempt to
disconnect on vdsm" - even if we fail to disconnect on vdsm there's no meaning
to keep it in the db.
Line 18:
Line 19: if(isStorageDisconnectSucceeded) {
Line 20: String connectionId = getConnection().getid();
Line 21: if(StringUtils.isNotEmpty(connectionId)) {
--
To view, visit http://gerrit.ovirt.org/11936
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iea5468371514bd2c7bc043a6c5520e2864a09fe8
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
