Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
f42552b
CSTACKEX-18_2: NFS3 snapshot changes
rajiv-jain-netapp Feb 19, 2026
8894248
CSTACK-18_2: fixing junit dependent changes
rajiv-jain-netapp Feb 19, 2026
3f0019a
STACK-18_2: fixes
rajiv-jain-netapp Feb 20, 2026
9b79f46
CSTACKEX-18_2: adding VM snapshot logic
rajiv-jain-netapp Feb 20, 2026
7a0d61e
CSTACKEX-18_2: fix junit issues
rajiv-jain-netapp Feb 20, 2026
7c3419e
CSTACKEX-18_2: fixes for vm snapshot workflow
rajiv-jain-netapp Feb 21, 2026
d2b6a27
CSTACKEX-18_2: fixing the behaviour for the VM level snapshot when qu…
rajiv-jain-netapp Feb 21, 2026
c5d5428
CSTACKEX-18_2: incorporating the review comments.
rajiv-jain-netapp Feb 24, 2026
3f18c11
CSTACKEX-18_2: transient fixes post incorporating the comments
rajiv-jain-netapp Feb 24, 2026
723561b
CSTACKEX-18_2: Incorporate review comments
rajiv-jain-netapp Feb 24, 2026
09968db
CSTACKEX-18_2: quiecing VM would be done based on user input for VM l…
rajiv-jain-netapp Feb 25, 2026
0a1a9c4
CSTACKEX-18_2: ONTAP plugin can not handle memory snapshot with stora…
rajiv-jain-netapp Feb 26, 2026
49df4c3
CSTACKEX-18_2: junit fix
rajiv-jain-netapp Feb 26, 2026
776b9a2
CSTACKEX-18_2: junit fix2
rajiv-jain-netapp Feb 26, 2026
c04e223
CSTACKEX-18_2: ensure that ONTAP volume related calls are served by c…
rajiv-jain-netapp Feb 26, 2026
186e59b
CSTACKEX-18_2: using flexvolume snapshot to get snapshot workflows fo…
rajiv-jain-netapp Feb 26, 2026
1020a2c
CSTACKEX-18_2: using flexvolume snapshot even for CS volume snapshot …
rajiv-jain-netapp Feb 27, 2026
672d7a4
CSTACKEX-18_2: we are taking snapshot for volume with flexvolume snap…
rajiv-jain-netapp Feb 28, 2026
9c63c61
CSTACKEX-18_2: junit fixes with recent refactor
rajiv-jain-netapp Feb 28, 2026
79730ed
CSTACKEX-18_2: fixing snapshot delete condition fix
rajiv-jain-netapp Mar 2, 2026
ae96e9b
CSTACKEX-18_2: delete snapshot should be done over plugin path not on…
rajiv-jain-netapp Mar 2, 2026
1b0bba9
CSTACKEX-18_2: plugin has to consider VM for snapshot in running and …
rajiv-jain-netapp Mar 2, 2026
7780a93
CSTACKEX-18_2: revert snapshot fixes for API not found
rajiv-jain-netapp Mar 2, 2026
5bff41f
CSTACKEC-18_2: revertsnapshot workflow using private cli REST endpoint
rajiv-jain-netapp Mar 4, 2026
2abbed6
CSTACKEX-18_2: taking snapshot with memory option set as true
rajiv-jain-netapp Mar 5, 2026
2340400
CSTACKEX-18_2: add exception handling for any error coming from agent
rajiv-jain-netapp Mar 5, 2026
ce93705
CSTACKEX-18_2: reverting memory snapshot workflow and eroring out for…
rajiv-jain-netapp Mar 5, 2026
e1a6465
CSTACKEX-18_2: revert of memory snapshot implementation
rajiv-jain-netapp Mar 5, 2026
9138e20
CSTACKEX-18_2: rollback the object creation in case of failures
rajiv-jain-netapp Mar 5, 2026
5f9e51c
CSTACKEX-18_2: rollback in case of any failures
rajiv-jain-netapp Mar 9, 2026
142e0e6
CSTACKEX-18_2: comments and some changes
rajiv-jain-netapp Mar 9, 2026
aa74a5a
CSTACKEX-18_2: checkstyle fixes
rajiv-jain-netapp Mar 9, 2026
55447b7
CSTACKEX-18_2: junit fixes
rajiv-jain-netapp Mar 9, 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
Original file line number Diff line number Diff line change
Expand Up @@ -503,8 +503,9 @@ protected VMSnapshot takeVmSnapshotInternal(VMSnapshot vmSnapshot, Map<VolumeInf
return processCreateVmSnapshotAnswer(vmSnapshot, volumeInfoToSnapshotObjectMap, createDiskOnlyVMSnapshotAnswer, userVm, vmSnapshotVO, virtualSize, parentSnapshotVo);
}

