Skip to content

Commit

Permalink
core: Support virtio ISO from data domain in VM import
Browse files Browse the repository at this point in the history 
Path to a virtio ISO image used in VM import was constructed assuming
that the image is located in an ISO domain. To support ISO images
located in a data domain we need to pass the storage domain ID to
ConvertVmCommand and use prepareImage()/teardownImage() to obtain the
path to the image on the host where VM conversion is running.

Due to limitation of prepareImage()/teardownImage() on VDSM side, these
calls cannot be nested. That's why the ISO image located in a data
domain can be used exclusively by a single instance of ConvertVmCommand.
Usage of it by any running VM or by other ConvertVmCommand instances in
the same time is prohibited.

Change-Id: I835602a9019ecbcf4c63529a9d006bc43ee793be
Bug-Url: https://bugzilla.redhat.com/1721455
Signed-off-by: Shmuel Melamud <smelamud@redhat.com>
  • Loading branch information
@smelamud
smelamud committed Aug 8, 2022
1 parent a26f402 commit e70ef65
Show file tree
Hide file tree
Showing 11 changed files with 145 additions and 7 deletions.
2 changes: 1 addition & 1 deletion backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public abstract class VmCommand<T extends VmOperationParameterBase> extends Comm
@Inject
private SnapshotDao snapshotDao;
@Inject
private StorageDomainDao storageDomainDao;
protected StorageDomainDao storageDomainDao;
@Inject
private DiskDao diskDao;
@Inject
Expand Down
46 changes: 44 additions & 2 deletions ...er/modules/bll/src/main/java/org/ovirt/engine/core/bll/exportimport/ConvertVmCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
import org.ovirt.engine.core.bll.tasks.CommandCoordinatorUtil;
import org.ovirt.engine.core.bll.tasks.interfaces.CommandCallback;
import org.ovirt.engine.core.common.action.ConvertVmParameters;
import org.ovirt.engine.core.common.businessentities.StorageDomainType;
import org.ovirt.engine.core.common.businessentities.V2VJobInfo.JobStatus;
import org.ovirt.engine.core.common.businessentities.VDSStatus;
import org.ovirt.engine.core.common.businessentities.storage.DiskImage;
import org.ovirt.engine.core.common.errors.EngineException;
import org.ovirt.engine.core.common.errors.EngineMessage;
import org.ovirt.engine.core.common.vdscommands.ConvertVmVDSParameters;
Expand All @@ -26,6 +28,7 @@
import org.ovirt.engine.core.vdsbroker.ResourceManager;
import org.ovirt.engine.core.vdsbroker.VdsManager;
import org.ovirt.engine.core.vdsbroker.VmManager;
import org.ovirt.engine.core.vdsbroker.vdsbroker.PrepareImageReturn;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -41,6 +44,8 @@ public class ConvertVmCommand<T extends ConvertVmParameters> extends VmCommand<T

private ConvertVmCallback cachedCallback;

private StorageDomainType isoStorageDomainType;

public ConvertVmCommand(Guid commandId) {
super(commandId);
}
Expand Down Expand Up @@ -132,9 +137,44 @@ private ConvertVmVDSParameters buildConvertParameters() {
return parameters;
}

private StorageDomainType getIsoStorageDomainType() {
if (isoStorageDomainType == null) {
isoStorageDomainType = getParameters().getVirtioIsoStorageDomainId() != null
? storageDomainDao.get(getParameters().getVirtioIsoStorageDomainId()).getStorageDomainType()
: StorageDomainType.ISO;
}
return isoStorageDomainType;
}

protected String getVirtioIsoPath() {
return getParameters().getVirtioIsoName() == null ? null :
new File(getIsoPrefix(getStoragePoolId(), getVdsId()), getParameters().getVirtioIsoName()).getPath();
if (getParameters().getVirtioIsoName() == null) {
return null;
}
if (getIsoStorageDomainType() == StorageDomainType.ISO) {
return new File(getIsoPrefix(getStoragePoolId(), getVdsId()), getParameters().getVirtioIsoName()).getPath();
}
DiskImage isoImage = imagesHandler.getSnapshotLeaf(new Guid(getParameters().getVirtioIsoName()));
PrepareImageReturn preparedImage =
(PrepareImageReturn) imagesHandler
.prepareImage(getStoragePoolId(),
getParameters().getVirtioIsoStorageDomainId(),
isoImage.getId(),
isoImage.getImageId(),
getVdsId())
.getReturnValue();
return preparedImage.getImagePath();
}

private void teardownVirtioIso() {
if (getParameters().getVirtioIsoName() == null || getIsoStorageDomainType() == StorageDomainType.ISO) {
return;
}
DiskImage isoImage = imagesHandler.getSnapshotLeaf(new Guid(getParameters().getVirtioIsoName()));
imagesHandler.teardownImage(getStoragePoolId(),
getParameters().getVirtioIsoStorageDomainId(),
isoImage.getId(),
isoImage.getImageId(),
getVdsId());
}

////////////////////
Expand All @@ -149,12 +189,14 @@ protected void endWithFailure() {
setSucceeded(true);
} finally {
deleteV2VJob();
teardownVirtioIso();
}
}

protected void endSuccessfully() {
getVdsManager().removeV2VJobInfoForVm(getVmId());
getVmManager().setConvertProxyHostId(null);
teardownVirtioIso();
setSucceeded(true);
}

Expand Down
63 changes: 63 additions & 0 deletions ...main/java/org/ovirt/engine/core/bll/exportimport/ImportVmFromExternalProviderCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import org.ovirt.engine.core.bll.CommandActionState;
import org.ovirt.engine.core.bll.DisableInPrepareMode;
import org.ovirt.engine.core.bll.LockMessagesMatchUtil;
import org.ovirt.engine.core.bll.NonTransactiveCommandAttribute;
import org.ovirt.engine.core.bll.NonTransactiveCommandAttribute.CommandCompensationPhase;
import org.ovirt.engine.core.bll.SerialChildCommandsExecutionCallback;
Expand Down Expand Up @@ -45,9 +46,11 @@
import org.ovirt.engine.core.common.businessentities.OriginType;
import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotStatus;
import org.ovirt.engine.core.common.businessentities.StorageDomainStatus;
import org.ovirt.engine.core.common.businessentities.StorageDomainType;
import org.ovirt.engine.core.common.businessentities.StoragePoolStatus;
import org.ovirt.engine.core.common.businessentities.VDS;
import org.ovirt.engine.core.common.businessentities.VDSStatus;
import org.ovirt.engine.core.common.businessentities.VM;
import org.ovirt.engine.core.common.businessentities.VMStatus;
import org.ovirt.engine.core.common.businessentities.VmDynamic;
import org.ovirt.engine.core.common.businessentities.storage.DiskImage;
Expand All @@ -58,16 +61,20 @@
import org.ovirt.engine.core.common.errors.EngineMessage;
import org.ovirt.engine.core.common.job.Step;
import org.ovirt.engine.core.common.job.StepEnum;
import org.ovirt.engine.core.common.locks.LockingGroup;
import org.ovirt.engine.core.common.queries.IdQueryParameters;
import org.ovirt.engine.core.common.queries.QueryType;
import org.ovirt.engine.core.common.utils.Pair;
import org.ovirt.engine.core.common.utils.ValidationUtils;
import org.ovirt.engine.core.common.vdscommands.VDSCommandType;
import org.ovirt.engine.core.common.vdscommands.VdsAndVmIDVDSParametersBase;
import org.ovirt.engine.core.compat.Guid;
import org.ovirt.engine.core.dal.job.ExecutionMessageDirector;
import org.ovirt.engine.core.dao.ClusterDao;
import org.ovirt.engine.core.dao.DiskDao;
import org.ovirt.engine.core.dao.StorageDomainDao;
import org.ovirt.engine.core.dao.VdsDao;
import org.ovirt.engine.core.dao.VmDao;

