XFS cannot have nosuid when mounting

[baude]

Atomic/mount.py:
    Under certain conditions, nosuid was still being
    injected into the mount options.

[baude]

@rhatdan Assuming this passes checks (ill check in the morning), can you review my use of while conditions to "wait" on on the thinpool device to activate? I'd like your opinion. I'm also not super fond of using a sleep condition to buy time, but I also thought the one while loop should have an "out" in case the condition is never met. Do you think both should as well?

[rhatdan]

@rjvgoyal is there a better way.

@baude Looks like you got a chance at an Infinite loop here.

[baude]

@rhatdan When you mount with thinpool, if the filesystem is XFS, you cannot use nosuid (it must use nouuid). After talking with @rhvgoyal, he confirmed that and that it should be nouuid when using XFS and nothing for EXT4.

[baude]

@rhatdan Updated per our conversation today.

[rhvgoyal]

@baude I talked to few folks here who know much more about dmsetup and they said that --addnodeoncreate is for systems not having udev.

they think that by default by the time "dmsetup create --table<>" returned, udev must have settled down.

So please remove this and if problem is still happening, we need to debug why you are getting EBUSY during remove.

[rhvgoyal]

@baude

One more thing. "dmsetup remove" seems to support option --retry. Which will try device removal if it fails first time.

Following is from "man dmsetup".

If an attempt to remove a device fails, perhaps because a process run from a quick udev rule temporarily opened the device, the --retry option will cause the operation to be retried for a few seconds before failing.

May be give it a try and see if this solves the issue you are facing.

[baude]

Updated to include --retry on the dmsetup remove per @rhvgoyal

[baude]

Re-added --nosuid after conversation with @rhvgoyal; so now XFS mounts will have both 'nouuid' and 'nosuid' and we retain the --retry option.

[rhatdan]

I only see --retry?

[rhvgoyal]

nosuid and nouuid can coexist. So we really don't seem to have a need to remove nosuid. If you want to remove it explicitly for some other reason, then that can be done in separate PR. But that does not seem to be a requirement atleast for the failure baude was seeing.

[baude]

@rhatdan yes, the agreement yesterday was to only use --retry. The nouuid stuff is already in there from previous commits.

[rhatdan]

Ok, LGTM.

I think having NOSUID is good. We don't want new file systems with SUID binaries showing up on the host where unpriv processes might be able to take advantage.