Skip to content

extract OVA: join disks by tar member name; fix multi-disk imports #1

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

Merged
slavkap merged 1 commit into from
Jun 8, 2026
Merged

extract OVA: join disks by tar member name; fix multi-disk imports #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
79 changes: 20 additions & 59 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 @@ -8,13 +8,11 @@
import java.util.Map;
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;
import javax.inject.Inject;

import org.apache.commons.collections.MapUtils;
import org.apache.commons.lang.StringUtils;
import org.ovirt.engine.core.bll.ConcurrentChildCommandsExecutionCallback;
import org.ovirt.engine.core.bll.NonTransactiveCommandAttribute;
Expand Down Expand Up @@ -101,8 +99,8 @@ protected void executeVmCommand() {
prepareManagedBlockDisksForAnsibleExtract();
}
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 Down Expand Up @@ -213,50 +211,13 @@ private void prepareManagedBlockDisksForAnsibleExtract() {
}
}

private String buildOvaImageMappingsYamlForAnsible() {
Map<Guid, Guid> byDiskId = getParameters().getOvaSourceImageIdByDiskId();
if (MapUtils.isNotEmpty(byDiskId)) {
Map<Guid, Guid> currentImageIdToSource = new LinkedHashMap<>();
for (DiskImage d : getDiskList()) {
Guid sourceImageId = byDiskId.get(d.getId());
if (sourceImageId != null) {
currentImageIdToSource.put(d.getImageId(), sourceImageId);
}
}
if (!currentImageIdToSource.isEmpty()) {
return currentImageIdToSource.entrySet()
.stream()
.map(e -> String
.format("\\\"%s\\\": \\\"%s\\\"", e.getValue().toString(), e.getKey().toString()))
.collect(Collectors.joining(", ", "{", "}"));
}
}
Map<Guid, Guid> legacy = getParameters().getImageMappings();
if (MapUtils.isEmpty(legacy)) {
return "{}";
}
if (legacy.size() == 1 && getDiskList().size() == 1) {
Map.Entry<Guid, Guid> e = legacy.entrySet().iterator().next();
Guid sourceImageId = e.getValue();
Guid currentImageId = getDiskList().get(0).getImageId();
return "{" + String.format("\\\"%s\\\": \\\"%s\\\"",
sourceImageId.toString(),
currentImageId.toString()) + "}";
}
return legacy.entrySet()
.stream()
.map(e -> String.format("\\\"%s\\\": \\\"%s\\\"", e.getValue().toString(), e.getKey().toString()))
.collect(Collectors.joining(", ", "{", "}"));
}

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", buildOvaImageMappingsYamlForAnsible())
.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 @@ -317,27 +278,27 @@ private List<String> prepareImages() {
}

