Maor Lipchuk has posted comments on this change.
Change subject: core: update LUN device size using getVmStats
......................................................................
Patch Set 1:
(4 comments)
1) Are you also considering updating the LUN size when activating an unplugged
disk?
2) Better to add a reviewer from virt
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/LunDAODbFacadeImpl.java
Line 134: .addValue("serial", lun.getSerial())
Line 135: .addValue("lun_mapping", lun.getLunMapping())
Line 136: .addValue("vendor_id", lun.getVendorId())
Line 137: .addValue("product_id", lun.getProductId())
Line 138: .addValue("device_size", lun.getDeviceSize());
Maybe extend it to another method, to keep it DRY
Line 139: }
Line 140:
Line 141: @Override
Line 142: public MapSqlParameterMapper<LUNs> getBatchMapper() {
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
Line 93: private final Map<Guid, VmDynamic> _vmDynamicToSave = new
HashMap<>();
Line 94: private final Map<Guid, VmStatistics> _vmStatisticsToSave = new
HashMap<>();
Line 95: private final Map<Guid, List<VmNetworkInterface>>
_vmInterfaceStatisticsToSave = new HashMap<>();
Line 96: private final Map<Guid, DiskImageDynamic>
_vmDiskImageDynamicToSave = new HashMap<>();
Line 97: private final List<LUNs> _vmLunDisksToSave = new ArrayList<>();
variable name should not begin with underscore, it is not a java standard
Line 98: private final Map<VmDeviceId, VmDevice> vmDeviceToSave = new
HashMap<>();
Line 99: private final List<VmDevice> newVmDevices = new ArrayList<>();
Line 100: private final List<VmDeviceId> removedDeviceIds = new
ArrayList<>();
Line 101: private final Map<VM, VmDynamic> _vmsClientIpChanged = new
HashMap<>();
Line 999: for (VmDynamic vmDynamic : getPoweringUpVms()) {
Line 1000: VmInternalData vmInternalData =
getRunningVms().get(vmDynamic.getId());
Line 1001: if (vmInternalData != null) {
Line 1002: Map<String, LUNs> lunsMap =
vmInternalData.getLunsMap();
Line 1003: List<Disk> vmDisks =
getDbFacade().getDiskDao().getAllForVm(vmDynamic.getId(), true);
Suggestion: Maybe add a stored procedure or change the getAllForVm to also
filter disks by storageType
Line 1004: for (Disk disk : vmDisks) {
Line 1005: if (disk.getDiskStorageType() !=
DiskStorageType.LUN) {
Line 1006: continue;
Line 1007: }
Line 1008:
Line 1009: LUNs lunFromDB = ((LunDisk) disk).getLun();
Line 1010: LUNs lunFromMap = lunsMap.get(lunFromDB.getId());
Line 1011:
Line 1012: if (lunFromMap != null &&
lunFromMap.getDeviceSize() != 0
1. I think that If we have a lun in the DB which is not in the VM, then it will
be good idea to print an error log about it.
2. Why do we check if the device size is 0?
Line 1013: && lunFromMap.getDeviceSize() !=
lunFromDB.getDeviceSize()) {
Line 1014: // Found a mismatch - set LUN for update
Line 1015: log.infoFormat("Updated LUN device size -
ID: {0}, previous size: {1}, new size: {2}.",
Line 1016: lunFromDB.getLUN_id(),
lunFromDB.getDeviceSize(), lunFromMap.getDeviceSize());
--
To view, visit http://gerrit.ovirt.org/22978
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0d4d805ca333990ea1f612eb03a87f4a505f4a8
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
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
