Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add temporary mount point properties #985

Closed
foobarz opened this issue Sep 24, 2012 · 10 comments
Closed

Add temporary mount point properties #985

foobarz opened this issue Sep 24, 2012 · 10 comments
Milestone

Comments

@foobarz
Copy link

foobarz commented Sep 24, 2012

I have created a root pool, called "zfs-root" that my system is installed on and boots onto. zfs-root is using mountpoint=legacy and readonly=on. zfs-root pool is used directly as the root (/) filesystem.

When the system boots, zfs-root is mounted by default as readonly in the initramfs per the readonly=on property as expected and desired. When the system /sbin/init program runs on zfs-root, it remounts / to rw. This works fine. The zfs-root readonly property then has the value "off" (temporary).

Now, I created a zfs filesystem to use as maybe a virtual machine container, or user directory or whatever:

zfs create zfs-root/vm1

zfs-root/vm1 inherits mountpoint=legacy and readonly=on from zfs-root.

Then I make a directory as mountpoint:

mkdir /root/vm1

Now, I mount it:

mount -t zfs -o rw "zfs-root/vm1" /root/vm1

I just told mount to mount it read/write with "rw", but this what I get:

cat /proc/mounts
zfs-root/vm1 /root/vm1 zfs ro,relatime,xattr 0 0
cat /etc/mtab
zfs-root/vm1 /root/vm1 zfs rw 0 0

The kernel has actually mounted it ro, and I cannot write to it. mtab says rw, the option I gave to mount. Yet, it is not really rw. Information in mtab is wrong, but is what I asked for. This seems like a real bug: 1) I mounted for rw yet get ro. 2) kernel /proc/mount and userspace /etc/mtab do not agree.

I can remount:

mount -o remount,rw /root/vm1
cat /proc/mounts
zfs-root/vm1 /root/vm1 zfs rw,relatime,xattr 0 0
cat /etc/mtab
zfs-root/vm1 /root/vm1 zfs rw 0 0

This makes it rw, as originally requested from the first mount command.

Either this is a bug with how mountpoint=legacy and readonly=on are interacting, or it is a usage error on my part to set readonly=on while using mountpoint=legacy. I didn't see documentation about this conflict but maybe that is my error too. I am guessing that if one is using mountpoint=legacy, readonly=off (the default) should also be used. This could be documented for a dummy like me, or even take my possible suggestion: if mountpoint=legacy, then also readonly=legacy... a new value for readonly=property so they are consistent with each other. If mountpoint=legacy, readonly property gets locked to "legacy" also so you cannot change it really and must use "mount" command ro and rw options accordingly.

@behlendorf
Copy link
Contributor

I believe this is because we haven't implemented temporary mount point properties. This work might happen for 0.6.3 because properly supporting them would allow us to more easily integrate with the full xfstests framework.

From the zfs(8) man page:

   Temporary Mount Point Properties
       When a file system is  mounted,  either  through  mount(8)  for  legacy
       mounts  or  the  zfs  mount  command for normal file systems, its mount
       options are set according to its properties.  The  correlation  between
       properties and mount options is as follows:

             PROPERTY                MOUNT OPTION
              devices                 devices/nodevices
              exec                    exec/noexec
              readonly                ro/rw
              setuid                  setuid/nosetuid
              xattr                   xattr/noxattr

       In addition, these options can be set on a per-mount basis using the -o
       option, without affecting the property that is stored on disk. The val-
       ues  specified  on  the  command line override the values stored in the
       dataset. The -nosuid option is an alias for  nodevices,nosetuid.  These
       properties  are  reported as "temporary" by the zfs get command. If the
       properties are changed while the dataset is mounted,  the  new  setting
       overrides any temporary settings.

behlendorf referenced this issue in renelson/zfs Dec 14, 2013
	case is handled.

2). In both files, the header and the C code, these options are now
	accepted as passed through by mount in util-linux-2.24 when it execs mount.zfs:

	acl
	noacl
	posixacl
@behlendorf behlendorf removed this from the 0.6.4 milestone Oct 6, 2014
@behlendorf behlendorf added Bug - Minor and removed Type: Documentation Indicates a requested change to the documentation labels Oct 6, 2014
behlendorf added a commit to behlendorf/zfs that referenced this issue Sep 1, 2015
Add the required kernel side infrastructure to parse arbitrary
mount options.  This enables us to support temporary mount
options is largely the same way it's handled on other platforms.

See the 'Temporary Mount Point Properties' section of zfs(8)
for complete details.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#985
Closes openzfs#3351
behlendorf added a commit to behlendorf/zfs that referenced this issue Sep 1, 2015
Add the required kernel side infrastructure to parse arbitrary
mount options.  This enables us to support temporary mount
options in largely the same way it is handled on other platforms.

