Arik Hadas has uploaded a new change for review.
Change subject: core: cleanup in RunVmCommand
......................................................................
core: cleanup in RunVmCommand
1. remove '_' prefix from class fields
2. define a static field that stores "iso://" string
3. use #cdPathWindowsToLinux method instead of calling
ImagesHandler.cdPathWindowsToLinux explicitly in some places.
4. reference RunVmCommand#ISO_PREFIX from the test class, instead of
defining the iso prefix string again in the test class
5. use IsoDomainListSyncronizer#findActiveISODomain to find active iso
domain instead of having duplicated code
Change-Id: Ia37a78eacd6c415a9bf10920362140736c7d70fc
Signed-off-by: Arik Hadas <aha...@redhat.com>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
M
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
2 files changed, 29 insertions(+), 45 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/77/17577/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
index e2e7c7b..b4dce4c 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
@@ -45,9 +45,6 @@
import org.ovirt.engine.core.common.businessentities.RepoImage;
import org.ovirt.engine.core.common.businessentities.Snapshot;
import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType;
-import org.ovirt.engine.core.common.businessentities.StorageDomain;
-import org.ovirt.engine.core.common.businessentities.StorageDomainStatus;
-import org.ovirt.engine.core.common.businessentities.StorageDomainType;
import org.ovirt.engine.core.common.businessentities.VDS;
import org.ovirt.engine.core.common.businessentities.VM;
import org.ovirt.engine.core.common.businessentities.VMStatus;
@@ -78,8 +75,6 @@
import org.ovirt.engine.core.dal.job.ExecutionMessageDirector;
import org.ovirt.engine.core.dao.SnapshotDao;
import org.ovirt.engine.core.utils.NetworkUtils;
-import org.ovirt.engine.core.utils.linq.LinqUtils;
-import org.ovirt.engine.core.utils.linq.Predicate;
import org.ovirt.engine.core.utils.log.Log;
import org.ovirt.engine.core.utils.log.LogFactory;
@@ -90,8 +85,8 @@
implements QuotaVdsDependent {
private static final RunVmValidator runVmValidator = new RunVmValidator();
- private String _cdImagePath = "";
- private String _floppyImagePath = "";
+ private String cdImagePath = "";
+ private String floppyImagePath = "";
private boolean mResume;
/** Note: this field should not be used directly, use {@link
#isVmRunningStateless()} instead */
private Boolean cachedVmIsRunningStateless;
@@ -103,6 +98,8 @@
/** This flag is used to indicate that the disks might be dirty since the
memory
* from the active snapshot was restored so the memory should not be used
*/
private boolean memoryFromSnapshotIrrelevant;
+
+ public static final String ISO_PREFIX = "iso://";
protected RunVmCommand(Guid commandId) {
super(commandId);
@@ -133,12 +130,10 @@
private void initRunVmCommand() {
RunVmParams runVmParameters = getParameters();
if (!StringUtils.isEmpty(runVmParameters.getDiskPath())) {
- _cdImagePath =
ImagesHandler.cdPathWindowsToLinux(runVmParameters.getDiskPath(), getVm()
- .getStoragePoolId());
+ cdImagePath = cdPathWindowsToLinux(runVmParameters.getDiskPath());
}
if (!StringUtils.isEmpty(runVmParameters.getFloppyPath())) {
- _floppyImagePath =
ImagesHandler.cdPathWindowsToLinux(runVmParameters.getFloppyPath(), getVm()
- .getStoragePoolId());
+ floppyImagePath =
cdPathWindowsToLinux(runVmParameters.getFloppyPath());
}
if (getVm() != null) {
@@ -187,14 +182,13 @@
* @return String of the full file path.
*/
protected String getIsoPrefixFilePath(String url) {
- String isoPrefixName = "iso://";
// The initial Url.
String fullPathFileName = url;
// If file name got prefix of iso:// then set the path to the Iso
domain.
- int prefixLength = isoPrefixName.length();
- if (url.length() >= prefixLength && (url.substring(0,
prefixLength)).equalsIgnoreCase(isoPrefixName)) {
- fullPathFileName =
cdPathWindowsToLinux(url.substring(prefixLength, url.length()));
+ int prefixLength = ISO_PREFIX.length();
+ if (url.length() >= prefixLength && (url.substring(0,
prefixLength)).equalsIgnoreCase(ISO_PREFIX)) {
+ fullPathFileName =
cdPathWindowsToLinux(url.substring(prefixLength));
}
return fullPathFileName;
}
@@ -324,15 +318,15 @@
private void attachCd() {
Guid storagePoolId = getVm().getStoragePoolId();
- boolean isIsoFound =
(getIsoDomainListSyncronizer().findActiveISODomain(storagePoolId) != null);
- if (isIsoFound) {
+ // if iso domain found
+ if (getIsoDomainListSyncronizer().findActiveISODomain(storagePoolId)
!= null) {
if (StringUtils.isEmpty(getVm().getCdPath())) {
getVm().setCdPath(getVm().getIsoPath());
guestToolsVersionTreatment();
if (getVm().getBootSequence() != null &&
getVm().getBootSequence().containsSubsequence(BootSequence.D)) {
getVm().setCdPath(getVm().getIsoPath());
}
-
getVm().setCdPath(ImagesHandler.cdPathWindowsToLinux(getVm().getCdPath(),
getVm().getStoragePoolId()));
+ getVm().setCdPath(cdPathWindowsToLinux(getVm().getCdPath()));
}
} else if (!StringUtils.isEmpty(getVm().getIsoPath())) {
getVm().setCdPath("");
@@ -600,8 +594,8 @@
} else {
handleMemoryAdjustments();
VmHandler.updateDisksFromDb(getVm());
- getVm().setCdPath(_cdImagePath);
- getVm().setFloppyPath(_floppyImagePath);
+ getVm().setCdPath(cdImagePath);
+ getVm().setFloppyPath(floppyImagePath);
getVm().setKvmEnable(getParameters().getKvmEnable());
getVm().setRunAndPause(getParameters().getRunAndPause() == null ?
getVm().isRunAndPause() : getParameters().getRunAndPause());
getVm().setAcpiEnable(getParameters().getAcpiEnable());
@@ -675,17 +669,9 @@
boolean attachCd = false;
String selectedToolsVersion = "";
String selectedToolsClusterVersion = "";
- StorageDomain isoDomain = null;
+ Guid isoDomainId =
getIsoDomainListSyncronizer().findActiveISODomain(getVm().getStoragePoolId());
if (OsRepositoryImpl.INSTANCE.isWindows(getVm().getVmOsId())
- && (null != (isoDomain =
-
LinqUtils.firstOrNull(getStorageDomainDAO().getAllForStoragePool(getVm().getStoragePoolId()),
- new Predicate<StorageDomain>() {
- @Override
- public boolean eval(StorageDomain domain) {
- return domain.getStorageDomainType()
== StorageDomainType.ISO;
- }
- }))
- && isoDomain.getStatus() == StorageDomainStatus.Active
&& StringUtils.isEmpty(_cdImagePath))) {
+ && null != isoDomainId && StringUtils.isEmpty(cdImagePath)) {
// get cluster version of the vm tools
Version vmToolsClusterVersion = null;
@@ -700,8 +686,7 @@
// Fetch cached Iso files from active Iso domain.
List<RepoImage> repoFilesMap =
-
getIsoDomainListSyncronizer().getCachedIsoListByDomainId(isoDomain.getId(),
- ImageFileType.ISO);
+
getIsoDomainListSyncronizer().getCachedIsoListByDomainId(isoDomainId,
ImageFileType.ISO);
Version bestClusterVer = null;
int bestToolVer = 0;
for (RepoImage map : repoFilesMap) {
@@ -748,7 +733,7 @@
new
IrsBaseVDSCommandParameters(getVm().getStoragePoolId())).getReturnValue();
rhevToolsPath = isoDir + File.separator + rhevToolsPath;
-
getVm().setCdPath(ImagesHandler.cdPathWindowsToLinux(rhevToolsPath,
getVm().getStoragePoolId()));
+ getVm().setCdPath(cdPathWindowsToLinux(rhevToolsPath));
}
}
diff --git
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
index 8769d14..0c8559e 100644
---
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
+++
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
@@ -82,7 +82,6 @@
@Mock
private IsoDomainListSyncronizer isoDomainListSyncronizer;
- private static final String ISO_PREFIX = "iso://";
private static final String ACTIVE_ISO_PREFIX =
"/rhev/data-center/mnt/some_computer/f6bccab4-e2f5-4e02-bba0-5748a7bc07b6/images/11111111-1111-1111-1111-111111111111";
private static final String INACTIVE_ISO_PREFIX = "";
@@ -149,7 +148,7 @@
public void validateIsoPrefix() throws Exception {
String initrd = "initrd";
String kernel = "kernel";
- VM vm = createVmForTesting(ISO_PREFIX + initrd, ISO_PREFIX + kernel);
+ VM vm = createVmForTesting(RunVmCommand.ISO_PREFIX + initrd,
RunVmCommand.ISO_PREFIX + kernel);
assertEquals(vm.getInitrdUrl(), ACTIVE_ISO_PREFIX + "/" + initrd);
assertEquals(vm.getKernelUrl(), ACTIVE_ISO_PREFIX + "/" + kernel);
}
@@ -158,7 +157,7 @@
public void validateIsoPrefixForKernelAndNoPrefixForInitrd() throws
Exception {
String initrd = "initrd";
String kernel = "kernel";
- VM vm = createVmForTesting(initrd, ISO_PREFIX + kernel);
+ VM vm = createVmForTesting(initrd, RunVmCommand.ISO_PREFIX + kernel);
assertEquals(vm.getInitrdUrl(), initrd);
assertEquals(vm.getKernelUrl(), ACTIVE_ISO_PREFIX + "/" + kernel);
}
@@ -167,7 +166,7 @@
public void validateIsoPrefixForInitrdAndNoPrefixForKernel() throws
Exception {
String initrd = "initrd";
String kernel = "kernel";
- VM vm = createVmForTesting(ISO_PREFIX + initrd, kernel);
+ VM vm = createVmForTesting(RunVmCommand.ISO_PREFIX + initrd, kernel);
assertEquals(vm.getInitrdUrl(), ACTIVE_ISO_PREFIX + "/" + initrd);
assertEquals(vm.getKernelUrl(), kernel);
}
@@ -175,7 +174,7 @@
@Test
public void validateIsoPrefixNameForKernelAndNullForInitrd() throws
Exception {
String kernel = "kernel";
- VM vm = createVmForTesting(null, ISO_PREFIX + kernel);
+ VM vm = createVmForTesting(null, RunVmCommand.ISO_PREFIX + kernel);
assertEquals(vm.getInitrdUrl(), null);
assertEquals(vm.getKernelUrl(), ACTIVE_ISO_PREFIX + "/" + kernel);
}
@@ -189,8 +188,8 @@
@Test
public void validateIsoPrefixForOnlyIsoPrefixInKernelAndInitrd() throws
Exception {
- String initrd = ISO_PREFIX;
- String kernelUrl = ISO_PREFIX;
+ String initrd = RunVmCommand.ISO_PREFIX;
+ String kernelUrl = RunVmCommand.ISO_PREFIX;
VM vm = createVmForTesting(initrd, kernelUrl);
assertEquals(vm.getInitrdUrl(), "");
assertEquals(vm.getKernelUrl(), "");
@@ -208,7 +207,7 @@
@Test
public void validateIsoPrefixNameForInitrdAndNullForKernel() throws
Exception {
String initrd = "initrd";
- VM vm = createVmForTesting(ISO_PREFIX + initrd, null);
+ VM vm = createVmForTesting(RunVmCommand.ISO_PREFIX + initrd, null);
assertEquals(vm.getInitrdUrl(), ACTIVE_ISO_PREFIX + "/" + initrd);
assertEquals(vm.getKernelUrl(), null);
}
@@ -219,21 +218,21 @@
setIsoPrefixVDSMethod(INACTIVE_ISO_PREFIX);
String initrd = "initrd";
- VM vm = createVmForTesting(ISO_PREFIX + initrd, null);
+ VM vm = createVmForTesting(RunVmCommand.ISO_PREFIX + initrd, null);
assertEquals(vm.getInitrdUrl(), INACTIVE_ISO_PREFIX + "/" + initrd);
}
@Test
public void validateIsoPrefixWithTrippleSlash() throws Exception {
- String initrd = ISO_PREFIX + "/";
+ String initrd = RunVmCommand.ISO_PREFIX + "/";
VM vm = createVmForTesting(initrd, null);
assertEquals(vm.getInitrdUrl(), ACTIVE_ISO_PREFIX + "/");
}
@Test
public void validateIsoPrefixInTheMiddleOfTheInitrdAndKerenelName() throws
Exception {
- String initrd = "initrd " + ISO_PREFIX;
- String kernelUrl = "kernelUrl " + ISO_PREFIX;
+ String initrd = "initrd " + RunVmCommand.ISO_PREFIX;
+ String kernelUrl = "kernelUrl " + RunVmCommand.ISO_PREFIX;
VM vm = createVmForTesting(initrd, kernelUrl);
assertEquals(vm.getInitrdUrl(), initrd);
assertEquals(vm.getKernelUrl(), kernelUrl);
--
To view, visit http://gerrit.ovirt.org/17577
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia37a78eacd6c415a9bf10920362140736c7d70fc
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <aha...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
