Skip to content

Support import of VM/template for Managed block storage #1138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
slavkap wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Support import of VM/template for Managed block storage #1138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
561dc49
Support import VM/template for Managed block storage
slavkap Apr 15, 2026
c813cfc
core: pass tar member name as direct join key for OVA extract
ryan-ronnander May 5, 2026
fc3a11f
fix failing build
slavkap Jun 8, 2026
f46936d
refactor OVA import for managed block storage
slavkap Jun 15, 2026
e28e475
fix OVA import: disk-id tar mapping and mixed template disk profiles
slavkap Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 25 additions & 8 deletions backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@

import java.lang.reflect.Field;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;

import javax.annotation.PostConstruct;
import javax.inject.Inject;
import javax.inject.Singleton;

import org.apache.commons.collections.CollectionUtils;
import org.ovirt.engine.core.bll.context.CompensationContext;
import org.ovirt.engine.core.bll.storage.disk.image.DisksFilter;
import org.ovirt.engine.core.bll.storage.disk.image.ImagesHandler;
Expand Down Expand Up @@ -106,17 +108,33 @@ public void updateDisksFromDb(VmTemplate vmt) {
vmt.getDiskList().clear();
List<Disk> diskList = diskDao.getAllForVm(vmt.getId());
for (Disk dit : diskList) {
DiskImage diskImage = (DiskImage) dit;
vmt.getDiskTemplateMap().put(dit.getId(), diskImage);
vmt.getDiskImageMap().put(dit.getId(), diskImage);

DiskVmElement dve = diskVmElementDao.get(new VmDeviceId(dit.getId(), vmt.getId()));
dit.setDiskVmElements(Collections.singletonList(dve));
attachDiskImageToTemplate(vmt, dit);
}
}

vmt.getDiskList().add(diskImage);
public void addTemplateDisksFromDiskIds(VmTemplate vmt, Collection<Guid> diskIds) {
if (CollectionUtils.isEmpty(diskIds)) {
return;
}
for (Guid diskId : diskIds) {
Disk dit = diskDao.get(diskId);
if (!(dit instanceof DiskImage)) {
continue;
}
attachDiskImageToTemplate(vmt, dit);
}
}

private void attachDiskImageToTemplate(VmTemplate vmt, Disk dit) {
DiskImage diskImage = (DiskImage) dit;
Guid diskId = dit.getId();
vmt.getDiskTemplateMap().put(diskId, diskImage);
vmt.getDiskImageMap().put(diskId, diskImage);
DiskVmElement dve = diskVmElementDao.get(new VmDeviceId(diskId, vmt.getId()));
dit.setDiskVmElements(Collections.singletonList(dve));
vmt.getDiskList().add(diskImage);
}

