GutoVeronezi commented on a change in pull request #5349:
URL: https://github.com/apache/cloudstack/pull/5349#discussion_r694188928
##########
File path:
plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java
##########
@@ -287,20 +288,45 @@ public void copyAsync(DataObject srcdata, DataObject
destData, AsyncCompletionCa
}
CopyCommandResult result = new CopyCommandResult("", answer);
callback.complete(result);
+ } else if (srcdata.getType() == DataObjectType.SNAPSHOT &&
destData.getType() == DataObjectType.VOLUME) {
+ SnapshotObjectTO srcTO = (SnapshotObjectTO) srcdata.getTO();
+ CopyCommand cmd = new CopyCommand(srcTO, destData.getTO(),
StorageManager.PRIMARY_STORAGE_DOWNLOAD_WAIT.value(), true);
+ EndPoint ep = epSelector.select(srcdata, destData);
+ CopyCmdAnswer answer = null;
+ if (ep == null) {
+ String errMsg = "No remote endpoint to send command, check
if host or ssvm is down?";
+ s_logger.error(errMsg);
+ answer = new CopyCmdAnswer(errMsg);
+ } else {
+ answer = (CopyCmdAnswer) ep.sendMessage(cmd);
+ }
+ CopyCommandResult result = new CopyCommandResult("", answer);
+ callback.complete(result);
}
}
}
@Override
public boolean canCopy(DataObject srcData, DataObject destData) {
//BUG fix for CLOUDSTACK-4618
- DataStore store = destData.getDataStore();
- if (store.getRole() == DataStoreRole.Primary && srcData.getType() ==
DataObjectType.TEMPLATE
+ DataStore destStore = destData.getDataStore();
+ if (destStore.getRole() == DataStoreRole.Primary && srcData.getType()
== DataObjectType.TEMPLATE
&& (destData.getType() == DataObjectType.TEMPLATE ||
destData.getType() == DataObjectType.VOLUME)) {
- StoragePoolVO storagePoolVO =
primaryStoreDao.findById(store.getId());
+ StoragePoolVO storagePoolVO =
primaryStoreDao.findById(destStore.getId());
if (storagePoolVO != null && storagePoolVO.getPoolType() ==
Storage.StoragePoolType.CLVM) {
return true;
}
+ } else if (srcData.getType() == DataObjectType.SNAPSHOT &&
destData.getType() == DataObjectType.VOLUME) {
+ DataStore srcStore = srcData.getDataStore();
+ if (srcStore.getRole() == DataStoreRole.Primary &&
destStore.getRole() == DataStoreRole.Primary) {
+ StoragePoolVO srcStoragePoolVO =
primaryStoreDao.findById(srcStore.getId());
+ StoragePoolVO dstStoragePoolVO =
primaryStoreDao.findById(destStore.getId());
+ if (srcStoragePoolVO != null && srcStoragePoolVO.getPoolType()
== StoragePoolType.RBD
Review comment:
I think that what @DaanHoogland meant is:
```suggestion
if (srcStoragePoolVO != null &&
StoragePoolType.RBD.equals(srcStoragePoolVO.getPoolType())
```
Technically there are no differences in comparing `enums` with `equals` or
`==`, because `Enum#equals` implements `==`:
```java
public final boolean equals(Object other) {
return this==other;
}
```
As far as I know, there is no consensus about it, however I would go with
Daan and use `equals` to compare `enums`, as any other object we compare, and
let `==` only to primitive types.
##########
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -1639,6 +1673,74 @@ public Answer createVolumeFromSnapshot(final CopyCommand
cmd) {
}
}
+ private KVMPhysicalDisk createRBDvolumeFromRBDSnapshot(KVMPhysicalDisk
volume, String snapshotName, String name,
+ PhysicalDiskFormat format, long size, KVMStoragePool destPool, int
timeout) {
+
+ KVMStoragePool srcPool = volume.getPool();
+ KVMPhysicalDisk disk = null;
+ String newUuid = name;
+ int rbdFeatures = 61;
+
+ format = PhysicalDiskFormat.RAW;
+ disk = new KVMPhysicalDisk(destPool.getSourceDir() + "/" + newUuid,
newUuid, destPool);
+ disk.setFormat(format);
+ if (size > volume.getVirtualSize()) {
+ disk.setSize(size);
+ disk.setVirtualSize(size);
+ } else {
+ disk.setSize(volume.getVirtualSize());
+ disk.setVirtualSize(disk.getSize());
+ }
+
+ try {
+
+ Rados r = new Rados(srcPool.getAuthUserName());
+ r.confSet("mon_host", srcPool.getSourceHost() + ":" +
srcPool.getSourcePort());
+ r.confSet("key", srcPool.getAuthSecret());
+ r.confSet("client_mount_timeout", "30");
+ r.connect();
+
+ IoCTX io = r.ioCtxCreate(srcPool.getSourceDir());
+ Rbd rbd = new Rbd(io);
+ RbdImage srcImage = rbd.open(volume.getName());
+
+ List<RbdSnapInfo> snaps = srcImage.snapList();
+ boolean snapFound = false;
+ for (RbdSnapInfo snap : snaps) {
+ if (snapshotName.equals(snap.name)) {
+ snapFound = true;
+ break;
+ }
+ }
+
+ if (!snapFound) {
+ return null;
+ }
+ srcImage.snapProtect(snapshotName);
+
+ rbd.clone(volume.getName(), snapshotName, io, disk.getName(),
rbdFeatures, 0);
+ RbdImage diskImage = rbd.open(disk.getName());
+ if (disk.getVirtualSize() > volume.getVirtualSize()) {
+ diskImage.resize(disk.getVirtualSize());
+ }
+
+ diskImage.flatten();
+ rbd.close(diskImage);
+
+ srcImage.snapUnprotect(snapshotName);
+ rbd.close(srcImage);
+ r.ioCtxDestroy(io);
+ } catch (RadosException e) {
+ s_logger.error(String.format("Failed due to %s", e.getMessage()));
+ disk = null;
+ } catch (RbdException e) {
+ s_logger.error(String.format("Failed due to: %s", e.getMessage()));
+ disk = null;
+ }
Review comment:
We could join the catches and add the exception as to the log:
```suggestion
} catch (RbdException | RadosException e) {
s_logger.error(String.format("Failed due to %s",
e.getMessage()), e);
disk = null;
}
```
--
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]