logger.error("Disk-only VM snapshot for VM [{}] failed{}.", userVm.getUuid(), answer != null ? " due to" + answer.getDetails() : "");
throw new CloudRuntimeException(String.format("Disk-only VM snapshot for VM [%s] failed.", userVm.getUuid()));
String details = answer != null ? answer.getDetails() : String.format("No answer received from host [%s]. The host may be unreachable.", hostId);
Comment thread
suryag1201 marked this conversation as resolved.
logger.error("Disk-only VM snapshot for VM [{}] failed due to: {}.", userVm.getUuid(), details);
throw new CloudRuntimeException(String.format("Disk-only VM snapshot for VM [%s] failed due to: %s.", userVm.getUuid(), details));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ protected Answer takeDiskOnlyVmSnapshotOfRunningVm(CreateDiskOnlyVmSnapshotComma
return new CreateDiskOnlyVmSnapshotAnswer(cmd, false, errorMsg, null);
}
return new CreateDiskOnlyVmSnapshotAnswer(cmd, false, e.getMessage(), null);
} catch (Exception e) {
String errorMsg = String.format("Creation of disk-only VM snapshot for VM [%s] failed due to %s.", vmName, e.getMessage());
logger.error(errorMsg, e);
return new CreateDiskOnlyVmSnapshotAnswer(cmd, false, errorMsg, null);
} finally {
if (dm != null) {
try {
Expand Down Expand Up @@ -146,21 +150,13 @@ protected Answer takeDiskOnlyVmSnapshotOfStoppedVm(CreateDiskOnlyVmSnapshotComma
}
} catch (LibvirtException | QemuImgException e) {
logger.error("Exception while creating disk-only VM snapshot for VM [{}]. Deleting leftover deltas.", vmName, e);
for (VolumeObjectTO volumeObjectTO : volumeObjectTos) {
Pair<Long, String> volSizeAndNewPath = mapVolumeToSnapshotSizeAndNewVolumePath.get(volumeObjectTO.getUuid());
PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) volumeObjectTO.getDataStore();
KVMStoragePool kvmStoragePool = storagePoolMgr.getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid());

if (volSizeAndNewPath == null) {
continue;
}
try {
Files.deleteIfExists(Path.of(kvmStoragePool.getLocalPathFor(volSizeAndNewPath.second())));
} catch (IOException ex) {
logger.warn("Tried to delete leftover snapshot at [{}] failed.", volSizeAndNewPath.second(), ex);
}
}
cleanupLeftoverDeltas(volumeObjectTos, mapVolumeToSnapshotSizeAndNewVolumePath, storagePoolMgr);
return new Answer(cmd, e);
} catch (Exception e) {
logger.error("Unexpected exception while creating disk-only VM snapshot for VM [{}]. Deleting leftover deltas.", vmName, e);
cleanupLeftoverDeltas(volumeObjectTos, mapVolumeToSnapshotSizeAndNewVolumePath, storagePoolMgr);
return new CreateDiskOnlyVmSnapshotAnswer(cmd, false,
String.format("Creation of disk-only VM snapshot for VM [%s] failed due to %s.", vmName, e.getMessage()), null);
}

return new CreateDiskOnlyVmSnapshotAnswer(cmd, true, null, mapVolumeToSnapshotSizeAndNewVolumePath);
Expand Down Expand Up @@ -192,6 +188,23 @@ protected Pair<String, Map<String, Pair<Long, String>>> createSnapshotXmlAndNewV
return new Pair<>(snapshotXml, volumeObjectToNewPathMap);
}

protected void cleanupLeftoverDeltas(List<VolumeObjectTO> volumeObjectTos, Map<String, Pair<Long, String>> mapVolumeToSnapshotSizeAndNewVolumePath, KVMStoragePoolManager storagePoolMgr) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this refactoring somehow help our plugin functionality?

