Skip to content

Linstor: Fix migrate primary storage #9528

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
DaanHoogland merged 3 commits into from
Sep 9, 2024
Merged

Linstor: Fix migrate primary storage #9528

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5ea0308
linstor: Fix migrate primary storage
rp- Aug 7, 2024
a3e320d
kvm: add more volume info to KVMPhysicalDisk
rp- Aug 8, 2024
ebcf66a
linstor: use added KVMPhysicalDisk info for Aux properties
rp- Aug 8, 2024
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
27 changes: 24 additions & 3 deletions plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,18 @@

import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat;
import org.apache.cloudstack.utils.qemu.QemuObject;
import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
import org.apache.commons.lang3.StringUtils;

import java.util.ArrayList;
import java.util.List;

public class KVMPhysicalDisk {
private String path;
private String name;
private KVMStoragePool pool;
private final String name;
private final KVMStoragePool pool;
private String dispName;
Copy link
Copy Markdown
Contributor

@sureshanaparti sureshanaparti Aug 27, 2024

Choose a reason for hiding this comment

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

Suggested change
private String dispName;
private String displayName;

private String vmName;
private boolean useAsTemplate;

public static String RBDStringBuilder(String monHost, int monPort, String authUserName, String authSecret, String image) {
Expand Down Expand Up @@ -81,7 +84,9 @@ public KVMPhysicalDisk(String path, String name, KVMStoragePool pool) {

@Override
public String toString() {
return "KVMPhysicalDisk [path=" + path + ", name=" + name + ", pool=" + pool + ", format=" + format + ", size=" + size + ", virtualSize=" + virtualSize + "]";
return String.format("KVMPhysicalDisk %s",
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(
this, "path", "name", "pool", "format", "size", "virtualSize", "dispName", "vmName"));
}

public void setFormat(PhysicalDiskFormat format) {
Expand Down Expand Up @@ -135,4 +140,20 @@ public void setQemuEncryptFormat(QemuObject.EncryptFormat format) {
public void setUseAsTemplate() { this.useAsTemplate = true; }

public boolean useAsTemplate() { return this.useAsTemplate; }

public String getDispName() {
return dispName;
}

public void setDispName(String dispName) {
this.dispName = dispName;
}

public String getVmName() {
return vmName;
}

public void setVmName(String vmName) {
this.vmName = vmName;
}
}
5 changes: 5 additions & 0 deletions ...s/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ public Answer cloneVolumeFromBaseTemplate(final CopyCommand cmd) {
if (!storagePoolMgr.connectPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), path, details)) {
s_logger.warn("Failed to connect new volume at path: " + path + ", in storage pool id: " + primaryStore.getUuid());
}
BaseVol.setDispName(template.getName());

vol = storagePoolMgr.copyPhysicalDisk(BaseVol, path != null ? path : volume.getUuid(), primaryPool, cmd.getWaitInMillSeconds(), null, volume.getPassphrase(), volume.getProvisioningType());

Expand Down Expand Up @@ -524,6 +525,8 @@ public Answer copyVolumeFromImageCacheToPrimary(final CopyCommand cmd) {
final KVMPhysicalDisk volume = secondaryStoragePool.getPhysicalDisk(srcVolumeName);

volume.setFormat(PhysicalDiskFormat.valueOf(srcFormat.toString()));
volume.setDispName(srcVol.getName());
volume.setVmName(srcVol.getVmName());

final KVMPhysicalDisk newDisk = storagePoolMgr.copyPhysicalDisk(volume, path != null ? path : volumeName, primaryPool, cmd.getWaitInMillSeconds());

Expand Down Expand Up @@ -2486,6 +2489,8 @@ public Answer copyVolumeFromPrimaryToPrimary(CopyCommand cmd) {
}

volume.setFormat(PhysicalDiskFormat.valueOf(srcFormat.toString()));
volume.setDispName(srcVol.getName());
volume.setVmName(srcVol.getVmName());

String destVolumeName = null;
if (destPrimaryStore.isManaged()) {
Expand Down
5 changes: 4 additions & 1 deletion ...hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1402,7 +1402,10 @@ public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk disk, String name, KVMSt
*/

KVMStoragePool srcPool = disk.getPool();
PhysicalDiskFormat sourceFormat = disk.getFormat();
/* Linstor images are always stored as RAW, but Linstor uses qcow2 in DB,
to support snapshots(backuped) as qcow2 files. */
PhysicalDiskFormat sourceFormat = srcPool.getType() != StoragePoolType.Linstor ?
disk.getFormat() : PhysicalDiskFormat.RAW;
Copy link
Copy Markdown
Contributor

@sureshanaparti sureshanaparti Aug 27, 2024

Choose a reason for hiding this comment

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

can set format RAW in KVMPhysicalDisk for Linstor disks?

Copy link
Copy Markdown
Contributor Author

@rp- rp- Aug 27, 2024

Choose a reason for hiding this comment

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

You mean, setting it in KVMPhysicalDisk.setFormat() or KVMPhysicalDisk.getFormat()?

Copy link
Copy Markdown
Contributor Author

@rp- rp- Aug 28, 2024

Choose a reason for hiding this comment

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

@sureshanaparti ?

Copy link
Copy Markdown
Contributor Author

@rp- rp- Sep 2, 2024

Choose a reason for hiding this comment

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

@sureshanaparti ping

Copy link
Copy Markdown
Contributor

@sureshanaparti sureshanaparti Sep 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@rp- sorry, missed this. if the format is set while creating the linstor disk (KVMPhysicalDisk), you get it with disk.getFormat()

Copy link
Copy Markdown
Contributor

@sureshanaparti sureshanaparti Sep 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

check in LinstorStorageAdaptor.java - getPhysicalDisk() & createPhysicalDisk() - where KVMPhysicalDisk obj is created and RAW format is set (so, I think, no need to set the format RAW with StoragePoolType.Linstor pool type check again).

Copy link
Copy Markdown
Contributor Author

@rp- rp- Sep 5, 2024

Choose a reason for hiding this comment

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

Sadly it is, because the LinstorStorageAdaptor isn't used for migrating from another PrimaryStorage.
And so it will use the format sent from the management server and this doesn't match what is expected from the qemu-img convert command.

Copy link
Copy Markdown
Contributor Author

@rp- rp- Sep 5, 2024

Choose a reason for hiding this comment

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

If you want I can create the migration error and show you the exception/error log.

Copy link
Copy Markdown
Contributor

@sureshanaparti sureshanaparti Sep 5, 2024

Choose a reason for hiding this comment

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

ok, got it. maybe, you can revisit that later.

String sourcePath = disk.getPath();

KVMPhysicalDisk newDisk;
Expand Down
9 changes: 9 additions & 0 deletions .../volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,15 @@ public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk disk, String name, KVMSt
final KVMPhysicalDisk dstDisk = destPools.createPhysicalDisk(
name, QemuImg.PhysicalDiskFormat.RAW, provisioningType, disk.getVirtualSize(), null);

final DevelopersApi api = getLinstorAPI(destPools);
final String rscName = LinstorUtil.RSC_PREFIX + name;
try {
LinstorUtil.applyAuxProps(api, rscName, disk.getDispName(), disk.getVmName());
} catch (ApiException apiExc) {
s_logger.error(String.format("Error setting aux properties for %s", rscName));
logLinstorAnswers(apiExc.getApiCallRcList());
}

s_logger.debug(String.format("Linstor.copyPhysicalDisk: dstPath: %s", dstDisk.getPath()));
final QemuImgFile destFile = new QemuImgFile(dstDisk.getPath());
destFile.setFormat(dstDisk.getFormat());
Expand Down
37 changes: 8 additions & 29 deletions ...ava/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import com.linbit.linstor.api.model.ResourceDefinitionCloneRequest;
import com.linbit.linstor.api.model.ResourceDefinitionCloneStarted;
import com.linbit.linstor.api.model.ResourceDefinitionCreate;
import com.linbit.linstor.api.model.ResourceDefinitionModify;
import com.linbit.linstor.api.model.ResourceGroupSpawn;
import com.linbit.linstor.api.model.ResourceMakeAvailable;
import com.linbit.linstor.api.model.Snapshot;
Expand Down Expand Up @@ -62,8 +61,8 @@
import com.cloud.storage.DataStoreRole;
import com.cloud.storage.ResizeVolumePayload;
import com.cloud.storage.SnapshotVO;
import com.cloud.storage.Storage.StoragePoolType;
import com.cloud.storage.Storage;
import com.cloud.storage.Storage.StoragePoolType;
import com.cloud.storage.StorageManager;
import com.cloud.storage.StoragePool;
import com.cloud.storage.VMTemplateStoragePoolVO;
Expand Down Expand Up @@ -389,27 +388,6 @@ private void applyQoSSettings(StoragePoolVO storagePool, DevelopersApi api, Stri
}
}

private void applyAuxProps(DevelopersApi api, String rscName, String dispName, String vmName)
throws ApiException
{
ResourceDefinitionModify rdm = new ResourceDefinitionModify();
Properties props = new Properties();
if (dispName != null)
{
props.put("Aux/cs-name", dispName);
}
if (vmName != null)
{
props.put("Aux/cs-vm-name", vmName);
}
if (!props.isEmpty())
{
rdm.setOverrideProps(props);
ApiCallRcList answers = api.resourceDefinitionModify(rscName, rdm);
checkLinstorAnswersThrow(answers);
}
}

private String getRscGrp(StoragePoolVO storagePoolVO) {
return storagePoolVO.getUserInfo() != null && !storagePoolVO.getUserInfo().isEmpty() ?
storagePoolVO.getUserInfo() : "DfltRscGrp";
Expand All @@ -427,7 +405,8 @@ private String createResourceBase(
ApiCallRcList answers = api.resourceGroupSpawn(rscGrp, rscGrpSpawn);
checkLinstorAnswersThrow(answers);

applyAuxProps(api, rscName, volName, vmName);
answers = LinstorUtil.applyAuxProps(api, rscName, volName, vmName);
checkLinstorAnswersThrow(answers);

return LinstorUtil.getDevicePath(api, rscName);
} catch (ApiException apiEx)
Expand Down Expand Up @@ -499,7 +478,7 @@ private String cloneResource(long csCloneId, VolumeInfo volumeInfo, StoragePoolV
resizeResource(linstorApi, rscName, volumeInfo.getSize());
}

applyAuxProps(linstorApi, rscName, volumeInfo.getName(), volumeInfo.getAttachedVmName());
LinstorUtil.applyAuxProps(linstorApi, rscName, volumeInfo.getName(), volumeInfo.getAttachedVmName());
applyQoSSettings(storagePoolVO, linstorApi, rscName, volumeInfo.getMaxIops());

return LinstorUtil.getDevicePath(linstorApi, rscName);
Expand Down Expand Up @@ -551,7 +530,7 @@ private String createResourceFromSnapshot(long csSnapshotId, String rscName, Sto
answers = linstorApi.resourceSnapshotRestore(cloneRes, snapName, snapshotRestore);
checkLinstorAnswersThrow(answers);

applyAuxProps(linstorApi, rscName, volumeVO.getName(), null);
LinstorUtil.applyAuxProps(linstorApi, rscName, volumeVO.getName(), null);
applyQoSSettings(storagePoolVO, linstorApi, rscName, volumeVO.getMaxIops());

return LinstorUtil.getDevicePath(linstorApi, rscName);
Expand Down Expand Up @@ -833,7 +812,7 @@ public void copyAsync(DataObject srcData, DataObject dstData, AsyncCompletionCal
VolumeInfo volume = sinfo.getBaseVolume();
deleteSnapshot(
srcData.getDataStore(),
LinstorUtil.RSC_PREFIX + volume.getUuid(),
LinstorUtil.RSC_PREFIX + volume.getPath(),
LinstorUtil.RSC_PREFIX + sinfo.getUuid());
}
res = new CopyCommandResult(null, answer);
Expand Down Expand Up @@ -969,7 +948,7 @@ private Answer copyVolume(DataObject srcData, DataObject dstData) {
VolumeInfo srcVolInfo = (VolumeInfo) srcData;
final StoragePoolVO pool = _storagePoolDao.findById(srcVolInfo.getDataStore().getId());
final DevelopersApi api = LinstorUtil.getLinstorAPI(pool.getHostAddress());
final String rscName = LinstorUtil.RSC_PREFIX + srcVolInfo.getUuid();
final String rscName = LinstorUtil.RSC_PREFIX + srcVolInfo.getPath();

VolumeObjectTO to = (VolumeObjectTO) srcVolInfo.getTO();
// patch source format
Expand Down Expand Up @@ -1082,7 +1061,7 @@ protected Answer copySnapshot(DataObject srcData, DataObject destData) {
options.put("volumeSize", snapshotObject.getBaseVolume().getSize() + "");

try {
final String rscName = LinstorUtil.RSC_PREFIX + snapshotObject.getBaseVolume().getUuid();
final String rscName = LinstorUtil.RSC_PREFIX + snapshotObject.getBaseVolume().getPath();
String snapshotName = setCorrectSnapshotPath(api, rscName, snapshotObject);

CopyCommand cmd = new LinstorBackupSnapshotCommand(
Expand Down
24 changes: 24 additions & 0 deletions ...olume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@
import com.linbit.linstor.api.model.ApiCallRc;
import com.linbit.linstor.api.model.ApiCallRcList;
import com.linbit.linstor.api.model.Node;
import com.linbit.linstor.api.model.Properties;
import com.linbit.linstor.api.model.ProviderKind;
import com.linbit.linstor.api.model.Resource;
import com.linbit.linstor.api.model.ResourceDefinitionModify;
import com.linbit.linstor.api.model.ResourceGroup;
import com.linbit.linstor.api.model.ResourceWithVolumes;
import com.linbit.linstor.api.model.StoragePool;
Expand Down Expand Up @@ -236,4 +238,26 @@ public static String getDevicePath(DevelopersApi api, String rscName) throws Api
s_logger.error(errMsg);
throw new CloudRuntimeException("Linstor: " + errMsg);
}

public static ApiCallRcList applyAuxProps(DevelopersApi api, String rscName, String dispName, String vmName)
throws ApiException
{
ResourceDefinitionModify rdm = new ResourceDefinitionModify();
Properties props = new Properties();
if (dispName != null)
{
props.put("Aux/cs-name", dispName);
}
if (vmName != null)
{
props.put("Aux/cs-vm-name", vmName);
}
ApiCallRcList answers = new ApiCallRcList();
if (!props.isEmpty())
{
rdm.setOverrideProps(props);
answers = api.resourceDefinitionModify(rscName, rdm);
}
return answers;
}
}
4 changes: 2 additions & 2 deletions ...nstor/src/main/java/org/apache/cloudstack/storage/snapshot/LinstorVMSnapshotStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) {
final String snapshotName = vmSnapshotVO.getName();
final List<String> failedToDelete = new ArrayList<>();
for (VolumeObjectTO volumeObjectTO : volumeTOs) {
final String rscName = LinstorUtil.RSC_PREFIX + volumeObjectTO.getUuid();
final String rscName = LinstorUtil.RSC_PREFIX + volumeObjectTO.getPath();
String err = linstorDeleteSnapshot(api, rscName, snapshotName);

if (err != null)
Expand Down Expand Up @@ -292,7 +292,7 @@ private boolean revertVMSnapshotOperation(VMSnapshot vmSnapshot, long userVmId)
final String snapshotName = vmSnapshotVO.getName();

for (VolumeObjectTO volumeObjectTO : volumeTOs) {
final String rscName = LinstorUtil.RSC_PREFIX + volumeObjectTO.getUuid();
final String rscName = LinstorUtil.RSC_PREFIX + volumeObjectTO.getPath();
String err = linstorRevertSnapshot(api, rscName, snapshotName);
if (err != null) {
throw new CloudRuntimeException(String.format(
Expand Down