See the 'Temporary Mount Point Properties' section of zfs(8)
for complete details.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#985
Closes openzfs#3351
behlendorf added a commit to behlendorf/zfs that referenced this issue Sep 1, 2015
Add the required kernel side infrastructure to parse arbitrary
mount options.  This enables us to support temporary mount
options in largely the same way it is handled on other platforms.

See the 'Temporary Mount Point Properties' section of zfs(8)
for complete details.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#985
Closes openzfs#3351
@behlendorf behlendorf added this to the 0.6.5 milestone Sep 3, 2015
tomgarcia pushed a commit to tomgarcia/zfs that referenced this issue Sep 11, 2015
Add the required kernel side infrastructure to parse arbitrary
mount options.  This enables us to support temporary mount
options in largely the same way it is handled on other platforms.

See the 'Temporary Mount Point Properties' section of zfs(8)
for complete details.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#985
Closes openzfs#3351
@TemptorSent
Copy link

Looking at the code, it appears temporary mountpoint support was penciled in to this code MNTOPT_MNTPOINT, but its implementation is incomplete. Is there any reason this functionality can't be fleshed out and made functional based on what's there?

@behlendorf
Copy link
Contributor

behlendorf commented Mar 7, 2018

@TemptorSent could you be more specific? In what way is it currently incomplete?

@TemptorSent
Copy link

@behlendorf - As far as I can find, there is no means of setting the 'mountpoint' property on a temporary basis, as also mentioned in #4553.

The only place I see MNTOPT_MNTPOINT ("mntpoint") used currently is a 'hint' for snapshots. I found no mention of ZPROP_SRC_TEMPORARY in combination with the mountpoint property, so as far as I can tell, only the ZFS_PROP_{ATIME,RELATIME,DEVICES,EXEC,READONLY,SUID,XATTR,NBMAND} are actually handled as temporary properties.

Ideally, the mountpoint property should be able to be set temporarily at mount time AND be independent from the target mountpoint of the mount.zfs command. This would allow proper mountpoint inheritance for child datasets to work when mounting the parent in a temporary location, as well as properly supporting pivot_root from /sysroot to / without breaking subsequent automounting, such as
mount -t zfs -o zfsutil,mountpoint=/mnt/newsys rpool/ROOT/newsys /mnt/newsys for a chroot, but
mount -t zfs -o zfsutil,mountpoint=/ rpool/ROOT/mysys /sysroot ; ... ; cd /sysroot ; pivot_root . initroot ; exec chroot . ... for initramfs.

Allowing temporary override of the mountpoint using zfs mount -o mountpoint=/mnt/oldsys rpool/ROOT/oldsys would also be desirable and I believe implementation would follow logically using the added capability in mount.zfs.

@behlendorf
Copy link
Contributor

@TemptorSent I see where you're going with this. The short answer is that there's no technical reason this couldn't be added, it simply hasn't been done. I agree it would definitely be nice to have, we just need a developer to implement it.

@TemptorSent
Copy link

@behlendorf - Excellent, thank you. I wanted to make sure there was no technical reason not to proceed with implementing it before wasting my time :) While I'm at it, should support for other temporary properties be implemented as well?

@behlendorf
Copy link
Contributor

Sounds good, thanks! While you're here I think adding support for ZFS_PROP_CANMOUNT in addition to ZFS_PROP_MOUNTPOINT would be useful. We have several datasets which are used by Lustre which set canmount=off by default so they can't be inadvertantly mounted as a normal filesystem. Being able to explicitly mount them when needed with zfs mount -o canmount=on mountpoint=/mnt/ ... would be convenient!

@TemptorSent
Copy link

Okay great! I'll start identifying the sections to modify then, starting with updating the docs and tests.

@almereyda
Copy link

It appears reopening this alongside #4553 would seem feasible for the case described in the last post.

@Xasin
Copy link

Xasin commented Sep 29, 2022

@almereyda
I support reopening this issue.
Running:

zfs-2.0.3-9
zfs-kmod-2.0.3-9

On Debian 11 (bullseye) with standard zfsutils-linux package, the -o mountpoint option is still not available, but would be very useful for my situation in order to better deal with multiple OS on my laptop (for dev. installs)

I can temporarily modify the mountpoint property, but that's messy and more likely to break something.

pcd1193182 pushed a commit to pcd1193182/zfs that referenced this issue Sep 26, 2023
Bumps [syn](https://github.com/dtolnay/syn) from 2.0.22 to 2.0.23.
- [Release notes](https://github.com/dtolnay/syn/releases)
- [Commits](dtolnay/syn@2.0.22...2.0.23)

---
updated-dependencies:
- dependency-name: syn
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants