Liron Aravot has posted comments on this change.

Change subject: core: Register all the OVF disks to Storage
......................................................................


Patch Set 6:

(3 comments)

http://gerrit.ovirt.org/#/c/32743/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java:

Line 52: @NonTransactiveCommandAttribute(forceCompensation = true)
Line 53: public class AttachStorageDomainToPoolCommand<T extends 
AttachStorageDomainToPoolParameters> extends
Line 54:         StorageDomainCommandBase<T> {
Line 55:     private StoragePoolIsoMap map;
Line 56:     private List<DiskImage> ovfDisks = null;
no need for the = null;
Line 57: 
Line 58:     public AttachStorageDomainToPoolCommand(T parameters) {
Line 59:         this(parameters, null);
Line 60:     }


Line 248:             ovfStoreDiskImage.setShareable(true);
Line 249:             RegisterDiskParameters registerDiskParams =
Line 250:                     new RegisterDiskParameters(ovfStoreDiskImage, 
getParameters().getStorageDomainId());
Line 251:             String result = "succeeded";
Line 252:             if (!runInternalAction(VdcActionType.RegisterDisk, 
registerDiskParams, null).getSucceeded()) {
1. you should add a comment to indicate that the same transaction is used by 
register disk command always, otherwise there will be a problem if it'll be 
changed to non transactive by anyone.

2. you should pass the compensation context to the register command
Line 253:                 result = "failed";
Line 254:             }
Line 255:             log.infoFormat("Register new floating OVF_STORE disk with 
disk id {0} for storage domain {1} has {2}",
Line 256:                     ovfStoreDiskImage.getId(),


Line 310:         for (DiskImage ovfStoreDisk : ovfStoreDiskImages) {
Line 311:             boolean isBetterOvfDiskFound = false;
Line 312: 
Line 313:             // Check which disks are of OVF_STORE
Line 314:             String diskDecription = ovfStoreDisk.getDescription();
this string is no longer relevant, same goes for the comment above it
Line 315:             Map<String, Object> diskDescriptionMap;
Line 316:             try {
Line 317:                 diskDescriptionMap = 
JsonHelper.jsonToMap(diskDecription);
Line 318:             } catch (IOException e) {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9fc6666a64efa16968935cfceb10e19b8a4e2eef
Gerrit-PatchSet: 6
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

Reply via email to