Liron Aravot has posted comments on this change. Change subject: core: Initialize entities in DB from OVF disk on attach ......................................................................
Patch Set 1: (6 comments) partial review http://gerrit.ovirt.org/#/c/29041/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java: Line 96: connectAllHostsToPool(); Line 97: runVdsCommand(VDSCommandType.AttachStorageDomain, Line 98: new AttachStorageDomainVDSCommandParameters(getParameters().getStoragePoolId(), Line 99: getParameters().getStorageDomainId())); Line 100: final List<OvfEntityData> unregisteredEntitiesFromOvfDisk = getEntitiesFromStorageOvfDisk(); why don't we do it in the import domain flow? Line 101: executeInNewTransaction(new TransactionMethod<Object>() { Line 102: @Override Line 103: public Object runInTransaction() { Line 104: final StorageDomainType sdType = getStorageDomain().getStorageDomainType(); Line 114: updateStorageDomainFormat(getStorageDomain()); Line 115: } Line 116: Line 117: // Update unregistered entities Line 118: for (OvfEntityData ovf : unregisteredEntitiesFromOvfDisk) { haven't reviewed this part, see question below Line 119: getUnregisteredOVFDataDao().removeEntity(ovf.getEntityId(), Line 120: getParameters().getStorageDomainId()); Line 121: getUnregisteredOVFDataDao().saveOVFData(ovf); Line 122: log.infoFormat("Adding OVF data of entity id {0} and entity name {1}", Line 171: * @param disks Line 172: * - A list of disks Line 173: * @return A Pair which contains the best OVF disk to retrieve data from and its size. Line 174: */ Line 175: private Pair<DiskImage, Long> getLatestOVFDisk(List<Disk> disks) { we should have a sorted list, if reading from the first fails continue to the second and so on. Line 176: Date lastUpdate = new Date(); Line 177: boolean lastIsUpdated = false; Line 178: Long size = 0L; Line 179: Disk diskToGetOVF = null; Line 181: boolean isDiskLastUpdated = false; Line 182: // Check which disks are of OVF_STORE Line 183: String diskDecription = ((DiskImage) disk).getDescription(); Line 184: if (diskDecription.contains(OvfInfoFileConstants.OvfStoreDescriptionLabel)) { Line 185: Map<String, Object> diskDescriptionMap = buildJson(diskDecription); in case of description which isn't json, runtime exception will be thrown and nothing will run here. the logic should be simpler in my opinion so that it'll be better understood, performance is non issue here as we have very low number of ovf stores anyway - so if we iterate once or twice is a non issue. a. find relevant ovf disks on the domain b. sort the ovf disks c. start reading data from the disks. Line 186: if (!isDomainExistsInDiskDescription(diskDescriptionMap, getParameters().getStorageDomainId())) { Line 187: break; Line 188: } Line 189: Line 217: return new SimpleDateFormat(OvfParser.formatStrFromDiskDescription).parse(map.get(OvfInfoFileConstants.LastUpdated) Line 218: .toString()); Line 219: } catch (java.text.ParseException e) { Line 220: log.errorFormat("LastUpdate Date could not be parsed from disk desscription"); Line 221: e.printStackTrace(); ? Line 222: return null; Line 223: } Line 224: } Line 225: Line 226: private boolean isDomainExistsInDiskDescription(Map<String, Object> map, Guid storageDomainId) { Line 227: return map.get(OvfInfoFileConstants.Domains).toString().contains(storageDomainId.toString()); Line 228: } Line 229: Line 230: private Map<String, Object> buildJson(String json) { already have that on the jsonHelper Line 231: try { Line 232: return JsonHelper.jsonToMap(json); Line 233: } catch (IOException e) { Line 234: throw new RuntimeException("Exception while generating json containing ovf store info", e); -- To view, visit http://gerrit.ovirt.org/29041 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ica68974916821ac1c9fbe6f43400170d19d716c9 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
