Chris Morrissey has posted comments on this change.

Change subject: core: Added RegisterDiskCommand and GetUnregisteredDisksQuery
......................................................................


Patch Set 1: (7 inline comments)

Added responses to inline comments. Working on next patch set.

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUnregisteredDiskQuery.java
Line 32:             getQueryReturnValue().setSucceeded(true);
Line 33:         }
Line 34:     }
Line 35: 
Line 36:     public static DiskImage populateDisk(BackendInternal backend, Guid 
storageDomainId, Guid diskId) {
The public and static modifiers were an artifact of an earlier version. Will 
change to private.
Line 37:         Guid storagePoolId = getStoragePoolIdForDomain(backend, 
storageDomainId);
Line 38: 
Line 39:         // Now get the list of volumes for each new image.
Line 40:         StoragePoolDomainAndGroupIdBaseVDSCommandParameters 
getVolumesParameters = new StoragePoolDomainAndGroupIdBaseVDSCommandParameters(


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUnregisteredDisksQuery.java
Line 41:         List<Disk> unregisteredDisks = new ArrayList<Disk>();
Line 42:         for (Guid unregisteredDiskId : imagesList) {
Line 43:             String sessionId = getParameters().getSessionId();
Line 44:             if (sessionId == null) {
Line 45:                 sessionId = 
ThreadLocalParamsContainer.getHttpSessionId();
Yeah, they were accidentally left in. They will be removed in the next patch 
set.
Line 46:             }
Line 47:             GetUnregisteredDiskQueryParameters unregQueryParams = new 
GetUnregisteredDiskQueryParameters(
Line 48:                     unregisteredDiskId, getStorageDomainId());
Line 49:             VdcQueryReturnValue unregQueryReturn = 
getBackend().runInternalQuery(VdcQueryType.GetUnregisteredDisk,


Line 50:                     unregQueryParams);
Line 51:             if (unregQueryReturn.getSucceeded()) {
Line 52:                 unregisteredDisks.add((Disk) 
unregQueryReturn.getReturnValue());
Line 53:             } else {
Line 54:                 System.out.println("Could not get populated disk, 
reason: " + unregQueryReturn.getExceptionString());
Will do. Thanks.
Line 55:             }
Line 56:         }
Line 57:         getQueryReturnValue().setReturnValue(unregisteredDisks);
Line 58:     }


Line 61:         return Backend.getInstance().getResourceManager();
Line 62:     }
Line 63: 
Line 64:     protected Guid getStoragePoolId() {
Line 65:         return 
getDbFacade().getStoragePoolDao().getAllForStorageDomain(getStorageDomainId()).get(0).getId();
Good call. Fixed for next patch set.
Line 66:     }
Line 67: 
Line 68:     protected Guid getStorageDomainId() {
Line 69:         return getParameters().getStorageDomainId();


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RegisterDiskCommand.java
Line 31: 
Line 32:     @Override
Line 33:     protected boolean canDoAction() {
Line 34:         // Currently this only supports importing images and does not 
work with luns.
Line 35:         return getParameters().getDiskImage().getDiskStorageType() == 
DiskStorageType.IMAGE;
This method returns a boolean. Do you mean to log a reason why the action 
cannot be performed?
Line 36:     }
Line 37: 
Line 38:     @Override
Line 39:     protected void executeCommand() {


....................................................
File 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetUnregisteredDisksQueryTest.java
Line 34: import org.ovirt.engine.core.dao.DiskImageDAO;
Line 35: import org.ovirt.engine.core.utils.MockConfigRule;
Line 36: 
Line 37: @RunWith(MockitoJUnitRunner.class)
Line 38: public class GetUnregisteredDisksQueryTest
Yep, not sure how I missed that one. Will be fixed in the next patch.
Line 39:         extends
Line 40:         AbstractQueryTest<StorageDomainQueryParametersBase, 
GetUnregisteredDisksQuery<StorageDomainQueryParametersBase>> {
Line 41: 
Line 42:     @Rule


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainDisksResource.java
Line 51:             // First we need to query the backend to fill in all the 
information about the disk from the VDSM.
Line 52:             // We don't just use the information from the Disk object 
because it's missing a few things like creation
Line 53:             // date and last modified date.
Line 54:             GetUnregisteredDiskQueryParameters getDiskParams = new 
GetUnregisteredDiskQueryParameters(
Line 55:                     Guid.createGuidFromString(disk.getId()), 
storageDomainId);
Thanks for the tip. Will do.
Line 56:             VdcQueryReturnValue populateReturn = 
runQuery(VdcQueryType.GetUnregisteredDisk, getDiskParams);
Line 57:             if (populateReturn.getSucceeded()) {
Line 58:                 DiskImage unregisteredDisk = (DiskImage) 
populateReturn.getReturnValue();
Line 59:                 RegisterDiskParameters registerDiskParams = new 
RegisterDiskParameters(unregisteredDisk);


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I82de498fd9a8e25ed9e1dc5776f2fdf0c35b46da
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Chris Morrissey <cmorr...@netapp.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Chris Morrissey <cmorr...@netapp.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Itamar Heim <ih...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to