/**
* Lock the VM template with the given id in a new transaction, handling the compensation data using the given
* {@link CompensationContext}.
Expand Down Expand Up @@ -211,4 +229,3 @@ public ValidationResult isVmTemplateImagesReady(VmTemplate vmTemplate,
return ValidationResult.VALID;
}
}

108 changes: 74 additions & 34 deletions ...r/modules/bll/src/main/java/org/ovirt/engine/core/bll/exportimport/ExtractOvaCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@

import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

import javax.enterprise.inject.Instance;
import javax.enterprise.inject.Typed;
Expand All @@ -23,6 +25,7 @@
import org.ovirt.engine.core.bll.tasks.interfaces.CommandCallback;
import org.ovirt.engine.core.common.action.ConvertOvaParameters;
import org.ovirt.engine.core.common.businessentities.VmEntityType;
import org.ovirt.engine.core.common.businessentities.VmTemplate;
import org.ovirt.engine.core.common.businessentities.storage.DiskImage;
import org.ovirt.engine.core.common.businessentities.storage.VolumeFormat;
import org.ovirt.engine.core.common.errors.EngineError;
Expand All @@ -37,6 +40,7 @@
import org.ovirt.engine.core.common.vdscommands.VDSReturnValue;
import org.ovirt.engine.core.compat.CommandStatus;
import org.ovirt.engine.core.compat.Guid;
import org.ovirt.engine.core.dao.DiskDao;
import org.ovirt.engine.core.dao.VmDao;
import org.ovirt.engine.core.utils.EngineLocalConfig;
import org.ovirt.engine.core.vdsbroker.vdsbroker.PrepareImageReturn;
Expand All @@ -59,6 +63,8 @@ public class ExtractOvaCommand<T extends ConvertOvaParameters> extends VmCommand
@Inject
private VmDao vmDao;
@Inject
private DiskDao diskDao;
@Inject
@Typed(ConcurrentChildCommandsExecutionCallback.class)
private Instance<ConcurrentChildCommandsExecutionCallback> callbackProvider;

Expand All @@ -83,9 +89,10 @@ protected void init() {
protected void executeVmCommand() {
try {
updateDisksFromDb();
prepareDisksBeforeExtract();
List<String> diskPaths = prepareImages();
String diskPathToFormat = prepareDiskPathToFormat(getDiskList(), diskPaths);
boolean succeeded = runAnsibleImportOvaPlaybook(diskPathToFormat);
String disksJson = prepareDisksJson(getDiskList(), diskPaths);
boolean succeeded = runAnsibleImportOvaPlaybook(disksJson);
teardownImages();
if (!succeeded) {
log.error("Failed to extract OVA file");
Expand All @@ -100,33 +107,42 @@ protected void executeVmCommand() {
}
}

protected void prepareDisksBeforeExtract() {
}

private void updateDisksFromDb() {
if (getParameters().getVmEntityType() == VmEntityType.TEMPLATE) {
templateHandler.updateDisksFromDb(getVmTemplate());
VmTemplate vmt = getVmTemplate();
if (vmt == null) {
throw new EngineException(EngineError.GeneralException, "OVA extract: template entity not found");
}
templateHandler.updateDisksFromDb(vmt);
if (vmt.getDiskList().isEmpty()) {
templateHandler.addTemplateDisksFromDiskIds(vmt, getParameters().getTemplateDiskIdsForExtract());
}
if (vmt.getDiskList().isEmpty()) {
int rawCount = diskDao.getAllForVm(vmt.getId()).size();
log.error(
"OVA extract: no disks on template {} after reload (diskDao.getAllForVm returned {} row(s)); "
+ "cannot build target paths for extract_ova.py",
vmt.getId(),
rawCount);
throw new EngineException(
EngineError.GeneralException,
"OVA extract found no template disks in the engine database; check template id and disk attachment.");
}
} else {
vmHandler.updateDisksFromDb(getVm());
}
}

private Map<Guid, Guid> getImageMappings() {
return getParameters().getImageMappings() != null ?
getParameters().getImageMappings()
: Collections.emptyMap();
}

private boolean runAnsibleImportOvaPlaybook(String disksPathToFormat) {
private boolean runAnsibleImportOvaPlaybook(String disksJson) {
long timeout = TimeUnit.MINUTES.toSeconds(
EngineLocalConfig.getInstance().getInteger("ANSIBLE_PLAYBOOK_EXEC_DEFAULT_TIMEOUT"));
AnsibleCommandConfig commandConfig = new AnsibleCommandConfig()
.host(getVds())
.variable("ovirt_import_ova_path", getParameters().getOvaPath())
.variable("ovirt_import_ova_disks", disksPathToFormat)
.variable("ovirt_import_ova_image_mappings",
getImageMappings().entrySet()
.stream()
.map(e -> String
.format("\\\"%s\\\": \\\"%s\\\"", e.getValue().toString(), e.getKey().toString()))
.collect(Collectors.joining(", ", "{", "}")))
.variable("ovirt_import_ova_disks", disksJson)
.variable("ansible_timeout", timeout)
// /var/log/ovirt-engine/ova/ovirt-import-ova-ansible-{hostname}-{correlationid}-{timestamp}.log
.logFileDirectory(IMPORT_OVA_LOG_DIRECTORY)
Expand Down Expand Up @@ -182,49 +198,73 @@ private String encode(String str) {
*/
private List<String> prepareImages() {
return getDiskList().stream()
.map(this::prepareImage)
.map(PrepareImageReturn::getImagePath)
.map(this::prepareImagePath)
.collect(Collectors.toList());
}

