Alon Bar-Lev has posted comments on this change.
Change subject: bootstrap: send complete bootstrap from engine
......................................................................
Patch Set 11: (15 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsInstaller.java
Line 55: private Guid _fileGuid = new Guid();
Line 56: private final VDS _vds;
Line 57: private String serverInstallationTime;
Line 58:
Line 59: private static CachedTar s_bootstrapPackage;
How do you distinguish within code the implication of modifying a symbol? Is it
local to class or global?
There must be some naming convention to help people understand correctly code
snip.
Line 60: private String _bootstrapCommand;
Line 61:
Line 62: protected final VdsInstallerSSH _wrapper = new VdsInstallerSSH();
Line 63: private final OpenSslCAWrapper _caWrapper = new OpenSslCAWrapper();
Line 112: vds,
Line 113: Config.<String> GetValue(ConfigValues.BootstrapCommand)
Line 114: );
Line 115:
Line 116: if (s_bootstrapPackage == null) {
Thought about this, and I don't care to if it is created several time as long
as we keep one instance.
But as we need initialization and Config access, we should do this not in
static context, synchronize is the alternative, but not required in this case.
Line 117: String cache = System.getenv("ENGINE_CACHE");
Line 118: if (cache == null) {
Line 119: log.warn("ENGINE_CACHE environment not found using
tmpdir");
Line 120: cache = System.getProperty("java.io.tmpdir");
Line 121: }
Line 122: s_bootstrapPackage = new CachedTar(
Line 123: new File(cache, Config.<String>
GetValue(ConfigValues.BootstrapPackageName)),
Line 124: new File(Config.<String>
GetValue(ConfigValues.BootstrapPackageDirectory))
Line 125: );
It won't be simpler as I need the new File anyway to concat the path.
Line 126: }
Line 127:
Line 128: VdsGroupDAO vdsGroupDao =
DbFacade.getInstance().getVdsGroupDAO();
Line 129: Guid vdsGroupId = vds.getvds_group_id();
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
Line 1430: BootstrapCommand(373),
Line 1431:
Line 1432: @TypeConverterAttribute(Integer.class)
Line 1433: @DefaultValueAttribute("10000")
Line 1434: BootstrapCacheRefreshInterval(374),
Why not? What do we lose? Why not put all constants as configurable upon need?
Line 1435: @TypeConverterAttribute(String.class)
Line 1436: @DefaultValueAttribute("/usr/share/vdsm-bootstrap/interface-2")
Line 1437: BootstrapPackageDirectory(375),
Line 1438: @TypeConverterAttribute(String.class)
Line 1436: @DefaultValueAttribute("/usr/share/vdsm-bootstrap/interface-2")
Line 1437: BootstrapPackageDirectory(375),
Line 1438: @TypeConverterAttribute(String.class)
Line 1439: @DefaultValueAttribute("vdsm-bootstrap-2.tar")
Line 1440: BootstrapPackageName(376),
Again, we lose nothing. If someone want to name it differently so be it.
Line 1441:
Line 1442: Invalid(65535);
Line 1443:
Line 1444: private int intValue;
....................................................
File backend/manager/modules/utils/pom.xml
Line 56: <dependency>
Line 57: <groupId>javatar</groupId>
Line 58: <artifactId>javatar</artifactId>
Line 59: <version>${javatar.version}</version>
Line 60: </dependency>
I've done the same as sshd...
Done.
Line 61:
Line 62: <dependency>
Line 63: <groupId>${engine.groupId}</groupId>
Line 64: <artifactId>engineencryptutils</artifactId>
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/hostinstall/CachedTar.java
Line 76: }
Line 77: }
Line 78: }
Line 79:
Line 80: private void ensure() throws IOException {
No I don't, rename is atomic.
Line 81: if (!this.archive.exists()) {
Line 82: log.info(
Line 83: String.format(
Line 84: "Tarball '%1$s' is missing, creating",
Line 81: if (!this.archive.exists()) {
Line 82: log.info(
Line 83: String.format(
Line 84: "Tarball '%1$s' is missing, creating",
Line 85: this.archive
Done
Line 86: )
Line 87: );
Line 88: this.nextCheckTime = System.currentTimeMillis() +
this.refreshInterval;
Line 89: create();
Line 93: if (archive.lastModified() <=
DirectoryTimestamp.getTimestamp(this.dir)) {
Line 94: log.info(
Line 95: String.format(
Line 96: "Tarball '%1$s' is out of date, re-creating",
Line 97: this.archive
Done
Line 98: )
Line 99: );
Line 100: create();
Line 101: }
Line 119: /**
Line 120: * Get file of archive, without enforcing cache.
Line 121: * @return File name.
Line 122: *
Line 123: * This should be used only if file is not to be accessed
(messages).
I know...
Done.
Line 124: */
Line 125: public File getFileNoUse() {
Line 126: return this.archive;
Line 127: }
Line 139: * Get stream of archive.
Line 140: * @return InputStream.
Line 141: */
Line 142: public InputStream getStream() throws IOException {
Line 143: return new FileInputStream(getFile());
Done
Line 144: }
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/hostinstall/DirectoryTimestamp.java
Line 21: * Returns the maximum timestamp of directory tree.
Line 22: * @param file directory/file name.
Line 23: * @return max timestamp.
Line 24: */
Line 25: public static long getTimestamp(File file) {
Done
Line 26: if (file.isDirectory()) {
Line 27: long m = 0;
Line 28: for (String name : file.list()) {
Line 29: m = Math.max(m, getTimestamp(new File(file, name)));
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/hostinstall/Tar.java
Line 11: import org.apache.commons.logging.LogFactory;
Line 12:
Line 13: import com.ice.tar.InvalidHeaderException;
Line 14: import com.ice.tar.TarEntry;
Line 15: import com.ice.tar.TarOutputStream;
1. This is what we have at rhel and fedora.
2. zip does not support file attribute (execute and permissions).
3. It is working great... tar hasn't been changed for a long time as well.
I will look into this one as well.
Line 16:
Line 17: /**
Line 18: * A simple recursive tar based on javatar.
Line 19: */
....................................................
File packaging/fedora/engine-service.py
Line 334: if os.path.exists(engineTmpDir):
Line 335: shutil.rmtree(engineTmpDir)
Line 336: os.mkdir(engineTmpDir)
Line 337: os.chown(engineTmpDir, engineUid, engineGid)
Line 338: if not os.path.exists(engineCacheDir):
Done
Line 339: os.mkdir(engineCacheDir)
Line 340: os.chown(engineCacheDir, engineUid, engineGid)
Line 341:
Line 342: # Generate the main configuration from the template and copy it
to the
....................................................
File pom.xml
Line 45: <dozer.version>5.2.0</dozer.version>
Line 46: <cxf.version>2.2.7</cxf.version>
Line 47: <mina-core.version>2.0.1</mina-core.version>
Line 48: <sshd-core.version>0.7.0</sshd-core.version>
Line 49: <javatar.version>2.5</javatar.version>
Done
Line 50: <slf4j-jdk14.version>1.5.11</slf4j-jdk14.version>
Line 51: <gwt.version>2.2.0</gwt.version>
Line 52: <gwt.plugin.version>1.3.2.google</gwt.plugin.version>
Line 53: <findbugs.version>2.5.1</findbugs.version>
--
To view, visit http://gerrit.ovirt.org/6963
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f4a09ca9e66f0c9f5f4f7b283a5f43986b7e603
Gerrit-PatchSet: 11
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: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsl...@redhat.com>
Gerrit-Reviewer: Eyal Edri <ee...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Ofer Schreiber <oschr...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
