Juan Hernandez has posted comments on this change.

Change subject: host-deploy: use cpio instead of tar for bundle transfer
......................................................................


Patch Set 1: (6 inline comments)

As we are changing default values of configuration parameters what happens if 
the user has changed any of those values?

....................................................
File 
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/archivers/CachedCpio.java
Line 13: /**
Line 14:  * Handles cache tar file based on directory.
Line 15:  *
Line 16:  * Cache tar based on directory structure. If files are changed
Line 17:  * recreate tar file. Test file change once per interval.
Update this comment, it still talks about tar.
Line 18:  */
Line 19: public class CachedCpio {
Line 20: 
Line 21:     private static final Log log = LogFactory.getLog(CachedCpio.class);


Line 23:     private long _refreshInterval = 10000;
Line 24:     private long _nextCheckTime = 0;
Line 25: 
Line 26:     private File _archive;
Line 27:     private File _dir;
This introduction of _ as prefixes is out of the scope of this patch, and 
against the practice used in the rest of the engine.
Line 28: 
Line 29:     private void create(long timestamp) throws IOException {
Line 30:         File temp = null;
Line 31:         try {


Line 110:     }
Line 111: 
Line 112:     /**
Line 113:      * Constructor.
Line 114:      * @param archive name of tar to cache.
This comment isn't updated.
Line 115:      * @param dir base directory.
Line 116:      */
Line 117:     public CachedCpio(File archive, File dir) {
Line 118:         _archive = archive;


....................................................
File 
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/archivers/Cpio.java
Line 20: public class Cpio {
Line 21: 
Line 22:     private static final Log log = LogFactory.getLog(Cpio.class);
Line 23: 
Line 24:     private static void _recurse(
Method names should not start with _ (neither with s_ if they are static).
Line 25:         CpioArchiveOutputStream archive,
Line 26:         File file,
Line 27:         String base
Line 28:     ) throws SecurityException, IOException {


Line 79:      *  @param os output stream to write into.
Line 80:      *  @param base base directory.
Line 81:      *
Line 82:      *  Only regular files and directories are supported.
Line 83:      *  Files will be owner rw and optional execute bit.
In Javadoc comments should go before @param, otherwise they are considered part 
of the last @param.
Line 84:      */
Line 85:     public static void doArchive(
Line 86:         OutputStream os,
Line 87:         File base


....................................................
File 
backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/archivers/CpioTest.java
Line 63:         File tmpDir1 = null;
Line 64:         File tmpDir2 = null;
Line 65: 
Line 66:         try {
Line 67:             tmpCpio = File.createTempFile("test1", "tar");
If this is a cpio file the extension should not be "tar".
Line 68:             tmpDir1 = File.createTempFile("test1", "tmp");
Line 69:             tmpDir1.delete();
Line 70:             tmpDir1.mkdir();
Line 71:             tmpDir2 = File.createTempFile("test1", "tmp");


-- 
To view, visit http://gerrit.ovirt.org/17396
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1386e7d688a9ec7e28519fb407478fd17cbab4ca
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to