/**
* @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"));
Map<String, String> diskPathToImageId = IntStream.range(0, diskList.size())
.boxed()
.collect(Collectors.toMap(i -> diskPaths.get(i),
i -> diskList.get(i).getImageId().toString()));
Map<String, Object> spec = new HashMap<>();
spec.put("pathToFormat", diskPathToFormat);
spec.put("pathToImageId", diskPathToImageId);
String json;
private String prepareDisksJson(List<DiskImage> diskList, List<String> diskPaths) {
List<String> tarNames = getParameters().getOvaTarNamesByIndex();
if (tarNames == null || tarNames.size() != diskList.size()) {
throw new EngineException(
EngineError.GeneralException,
"OVA extract: ovaTarNamesByIndex missing or size mismatch with disk list");
}
Map<String, Map<String, String>> entries = new LinkedHashMap<>();
for (int i = 0; i < diskList.size(); i++) {
Map<String, String> entry = new HashMap<>();
entry.put("path", diskPaths.get(i));
entry.put("format", diskList.get(i).getVolumeFormat() == VolumeFormat.COW ? "qcow2" : "raw");
entries.put(tarNames.get(i), entry);
}
try {
json = new ObjectMapper().writeValueAsString(spec);
return encode(new ObjectMapper().writeValueAsString(entries));
} catch (IOException e) {
throw new RuntimeException("failed to serialize disk info");
}
return encode(json);
}

private List<DiskImage> getDiskList() {
Expand Down
75 changes: 27 additions & 48 deletions ...ules/bll/src/main/java/org/ovirt/engine/core/bll/exportimport/ImportVmFromOvaCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
package org.ovirt.engine.core.bll.exportimport;

import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import javax.inject.Inject;

import org.apache.commons.collections.CollectionUtils;
import org.ovirt.engine.core.bll.CommandActionState;
import org.ovirt.engine.core.bll.DisableInPrepareMode;
import org.ovirt.engine.core.bll.NonTransactiveCommandAttribute;
Expand Down Expand Up @@ -123,52 +122,21 @@ private void attachManagedBlockVolumesToProxyHostForOvaConversion() {
getVmId());
}

private Map<Guid, Guid> buildOvaImageMappingsForConversion() {
if (!getParameters().isImportAsNewEntity() || newDiskIdForDisk.isEmpty()) {
return getParameters().getImageMappings();
}
Map<Guid, Guid> imageMappings = new HashMap<>();
for (DiskImage disk : getDisks()) {
DiskImage old = newDiskIdForDisk.get(disk.getId());
if (old != null) {
imageMappings.put(disk.getImageId(), old.getImageId());
}
}
return imageMappings.isEmpty() ? getParameters().getImageMappings() : imageMappings;
}

private Map<Guid, Guid> buildOvaSourceImageIdByDiskId() {
if (!getParameters().isImportAsNewEntity() || newDiskIdForDisk.isEmpty()) {
return null;
}
return OvaImportManagedBlockSupport.ovaSourceImageIdByDiskId(
getDisks(),
diskId -> {
DiskImage old = newDiskIdForDisk.get(diskId);
return old != null ? old.getImageId() : null;
});
}

@Override
protected void processImages() {
List<Guid> ovfTarImageIdsInOrder = null;
if (CommandActionState.EXECUTE.equals(getActionState())
&& !getParameters().isImportAsNewEntity()
&& OvaImportManagedBlockSupport.isManagedBlockDestination(getStorageDomain())
&& getVm().getOrigin() == OriginType.OVIRT) {
ovfTarImageIdsInOrder =
getVm().getImages().stream().map(DiskImage::getImageId).collect(Collectors.toList());
}
// Capture OVF tar names (image.imageId == <File ovf:href>) before super.processImages
// mints fresh disk ids; persist via imageMappings (command is reloaded before convert()).
List<Guid> ovfTarImageIds = getVm().getImages().stream()
.map(DiskImage::getImageId)
.collect(Collectors.toList());
super.processImages();
if (CollectionUtils.isNotEmpty(ovfTarImageIdsInOrder)) {
List<DiskImage> live = getDisks();
if (live.size() == ovfTarImageIdsInOrder.size()) {
Map<Guid, Guid> legacy = new HashMap<>();
for (int i = 0; i < live.size(); i++) {
legacy.put(live.get(i).getImageId(), ovfTarImageIdsInOrder.get(i));
}
getParameters().setImageMappings(legacy);
List<DiskImage> live = getDisks();
if (live.size() == ovfTarImageIds.size()) {
Map<Guid, Guid> tarNameByDiskId = new LinkedHashMap<>();
for (int i = 0; i < live.size(); i++) {
tarNameByDiskId.put(live.get(i).getId(), ovfTarImageIds.get(i));
}
getParameters().setImageMappings(tarNameByDiskId);
}
}

Expand All @@ -191,7 +159,6 @@ private ConvertOvaParameters buildConvertOvaParameters() {
parameters.setOvaPath(getParameters().getOvaPath());
parameters.setVmName(getVmName());
parameters.setDisks(getDisks());
parameters.setImageMappings(buildOvaImageMappingsForConversion());
parameters.setStoragePoolId(getStoragePoolId());
parameters.setStorageDomainId(getStorageDomainId());
parameters.setProxyHostId(getParameters().getProxyHostId());
Expand All @@ -203,7 +170,7 @@ private ConvertOvaParameters buildConvertOvaParameters() {
parameters.setParentParameters(getParameters());
parameters.setEndProcedure(EndProcedure.COMMAND_MANAGED);
parameters.setPreAttachedManagedBlockDevicesByDiskId(preAttachedManagedBlockDeviceMapForOvaChildCommands());
parameters.setOvaSourceImageIdByDiskId(buildOvaSourceImageIdByDiskId());
parameters.setOvaTarNamesByIndex(buildOvaTarNamesByIndex());
return parameters;
}

Expand All @@ -212,7 +179,6 @@ private ConvertOvaParameters buildExtractOvaParameters() {
parameters.setOvaPath(getParameters().getOvaPath());
parameters.setVmName(getVmName());
parameters.setDisks(getDisks());
parameters.setImageMappings(buildOvaImageMappingsForConversion());
parameters.setStoragePoolId(getStoragePoolId());
parameters.setStorageDomainId(getStorageDomainId());
parameters.setProxyHostId(getParameters().getProxyHostId());
Expand All @@ -221,10 +187,23 @@ private ConvertOvaParameters buildExtractOvaParameters() {
parameters.setParentParameters(getParameters());
parameters.setEndProcedure(EndProcedure.COMMAND_MANAGED);
parameters.setPreAttachedManagedBlockDevicesByDiskId(preAttachedManagedBlockDeviceMapForOvaChildCommands());
parameters.setOvaSourceImageIdByDiskId(buildOvaSourceImageIdByDiskId());
parameters.setOvaTarNamesByIndex(buildOvaTarNamesByIndex());
return parameters;
}

private List<String> buildOvaTarNamesByIndex() {
Map<Guid, Guid> tarNameByDiskId = getParameters().getImageMappings();
if (tarNameByDiskId == null || tarNameByDiskId.isEmpty()) {
return null;
}
return getDisks().stream()
.map(d -> {
Guid tarName = tarNameByDiskId.get(d.getId());
return tarName != null ? tarName.toString() : null;
})
.collect(Collectors.toList());
}

private Map<Guid, Map<String, Object>> preAttachedManagedBlockDeviceMapForOvaChildCommands() {
if (!OvaImportManagedBlockSupport.isManagedBlockDestination(getStorageDomain())) {
return null;
Expand Down
41 changes: 28 additions & 13 deletions .../src/main/java/org/ovirt/engine/core/bll/exportimport/ImportVmTemplateFromOvaCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -197,10 +198,24 @@ protected void addDisksToDb() {

@Override
protected void copyImagesToTargetDomain() {
// OVF tar names per disk via originalDiskImageIdMap (populated by initImportClonedTemplateDisks
// for both Clone and !Clone); persist via diskMappings (command is reloaded before convert()).
List<Guid> ovfTarImageIds = getImages().stream()
.map(d -> {
Guid orig = getOriginalDiskImageIdMap(d.getId());
return orig != null ? orig : d.getImageId();
})
.collect(Collectors.toList());
List<Guid> createdDiskIds = new ArrayList<>();
getImages().stream().map(this::adjustDisk).forEach(img -> createdDiskIds.add(createDisk(img)));
getParameters().setTemplateDiskIdsForOvaExtract(createdDiskIds);
getParameters().setDiskMappings(getImageMappings());
if (createdDiskIds.size() == ovfTarImageIds.size()) {
Map<Guid, Guid> tarNameByDiskId = new LinkedHashMap<>();
for (int i = 0; i < createdDiskIds.size(); i++) {
tarNameByDiskId.put(createdDiskIds.get(i), ovfTarImageIds.get(i));
}
getParameters().setDiskMappings(tarNameByDiskId);
}
}

protected DiskImage adjustDisk(DiskImage image) {
Expand Down Expand Up @@ -256,16 +271,17 @@ private Map<Guid, Map<String, Object>> preAttachedManagedBlockDeviceMapForOvaChi
return OvaImportManagedBlockSupport.preAttachedManagedBlockDevicesByDiskId(getVmTemplate().getDiskList());
}

private Map<Guid, Guid> buildOvaSourceImageIdByDiskId() {
return OvaImportManagedBlockSupport.ovaSourceImageIdByDiskId(
getVmTemplate().getDiskList(),
this::getOriginalDiskImageIdMap);
}

protected Map<Guid, Guid> getImageMappings() {
return getImages().stream().collect(Collectors.toMap(
DiskImage::getImageId,
d -> getOriginalDiskImageIdMap(d.getId())));
private List<String> buildOvaTarNamesByIndex() {
Map<Guid, Guid> tarNameByDiskId = getParameters().getImageMappings();
if (tarNameByDiskId == null || tarNameByDiskId.isEmpty()) {
return null;
}
return getVmTemplate().getDiskList().stream()
.map(d -> {
Guid tarName = tarNameByDiskId.get(d.getId());
return tarName != null ? tarName.toString() : null;
})
.collect(Collectors.toList());
}

private void convert() {
Expand All @@ -287,7 +303,6 @@ private ConvertOvaParameters buildExtractOvaParameters() {
parameters.setOvaPath(getParameters().getOvaPath());
parameters.setVmName(getVmTemplateName());
parameters.setDisks(getVmTemplate().getDiskList());
parameters.setImageMappings(getParameters().getImageMappings());
parameters.setStoragePoolId(getStoragePoolId());
Guid extractStorageDomainId = getStorageDomainId();
if (Guid.isNullOrEmpty(extractStorageDomainId)) {
Expand All @@ -307,7 +322,7 @@ private ConvertOvaParameters buildExtractOvaParameters() {
parameters.setEndProcedure(EndProcedure.COMMAND_MANAGED);
parameters.setVmEntityType(VmEntityType.TEMPLATE);
parameters.setPreAttachedManagedBlockDevicesByDiskId(preAttachedManagedBlockDeviceMapForOvaChildCommands());
parameters.setOvaSourceImageIdByDiskId(buildOvaSourceImageIdByDiskId());
parameters.setOvaTarNamesByIndex(buildOvaTarNamesByIndex());
parameters.setTemplateDiskIdsForExtract(getParameters().getTemplateDiskIdsForOvaExtract());
return parameters;
}
Expand Down
13 changes: 0 additions & 13 deletions ...ll/src/main/java/org/ovirt/engine/core/bll/exportimport/OvaImportManagedBlockSupport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,19 +124,6 @@ public static Map<Guid, Map<String, Object>> preAttachedManagedBlockDevicesByDis
return out.isEmpty() ? null : out;
}

public static Map<Guid, Guid> ovaSourceImageIdByDiskId(
Iterable<DiskImage> disks,
Function<Guid, Guid> currentDiskIdToOvaSourceImageId) {
Map<Guid, Guid> out = new HashMap<>();
for (DiskImage disk : disks) {
Guid sourceImageId = currentDiskIdToOvaSourceImageId.apply(disk.getId());
if (sourceImageId != null) {
out.put(disk.getId(), sourceImageId);
}
}
return out.isEmpty() ? null : out;
}

@FunctionalInterface
public interface InternalActionInvoker {
void invoke(ActionType actionType, ActionParametersBase parameters, CommandContext commandContext);
Expand Down
Loading