Vered Volansky has uploaded a new change for review. Change subject: restapi: Fix NPE when live migrating a disk ......................................................................
restapi: Fix NPE when live migrating a disk Empty Guid was passed as source domain id to the MoveDisksCommand and onward. This is fixed and the actual source domain is extracted from the disk entity when moving/migrating/copying a disk. The source domain id is then used in the parameters classes. This was mocked out in the relevant tests. Change-Id: I570a4c81605b63a860c012e9071e46c34c793635 Bug-Url: https://bugzilla.redhat.com/1103499 Signed-off-by: Vered Volansky <[email protected]> --- M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDiskResource.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResource.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendDisksResourceTest.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendDiskResourceTest.java 4 files changed, 31 insertions(+), 6 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/39/31339/1 diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDiskResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDiskResource.java index 7a1fd28..3a7a05f 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDiskResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDiskResource.java @@ -68,23 +68,31 @@ public Response move(Action action) { validateParameters(action, "storageDomain.id|name"); Guid storageDomainId = getStorageDomainId(action); - Guid imageId = asGuid(get().getImageId()); + Disk disk = get(); + Guid imageId = asGuid(disk.getImageId()); + Guid sourceStorageDomainId = getSourceStorageDomainId(disk); MoveDisksParameters params = new MoveDisksParameters(Collections.singletonList(new MoveDiskParameters( imageId, - Guid.Empty, + sourceStorageDomainId, storageDomainId))); return doAction(VdcActionType.MoveDisks, params, action); + } + + protected Guid getSourceStorageDomainId(Disk disk) { + return asGuid(disk.getStorageDomains().getStorageDomains().get(0).getId()); } @Override public Response copy(Action action) { validateParameters(action, "storageDomain.id|name"); Guid storageDomainId = getStorageDomainId(action); - Guid imageId = asGuid(get().getImageId()); + Disk disk = get(); + Guid imageId = asGuid(disk.getImageId()); + Guid sourceStorageDomainId = getSourceStorageDomainId(disk); MoveOrCopyImageGroupParameters params = new MoveOrCopyImageGroupParameters(imageId, - Guid.Empty, + sourceStorageDomainId, storageDomainId, ImageOperation.Copy); return doAction(VdcActionType.MoveOrCopyDisk, params, action); diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResource.java index 2f3da7a..41671e4 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResource.java @@ -90,11 +90,13 @@ public Response move(Action action) { validateParameters(action, "storageDomain.id|name"); Guid storageDomainId = getStorageDomainId(action); - Guid imageId = asGuid(getDisk().getImageId()); + Disk disk = getDisk(); + Guid sourceStorageDomainId = getSourceStorageDomainId(disk); + Guid imageId = asGuid(disk.getImageId()); MoveDisksParameters params = new MoveDisksParameters(Collections.singletonList(new MoveDiskParameters( imageId, - Guid.Empty, + sourceStorageDomainId, storageDomainId))); return doAction(VdcActionType.MoveDisks, params, action); } @@ -103,6 +105,10 @@ return performGet(VdcQueryType.GetDiskByDiskId, new IdQueryParameters(guid)); } + protected Guid getSourceStorageDomainId(Disk disk) { + return asGuid(disk.getStorageDomains().getStorageDomains().get(0).getId()); + } + @Override public Disk get() { return super.get();//explicit call solves REST-Easy confusion diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendDisksResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendDisksResourceTest.java index 60d2681..2892aee 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendDisksResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendDisksResourceTest.java @@ -107,6 +107,9 @@ expect(entity.getDiskStorageType()).andReturn(DiskStorageType.IMAGE).anyTimes(); expect(entity.getImageId()).andReturn(GUIDS[1]).anyTimes(); expect(entity.getReadOnly()).andReturn(true).anyTimes(); + ArrayList<Guid> sdIds = new ArrayList<>(); + sdIds.add(Guid.Empty); + expect(entity.getStorageIds()).andReturn(sdIds).anyTimes(); return setUpStatisticalEntityExpectations(entity); } diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendDiskResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendDiskResourceTest.java index 7d9e805..a1eda11 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendDiskResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendDiskResourceTest.java @@ -24,6 +24,8 @@ import org.ovirt.engine.core.common.queries.VdcQueryType; import org.ovirt.engine.core.compat.Guid; +import java.util.ArrayList; + public class BackendDiskResourceTest extends AbstractBackendSubResourceTest<Disk, org.ovirt.engine.core.common.businessentities.Disk, BackendDiskResource>{ protected static final Guid DISK_ID = GUIDS[1]; @@ -133,8 +135,14 @@ entity.setBoot(false); entity.setShareable(false); entity.setPropagateErrors(PropagateErrors.On); + + ArrayList<Guid> sdIds = new ArrayList<>(); + sdIds.add(Guid.Empty); + entity.setStorageIds(sdIds); + return setUpStatisticalEntityExpectations(entity); } + static org.ovirt.engine.core.common.businessentities.Disk setUpStatisticalEntityExpectations(DiskImage entity) { entity.setReadRate(1); entity.setWriteRate(2); -- To view, visit http://gerrit.ovirt.org/31339 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I570a4c81605b63a860c012e9071e46c34c793635 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5 Gerrit-Owner: Vered Volansky <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
