lge commented on PR #8887:
URL: https://github.com/apache/cloudstack/pull/8887#issuecomment-2076650461
Would it be acceptable to "mark" a mount point as such, and make it
immutable?
The goal is to make it impossible to "accidentally" create stuff inside the
mount point that should have been created inside the supposedly mounted file
system.
The idea is to place an indicator file into the mount point directory (not
the mounted file system), remove all permissions from the mount point (that
will be overridden by the actual mount, once that happens), and make it
immutable just in case (because root tends to ignore permissions).
If you like it, it should probably be done more generically for all `mkdir`d
mount points, and not only for libvirt nfs storage pools, but I don't know my
way around cloud stack code good enough yet to find the "proper" location for
this.
Suggestion below. I'm not Java native, so consider this "pseudo code" if it
is not sufficiently java-ish...
```java
diff --git
a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
index 3002fea41d..0afc232828 100644
---
a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
+++
b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
@@ -32,6 +32,7 @@ import org.apache.cloudstack.utils.qemu.QemuImgException;
import org.apache.cloudstack.utils.qemu.QemuImgFile;
import org.apache.cloudstack.utils.qemu.QemuObject;
import org.apache.commons.codec.binary.Base64;
+import org.apache.commons.lang.SystemUtils;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
import org.libvirt.Connect;
@@ -276,7 +277,29 @@ public class LibvirtStorageAdaptor implements
StorageAdaptor {
private StoragePool createNetfsStoragePool(PoolType fsType, Connect
conn, String uuid, String host, String path) throws LibvirtException {
String targetPath = _mountPoint + File.separator + uuid;
LibvirtStoragePoolDef spd = new LibvirtStoragePoolDef(fsType, uuid,
uuid, host, path, targetPath);
- _storageLayer.mkdir(targetPath);
+ if (SystemUtils.IS_OS_LINUX) {
+ // targetPath better not contain funny characters!
+ // maybe s/'/'\''/g first?
+ int createMntPointResult =
Script.runSimpleBashScriptForExitValue(
+ // That is a directory already? Good enough.
+ "! test -L '" + targetPath + "' && test -d '" +
targetPath + "' && exit 0;" +
+ // we really want that mount point
+ "mkdir -p '" + targetPath + "' || exit;" +
+ // optionally /try/ to mark it as mount point and
make it immutable,
+ // so we won't "accidentally" create stuff inside
the mount point
+ // that should have been created in the mounted
file system
+ "touch '" + targetPath +
"/__MOUNT_POINT_ONLY__'; " +
+ "chmod 0 '" + targetPath + "'; " +
+ "chattr +i '" + targetPath + "'; " +
+ // mkdir was successful: propagate exit code 0
+ "exit 0");
+ if (createMntPointResult != 0) {
+ logger.error("Unable to create mount point " +
targetPath);
+ // What now? Just let Libvirt try itself and fail below.
+ }
+ } else {
+ _storageLayer.mkdir(targetPath);
+ }
StoragePool sp = null;
try {
logger.debug(spd.toString());
```
Is libvirt on non IS_OS_LINUX even a thing?
Possibly check for existence of the directory in a java native way first, or
use JNA, if calling the shell looks too heavy.
Bonus points to avoid the race between the first one to "mkdir", someone
else winning the race to mount some file system on that mount point, and then
the `chattr +i` happening on the top level directory of the mounted file system
instead of the mount point ;-)
--
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]
