Daniel Erez has posted comments on this change. Change subject: core: update LUN device size using getVmStats ......................................................................
Patch Set 1: (4 comments) Regarding update on disk activate - updating LUN size could be a relativity heavy operation on vdsm side, hence, the suggested patch [1] updates device size only upon running a VM. Therefore, we fetch updated data only using getVmStats and updating on disk plug won't be relevant. [1] http://gerrit.ovirt.org/#/c/22976/ .................................................... 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()); It's being done similarly in most DbFacadeImpl, but i'll see how I can extract it as you suggested. 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<>(); Seems like a standard in the file :). But Sure.. 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); Yeah, thought about it, wasn't sure it worth it for this patch context. Will add it in a following patch. 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. What do you mean? Both list can contain only LUNs from a VM: 'lunsMap' is retrieved from getVmStats and 'vmDisks' is retrieved from the DB by a specified VM id. 2. Currently, all LUNs from getVmStats have a size of 0 (should be fixed in vdsm by [1]). Added this check just for preventing redundant DB updates. [1] http://gerrit.ovirt.org/#/c/22976/ 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: Daniel Erez <de...@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
