Liron Aravot 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);
Alissa, I'll answer one by one because some questions were raised :-)
1. "the second part doesn't check if lun exists in db", the second part is the
else clause of the first if that does check if the lun exist in the db.
2. "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" - this method gets a lun, and volume group id and
makes sure that when we exist the method, there will be a lun in the db with
the given volume group id. it doesn't do any validation (lun can also be null
for example), the parameters should be checked in the canDoAction, yes - this
method is public, but it's enforced by the use of storage helpers..do we want
to change and add validation check on all those method? it's debateable - it's
either we will get an NPE or will raise an exception ourselves, can be
inspected later on.
3. "Perhaps it's better to check first if vg is null or empty before the
insert" - answered regarding the values check in 2. it's checked only when we
update because if the lun already saved it's either saved with vg or "" vg, if
it's with vg no need to update it and also if its saved with "" no need to
update it.
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: Daniel Paikov <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
