Alissa Bonas has posted comments on this change.
Change subject: core: persist LUN with correct volume_group_id
......................................................................
Patch Set 1: (1 inline comment)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java
Line 236: }
Line 237:
Line 238: public static void proceedLUNInDb(final LUNs lun, StorageType
storageType, String volumeGroupId) {
Line 239: if (getLunDao().get(lun.getLUN_id()) == null) {
Line 240: lun.setvolume_group_id(volumeGroupId);
The logic in this method is a bit unclear to me... (before your change as well).
Because it says "if there is no lun in db then add it there , but if the volume
group is not empty - update the volume group id in lun". the second part
doesn't check if lun exists in db, it just goes and tries to update the vg
there.
In this proposed change, adding the volume group id to the new lun which didn't
exist before means inserting volume group id to db even if it's null or empty
string in the first if... what benefit does it add?
Perhaps it's better to check first if vg is null or empty before the insert
(but then again, this is what the "else if" part does - but only for existing
luns)
There is also a call to this method couple of rows above, where the volume
group is "" :
proceedLUNInDb(lun,storageType,"");
Line 241: getLunDao().save(lun);
Line 242: } else if (!volumeGroupId.isEmpty()){
Line 243: getLunDao().updateLUNsVolumeGroupId(lun.getLUN_id(),
volumeGroupId);
Line 244: }
--
To view, visit http://gerrit.ovirt.org/10630
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4404b0d17b107208e5ec00c688db305ba0e65fe
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
