slavkap commented on code in PR #14389:
URL: https://github.com/apache/cloudstack/pull/14389#discussion_r4247503994
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVolumeCommandWrapper.java:
##########
@@ -51,7 +51,15 @@ public final class LibvirtCheckVolumeCommandWrapper extends
CommandWrapper<Check
Storage.StoragePoolType.Filesystem,
Storage.StoragePoolType.NetworkFilesystem,
Storage.StoragePoolType.SharedMountPoint,
- Storage.StoragePoolType.RBD);
+ Storage.StoragePoolType.RBD,
+ Storage.StoragePoolType.StorPool);
+
+ private static final List<Storage.StoragePoolType>
RAW_BLOCK_STORAGE_POOL_TYPES = Arrays.asList(
+ Storage.StoragePoolType.RBD,
+ Storage.StoragePoolType.StorPool);
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]
