Liron Aravot has posted comments on this change.

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


Patch Set 7:

(3 comments)

http://gerrit.ovirt.org/#/c/32743/7/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 195:     protected List<OvfEntityData> getEntitiesFromStorageOvfDisk() {
Line 196:         List<OvfEntityData> ovfEntitiesFromTar = 
Collections.emptyList();
Line 197: 
Line 198:         // Initialize the list with all the ovfDisks in the specified 
Storage Domain.
Line 199:         List<DiskImage> ovfStoreDiskImages = new 
ArrayList(getAllOVFDisks());
please move to the correct patch and add a comment
Line 200:         if (!ovfStoreDiskImages.isEmpty()) {
Line 201:             Pair<DiskImage, Long> ovfDiskAndSize = 
getLatestOVFDisk(ovfStoreDiskImages);
Line 202:             DiskImage ovfDisk = ovfDiskAndSize.getFirst();
Line 203:             if (ovfDisk != null) {


Line 247:             ovfStoreDiskImage.setShareable(true);
Line 248:             RegisterDiskParameters registerDiskParams =
Line 249:                     new RegisterDiskParameters(ovfStoreDiskImage, 
getParameters().getStorageDomainId());
Line 250:             String result = "succeeded";
Line 251:             if (!runInternalAction(VdcActionType.RegisterDisk, 
registerDiskParams, cloneContext()).getSucceeded()) {
we should open bug for internal commands/compensation in general, let's not 
forget about it - because we pass the context to the internal command, failure 
will clear the compensation context while the execution of the parent command 
might ignore the failure.
Line 252:                 result = "failed";
Line 253:             }
Line 254:             log.infoFormat("Register new floating OVF_STORE disk with 
disk id {0} for storage domain {1} has {2}",
Line 255:                     ovfStoreDiskImage.getId(),


http://gerrit.ovirt.org/#/c/32743/7/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommandTest.java
File 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommandTest.java:

Line 114:         mockGetStorageDomainInfoVdsCommand(storageDomain);
Line 115:         mockAttachStorageDomainVdsCommand();
Line 116:         when(vdsDAO.get(any(Guid.class))).thenReturn(vds);
Line 117:         
doReturn(getUnregisteredList()).when(cmd).getEntitiesFromStorageOvfDisk();
Line 118:         doReturn(getUnregisteredList()).when(cmd).getAllOVFDisks();
please change to return correct value if it's even necessary
Line 119:         doAnswer(new Answer<Object>() {
Line 120:             @Override
Line 121:             public Object answer(InvocationOnMock invocation) throws 
Throwable {
Line 122:                 map = (StoragePoolIsoMap) 
invocation.getArguments()[0];


-- 
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: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to