/**
* @return a json with the corresponding mounted disks paths and formats
* @return JSON map keyed by OVA tar member name: {tarName: {path, format}}
*/
private String prepareDiskPathToFormat(List<DiskImage> diskList, List<String> diskPaths) {
Map<String, String> diskPathToFormat = IntStream.range(0, diskList.size())
.boxed()
.collect(Collectors.toMap(i -> diskPaths.get(i),
i -> diskList.get(i).getVolumeFormat() == VolumeFormat.COW ? "qcow2" : "raw"));
String json;
private String prepareDisksJson(List<DiskImage> diskList, List<String> diskPaths) {

@peter-boden peter-boden Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method pairs ovaTarNamesByIndex with diskPaths/diskList by index. These two lists come from different places and are not guaranteed to share the same order:

  • ovaTarNamesByIndex is built in the parent (ImportVmFromOvaCommand.buildOvaTarNamesByIndex, ImportVmTemplateFromOvaCommand equivalent) from imageMappings, a Map<diskId, tarName> that is already keyed by disk id.
  • diskList in the child is reloaded via updateDisksFromDb() -> diskDao.getAllForVm -> GetDisksVmGuid, which has no ORDER BY and joins a multi-table view. Order is not guaranteed.

When the two orders diverge, disk i's tar name gets paired with disk j's path, so the wrong OVA member is extracted into the wrong volume. Silent, no error. Single-disk imports can't hit this (index 0 to index 0 always matches),
which is likely why the 1/2/3-disk LINSTOR test passed but real-world multi-disk still risks corruption depending on plan/heap order.

Compounding issue: extract_ova.py has no coverage check. If a tar name the engine hands over never matches a member in the OVA, that disk is silently skipped and the playbook returns rc 0 with an empty volume.

Suggested fix: stop pairing by index.

  1. On ConvertOvaParameters, replace List<String> ovaTarNamesByIndex with Map<Guid, String> diskIdToTarName. The parent already has the disk-id keyed map (imageMappings), just pass it through instead of flattening to a list.
  2. In prepareDisksJson, look up the tar name via diskIdToTarName.get(diskList.get(i).getId()) instead of tarNames.get(i). Throw if a lookup misses or a tar name is duplicated, don't silently drop a disk.
  3. In extract_ova.py, track a remaining = set(disks) and discard on match, sys.exit(1) if anything is left after the loop.

This removes the ordering dependency entirely, no ORDER BY/sort needed anywhere, and adds loud-failure behavior for a mismatch instead of a silent bad import.

@slavkap slavkap Aug 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching both issues @peter-boden! For extract, we now pass diskIdToTarName (Map<Guid, String>) instead of pairing by index, fail on miss/duplicate in prepareDisksJson, and extract_ova.py exits 1 if any tar name is unmatched. For mixed template import, MbsImportVmTemplateFromOvaCommand no longer overrides setAndValidateDiskProfiles() with return true; it supplies a per-disk skip predicate so only MBS-destined disks skip profiles, using destination-based lookup in validate() (before adjustDisk/copyFrom). Destination resolution goes through getDestinationDomainIdForDisk() keyed by original OVF disk id so non-MBS disks in a mixed template still get profiles and the correct storage domain. Tested with mixed MBS (with StorPool) + NFS template OVA import.

Map<Guid, String> diskIdToTarName = getParameters().getDiskIdToTarName();
if (diskIdToTarName == null || diskIdToTarName.isEmpty()) {
throw new EngineException(
EngineError.GeneralException,
"OVA extract: diskIdToTarName missing");
}
if (diskList.size() != diskPaths.size()) {
throw new EngineException(
EngineError.GeneralException,
"OVA extract: disk list and path list size mismatch");
}
Map<String, Map<String, String>> entries = new LinkedHashMap<>();
Set<String> usedTarNames = new HashSet<>();
for (int i = 0; i < diskList.size(); i++) {
DiskImage disk = diskList.get(i);
String tarName = diskIdToTarName.get(disk.getId());
if (tarName == null) {
throw new EngineException(
EngineError.GeneralException,
String.format("OVA extract: no tar name mapping for disk '%s'", disk.getId()));
}
if (!usedTarNames.add(tarName)) {
throw new EngineException(
EngineError.GeneralException,
String.format("OVA extract: duplicate tar name '%s'", tarName));
}
Map<String, String> entry = new HashMap<>();
entry.put("path", diskPaths.get(i));
entry.put("format", disk.getVolumeFormat() == VolumeFormat.COW ? "qcow2" : "raw");
entries.put(tarName, entry);
}
try {
json = new ObjectMapper().writeValueAsString(diskPathToFormat);
return encode(new ObjectMapper().writeValueAsString(entries));
} catch (IOException e) {
throw new RuntimeException("failed to serialize disk info");
}
return encode(json);
}

private List<DiskImage> getDiskList() {
protected List<DiskImage> getDiskList() {
return getParameters().getVmEntityType() == VmEntityType.TEMPLATE ?
getVmTemplate().getDiskList()
: getVm().getDiskList();
}

private PrepareImageReturn prepareImage(DiskImage image) {
protected String prepareImagePath(DiskImage image) {
VDSReturnValue vdsRetVal = imagesHandler.prepareImage(
image.getStoragePoolId(),
image.getStorageIds().get(0),
image.getId(),
image.getImageId(),
getParameters().getProxyHostId());
return (PrepareImageReturn) vdsRetVal.getReturnValue();
return ((PrepareImageReturn) vdsRetVal.getReturnValue()).getImagePath();
}

private void teardownImages() {
getDiskList().forEach(this::teardownImage);
}

private void teardownImage(DiskImage image) {
protected void teardownImage(DiskImage image) {
imagesHandler.teardownImage(
image.getStoragePoolId(),
image.getStorageIds().get(0),
Expand Down
4 changes: 3 additions & 1 deletion .../src/main/java/org/ovirt/engine/core/bll/exportimport/ImportVmFromExternalUrlCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,9 @@ protected VolumeType getAutoDetectedVolumeType(DiskImage disk) {

@Override
protected ActionType getImportActionType() {
return ActionType.ImportVmFromOva;
return OvaImportManagedBlockSupport.isManagedBlockDestination(getStorageDomain())
? ActionType.MbsImportVmFromOva
: ActionType.ImportVmFromOva;
}

@Override
Expand Down
Loading