for (VolumeObjectTO volumeObjectTO : volumeObjectTos) {
Pair<Long, String> volSizeAndNewPath = mapVolumeToSnapshotSizeAndNewVolumePath.get(volumeObjectTO.getUuid());
PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) volumeObjectTO.getDataStore();
KVMStoragePool kvmStoragePool = storagePoolMgr.getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid());

if (volSizeAndNewPath == null) {
continue;
}
try {
Files.deleteIfExists(Path.of(kvmStoragePool.getLocalPathFor(volSizeAndNewPath.second())));
} catch (IOException ex) {
logger.warn("Tried to delete leftover snapshot at [{}] failed.", volSizeAndNewPath.second(), ex);
}
}
}

protected long getFileSize(String path) {
return new File(path).length();
}
Expand Down
10 changes: 10 additions & 0 deletions plugins/storage/volume/ontap/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,16 @@
<artifactId>cloud-engine-storage-volume</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.apache.cloudstack</groupId>
<artifactId>cloud-engine-storage-snapshot</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.apache.cloudstack</groupId>
<artifactId>cloud-server</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>io.swagger</groupId>
<artifactId>swagger-annotations</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import com.cloud.storage.Volume;
import com.cloud.storage.VolumeVO;
import com.cloud.storage.ScopeType;
import com.cloud.storage.dao.SnapshotDetailsDao;
import com.cloud.storage.dao.SnapshotDetailsVO;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.storage.dao.VolumeDetailsDao;
import com.cloud.utils.Pair;
Expand All @@ -47,18 +49,22 @@
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
import org.apache.cloudstack.storage.command.CommandResult;
import org.apache.cloudstack.storage.command.CreateObjectAnswer;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.cloudstack.storage.feign.model.FileInfo;
import org.apache.cloudstack.storage.feign.model.Lun;
import org.apache.cloudstack.storage.service.SANStrategy;
import org.apache.cloudstack.storage.service.StorageStrategy;
import org.apache.cloudstack.storage.service.UnifiedSANStrategy;
import org.apache.cloudstack.storage.service.model.AccessGroup;
import org.apache.cloudstack.storage.service.model.CloudStackVolume;
import org.apache.cloudstack.storage.service.model.ProtocolType;
import org.apache.cloudstack.storage.to.SnapshotObjectTO;
import org.apache.cloudstack.storage.utils.Constants;
import org.apache.cloudstack.storage.utils.Utility;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

