Skip to content

Commit

Permalink
core: Uploading an image with an existing disk id
Browse files Browse the repository at this point in the history
While uploading users can choose disk-id for thier uplaod and if
the disk-id was already existing the engine will make a new image
in lock state and fail the prosces. now it will check the disk-id before
creating and will fail if there is another disk-id with the same number.

Bug-Url: https://bugzilla.redhat.com/2095215
Signed-off-by: Artiom Divak <adivak@redhat.com>
  • Loading branch information
ArtiomDivak authored and ahadas committed Jul 14, 2022
1 parent 1691f23 commit c07ec66
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
import org.ovirt.engine.core.compat.Guid;
import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector;
import org.ovirt.engine.core.dao.BaseDiskDao;
import org.ovirt.engine.core.dao.DiskDao;
import org.ovirt.engine.core.dao.DiskLunMapDao;
import org.ovirt.engine.core.dao.DiskVmElementDao;
import org.ovirt.engine.core.dao.ImageDao;
Expand Down Expand Up @@ -118,6 +119,8 @@ public class AddDiskCommand<T extends AddDiskParameters> extends AbstractDiskVmC
private Instance<AddDiskCommandCallback> callbackProvider;
@Inject
private ImageDao imageDao;
@Inject
private DiskDao diskDao;

@Inject
private MultiLevelAdministrationHandler multiLevelAdministrationHandler;
Expand Down Expand Up @@ -300,7 +303,8 @@ protected boolean checkIfImageDiskCanBeAdded(VM vm, DiskVmElementValidator diskV
canAddShareableDisk() &&
validate(diskVmElementValidator.isVirtIoScsiValid(vm)) &&
validate(diskVmElementValidator.isDiskInterfaceSupported(getVm())) &&
validateDiskImageNotExisting(((DiskImage) getParameters().getDiskInfo()).getImageId());
validateDiskImageNotExisting(((DiskImage) getParameters().getDiskInfo()).getImageId()) &&
validateDiskNotExisting(getParameters().getDiskInfo().getId());

if (returnValue && vm != null) {
StoragePool sp = getStoragePool(); // Note this is done according to the VM's spId.
Expand All @@ -314,6 +318,17 @@ protected boolean checkIfImageDiskCanBeAdded(VM vm, DiskVmElementValidator diskV
return returnValue;
}

private boolean validateDiskNotExisting(Guid diskId) {
Disk disk = diskDao.get(diskId);

if (disk != null && !Guid.isNullOrEmpty(disk.getId())) {
return failValidation(EngineMessage.ACTION_TYPE_FAILED_DISK_ALREADY_EXISTS,
String.format("$diskAlias %s", disk.getDiskAlias()),
String.format("$diskID %s", diskId));
}
return true;
}

private boolean validateDiskImageNotExisting(Guid imageId) {
Image image = imageDao.get(imageId);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
import org.ovirt.engine.core.common.osinfo.OsRepository;
import org.ovirt.engine.core.compat.Guid;
import org.ovirt.engine.core.compat.Version;
import org.ovirt.engine.core.dao.DiskDao;
import org.ovirt.engine.core.dao.DiskLunMapDao;
import org.ovirt.engine.core.dao.DiskVmElementDao;
import org.ovirt.engine.core.dao.ImageDao;
Expand Down Expand Up @@ -136,6 +137,9 @@ public static Stream<MockConfigDescriptor<?>> mockConfiguration() {
@Mock
private ImageDao imageDao;

@Mock
private DiskDao diskDao;

/**
* The command under test.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ public enum EngineMessage {
ACTION_TYPE_FAILED_IMAGE_TYPE_DOES_NOT_EXIST(ErrorType.BAD_PARAMETERS),
ACTION_TYPE_FAILED_TEMPLATE_IS_DISABLED(ErrorType.CONFLICT),
ACTION_TYPE_FAILED_TEMPLATE_NOT_EXISTS_IN_CURRENT_DC(ErrorType.BAD_PARAMETERS),
ACTION_TYPE_FAILED_DISK_ALREADY_EXISTS(ErrorType.CONFLICT),
ACTION_TYPE_FAILED_IMAGE_ALREADY_EXISTS(ErrorType.CONFLICT),
ACTION_TYPE_FAILED_TEMPLATE_GUID_ALREADY_EXISTS(ErrorType.CONFLICT),
ACTION_TYPE_FAILED_TEMPLATE_ID_CANT_BE_CHANGED(ErrorType.BAD_PARAMETERS),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ ACTION_TYPE_FAILED_ILLEGAL_OS_TYPE_IS_NOT_SUPPORTED_BY_ARCHITECTURE_TYPE=Cannot
ACTION_TYPE_FAILED_ILLEGAL_VM_DISPLAY_TYPE_IS_NOT_SUPPORTED_BY_OS=Cannot ${action} ${type}. Selected display type is not supported by the operating system.
ACTION_TYPE_FAILED_ILLEGAL_VM_DISPLAY_TYPE_IS_NOT_SUPPORTED_BY_FIRMWARE=Cannot ${action} ${type}. Selected display type is not supported by the firmware.
ACTION_TYPE_FAILED_ILLEGAL_WATCHDOG_MODEL_IS_NOT_SUPPORTED_BY_OS=Cannot ${action} ${type}. Selected watchdog model is not supported by the operating system.
ACTION_TYPE_FAILED_DISK_ALREADY_EXISTS=Cannot ${action} ${type}. The selected disk ID\: ${diskID} is already used by the following existing disk\: ${diskAlias}.
ACTION_TYPE_FAILED_IMAGE_ALREADY_EXISTS=Cannot ${action} ${type}. One of the Template Images already exists.
ACTION_TYPE_FAILED_IMAGE_DOWNLOAD_ERROR=Cannot ${action} ${type}. The image content could not be detected. Please try to re-import the image.
ACTION_TYPE_FAILED_CANNOT_FIND_SPECIFIED_IMAGE=Cannot ${action} ${type}. The specified image wasn't found.
Expand Down Expand Up @@ -1717,4 +1718,4 @@ ACTION_TYPE_FAILED_CONVERT_DISK_NO_FORMAT_CHANGE=Cannot ${action} ${type}. Disk
ACTION_TYPE_FAILED_CONVERT_DISK_NO_ALLOCATION_CHANGE=Cannot ${action} ${type}. Disk is already in allocation ${allocation}
ACTION_TYPE_FAILED_DISK_WITH_CHAIN=Cannot ${action} ${type}. Disks with multiple snapshots are not supported.
ACTION_TYPE_FAILED_HOSTED_ENGINE_VM_BACKUP_NOT_SUPPORTED=Cannot ${action} ${type}. Backup of Hosted Engine VM is not supported
ACTION_TYPE_FAILED_ACTIVE_IMAGE_TRANSFER_FOR_VM_BACKUP=Cannot ${action} ${type}. There is an active image transfer for VM backup
ACTION_TYPE_FAILED_ACTIVE_IMAGE_TRANSFER_FOR_VM_BACKUP=Cannot ${action} ${type}. There is an active image transfer for VM backup

0 comments on commit c07ec66

Please sign in to comment.