slavkap commented on code in PR #14389:
URL: https://github.com/apache/cloudstack/pull/14389#discussion_r4247499419
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVolumeCommandWrapper.java:
##########
@@ -63,27 +71,37 @@ public Answer execute(final CheckVolumeCommand command,
final LibvirtComputingRe
try {
if
(STORAGE_POOL_TYPES_SUPPORTED.contains(storageFilerTO.getType())) {
- final KVMPhysicalDisk vol = pool.getPhysicalDisk(srcFile);
- final String path = vol.getPath();
- final boolean isRbd =
Storage.StoragePoolType.RBD.equals(storageFilerTO.getType());
-
- Map<VolumeOnStorageTO.Detail, String> volumeDetails =
getVolumeDetails(pool, vol);
- if (MapUtils.isEmpty(volumeDetails)) {
- return new Answer(command, false, "Unable to read the
volume on the storage pool");
+ final boolean shouldAttachTheVolume =
POOL_NEEDS_TO_EXPOSE_THE_VOLUME.contains(storageFilerTO.getType());
+ if (shouldAttachTheVolume &&
!pool.connectPhysicalDisk(srcFile, new HashMap<>())) {
+ return new Answer(command, false, "Unable to attach the
volume on the storage pool");
}
+ try {
+ final KVMPhysicalDisk vol = pool.getPhysicalDisk(srcFile);
+ final String path = vol.getPath();
+ final boolean isRawBlockPool =
RAW_BLOCK_STORAGE_POOL_TYPES.contains(storageFilerTO.getType());
+
+ Map<VolumeOnStorageTO.Detail, String> volumeDetails =
getVolumeDetails(pool, vol);
+ if (MapUtils.isEmpty(volumeDetails)) {
+ return new Answer(command, false, "Unable to read the
volume on the storage pool");
+ }
- if (!isRbd) {
- try {
- KVMPhysicalDisk.checkQcow2File(path);
- } catch (final CloudRuntimeException e) {
- return new CheckVolumeAnswer(command, false, "", 0,
volumeDetails);
+ if (!isRawBlockPool) {
+ try {
+ KVMPhysicalDisk.checkQcow2File(path);
+ } catch (final CloudRuntimeException e) {
+ return new CheckVolumeAnswer(command, false, "",
0, volumeDetails);
+ }
}
- }
- // Images on RBD are raw and the path is an image name that
qemu-img cannot open
- // without the rbd: URI, so take the size libvirt already
reported for the volume.
- long size = isRbd ? vol.getVirtualSize() :
KVMPhysicalDisk.getVirtualSizeFromFile(path);
- return new CheckVolumeAnswer(command, true, "", size,
volumeDetails);
+ // Images on RBD are raw and the path is an image name
that qemu-img cannot open
+ // without the rbd: URI, so take the size libvirt already
reported for the volume
+ long size = isRawBlockPool ? vol.getVirtualSize() :
KVMPhysicalDisk.getVirtualSizeFromFile(path);
Review Comment:
This is intentional, not inconsistent. `format=QCOW2` in CloudStack's DB
for StorPool volumes is a long-standing labeling convention (see
`StorPoolDataMotionStrategy.setFormat(ImageFormat.QCOW2)`, which predates this
PR) — it doesn't mean the bytes on the StorPool block device are actually
structured as a QCOW2 container. On disk, a StorPool volume is a raw block
device, same as RBD, so grouping it with RBD in `RAW_BLOCK_STORAGE_POOL_TYPES`
(skip QCOW2 structural validation, read size via the libvirt-reported virtual
size) is correct. Also worth noting checkQcow2File() is a no-op for any path
without a .qcow2 extension, so it wouldn't have done anything differently for
StorPool paths anyway. We'll leave this as is.
--
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]
