Alona Kaplan has posted comments on this change. Change subject: core: Removal of labelled network from DC inconsistent with removal from cluster ......................................................................
Patch Set 1: (1 comment) http://gerrit.ovirt.org/#/c/30795/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/cluster/NetworkHelper.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/cluster/NetworkHelper.java: Line 135: ArrayList<VdcActionParametersBase> parameters = builder.buildParameters(nics); Line 136: Line 137: if (!parameters.isEmpty()) { Line 138: NetworkParametersBuilder.updateParametersSequencing(parameters); Line 139: Backend.getInstance().runInternalMultipleActions(VdcActionType.PersistentSetupNetworks, parameters, context); > As I understand from http://www.ovirt.org/Wiki/Engine_Command_changes the c After rethinking it, it is even better to pass 'cloneContextAndDetachFromParent(context)'. Since you don't want the outer command to rollback in case of failure in removing network from hosts. public CommandContext cloneContextAndDetachFromParent(CommandContext context) { return context.clone().withoutCompensationContext().withoutExecutionContext().withoutLock(); } Since 'PersistentSetupNetworksCommand' already does it when calling its internal commands, there is no real bug. But I think it is not a good practice to reside on it. Line 140: } Line 141: } -- To view, visit http://gerrit.ovirt.org/30795 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9ebc162324ff2ed98374d6a9a3bd9d87bb69db3f Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Martin Mucha <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
