Liron Aravot has posted comments on this change. Change subject: core: Requesting to consume quota while extending a disk fix. ......................................................................
Patch Set 2: (2 comments) https://gerrit.ovirt.org/#/c/39330/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java: Line 687: DiskImage newDiskImage = (DiskImage) getNewDisk(); Line 688: Line 689: boolean emptyOldQuota = oldDiskImage.getQuotaId() == null || Guid.Empty.equals(oldDiskImage.getQuotaId()); Line 690: boolean differentNewQuota = !emptyOldQuota && !oldDiskImage.getQuotaId().equals(newDiskImage.getQuotaId()); Line 691: long diskExtendingDiff = newDiskImage.getSizeInGigabytes() - oldDiskImage.getSizeInGigabytes(); if the user insert new size lower then the old size, you'll attempt to consume quota with negative value (and then fail on the canDoAction()). how does the system acts on that scenario? (consumption of negative quota) Line 692: Line 693: if (emptyOldQuota || differentNewQuota ) { Line 694: list.add(generateQuotaConsumeParameters(newDiskImage, newDiskImage.getSizeInGigabytes())); Line 695: } else if (diskExtendingDiff != 0) { Line 692: Line 693: if (emptyOldQuota || differentNewQuota ) { Line 694: list.add(generateQuotaConsumeParameters(newDiskImage, newDiskImage.getSizeInGigabytes())); Line 695: } else if (diskExtendingDiff != 0) { Line 696: list.add(generateQuotaConsumeParameters(newDiskImage, diskExtendingDiff)); what if there is no quota defined for the disk? (getQuotaId() == nul) you'll attempt to consume quota will null id, that might lead to a npe. Line 697: } Line 698: Line 699: if (differentNewQuota) { Line 700: list.add(new QuotaStorageConsumptionParameter( -- To view, visit https://gerrit.ovirt.org/39330 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib31206b0a9172b35217845b01253951d93f380f1 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Amit Aviram <aavi...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Amit Aviram <aavi...@redhat.com> Gerrit-Reviewer: Candace Sheremeta <csher...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Greg Padgett <gpadg...@redhat.com> Gerrit-Reviewer: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: Vered Volansky <vvola...@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
