Shmuel Leib Melamud has posted comments on this change. Change subject: restapi: Remove VM pool if no VMs can be created ......................................................................
Patch Set 4: (3 comments) https://gerrit.ovirt.org/#/c/39336/4//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2015-03-30 13:47:03 +0300 Line 4: Commit: Shmuel Melamud <smela...@redhat.com> Line 5: CommitDate: 2015-04-06 10:50:35 +0300 Line 6: Line 7: restapi: Remove VM pool if no VMs can be created > the change is in the backend, not api so should start with 'core' instead o Done Line 8: Line 9: If VM pool is created via REST API and one of VM parameters is Line 10: incorrect, the VM Pool is created, but VM creation fails. In this case Line 11: VM pool is also removed, because VM pool without VMs shouldn't exist. https://gerrit.ovirt.org/#/c/39336/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmPoolWithVmsCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmPoolWithVmsCommand.java: Line 67: } Line 68: Line 69: @Override Line 70: protected void onNoVmsAdded(Guid poolId) { Line 71: getVmPoolDAO().remove(poolId); > this should also make the audit log to log 'failed' instead of success... Success is logged into audit log only if isAddVmsSucceded() i.e. all of the VMs were added successfully. onNoVmsAdded() is called when no of the VMs were added successfully, which means isAddVmsSucceded() will be false. Line 72: } Line 73: Line 74: @Override Line 75: public AuditLogType getAuditLogTypeValue() { https://gerrit.ovirt.org/#/c/39336/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommand.java: Line 62: private HashMap<Guid, DiskImage> diskInfoDestinationMap; Line 63: private Map<Guid, List<DiskImage>> storageToDisksMap; Line 64: private Map<Guid, StorageDomain> destStorages = new HashMap<>(); Line 65: private boolean addVmsSucceeded = true; Line 66: private boolean vmsAdded = false; > maybe document the difference between this member and 'addVmsSucceeded' abo Done Line 67: private NameForVmInPoolGenerator nameForVmInPoolGenerator; Line 68: Line 69: /** Line 70: * Constructor for command creation when compensation is applied on startup -- To view, visit https://gerrit.ovirt.org/39336 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I36b137a905ea1364255af2bcdb11b27ed5198342 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shmuel Leib Melamud <smela...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Shmuel Leib Melamud <smela...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
