Maor Lipchuk has uploaded a new change for review. Change subject: core: Improve attach process when register disks ......................................................................
core: Improve attach process when register disks Remove redundant failure log when disk is failed to be registered as part of the attach process. Disks will not be registered if they have snapshots, for example, although the attach process will still succeed and should not be related to this. As part of the fix, there is also an improvement of the images registration flow, this are the improvements: 1. Avoid redundant validation for Storage Domain availability, since it is already being validated in the CDA 2. Avoid feching for existing images in the Storage Domain, since the Storage Domain was only attached and does not contain any images in the setup yet. 3. Avoid calling two internal query command. Change-Id: If302d623ab13217776e0cb6ec49dda9cc59b7b39 Bug-Url: https://bugzilla.redhat.com/1136808 Signed-off-by: Maor Lipchuk <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java 1 file changed, 89 insertions(+), 5 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/54/33154/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java index 01d3a1e..66511e1 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java @@ -21,6 +21,7 @@ import org.ovirt.engine.core.common.action.VdcReturnValueBase; import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.DiskInterface; import org.ovirt.engine.core.common.businessentities.OvfEntityData; import org.ovirt.engine.core.common.businessentities.StorageDomainOvfInfo; import org.ovirt.engine.core.common.businessentities.StorageDomainOvfInfoStatus; @@ -33,13 +34,14 @@ import org.ovirt.engine.core.common.errors.VdcBLLException; import org.ovirt.engine.core.common.errors.VdcBllErrors; import org.ovirt.engine.core.common.errors.VdcBllMessages; -import org.ovirt.engine.core.common.queries.GetUnregisteredDisksQueryParameters; -import org.ovirt.engine.core.common.queries.VdcQueryType; import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.common.vdscommands.AttachStorageDomainVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.DetachStorageDomainVDSCommandParameters; +import org.ovirt.engine.core.common.vdscommands.GetImageInfoVDSCommandParameters; +import org.ovirt.engine.core.common.vdscommands.GetImagesListVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.HSMGetStorageDomainInfoVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.ImageHttpAccessVDSCommandParameters; +import org.ovirt.engine.core.common.vdscommands.StoragePoolDomainAndGroupIdBaseVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.common.vdscommands.VDSReturnValue; import org.ovirt.engine.core.compat.Guid; @@ -273,14 +275,96 @@ } } + private List<Disk> getUnregisteredDisks() { + Guid storagePoolId = getVds().getStoragePoolId(); + Guid storageDomainId = getParameters().getStorageDomainId(); + List<Disk> unregisteredDisks = new ArrayList<Disk>(); + List<Guid> imagesFromVds = null; + try { + // Get all of the images on the storage domain + VDSReturnValue vdsReturnValue = + runVdsCommand(VDSCommandType.GetImagesList, new GetImagesListVDSCommandParameters(storageDomainId, + storagePoolId)); + if (!vdsReturnValue.getSucceeded()) { + log.warn("Failed to get a list of images for Storage Domain to register OVF disks."); + return unregisteredDisks; + } + imagesFromVds = (List<Guid>) vdsReturnValue.getReturnValue(); + } catch (VdcBLLException e) { + log.warnFormat("Failed to get a list of images for Storage Domain to register OVF disks. Error: {0}", e); + } + for (Guid unregisteredDiskId : imagesFromVds) { + populateUnregisteredDisksListWithUnregisteredDisk(storagePoolId, + storageDomainId, + unregisteredDisks, + unregisteredDiskId); + + } + return unregisteredDisks; + } + + private void populateUnregisteredDisksListWithUnregisteredDisk(Guid storagePoolId, + Guid storageDomainId, + List<Disk> unregisteredDisks, + Guid unregisteredDiskId) { + // Get the list of volumes for each new image. + StoragePoolDomainAndGroupIdBaseVDSCommandParameters getVolumesParameters = + new StoragePoolDomainAndGroupIdBaseVDSCommandParameters( + storagePoolId, storageDomainId, unregisteredDiskId); + VDSReturnValue imageInfoReturn = null; + VDSReturnValue volumesListReturn = null; + try { + volumesListReturn = runVdsCommand(VDSCommandType.GetVolumesList, + getVolumesParameters); + if (!volumesListReturn.getSucceeded()) { + logUnregisterDiskFailure(unregisteredDiskId.toString(), volumesListReturn.getExceptionString()); + return; + } + List<Guid> volumesList = (List<Guid>) volumesListReturn.getReturnValue(); + + // We can't deal with snapshots, so there should only be a single volume associated with the + // image. If there are multiple volumes, skip the image and move on to the next. + if (volumesList.size() != 1) { + log.info("Could not get populated disk since the disk contains snapshots."); + return; + } + + // Get the information about the volume from VDSM. + GetImageInfoVDSCommandParameters imageInfoParameters = new GetImageInfoVDSCommandParameters( + storagePoolId, storageDomainId, unregisteredDiskId, volumesList.get(0)); + imageInfoReturn = runVdsCommand(VDSCommandType.GetImageInfo, imageInfoParameters); + } catch (VdcBLLException e) { + logUnregisterDiskFailure(unregisteredDiskId.toString(), e.getMessage()); + } + + if (!imageInfoReturn.getSucceeded()) { + logUnregisterDiskFailure(unregisteredDiskId.toString(), volumesListReturn.getExceptionString()); + return; + } + + DiskImage newDiskImage = (DiskImage) imageInfoReturn.getReturnValue(); + + // The disk image won't have an interface set on it. Set it to IDE by default. When the + // disk is attached to a VM, its interface can be changed to the appropriate value for that VM. + newDiskImage.setDiskInterface(DiskInterface.IDE); + newDiskImage.setStoragePoolId(storagePoolId); + unregisteredDisks.add(newDiskImage); + } + + private void logUnregisterDiskFailure(String diskId, String exception) { + String addedInfoOnFailure = null; + if (exception != null) { + addedInfoOnFailure = " reason: " + exception; + } + log.infoFormat("Could not get populated disk id {0}. {1}", diskId, addedInfoOnFailure); + } + protected List<DiskImage> getAllOVFDisks() { if (ovfDisks == null) { ovfDisks = new ArrayList<>(); // Get all unregistered disks. - List<Disk> unregisteredDisks = getBackend().runInternalQuery(VdcQueryType.GetUnregisteredDisks, - new GetUnregisteredDisksQueryParameters(getParameters().getStorageDomainId(), - getVds().getStoragePoolId())).getReturnValue(); + List<Disk> unregisteredDisks = getUnregisteredDisks(); for (Disk disk : unregisteredDisks) { DiskImage ovfStoreDisk = (DiskImage) disk; String diskDecription = ovfStoreDisk.getDescription(); -- To view, visit http://gerrit.ovirt.org/33154 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: If302d623ab13217776e0cb6ec49dda9cc59b7b39 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