@DisableInPrepareMode
@NonTransactiveCommandAttribute(forceCompensation = true, compensationPhase = CommandCompensationPhase.END_COMMAND)
Expand All @@ -92,11 +99,17 @@ public class ImportVmFromExternalProviderCommand<T extends ImportVmFromExternalP
@Inject
private DiskDao diskDao;
@Inject
private VmDao vmDao;
@Inject
private StorageDomainDao storageDomainDao;
@Inject
private ImportUtils importUtils;
@Inject
@Typed(SerialChildCommandsExecutionCallback.class)
private Instance<SerialChildCommandsExecutionCallback> callbackProvider;

private StorageDomainType virtioIsoStorageDomainType;

public ImportVmFromExternalProviderCommand(Guid cmdId) {
super(cmdId);
}
Expand Down Expand Up @@ -189,17 +202,39 @@ protected boolean validate() {

if (getVm().getOrigin() != OriginType.KVM &&
getParameters().getVirtioIsoName() != null &&
getVirtioIsoStorageDomainType() == StorageDomainType.ISO &&
getActiveIsoDomainId() == null) {
return failValidation(EngineMessage.ERROR_CANNOT_FIND_ISO_IMAGE_PATH);
}

if (!validateVirtioIsoDiskUsage()) {
return false;
}

if (!validate(vmHandler.validateMaxMemorySize(getVm().getStaticData(), getEffectiveCompatibilityVersion()))) {
return false;
}

return true;
}

private boolean validateVirtioIsoDiskUsage() {
Map<Boolean, List<VM>> vms = vmDao.getForDisk(new Guid(getParameters().getVirtioIsoName()), true);

// We cannot measure active images used by VMs
if (isAttachedToRunningVMs(vms)) {
return failValidation(EngineMessage.ACTION_TYPE_FAILED_VIRTIO_ISO_IMAGE_BEING_USED);
}

return true;
}

private boolean isAttachedToRunningVMs(Map<Boolean, List<VM>> vms) {
return !vms.isEmpty() && vms.computeIfAbsent(Boolean.TRUE, b -> Collections.emptyList())
.stream()
.anyMatch(VM::isRunning);
}

