weizhouapache commented on code in PR #13053:
URL: https://github.com/apache/cloudstack/pull/13053#discussion_r3403449246
##########
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java:
##########
@@ -325,8 +326,13 @@ public StrategyPriority canHandle(Long vmId, Long
rootPoolId, boolean snapshotMe
List<VolumeVO> volumes = volumeDao.findByInstance(vmId);
for (VolumeVO volume : volumes) {
StoragePoolVO storagePoolVO =
storagePool.findById(volume.getPoolId());
+ if (storagePoolVO.isManaged() &&
DataStoreProvider.ONTAP_PLUGIN_NAME.equals(storagePoolVO.getStorageProviderName()))
{
Review Comment:
@rajiv-jain-netapp
This check is used in several places, so it would be good to extract it into
a separate method.
I also noticed that `NetworkFileSystem` or `Iscsi` storage pools are created
(#12563). However, as you've pointed out, CloudStack supports many operations
on traditional NFS storage, and some of those operations are not supported by
NetApp ONTAP.
Have you considered introducing a new storage pool type, for example
`NetAppNFS` or `NetAppIscsi` ? That might make it easier to distinguish the
capabilities and handle NetApp-specific behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
