Tomas Jelinek has uploaded a new change for review.

Change subject: core: Allow creation of new disks as copy of any existing disk
......................................................................

core: Allow creation of new disks as copy of any existing disk

Allowed to make a copy of a VM disk

Change-Id: I07eece05b1880011f1a1b4300d07f0bd1ebf30bd
Bug-Url: https://bugzilla.redhat.com/1132066
Signed-off-by: Tomas Jelinek <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
M 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommandTest.java
M 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyImageGroupParameters.java
M 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/CopyImageVDSCommand.java
6 files changed, 56 insertions(+), 71 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/34/38334/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
index 8273e1f..b45fcfa 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
@@ -88,7 +88,6 @@
     protected boolean canDoAction() {
         return isImageExist()
                 && checkOperationIsCorrect()
-                && canFindVmOrTemplate()
                 && isDiskUsedAsOvfStore()
                 && isImageNotLocked()
                 && isSourceAndDestTheSame()
@@ -97,7 +96,6 @@
                 && checkTemplateInDestStorageDomain()
                 && validateSpaceRequirements()
                 && checkCanBeMoveInVm()
-                && checkIfNeedToBeOverride()
                 && setAndValidateDiskProfiles();
     }
 
@@ -140,11 +138,6 @@
      * @return
      */
     protected boolean checkOperationIsCorrect() {
-        if (getParameters().getOperation() == ImageOperation.Copy
-                && (getImage().getVmEntityType() == null || 
!getImage().getVmEntityType().isTemplateType())) {
-            return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_DISK_IS_NOT_TEMPLATE_DISK);
-        }
-
         if (getParameters().getOperation() == ImageOperation.Move
                 && getImage().getVmEntityType() != null && 
getImage().getVmEntityType().isTemplateType()) {
             return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_DISK_IS_NOT_VM_DISK);
@@ -182,14 +175,6 @@
 
     protected List<DiskImage> getAllImageSnapshots() {
         return ImagesHandler.getAllImageSnapshots(getImage().getImageId());
-    }
-
-    protected boolean checkIfNeedToBeOverride() {
-        if (getParameters().getOperation() == ImageOperation.Copy && 
!getParameters().getForceOverride()
-                && 
getImage().getStorageIds().contains(getStorageDomain().getId())) {
-            return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_IMAGE_ALREADY_EXISTS);
-        }
-        return true;
     }
 
     /**
@@ -265,6 +250,17 @@
             getReturnValue().setFault(vdcRetValue.getFault());
         } else {
             setSucceeded(true);
+            if (getParameters().getOperation() == ImageOperation.Copy) {
+//                getImageStorageDomainMapDao().save(
+//                        new image_storage_domain_map(image.getImageId(),
+//                                getParameters().getStorageDomainId(),
+//                                getParameters().getQuotaId(),
+//                                getParameters().getDiskProfileId()));
+
+                ImagesHandler.addDiskImageWithNoVmDevice(getImage());
+
+            }
+
             
getReturnValue().getVdsmTaskIdList().addAll(vdcRetValue.getInternalVdsmTaskIdList());
         }
     }
@@ -340,32 +336,39 @@
     private void overrideParameters() {
         if (getParameters().getOperation() == ImageOperation.Copy) {
             getParameters().setUseCopyCollapse(true);
-            getParameters().setAddImageDomainMapping(true);
+            getParameters().setAddImageDomainMapping(false);
+
+            Guid newId = Guid.newGuid();
+            Guid newGroupId = Guid.newGuid();
+
+            DiskImage image = getImage();
+
+            image.setId(newGroupId);
+            image.setImageId(newId);
+
+            image.setDiskAlias(getDiskAlias());
+            image.setStorageIds(new ArrayList<Guid>());
+            image.getStorageIds().add(getParameters().getStorageDomainId());
+            image.setQuotaId(getParameters().getQuotaId());
+            image.setDiskProfileId(getParameters().getDiskProfileId());
+
+            getParameters().setDestinationImageId(newId);
+            getParameters().setImageGroupID(getParameters().getImageGroupID());
+            getParameters().setDestImageGroupId(newGroupId);
         } else {
             getParameters().setUseCopyCollapse(false);
+
+            getParameters().setDestinationImageId(getImageId());
+            getParameters().setImageGroupID(getImageGroupId());
+            getParameters().setDestImageGroupId(getImageGroupId());
         }
-        getParameters().setDestinationImageId(getImageId());
-        getParameters().setImageGroupID(getImageGroupId());
-        getParameters().setDestImageGroupId(getImageGroupId());
+
         getParameters().setVolumeFormat(getDiskImage().getVolumeFormat());
         getParameters().setVolumeType(getDiskImage().getVolumeType());
         getParameters().setCopyVolumeType(CopyVolumeType.SharedVol);
         getParameters().setParentCommand(getActionType());
         getParameters().setParentParameters(getParameters());
         getParameters().setDiskProfileId(getImage().getDiskProfileId());
-    }
-
-    /**
-     * The following method will determine if a provided vm/template exists
-     * @return
-     */
-    private boolean canFindVmOrTemplate() {
-        if (getParameters().getOperation() == ImageOperation.Copy) {
-            if (getVmTemplate() == null) {
-                return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST);
-            }
-        }
-        return true;
     }
 
     @Override