protected boolean validateStorageSpace() {
List<DiskImage> dummiesDisksList = createDiskDummiesForSpaceValidations(getVm().getImages());
return validate(getImportValidator().validateSpaceRequirements(dummiesDisksList));
Expand Down Expand Up @@ -428,6 +463,7 @@ private ConvertVmParameters buildConvertVmParameters() {
parameters.setStorageDomainId(getStorageDomainId());
parameters.setProxyHostId(getParameters().getProxyHostId());
parameters.setClusterId(getClusterId());
parameters.setVirtioIsoStorageDomainId(getParameters().getVirtioIsoStorageDomainId());
parameters.setVirtioIsoName(getParameters().getVirtioIsoName());
parameters.setNetworkInterfaces(getParameters().getVm().getInterfaces());
parameters.setParentCommand(getActionType());
Expand Down Expand Up @@ -468,6 +504,15 @@ public List<PermissionSubject> getPermissionCheckSubjects() {
return permissionList;
}

private StorageDomainType getVirtioIsoStorageDomainType() {
if (virtioIsoStorageDomainType == null) {
virtioIsoStorageDomainType = getParameters().getVirtioIsoStorageDomainId() != null
? storageDomainDao.get(getParameters().getVirtioIsoStorageDomainId()).getStorageDomainType()
: StorageDomainType.ISO;
}
return virtioIsoStorageDomainType;
}

protected Guid getActiveIsoDomainId() {
return isoDomainListSynchronizer.findActiveISODomain(getStoragePoolId());
}
Expand Down Expand Up @@ -575,4 +620,22 @@ private void deleteV2VJob() {
runVdsCommand(VDSCommandType.DeleteV2VJob,
new VdsAndVmIDVDSParametersBase(getVdsId(), getVmId()));
}

@Override
protected Map<String, Pair<String, String>> getExclusiveLocks() {
var parentLocks = super.getExclusiveLocks();
if (getVirtioIsoStorageDomainType() == StorageDomainType.ISO) {
return parentLocks;
}

Map<String, Pair<String, String>> locks = new HashMap<>();
if (parentLocks != null) {
locks.putAll(parentLocks);
}
locks.put(getParameters().getVirtioIsoName(),
LockMessagesMatchUtil.makeLockingPair(LockingGroup.DISK,
EngineMessage.ACTION_TYPE_FAILED_VIRTIO_ISO_IMAGE_BEING_USED));
return locks;
}

}
1 change: 1 addition & 0 deletions ...ules/bll/src/main/java/org/ovirt/engine/core/bll/exportimport/ImportVmFromOvaCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ private ConvertOvaParameters buildConvertOvaParameters() {
parameters.setStorageDomainId(getStorageDomainId());
parameters.setProxyHostId(getParameters().getProxyHostId());
parameters.setClusterId(getClusterId());
parameters.setVirtioIsoStorageDomainId(getParameters().getVirtioIsoStorageDomainId());
parameters.setVirtioIsoName(getParameters().getVirtioIsoName());
parameters.setNetworkInterfaces(getParameters().getVm().getInterfaces());
parameters.setParentCommand(getActionType());
Expand Down
9 changes: 9 additions & 0 deletions ...modules/common/src/main/java/org/ovirt/engine/core/common/action/ConvertVmParameters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ public class ConvertVmParameters extends VmOperationParameterBase {
private Guid storageDomainId;
private Guid proxyHostId;
private Guid clusterId;
private Guid virtioIsoStorageDomainId;
private String virtioIsoName;
private OriginType originType;
private String compatVersion;
Expand Down Expand Up @@ -102,6 +103,14 @@ public void setClusterId(Guid clusterId) {
this.clusterId = clusterId;
}

public Guid getVirtioIsoStorageDomainId() {
return virtioIsoStorageDomainId;
}

public void setVirtioIsoStorageDomainId(Guid virtioIsoStorageDomainId) {
this.virtioIsoStorageDomainId = virtioIsoStorageDomainId;
}

public String getVirtioIsoName() {
return virtioIsoName;
}
Expand Down
9 changes: 9 additions & 0 deletions ...main/java/org/ovirt/engine/core/common/action/ImportVmFromExternalProviderParameters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public enum Phase {
private String password;
private Guid proxyHostId;
private ArrayList<Guid> disks;
private Guid virtioIsoStorageDomainId;
private String virtioIsoName;
private String externalName;
private Phase importPhase = Phase.CREATE_DISKS;
Expand Down Expand Up @@ -72,6 +73,14 @@ public void setDisks(ArrayList<Guid> disks) {
this.disks = disks;
}

public Guid getVirtioIsoStorageDomainId() {
return virtioIsoStorageDomainId;
}

public void setVirtioIsoStorageDomainId(Guid virtioIsoStorageDomainId) {
this.virtioIsoStorageDomainId = virtioIsoStorageDomainId;
}

public String getVirtioIsoName() {
return virtioIsoName;
}
Expand Down
1 change: 1 addition & 0 deletions ...nager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/EngineMessage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -977,6 +977,7 @@ public enum EngineMessage {
ERROR_CANNOT_ADD_EXISTING_STORAGE_DOMAIN_CONNECTION_DATA_ILLEGAL(ErrorType.BAD_PARAMETERS),
ERROR_CANNOT_MANAGE_STORAGE_DOMAIN(ErrorType.NOT_SUPPORTED),
ERROR_CANNOT_FIND_ISO_IMAGE_PATH(ErrorType.BAD_PARAMETERS),
ACTION_TYPE_FAILED_VIRTIO_ISO_IMAGE_BEING_USED(ErrorType.CONFLICT),
ERROR_ISO_IMAGE_STATUS_ILLEGAL(ErrorType.CONFLICT),
ERROR_CANNOT_FIND_FLOPPY_IMAGE_PATH(ErrorType.BAD_PARAMETERS),
ERROR_CANNOT_REMOVE_SNAPSHOT_ILLEGAL_IMAGE(ErrorType.CONFLICT),
Expand Down
2 changes: 2 additions & 0 deletions frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1897,6 +1897,8 @@ public interface AppErrors extends ConstantsWithLookup {

String ERROR_CANNOT_FIND_ISO_IMAGE_PATH();

String ACTION_TYPE_FAILED_VIRTIO_ISO_IMAGE_BEING_USED();

String ERROR_CANNOT_REMOVE_SNAPSHOT_ILLEGAL_IMAGE();

String ERROR_ISO_IMAGE_STATUS_ILLEGAL();
Expand Down
1 change: 1 addition & 0 deletions ...min/modules/frontend/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,7 @@ ERROR_CANNOT_DETACH_LAST_STORAGE_DOMAIN=Cannot detach the master Storage Domain
ERROR_CANNOT_EXTEND_CONNECTION_FAILED=Cannot extend Storage Domain. Storage device ${lun} is unreachable from ${hostName}.
ERROR_CANNOT_FIND_FLOPPY_IMAGE_PATH=Invalid Floppy image path
ERROR_CANNOT_FIND_ISO_IMAGE_PATH=Invalid ISO image path
ACTION_TYPE_FAILED_VIRTIO_ISO_IMAGE_BEING_USED=Cannot ${action} ${type}. VirtIO-Drivers disk is being used
ERROR_CANNOT_REMOVE_SNAPSHOT_ILLEGAL_IMAGE=The requested snapshot has an invalid parent, either fix the snapshot definition or remove it manually to complete the process.
ERROR_ISO_IMAGE_STATUS_ILLEGAL=Cannot ${action} ${type}. The CDROM ISO image is in ${status} status.
ERROR_CANNOT_FORCE_REMOVE_STORAGE_POOL_WITH_VDS_NOT_IN_MAINTENANCE=Cannot ${action} ${type} while there are Hosts that are not in Maintenance mode.
Expand Down
9 changes: 7 additions & 2 deletions ...main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ImportVmFromExternalSourceModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,13 @@ private List<ActionParametersBase> buildImportVmFromExternalProviderParameters()
prm.setUsername(username);
prm.setPassword(password);
prm.setProxyHostId(proxyHostId);
prm.setVirtioIsoName(getIso().getIsChangable() && getIso().getSelectedItem() != null ?
getIso().getSelectedItem().getRepoImageId() : null);
if (getIso().getIsChangable() && getIso().getSelectedItem() != null) {
prm.setVirtioIsoStorageDomainId(getIso().getSelectedItem().getRepoDomainId());
prm.setVirtioIsoName(getIso().getSelectedItem().getRepoImageId());
} else {
prm.setVirtioIsoStorageDomainId(null);
prm.setVirtioIsoName(null);
}
prm.setExternalName(importVmData.getName());

if (getClusterQuota().getSelectedItem() != null &&
Expand Down
9 changes: 7 additions & 2 deletions ...monweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ImportVmFromOvaModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,13 @@ private List<ActionParametersBase> buildImportVmFromOvaParameters() {
String ovaFilename = vmNameToOva.get(importVmData.getName());
prm.setOvaPath(ovaFilename == null ? ovaPath : ovaPath + "/" + ovaFilename); //$NON-NLS-1$
prm.setProxyHostId(hostId);
prm.setVirtioIsoName(getIso().getIsChangable() && getIso().getSelectedItem() != null ?
getIso().getSelectedItem().getRepoImageId() : null);
if (getIso().getIsChangable() && getIso().getSelectedItem() != null) {
prm.setVirtioIsoStorageDomainId(getIso().getSelectedItem().getRepoDomainId());
prm.setVirtioIsoName(getIso().getSelectedItem().getRepoImageId());
} else {
prm.setVirtioIsoStorageDomainId(null);
prm.setVirtioIsoName(null);
}
prm.setExternalName(importVmData.getName());

if (getClusterQuota().getSelectedItem() != null &&
Expand Down

0 comments on commit e70ef65

Please sign in to comment.