Maor Lipchuk has posted comments on this change. Change subject: core: Adding support for OurMemoryTar ......................................................................
Patch Set 1: (10 comments) http://gerrit.ovirt.org/#/c/29039/1//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-06-23 04:34:07 +0300 Line 4: Commit: Maor Lipchuk <mlipc...@redhat.com> Line 5: CommitDate: 2014-06-23 05:26:27 +0300 Line 6: Line 7: core: Adding support for OurMemoryTar > s/OurMemoryTar/OutMemoryTar/ done Line 8: Line 9: Adding a utility to retrieve OVF data of entities from a tar file. Line 10: Line 11: Change-Id: Ie4fbc12337c16baca4be8a82d4a51b8b3ab0af4a http://gerrit.ovirt.org/#/c/29039/1/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/archivers/tar/OutMemoryTar.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/archivers/tar/OutMemoryTar.java: Line 17: private final static String ENTITY_NAME = "<Name>"; Line 18: private final static String END_ENTITY_NAME = "</Name>"; Line 19: private final static String OVF_FILE_EXT = ".ovf"; Line 20: Line 21: public static List<OvfEntityData> unTar(byte[] tarByte, Guid storageDomainId) { > This is a general utility, storage domain id shouldn't be related here. ple removed Line 22: List<OvfEntityData> ovfEntityDataFromTar = new ArrayList<>(); Line 23: InputStream tarInputStream = new ByteArrayInputStream(tarByte); Line 24: TarArchiveInputStream ovfDiskTarArchive = new TarArchiveInputStream(tarInputStream); Line 25: TarArchiveEntry tarEntry = null; Line 18: private final static String END_ENTITY_NAME = "</Name>"; Line 19: private final static String OVF_FILE_EXT = ".ovf"; Line 20: Line 21: public static List<OvfEntityData> unTar(byte[] tarByte, Guid storageDomainId) { Line 22: List<OvfEntityData> ovfEntityDataFromTar = new ArrayList<>(); > better to return a map here, Key is the guid and the value is the ovf. done Line 23: InputStream tarInputStream = new ByteArrayInputStream(tarByte); Line 24: TarArchiveInputStream ovfDiskTarArchive = new TarArchiveInputStream(tarInputStream); Line 25: TarArchiveEntry tarEntry = null; Line 26: try { Line 22: List<OvfEntityData> ovfEntityDataFromTar = new ArrayList<>(); Line 23: InputStream tarInputStream = new ByteArrayInputStream(tarByte); Line 24: TarArchiveInputStream ovfDiskTarArchive = new TarArchiveInputStream(tarInputStream); Line 25: TarArchiveEntry tarEntry = null; Line 26: try { > i'd prefer that we used here java 7's try-with-resources, see how it's done done Line 27: tarEntry = ovfDiskTarArchive.getNextTarEntry(); Line 28: String ovfData; Line 29: int offset = 0; Line 30: while (tarEntry != null) { Line 32: // Get Size of the file and create a byte array for the size. Line 33: byte[] content = new byte[(int) tarEntry.getSize()]; Line 34: Line 35: // Read file from the archive into byte array. Line 36: ovfDiskTarArchive.read(content, offset, content.length - offset); > this is unefficient and unneeded, done Line 37: ovfData = new String(content); Line 38: Line 39: // Get an ovf object. Line 40: OvfEntityData ovfEntityData = Line 47: } Line 48: tarEntry = ovfDiskTarArchive.getNextTarEntry(); Line 49: } Line 50: } catch (IOException e) { Line 51: e.printStackTrace(); > ? done this with your suggestion regarding java 7 Line 52: } finally { Line 53: closeArchive(ovfDiskTarArchive); Line 54: } Line 55: return ovfEntityDataFromTar; Line 54: } Line 55: return ovfEntityDataFromTar; Line 56: } Line 57: Line 58: private static Guid getEntityId(TarArchiveEntry tarEntry) { > please change this method to get the file name string. done Line 59: Guid entityId = Guid.createGuidFromString(tarEntry.getName().substring(0, tarEntry.getName().indexOf(".ovf"))); Line 60: return entityId; Line 61: } Line 62: Line 55: return ovfEntityDataFromTar; Line 56: } Line 57: Line 58: private static Guid getEntityId(TarArchiveEntry tarEntry) { Line 59: Guid entityId = Guid.createGuidFromString(tarEntry.getName().substring(0, tarEntry.getName().indexOf(".ovf"))); > *Please replace with FileNameUtilites.getBaseName(ovfFileName) done Line 60: return entityId; Line 61: } Line 62: Line 63: private static String getEntityName(String ovfData) { Line 75: } Line 76: return vmEntityType; Line 77: } Line 78: Line 79: private static OvfEntityData initOvfEntityData(Guid storageDomainId, > /s/init/create done Line 80: String ovfData, Line 81: VmEntityType vmEntityType, Line 82: String entityName, Line 83: Guid entityId) { Line 93: private static void closeArchive(TarArchiveInputStream ovfDiskTarArchive) { Line 94: try { Line 95: ovfDiskTarArchive.close(); Line 96: } catch (IOException e) { Line 97: e.printStackTrace(); > ? done for java7 Line 98: } Line 99: } -- To view, visit http://gerrit.ovirt.org/29039 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie4fbc12337c16baca4be8a82d4a51b8b3ab0af4a Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org 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