Expand All @@ -80,14 +86,16 @@ public class OntapPrimaryDatastoreDriver implements PrimaryDataStoreDriver {
@Inject private VMInstanceDao vmDao;
@Inject private VolumeDao volumeDao;
@Inject private VolumeDetailsDao volumeDetailsDao;
@Inject private SnapshotDetailsDao snapshotDetailsDao;

@Override
public Map<String, String> getCapabilities() {
s_logger.trace("OntapPrimaryDatastoreDriver: getCapabilities: Called");
Map<String, String> mapCapabilities = new HashMap<>();
// RAW managed initial implementation: snapshot features not yet supported
// TODO Set it to false once we start supporting snapshot feature
mapCapabilities.put(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString(), Boolean.FALSE.toString());
mapCapabilities.put(DataStoreCapabilities.CAN_CREATE_VOLUME_FROM_SNAPSHOT.toString(), Boolean.FALSE.toString());
mapCapabilities.put(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString(), Boolean.TRUE.toString());
mapCapabilities.put(DataStoreCapabilities.CAN_CREATE_VOLUME_FROM_SNAPSHOT.toString(), Boolean.TRUE.toString());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We are supporting volume creation from snapshot with this PR?

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.

volume creation usecase is not yet ready as part of this PR

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why do we need to set this as true as we are currently not supporting volume creation from snapshot.

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.

Without this property set to true, CloudStack initiates both the snapshot creation and the corresponding copy operation to secondary storage over the data path, as it assumes that snapshots must reside on secondary storage by default.
When only the storage-system-snapshot property is enabled, CloudStack successfully invokes the takeSnapshot() method in the plugin driver class. However, the snapshot remains on the primary storage pool and is not transferred to secondary storage.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

So, with this implementation, snapshots reside in primary storage itself?

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.

What I’m trying to explain is that if we don’t keep the second flag set to true, then only the plugin‑created snapshot will remain on the primary. In that case, the orchestrator will call the host agent again to create another snapshot and copy it to the secondary.
To avoid this unnecessary duplicate operation, I set both flags to true. This ensures that the orchestrator does not rely on the host-agent portion of the workflow, and the snapshot created by the plugin is the one that gets copied to the secondary.

return mapCapabilities;
}

Expand Down Expand Up @@ -524,7 +532,88 @@ public long getUsedIops(StoragePool storagePool) {

@Override
public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback<CreateCmdResult> callback) {
s_logger.info("takeSnapshot : entered with snapshot id: " + snapshot.getId() + " and name: " + snapshot.getName());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Lets have an intuitive log message instead of a method entry logger?

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.

Could you provide a few examples?
Since I am also logging the method arguments, I felt that this approach would give me better insight into the method invocation flow, rather than relying solely on static logging.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Instead of adding an entry log we can add a log post line 544. For example, it could be like
s_logger.info("takeSnapshot: Snapshot with ID: " + snapshot.getId() + " and name: " + snapshot.getName() + " requested for volume with ID: " + volumeVO.getId() + " and name: " + volumeVO.getName())
It's just an example but I feel this could give us more outlook on whats happening.

CreateCmdResult result;

try {
VolumeInfo volumeInfo = snapshot.getBaseVolume();

VolumeVO volumeVO = volumeDao.findById(volumeInfo.getId());
if(volumeVO == null) {
throw new CloudRuntimeException("takeSnapshot: VolumeVO not found for id: " + volumeInfo.getId());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please don't add method names in exceptions, as we don't have any cleanup framework before showing them on UI

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.

I understand. I was watching such instances, some of which are still overlooked.

}

/** we are keeping file path at volumeVO.getPath() */

StoragePoolVO storagePool = storagePoolDao.findById(volumeVO.getPoolId());
if(storagePool == null) {
s_logger.error("takeSnapshot : Storage Pool not found for id: " + volumeVO.getPoolId());
throw new CloudRuntimeException("takeSnapshot : Storage Pool not found for id: " + volumeVO.getPoolId());
}
Map<String, String> poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(volumeVO.getPoolId());
StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(poolDetails);

CloudStackVolume cloudStackVolume = null;
long usedBytes = getUsedBytes(storagePool);
long capacityBytes = storagePool.getCapacityBytes();

// Only proceed for NFS3 protocol
if (ProtocolType.NFS3.name().equalsIgnoreCase(poolDetails.get(Constants.PROTOCOL))) {
Comment thread
sandeeplocharla marked this conversation as resolved.
Outdated
Map<String, String> cloudStackVolumeRequestMap = new HashMap<>();
cloudStackVolumeRequestMap.put(Constants.VOLUME_UUID, poolDetails.get(Constants.VOLUME_UUID));
cloudStackVolumeRequestMap.put(Constants.FILE_PATH, volumeVO.getPath());
cloudStackVolume = storageStrategy.getCloudStackVolume(cloudStackVolumeRequestMap);
if (cloudStackVolume == null || cloudStackVolume.getFile() == null) {
throw new CloudRuntimeException("takeSnapshot: Failed to get source file to take snapshot");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should we have method names only in loggers and not have them in exceptions? Management server might show this as an error on the UI?

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.

acknowledged

}
s_logger.info("takeSnapshot : entered after getting cloudstack volume with file path: " + cloudStackVolume.getFile().getPath() + " and size: " + cloudStackVolume.getFile().getSize());
long fileSize = cloudStackVolume.getFile().getSize();
usedBytes += fileSize;
}
Comment thread
piyush5netapp marked this conversation as resolved.



if (usedBytes > capacityBytes) {
throw new CloudRuntimeException("Insufficient space remains in this primary storage to take a snapshot");
}

storagePool.setUsedBytes(usedBytes);

SnapshotObjectTO snapshotObjectTo = (SnapshotObjectTO)snapshot.getTO();

String snapshotName = volumeInfo.getName() + "-" + snapshot.getUuid();

int maxSnapshotNameLength = 64;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think, we can have more character in snapshot name

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.

moved it to contants for now, will update the length by checking it, if required.

int trimRequired = snapshotName.length() - maxSnapshotNameLength;

if (trimRequired > 0) {
snapshotName = StringUtils.left(volumeInfo.getName(), (volumeInfo.getName().length() - trimRequired)) + "-" + snapshot.getUuid();
}

CloudStackVolume snapCloudStackVolumeRequest = snapshotCloudStackVolumeRequestByProtocol(poolDetails, volumeVO.getPath(), snapshotName);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As per our recent discussion, maybe all of this code has to be offloaded to protocol specific strategy class

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.

we already have it below while calling actual snapshotCloudStackVolume() method. Request preparation we kept here for all the strategy calls.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Part of the discussion was to not create any request calls in driver class, push everything necessary to strategy classes and create the request necessary in the respective strategy class itself. BTW, the AI is on me to move the iSCSI specific request creation also into the respective strategy class. Will plan to pick it up soon but see if you can start this effort with snapshots?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As per the implementation, I understand that we intend to follow the current way of handling the code. In which ONTAP specific protocol related changes would be in strategy classes and the cloudstack specific classes such as Driver class, would handle the cloudstack specific code.

CloudStackVolume cloneCloudStackVolume = storageStrategy.snapshotCloudStackVolume(snapCloudStackVolumeRequest);

updateSnapshotDetails(snapshot.getId(), volumeInfo.getId(), poolDetails.get(Constants.VOLUME_UUID), cloneCloudStackVolume.getFile().getPath(), volumeVO.getPoolId(), fileSize);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If iSCSI needs different things to be updated in DB, we might get an if-else on protocol here also.

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.

yes, we have to handle that with iscsi protocol.


snapshotObjectTo.setPath(Constants.ONTAP_SNAP_ID +"="+cloneCloudStackVolume.getFile().getPath());
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is "=" intentional ?


/** Update size for the storage-pool including snapshot size */
storagePoolDao.update(volumeVO.getPoolId(), storagePool);

CreateObjectAnswer createObjectAnswer = new CreateObjectAnswer(snapshotObjectTo);

result = new CreateCmdResult(null, createObjectAnswer);

result.setResult(null);
}
catch (Exception ex) {
s_logger.error("takeSnapshot: Failed due to ", ex);
result = new CreateCmdResult(null, new CreateObjectAnswer(ex.toString()));

result.setResult(ex.toString());
}

callback.complete(result);
}

@Override
Expand Down Expand Up @@ -622,4 +711,87 @@ private CloudStackVolume createDeleteCloudStackVolumeRequest(StoragePool storage
return cloudStackVolumeDeleteRequest;

}

private CloudStackVolume getCloudStackVolumeRequestByProtocol(Map<String, String> details, String filePath) {
CloudStackVolume cloudStackVolumeRequest = null;
ProtocolType protocolType = null;
String protocol = null;

try {
protocol = details.get(Constants.PROTOCOL);
protocolType = ProtocolType.valueOf(protocol);
} catch (IllegalArgumentException e) {
throw new CloudRuntimeException("getCloudStackVolumeRequestByProtocol: Protocol: "+ protocol +" is not valid");
}
switch (protocolType) {
case NFS3:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Even this also should be in the respective strategy class itself?

cloudStackVolumeRequest = new CloudStackVolume();
FileInfo fileInfo = new FileInfo();
fileInfo.setPath(filePath);
cloudStackVolumeRequest.setFile(fileInfo);
String volumeUuid = details.get(Constants.VOLUME_UUID);
cloudStackVolumeRequest.setFlexVolumeUuid(volumeUuid);
break;
default:
throw new CloudRuntimeException("createCloudStackVolumeRequestByProtocol: Unsupported protocol " + protocol);
}
return cloudStackVolumeRequest;
}

private CloudStackVolume snapshotCloudStackVolumeRequestByProtocol(Map<String, String> details,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this method is not used anywhere

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.

I am calling this method in takeSnapshot()

String sourcePath,
String destinationPath) {
CloudStackVolume cloudStackVolumeRequest = null;
ProtocolType protocolType = null;
String protocol = null;

try {
protocol = details.get(Constants.PROTOCOL);
protocolType = ProtocolType.valueOf(protocol);
} catch (IllegalArgumentException e) {
throw new CloudRuntimeException("getCloudStackVolumeRequestByProtocol: Protocol: "+ protocol +" is not valid");
}
switch (protocolType) {
case NFS3:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same here

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.

As mentioned above, this was addressed elaborately in our discussion.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please take a relook? This was also planned to be pushed onto the strategy classes to be dealt with according to the requirement

cloudStackVolumeRequest = new CloudStackVolume();
FileInfo fileInfo = new FileInfo();
fileInfo.setPath(sourcePath);
cloudStackVolumeRequest.setFile(fileInfo);
String volumeUuid = details.get(Constants.VOLUME_UUID);
cloudStackVolumeRequest.setFlexVolumeUuid(volumeUuid);
cloudStackVolumeRequest.setDestinationPath(destinationPath);
break;
default:
throw new CloudRuntimeException("createCloudStackVolumeRequestByProtocol: Unsupported protocol " + protocol);

}
return cloudStackVolumeRequest;
}

/**
*
* @param csSnapshotId: generated snapshot id from cloudstack
* @param csVolumeId: Source CS volume id
* @param ontapVolumeUuid: storage flexvolume id
* @param ontapNewSnapshot: generated snapshot id from ONTAP
* @param storagePoolId: primary storage pool id
* @param ontapSnapSize: Size of snapshot CS volume(LUN/file)
*/
private void updateSnapshotDetails(long csSnapshotId, long csVolumeId, String ontapVolumeUuid, String ontapNewSnapshot, long storagePoolId, long ontapSnapSize) {
Comment thread
piyush5netapp marked this conversation as resolved.
Outdated
SnapshotDetailsVO snapshotDetail = new SnapshotDetailsVO(csSnapshotId, Constants.SRC_CS_VOLUME_ID, String.valueOf(csVolumeId), false);
snapshotDetailsDao.persist(snapshotDetail);

snapshotDetail = new SnapshotDetailsVO(csSnapshotId, Constants.BASE_ONTAP_FV_ID, String.valueOf(ontapVolumeUuid), false);
snapshotDetailsDao.persist(snapshotDetail);

snapshotDetail = new SnapshotDetailsVO(csSnapshotId, Constants.ONTAP_SNAP_ID, String.valueOf(ontapNewSnapshot), false);
snapshotDetailsDao.persist(snapshotDetail);

snapshotDetail = new SnapshotDetailsVO(csSnapshotId, Constants.PRIMARY_POOL_ID, String.valueOf(storagePoolId), false);
snapshotDetailsDao.persist(snapshotDetail);

snapshotDetail = new SnapshotDetailsVO(csSnapshotId, Constants.ONTAP_SNAP_SIZE, String.valueOf(ontapSnapSize), false);
snapshotDetailsDao.persist(snapshotDetail);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@

import feign.QueryMap;
import org.apache.cloudstack.storage.feign.model.ExportPolicy;
import org.apache.cloudstack.storage.feign.model.FileClone;
import org.apache.cloudstack.storage.feign.model.FileInfo;
import org.apache.cloudstack.storage.feign.model.response.JobResponse;
import org.apache.cloudstack.storage.feign.model.response.OntapResponse;
import feign.Headers;
import feign.Param;
Expand All @@ -32,7 +34,7 @@
public interface NASFeignClient {

// File Operations
@RequestLine("GET /api/storage/volumes/{volumeUuid}/files/{path}")
@RequestLine("GET /api/storage/volumes/{volumeUuid}/files/{path}?return_metadata=true")
@Headers({"Authorization: {authHeader}"})
OntapResponse<FileInfo> getFileResponse(@Param("authHeader") String authHeader,
@Param("volumeUuid") String volumeUUID,
Expand All @@ -58,6 +60,11 @@ void createFile(@Param("authHeader") String authHeader,
@Param("path") String filePath,
FileInfo file);

@RequestLine("POST /api/storage/file/clone")
@Headers({"Authorization: {authHeader}"})
JobResponse cloneFile(@Param("authHeader") String authHeader,
FileClone fileClone);

// Export Policy Operations
@RequestLine("POST /api/protocols/nfs/export-policies")
@Headers({"Authorization: {authHeader}"})
Expand Down
Loading
Loading