Maor Lipchuk has posted comments on this change. Change subject: core: Initialize entities in DB from OVF disk on attach ......................................................................
Patch Set 1: (5 comments) 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? since we will have problem when calling getImagesList at GetUnregisteredDisks Line 101: executeInNewTransaction(new TransactionMethod<Object>() { Line 102: @Override Line 103: public Object runInTransaction() { Line 104: final StorageDomainType sdType = getStorageDomain().getStorageDomainType(); 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 t This will be done in a later phase, we will have to create a new class for that sine the parameters in the disk description is not part of the disk entity 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 a That is why we check if the disk description exists. also that is the correct behaviour for non json string in OVF STORE, what other behaviour would you expect? We can make it simpler since we use Map of Strings combined with the Disk entity a is already done here, regarding b, this will be in another phase 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(); > ? if date fails to translate we should put the error in the log 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 what if we fail? we should put the error in the log 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: Maor Lipchuk <[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
