Vered Volansky has posted comments on this change.
Change subject: wip: core: remove as sync verb.
......................................................................
Patch Set 4: (7 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyImageGroupCommand.java
Line 190:
Line 191: @Override
Line 192: protected void revertTasks() {
Line 193: // Revert should be performed only for AddVmFromSnapshot at
this point.
Line 194: if (getParameters().getParentCommand() ==
VdcActionType.AddVmFromSnapshot || getParameters().getParentCommand() ==
VdcActionType.ImportVm) {
I know this was already here, but maybe in another patch - getParameters()
.getParentCommand() == VdcActionType.ImportVm test is done twice. Once here and
again in 198, so it's redundant here.
Line 195: Guid destImageId =
getParameters().getDestinationImageId();
Line 196: RemoveImageParameters removeImageParams =
Line 197: new RemoveImageParameters(destImageId);
Line 198: if (getParameters().getParentCommand() ==
VdcActionType.ImportVm) {
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java
Line 312: }
Line 313:
Line 314: @Override
Line 315: public AuditLogType getAuditLogTypeValue() {
Line 316: switch (getActionState()) {
if-else is more appropriate here now.
Line 317: case EXECUTE:
Line 318: return getSucceeded() ?
AuditLogType.USER_FINISHED_REMOVE_DISK
Line 319: : AuditLogType.USER_FINISHED_FAILED_REMOVE_DISK;
Line 320: default:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java
Line 112:
Line 113: try {
Line 114: VM vm = getVmForNonShareableDiskImage(diskImage);
Line 115: // if the disk is not part of a vm (floating), there are
no snapshots to update
Line 116: // so no lock is required.
Comment is no longer relevant.
Line 117: if (getParameters().isRemoveFromSnapshots() && vm !=
null) {
Line 118: lockVmSnapshotsWithWait(vm);
Line 119: updatedSnapshots =
prepareSnapshotConfigWithoutImage(diskImage.getId());
Line 120: } else {
Line 117: if (getParameters().isRemoveFromSnapshots() && vm !=
null) {
Line 118: lockVmSnapshotsWithWait(vm);
Line 119: updatedSnapshots =
prepareSnapshotConfigWithoutImage(diskImage.getId());
Line 120: } else {
Line 121: updatedSnapshots = Collections.emptyList();
You can initialize on declaration and avoid this else.
Line 122: }
Line 123:
Line 124:
TransactionSupport.executeInScope(TransactionScopeOption.Required,
Line 125: new TransactionMethod<Object>() {
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmTemplateFromImportExportCommand.java
Line 128: }
Line 129:
Line 130: @Override
Line 131: public AuditLogType getAuditLogTypeValue() {
Line 132: switch (getActionState()) {
I think if-else is more appropriate here.
Line 133: case EXECUTE:
Line 134: return getSucceeded() ?
AuditLogType.IMPORTEXPORT_REMOVE_TEMPLATE
Line 135: :
AuditLogType.IMPORTEXPORT_REMOVE_TEMPLATE_FAILED;
Line 136: default:
....................................................
Commit Message
Line 5: CommitDate: 2013-04-07 12:00:21 +0300
Line 6:
Line 7: wip: core: remove as sync verb.
Line 8:
Line 9: When performing DeleteImageGroup, the engine can treat the call as
I don't really understand the commit message. Please rephrase.
Specifics - can treat? it's not heuristics, either it does or it does under
certain conditions. It's also not conclusive if this is before or after this
patch.
s/it the/the .
Also break the sentence somewhere, this shouldn't be all in one sentence.
Line 10: synchronous call as afterwards it the image is considered to be non
Line 11: exist on the storage side - when receiving an ImageDoesNotExist error
Line 12: from vdsm - proceed with removing the image.
Line 13:
Line 7: wip: core: remove as sync verb.
Line 8:
Line 9: When performing DeleteImageGroup, the engine can treat the call as
Line 10: synchronous call as afterwards it the image is considered to be non
Line 11: exist on the storage side - when receiving an ImageDoesNotExist error
s/exist/existent
Line 12: from vdsm - proceed with removing the image.
Line 13:
Line 14: Change-Id: Ic822a92acb8231d2ca84ae092bd98659507925d8
Line 15: Bug-Url: https://bugzilla.redhat.com/884635
--
To view, visit http://gerrit.ovirt.org/13611
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic822a92acb8231d2ca84ae092bd98659507925d8
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Alissa Bonas <abo...@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 Ar <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
