Chris Morrissey has posted comments on this change.
Change subject: core: Added RegisterDiskCommand and GetUnregisteredDisksQuery
......................................................................
Patch Set 1: (5 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUnregisteredDiskQuery.java
Line 40: StoragePoolDomainAndGroupIdBaseVDSCommandParameters
getVolumesParameters = new StoragePoolDomainAndGroupIdBaseVDSCommandParameters(
Line 41: storagePoolId, storageDomainId, diskId);
Line 42: VDSReturnValue volumesListReturn =
backend.getResourceManager().RunVdsCommand(VDSCommandType.GetVolumesList,
Line 43: getVolumesParameters);
Line 44: if (volumesListReturn.getSucceeded()) {
While less indentation is good, putting a negative check and throwing versus a
positive check and continuing the logical flow seems to give better readability.
Line 45: @SuppressWarnings("unchecked")
Line 46: List<Guid> volumesList = (List<Guid>)
volumesListReturn.getReturnValue();
Line 47:
Line 48: // We can't deal with snapshots, so there should only be a
single volume associated with the
Line 82: protected static Guid getStoragePoolIdForDomain(BackendInternal
backend, Guid storageDomainId) {
Line 83: // Retrieve the storage pools for the storage domain. This is
needed when getting the list of volumes in the
Line 84: // unregistered images we find in the domain.
Line 85: VdcQueryReturnValue returnValue =
backend.runInternalQuery(VdcQueryType.GetStoragePoolsByStorageDomainId,
Line 86: new StorageDomainQueryParametersBase(storageDomainId));
I'll try this out. Good suggestion.
Line 87: @SuppressWarnings("unchecked")
Line 88: List<storage_pool> storagePools = (List<storage_pool>)
returnValue.getReturnValue();
Line 89:
Line 90: if (storagePools != null && !storagePools.isEmpty()) {
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUnregisteredDisksQuery.java
Line 44: if (sessionId == null) {
Line 45: sessionId =
ThreadLocalParamsContainer.getHttpSessionId();
Line 46: }
Line 47: GetUnregisteredDiskQueryParameters unregQueryParams = new
GetUnregisteredDiskQueryParameters(
Line 48: unregisteredDiskId, getStorageDomainId());
Good idea. In fact, I went ahead and added the storage pool id as a parameter
to both queries. This also makes more sense as the rest api
backendstoragedomaindisks resource currently using these queries can just do
one lookup and pass it in as a parameter to both.
Line 49: VdcQueryReturnValue unregQueryReturn =
getBackend().runInternalQuery(VdcQueryType.GetUnregisteredDisk,
Line 50: unregQueryParams);
Line 51: if (unregQueryReturn.getSucceeded()) {
Line 52: unregisteredDisks.add((Disk)
unregQueryReturn.getReturnValue());
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();
Actually, I moved this logic out of the query and now require the storage pool
ID be passed in the parameters.
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;
Thanks for the pointer Tal. I was able to figure it out looking at other code
and have it fixed in the next patch.
Line 36: }
Line 37:
Line 38: @Override
Line 39: protected void executeCommand() {
--
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 <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Chris Morrissey <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