@@ -408,7 +411,7 @@
     }
 
     public String getDiskAlias() {
-        return getImage().getDiskAlias();
+        return StringUtils.isEmpty(getParameters().getNewAlias()) ? 
getImage().getDiskAlias() : getParameters().getNewAlias();
     }
 
     protected boolean setAndValidateDiskProfiles() {
diff --git 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommandTest.java
 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommandTest.java
index ff1b44f..70dd6fe 100644
--- 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommandTest.java
+++ 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommandTest.java
@@ -122,30 +122,6 @@
     }
 
     @Test
-    public void canDoActionWrongDiskImageTypeVm() throws Exception {
-        initializeCommand(ImageOperation.Copy);
-        initVmDiskImage(false);
-        doReturn(null).when(command).getTemplateForImage();
-        command.defineVmTemplate();
-        assertFalse(command.canDoAction());
-        assertTrue(command.getReturnValue()
-                .getCanDoActionMessages()
-                
.contains(VdcBllMessages.ACTION_TYPE_FAILED_DISK_IS_NOT_TEMPLATE_DISK.toString()));
-    }
-
-    @Test
-    public void canDoActionCanNotFindTemplate() throws Exception {
-        initializeCommand(ImageOperation.Copy);
-        initTemplateDiskImage();
-        doReturn(null).when(command).getTemplateForImage();
-        command.defineVmTemplate();
-        assertFalse(command.canDoAction());
-        assertTrue(command.getReturnValue()
-                .getCanDoActionMessages()
-                
.contains(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST.toString()));
-    }
-
-    @Test
     public void canDoActionSameSourceAndDest() throws Exception {
         destStorageId = srcStorageId;
         initializeCommand(ImageOperation.Move);
diff --git 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyImageGroupParameters.java
 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyImageGroupParameters.java
index 171e574..a8ab0a0 100644
--- 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyImageGroupParameters.java
+++ 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyImageGroupParameters.java
@@ -20,6 +20,7 @@
     private Guid destImageGroupId;
     private ImageDbOperationScope revertDbOperationScope;
     private boolean shouldLockImageOnRevert;
+    private String newAlias;
 
     public MoveOrCopyImageGroupParameters() {
         operation = ImageOperation.Unassigned;
@@ -155,4 +156,12 @@
     public void setShouldLockImageOnRevert(boolean shouldLockImageOnRevert) {
         this.shouldLockImageOnRevert = shouldLockImageOnRevert;
     }
+
+    public String getNewAlias() {
+        return newAlias;
+    }
+
+    public void setNewAlias(String newAlias) {
+        this.newAlias = newAlias;
+    }
 }
diff --git 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
index 91f30a6..6c033ad 100644
--- 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
+++ 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
@@ -241,7 +241,6 @@
     ACTION_TYPE_FAILED_IMAGE_TYPE_DOES_NOT_EXIST(ErrorType.BAD_PARAMETERS),
     ACTION_TYPE_FAILED_TEMPLATE_IS_DISABLED(ErrorType.CONFLICT),
     
ACTION_TYPE_FAILED_TEMPLATE_NOT_EXISTS_IN_CURRENT_DC(ErrorType.BAD_PARAMETERS),
-    ACTION_TYPE_FAILED_IMAGE_ALREADY_EXISTS(ErrorType.CONFLICT),
     ACTION_TYPE_FAILED_TEMPLATE_NAME_ALREADY_EXISTS(ErrorType.CONFLICT),
     ACTION_TYPE_FAILED_TEMPLATE_GUID_ALREADY_EXISTS(ErrorType.CONFLICT),
     ACTION_TYPE_FAILED_ROLE_IS_READ_ONLY(ErrorType.CONFLICT),
@@ -858,7 +857,6 @@
     ACTION_TYPE_FAILED_NETWORK_NOT_IN_CLUSTER(ErrorType.CONFLICT),
     
ACTION_TYPE_FAILED_INTERFACE_NETWORK_NOT_CONFIGURED(ErrorType.BAD_PARAMETERS),
     ACTION_TYPE_FAILED_DISK_IS_NOT_VM_DISK(ErrorType.BAD_PARAMETERS),
-    ACTION_TYPE_FAILED_DISK_IS_NOT_TEMPLATE_DISK(ErrorType.BAD_PARAMETERS),
     ACTION_TYPE_FAILED_DISK_NOT_EXIST(ErrorType.BAD_PARAMETERS),
     ACTION_TYPE_FAILED_DISKS_NOT_EXIST(ErrorType.BAD_PARAMETERS),
     ACTION_TYPE_FAILED_DISK_SNAPSHOTS_NOT_EXIST(ErrorType.BAD_PARAMETERS),
diff --git 
a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties 
b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
index cbd7db5..56e53a9 100644
--- 
a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
+++ 
b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
@@ -257,7 +257,6 @@
 ACTION_TYPE_FAILED_TEMPLATE_IS_DISABLED=Cannot ${action} ${type}. The Template 
is disabled, please try to enable the template first and try again.
 ACTION_TYPE_FAILED_TEMPLATE_NOT_EXISTS_IN_CURRENT_DC=The specified Template 
doesn't exist in the current Data Center.
 ACTION_TYPE_FAILED_TEMPLATE_CANNOT_BE_CREATED_WITH_EMPTY_DISK_ALIAS=Cannot 
${action} ${type} with an empty disk alias.
-ACTION_TYPE_FAILED_IMAGE_ALREADY_EXISTS=Cannot ${action} ${type}. One of the 
Template Images already exists.
 ACTION_TYPE_FAILED_TEMPLATE_GUID_ALREADY_EXISTS=Cannot ${action} ${type}. A 
Template with the same identifier already exists.
 ACTION_TYPE_FAILED_CANDIDATE_ALREADY_EXISTS=Cannot ${action} ${type}. The 
export candidate already exists in the specified path.\n\
 - Use the 'Force Override' option to override the existing file.
@@ -1049,7 +1048,6 @@
 ACTION_TYPE_FAILED_NO_DISKS_SPECIFIED=Cannot ${action} ${type}. No disks have 
been specified.
 ACTION_TYPE_FAILED_CANT_MOVE_SHAREABLE_DISK_TO_GLUSTERFS=Cannot ${action} 
${type}. ${diskAlias} is shareable, which Gluster domains do not support.
 ACTION_TYPE_FAILED_DISK_IS_NOT_VM_DISK=Cannot ${action} ${type}. The following 
disk(s) are not attached to any VM: ${diskAliases}.
-ACTION_TYPE_FAILED_DISK_IS_NOT_TEMPLATE_DISK=Cannot ${action} ${type}. The 
selected disk is not a template disk. Only template disks can be copied.
 ACTION_TYPE_FAILED_SOURCE_AND_TARGET_SAME=Cannot ${action} ${type}. The source 
and target storage domains are the same.
 ACTION_TYPE_FAILED_CANNOT_MOVE_TEMPLATE_DISK=Cannot ${action} ${type}. 
Template disks cannot be moved.
 ACTION_TYPE_FAILED_BASE_TEMPLATE_DOES_NOT_EXIST=Cannot ${action} ${type}. Base 
Template does not exist for this Template Version.
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/CopyImageVDSCommand.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/CopyImageVDSCommand.java
index 2b1d58f..432d286 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/CopyImageVDSCommand.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/CopyImageVDSCommand.java
@@ -25,16 +25,17 @@
          */
         setReturnValue(Guid.Empty);
 
-        log.info("-- executeIrsBrokerCommand: calling 'copyImage' with two new 
parameters: description and UUID. Parameters:");
-        log.info("++ sdUUID={}", getParameters().getStorageDomainId());
-        log.info("++ spUUID={}", getParameters().getStoragePoolId());
-        log.info("++ vmGUID={}", getParameters().getVmId());
-        log.info("++ srcImageGUID={}", getParameters().getImageGroupId());
-        log.info("++ srcVolUUID={}", getParameters().getImageId());
-        log.info("++ dstImageGUID={}", getParameters().getdstImageGroupId());
-        log.info("++ dstVolUUID={}", getParameters().getDstImageId());
-        log.info("++ descr={}", getParameters().getImageDescription());
-        log.info("++ dstSdUUID={}", getParameters().getDstStorageDomainId());
+        StringBuilder sb = new StringBuilder("");
+        sb.append("-- executeIrsBrokerCommand: calling 'copyImage' with two 
new parameters: description and UUID. Parameters:");
+        sb.append("++ sdUUID={}" + getParameters().getStorageDomainId());
+        sb.append("++ spUUID={}" + getParameters().getStoragePoolId());
+        sb.append("++ vmGUID={}" + getParameters().getVmId());
+        sb.append("++ srcImageGUID={}" + getParameters().getImageGroupId());
+        sb.append("++ srcVolUUID={}" + getParameters().getImageId());
+        sb.append("++ dstImageGUID={}" + getParameters().getdstImageGroupId());
+        sb.append("++ dstVolUUID={}" + getParameters().getDstImageId());
+        sb.append("++ descr={}" + getParameters().getImageDescription());
+        sb.append("++ dstSdUUID={}" + getParameters().getDstStorageDomainId());
 
         // NOTE: The 'uuidReturn' variable will contain the taskID and not the
         // created image id!


-- 
To view, visit https://gerrit.ovirt.org/38334
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I07eece05b1880011f1a1b4300d07f0bd1ebf30bd
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to