Mount xfs filesystem with pquota option

[mbarnes]

When transitioning from devicemapper to overlay2, we lose the ability to specify a disk quota for all containers (dm.basesize).

~~The closest analogue I've found for overlay2 is this feature, which allows a disk quota to be set per-container. e.g. docker run --storage-opt size=3g -it fedora /bin/bash~~

Update: I've since discovered this PR that got merged in June -- Add overlay2.size daemon storage-opt -- which allows disk quota to be specified in the container-storage-setup config:

EXTRA_STORAGE_OPTIONS="--storage-opt overlay2.size=3G"

--

However, it requires the XFS filesystem to be mounted with project disk quota accounting enabled (mount -o pquota). container-storage-setup does not currently do this.

This 1st attempt is a simple but naive solution. From what I can tell, it seems safe to assume the filesystem being mounted is XFS. What I'm not sure about is whether there's a down side to just always enabling pquota.

An alternate solution might be to introduce a new mount option variable for /etc/sysconfig/docker-storage-setup. Perhaps:

CONTAINER_ROOT_LV_MOUNT_OPTIONS=pquota

[rhatdan]

@rhvgoyal WDYT? If it does not effect anything, I would rather just do it by default.

Should we also do this by default in CRI-O?

[rhvgoyal]

@rhatdan, I will check with file system team if there is a downside of enabling pquota by default. If penalty is not significant, then enabling it by default makes sense to me.

[rhvgoyal]

@mbarnes does this patch really work for you? I think mount_extra_volume() is used in non-compatibility mode. That's new set of commands and are not used by docker by default.

compatibility mode uses a systemd mount unit file to mount extra volume and that will require changes to make sure systemd mounts extra volume with option "pquota".

[mbarnes]

@rhvgoyal : Hmm, that's a good point.

The patch works after running atomic storage modify --driver=overlay2.

But, indeed, the mount option does not survive a reboot because of the compatibility mode approach. I guess we need both. I'll rework the patch.

[mbarnes]

:arrow_up: Rebased the patch, since it's only a two-liner.

Tested as follows:

# atomic storage reset
# atomic storage modify --driver=overlay2 --lvname=docker-root-lv --rootfs=/var/lib/docker
# mount | grep docker
/dev/mapper/docker_vg-docker--root--lv on /var/lib/docker type xfs (rw,relatime,seclabel,attr2,inode64,prjquota)
                                                                                                       ^^^^^^^^
# systemctl reboot

... reboots ...

# mount | grep docker
/dev/mapper/docker_vg-docker--root--lv on /var/lib/docker type xfs (rw,relatime,seclabel,attr2,inode64,prjquota)
                                                                                                       ^^^^^^^^

However, I notice in the code the modified unit file (with Options=pquota) will not be generated if the unit file already exists. So some users may get stuck with the noquota default. Maybe the unit file should just always get replaced on boot, to make sure users pick up changes like this?

[rhvgoyal]

That will be the case only for upgrade only. Yes we don't have a way to enforce this over upgrade. Fresh installations will work just fine.

[rhvgoyal]

Or setting up new storage after reset will work fine too.

[rhvgoyal]

LGTM

[rhvgoyal]

@rh-atomic-bot r+ 368e288

[rh-atomic-bot]

:hourglass: Testing commit 368e288 with merge 0baaa99...

[rh-atomic-bot]

:sunny: Test successful - status-redhatci Approved by: rhvgoyal Pushing 0baaa99d1b5b313c1939c3faccb50182600d3273 to master...

[rhatdan]

Time to create a new package?

[rhvgoyal]

yep, time for new build.