Liron Aravot has posted comments on this change. Change subject: engine: Remove IsValidVDSCommand (Preporation for removing global lock on SPM ops) ......................................................................
Patch Set 2: I would prefer that you didn't submit this (3 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/IsoDomainListSyncronizer.java Line 238: FileTypeExtension fileTypeExt) { Line 239: boolean refreshSucceeded = false; Line 240: // Setting the indication to the indication whether the storage pool is valid. Line 241: boolean updateFromVDSMSucceeded = true; Line 242: why is it done now without the check of the storage pool status? Line 243: // If the SPM and the storage pool are valid, try to refresh the Iso list by fetching it from the SPM. Line 244: if (fileTypeExt == FileTypeExtension.ISO) { Line 245: updateFromVDSMSucceeded = updateIsoListFromVDSM(storagePoolId, storageDomainId); Line 246: } else if (fileTypeExt == FileTypeExtension.Floppy) { .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java Line 84: try { Line 85: updateVmInSpm(getVm().getstorage_pool_id(), Line 86: Arrays.asList(getVm())); Line 87: } catch (Exception e) { Line 88: // DO nothing updateVmInSpm can fail for various reasons, right now the behaviour is to fail the actions when this operation fails as the ovf wasn't updated. can you please explain how this change and behaviour change is done in this patch? how about logging so at least we can observe what went wrong? audit log? Line 89: } Line 90: setSucceeded(true); Line 91: } Line 92: .................................................... File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommandTestAbstract.java Line 272: */ Line 273: private storage_pool mockStoragePool() { Line 274: storage_pool storage_pool = new storage_pool(); Line 275: storage_pool.setstatus(StoragePoolStatus.Up); Line 276: can this line be removed? Line 277: return storage_pool; Line 278: } Line 279: Line 280: private static void setDiskList(VmTemplate vmTemplate) { -- To view, visit http://gerrit.ovirt.org/9097 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I73c0fa0fd66f2f14132594a0b0450a7ecd7cf166 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Michael Kublin <mkub...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Ayal Baron <aba...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches